Backport 38834 to v2.7 branch #39054
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mbolivar-nordic
requested review from
andyross,
dcpleung,
nashif and
tbursztyka
as code owners
github-actions
bot
added
area: API
|
fun following the links to know what this PR is supposed to do without any description... |
cfriedt
approved these changes
Oct 2, 2021
cfriedt
requested changes
Oct 2, 2021
This reverts commit 4c32e21 with some manual conflict resolution. It's not clear that the supported devices are being properly computed, so let's revert this for v2.7.0 until we've had more time to think it through. Signed-off-by: Martí Bolívar <[email protected]>
This reverts commit b01e41c. It's not clear that the supported devices are being properly computed, so let's revert this for v2.7.0 until we've had more time to think it through. Signed-off-by: Martí Bolívar <[email protected]>
This reverts commit 0c6588f. It's not clear that the supported devices are being properly computed, so let's revert this for v2.7.0 until we've had more time to think it through. Signed-off-by: Martí Bolívar <[email protected]>
This reverts commit ec331c6. Although it's a valid simplification under the assumption that we're going to be padding the array out anyway, it would use extra ROM if we fix the build system issues that are currently forcing gen_handles.py to introduce extra padding in the handles arrays for linker pass 2. On the (perhaps optimistic) assumption that we're going to fix the build system, let's get rid of a commit that would get in the way. The extra "complexity" in device_required_handles_get() is trivial. This gets rid of a comment describing the linker passes, but the structure of the comment is a bit misleading (and it contains incorrect information for the results of pass 2: the terminator at the end is DEVICE_HANDLE_ENDS, not DEVICE_HANDLE_NULL). Signed-off-by: Martí Bolívar <[email protected]>
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]>
mbolivar-nordic
force-pushed
the
backport-38834-to-v2.7-branch
branch
from
b2f3117 to
a6de38c
Compare
|
I botched the rebase last time; it's passing now |
cfriedt
approved these changes
Oct 4, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #39022
Fixes: #38181