Skip to content

Commit 9877834

Browse files
mbolivar-nordiccfriedt
authored andcommitted
device: add fudge factor for handle padding
When CONFIG_USERSPACE is enabled, the ELF file from linker pass 1 is used to create a hash table that identifies kernel objects by address. We therefore can't allow the size of any object in the pass 2 ELF to change in a way that would change those addresses, or we would create a garbage hash table. Simultaneously (and regardless of CONFIG_USERSPACE's value), gen_handles.py must transform arrays of handles from their pass 1 values to their pass 2 values; see the file's docstring for more details on that transformation. The way this works is that gen_handles.py just pads out each pass 2 array so its length is the same as its pass 1 value. The padding value is a repeated run of DEVICE_HANDLE_ENDS values. This value is the terminator which we look for at runtime in places like device_required_handles_get(), so there must be at least one, and we error out in gen_handles.py if there's no room in the pass 2 array for at least one such value. (If there is extra room, we just keep inserting extra DEVICE_HANDLE_ENDS values to pad the array to its original length.) However, it is possible that a device has more direct dependencies in the pass 2 handles array than its corresponding devicetree node had in the pass 1 array. When this happens, users have no recourse, so that's a potential showstopper. To work around this possibility for now, add a new config option, CONFIG_DEVICE_HANDLE_PADDING, whose value defaults to 0. When nonzero, it is a count of padding handles that are inserted into each device handles array. When gen_handles.py errors out due to lack of room, its error message now tells the user how much to increase CONFIG_DEVICE_HANDLE_PADDING by to work around the problem. It looks like a real fix for this is to allocate kernel objects whose addresses are required for hash tables in CONFIG_USERSPACE=y configurations *before* the handle arrays. The handle arrays could then be resized as needed in pass 2, which saves ROM by avoiding unnecessary padding, and would avoid the need for CONFIG_DEVICE_HANDLE_PADDING altogether. However, this 'real fix' is not available and we are facing a deadline to get a temporary solution in for Zephyr v2.7.0, so this is a good enough workaround for now. Signed-off-by: Martí Bolívar <[email protected]>
1 parent ea2bbb8 commit 9877834

File tree

3 files changed

+69
-2
lines changed

3 files changed

+69
-2
lines changed

include/device.h

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,50 @@ static inline bool device_is_ready(const struct device *dev)
686686
Z_DEVICE_STATE_DEFINE(node_id, dev_name) \
687687
Z_DEVICE_DEFINE_PM_SLOT(dev_name)
688688

689+
/* Helper macros needed for CONFIG_DEVICE_HANDLE_PADDING. These should
690+
* be deleted when that option is removed.
691+
*
692+
* This is implemented "by hand" -- rather than using a helper macro
693+
* like UTIL_LISTIFY() -- because we need to allow users to wrap
694+
* DEVICE_DT_DEFINE with UTIL_LISTIFY, like this:
695+
*
696+
* #define DEFINE_FOO_DEVICE(...) DEVICE_DT_DEFINE(...)
697+
* UTIL_LISTIFY(N, DEFINE_FOO_DEVICE)
698+
*
699+
* If Z_DEVICE_HANDLE_PADDING uses UTIL_LISTIFY, this type of code
700+
* would fail, because the UTIL_LISTIFY token within the
701+
* Z_DEVICE_DEFINE_HANDLES expansion would not be expanded again,
702+
* since it appears in a context where UTIL_LISTIFY is already being
703+
* expanded. Standard C does not reexpand macros appearing in their
704+
* own expansion; this would lead to infinite recursions in general.
705+
*/
706+
#define Z_DEVICE_HANDLE_PADDING \
707+
Z_DEVICE_HANDLE_PADDING_(CONFIG_DEVICE_HANDLE_PADDING)
708+
#define Z_DEVICE_HANDLE_PADDING_(count) \
709+
Z_DEVICE_HANDLE_PADDING__(count)
710+
#define Z_DEVICE_HANDLE_PADDING__(count) \
711+
Z_DEVICE_HANDLE_PADDING_ ## count
712+
#define Z_DEVICE_HANDLE_PADDING_10 \
713+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_9
714+
#define Z_DEVICE_HANDLE_PADDING_9 \
715+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_8
716+
#define Z_DEVICE_HANDLE_PADDING_8 \
717+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_7
718+
#define Z_DEVICE_HANDLE_PADDING_7 \
719+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_6
720+
#define Z_DEVICE_HANDLE_PADDING_6 \
721+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_5
722+
#define Z_DEVICE_HANDLE_PADDING_5 \
723+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_4
724+
#define Z_DEVICE_HANDLE_PADDING_4 \
725+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_3
726+
#define Z_DEVICE_HANDLE_PADDING_3 \
727+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_2
728+
#define Z_DEVICE_HANDLE_PADDING_2 \
729+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_1
730+
#define Z_DEVICE_HANDLE_PADDING_1 \
731+
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_0
732+
#define Z_DEVICE_HANDLE_PADDING_0 EMPTY
689733

690734
/* Initial build provides a record that associates the device object
691735
* with its devicetree ordinal, and provides the dependency ordinals.
@@ -722,6 +766,7 @@ BUILD_ASSERT(sizeof(device_handle_t) == 2, "fix the linker scripts");
722766
)) \
723767
DEVICE_HANDLE_SEP, \
724768
Z_DEVICE_EXTRA_HANDLES(__VA_ARGS__) \
769+
Z_DEVICE_HANDLE_PADDING \
725770
};
726771

727772
#ifdef CONFIG_PM_DEVICE

kernel/Kconfig

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,22 @@ config WAITQ_DUMB
288288

289289
endchoice # WAITQ_ALGORITHM
290290

291+
config DEVICE_HANDLE_PADDING
292+
int "Number of additional device handles to use for padding"
293+
default 0
294+
range 0 10
295+
help
296+
This is a "fudge factor" which works around build system
297+
limitations. It is safe to ignore unless you get a specific
298+
build system error about it.
299+
300+
The option's value is the number of superfluous device handle
301+
values which are introduced into the array pointed to by each
302+
device's 'handles' member during the first linker pass.
303+
304+
Each handle uses 2 bytes, so a value of 3 would use an extra
305+
6 bytes of ROM for every device.
306+
291307
menu "Kernel Debugging and Metrics"
292308

293309
config INIT_STACKS

scripts/gen_handles.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,15 @@ def main():
311311
# address. We can't allow the size of any object in the
312312
# final elf to change. We also must make sure at least one
313313
# DEVICE_HANDLE_ENDS is inserted.
314-
assert len(hdls) < len(hs.handles), "%s no DEVICE_HANDLE_ENDS inserted" % (dev.sym.name,)
315-
while len(hdls) < len(hs.handles):
314+
padding = len(hs.handles) - len(hdls)
315+
assert padding > 0, \
316+
(f"device {dev.sym.name}: "
317+
"linker pass 1 left no room to insert DEVICE_HANDLE_ENDS. "
318+
"To work around, increase CONFIG_DEVICE_HANDLE_PADDING by " +
319+
str(1 + (-padding)))
320+
while padding > 0:
316321
hdls.append(DEVICE_HANDLE_ENDS)
322+
padding -= 1
317323
assert len(hdls) == len(hs.handles), "%s handle overflow" % (dev.sym.name,)
318324

319325
lines = [

0 commit comments

Comments
 (0)