Skip to content

Commit

Permalink
device: add fudge factor for handle padding
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
mbolivar-nordic authored and cfriedt committed Oct 1, 2021
1 parent 2c99173 commit a627666
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 2 deletions.
45 changes: 45 additions & 0 deletions include/device.h
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,50 @@ static inline bool device_is_ready(const struct device *dev)
Z_DEVICE_STATE_DEFINE(node_id, dev_name) \
Z_DEVICE_DEFINE_PM_SLOT(dev_name)

/* Helper macros needed for CONFIG_DEVICE_HANDLE_PADDING. These should
* be deleted when that option is removed.
*
* This is implemented "by hand" -- rather than using a helper macro
* like UTIL_LISTIFY() -- because we need to allow users to wrap
* DEVICE_DT_DEFINE with UTIL_LISTIFY, like this:
*
* #define DEFINE_FOO_DEVICE(...) DEVICE_DT_DEFINE(...)
* UTIL_LISTIFY(N, DEFINE_FOO_DEVICE)
*
* If Z_DEVICE_HANDLE_PADDING uses UTIL_LISTIFY, this type of code
* would fail, because the UTIL_LISTIFY token within the
* Z_DEVICE_DEFINE_HANDLES expansion would not be expanded again,
* since it appears in a context where UTIL_LISTIFY is already being
* expanded. Standard C does not reexpand macros appearing in their
* own expansion; this would lead to infinite recursions in general.
*/
#define Z_DEVICE_HANDLE_PADDING \
Z_DEVICE_HANDLE_PADDING_(CONFIG_DEVICE_HANDLE_PADDING)
#define Z_DEVICE_HANDLE_PADDING_(count) \
Z_DEVICE_HANDLE_PADDING__(count)
#define Z_DEVICE_HANDLE_PADDING__(count) \
Z_DEVICE_HANDLE_PADDING_ ## count
#define Z_DEVICE_HANDLE_PADDING_10 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_9
#define Z_DEVICE_HANDLE_PADDING_9 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_8
#define Z_DEVICE_HANDLE_PADDING_8 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_7
#define Z_DEVICE_HANDLE_PADDING_7 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_6
#define Z_DEVICE_HANDLE_PADDING_6 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_5
#define Z_DEVICE_HANDLE_PADDING_5 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_4
#define Z_DEVICE_HANDLE_PADDING_4 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_3
#define Z_DEVICE_HANDLE_PADDING_3 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_2
#define Z_DEVICE_HANDLE_PADDING_2 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_1
#define Z_DEVICE_HANDLE_PADDING_1 \
DEVICE_HANDLE_ENDS, Z_DEVICE_HANDLE_PADDING_0
#define Z_DEVICE_HANDLE_PADDING_0 EMPTY

/* Initial build provides a record that associates the device object
* with its devicetree ordinal, and provides the dependency ordinals.
Expand Down Expand Up @@ -735,6 +779,7 @@ BUILD_ASSERT(sizeof(device_handle_t) == 2, "fix the linker scripts");
)) \
DEVICE_HANDLE_SEP, \
Z_DEVICE_EXTRA_HANDLES(__VA_ARGS__) \
Z_DEVICE_HANDLE_PADDING \
};

#ifdef CONFIG_PM_DEVICE
Expand Down
16 changes: 16 additions & 0 deletions kernel/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,22 @@ config WAITQ_DUMB

endchoice # WAITQ_ALGORITHM

config DEVICE_HANDLE_PADDING
int "Number of additional device handles to use for padding"
default 0
range 0 10
help
This is a "fudge factor" which works around build system
limitations. It is safe to ignore unless you get a specific
build system error about it.

The option's value is the number of superfluous device handle
values which are introduced into the array pointed to by each
device's 'handles' member during the first linker pass.

Each handle uses 2 bytes, so a value of 3 would use an extra
6 bytes of ROM for every device.

menu "Kernel Debugging and Metrics"

config INIT_STACKS
Expand Down
10 changes: 8 additions & 2 deletions scripts/gen_handles.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,15 @@ def main():
# address. We can't allow the size of any object in the
# final elf to change. We also must make sure at least one
# DEVICE_HANDLE_ENDS is inserted.
assert len(hdls) < len(hs.handles), "%s no DEVICE_HANDLE_ENDS inserted" % (dev.sym.name,)
while len(hdls) < len(hs.handles):
padding = len(hs.handles) - len(hdls)
assert padding > 0, \
(f"device {dev.sym.name}: "
"linker pass 1 left no room to insert DEVICE_HANDLE_ENDS. "
"To work around, increase CONFIG_DEVICE_HANDLE_PADDING by " +
str(1 + (-padding)))
while padding > 0:
hdls.append(DEVICE_HANDLE_ENDS)
padding -= 1
assert len(hdls) == len(hs.handles), "%s handle overflow" % (dev.sym.name,)

lines = [
Expand Down

0 comments on commit a627666

Please sign in to comment.