kernel: assorted 64-bit fixes for user mode #20943
Merged
Conversation
This file contains hidden or 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
andrewboie
requested review from
MaureenHelm,
andyross,
carlescufi,
galak and
nashif
as code owners
zephyrbot
added
area: X86
|
All checks passed. Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages. |
Contributor
Author
|
This doesn't need to go into 2.1 as current user mode targets are unaffected. |
andrewboie
force-pushed
the
64bit-fixes-user
branch
from
32f1d5f to
0e082e8
Compare
dcpleung
approved these changes
Nov 22, 2019
nashif
approved these changes
Nov 22, 2019
stephanosio
reviewed
Nov 25, 2019
Member
There was a problem hiding this comment.
Shouldn't we be really treating start_addr here as uintptr_t *, as _thread_stack_info defines start as uintptr_t?
p.s. Speaking of which, it seems _thread_stack_info still defines size as u32_t rather than size_t.
Contributor
Author
There was a problem hiding this comment.
Actually we should not be using uintptr_t * at all.
The start member of thread_stack_info is stored as a uintptr_t because it is a memory address. It does not point to a uintptr_t.
I did fix the size member to use size_t.
wentongwu
approved these changes
Nov 27, 2019
added 5 commits
December 11, 2019 14:50
We need a size_t and not a u32_t for partition sizes, for 64-bit compatibility. Additionally, app_memdomain.h was also casting the base address to a u32_t instead of a uintptr_t. Signed-off-by: Andrew Boie <[email protected]>
This has to be wide enough to store a pointer. Signed-off-by: Andrew Boie <[email protected]>
Just use the correct data type. Signed-off-by: Andrew Boie <[email protected]>
64-bit systems generate some compiler warnings about data type sizes, use uintptr_t where int/u32_t was being cast to void *. Signed-off-by: Andrew Boie <[email protected]>
We need a format code for struct packing that fits in a pointer value, "I" is fixed at 32-bit. The conversion of string to pointer value now prints 8 bytes. This works for 32-bit since the leading 4 digits are always zero. The replaced length check uses sizeof(void *) and not 4. Signed-off-by: Andrew Boie <[email protected]>
added 9 commits
December 11, 2019 14:50
These are not C strings, just pointers to kernel objects. Improves output when working with a debugger. Signed-off-by: Andrew Boie <[email protected]>
Use a size_t instead. Signed-off-by: Andrew Boie <[email protected]>
Always use size_t for size calculations, not u32_t. Signed-off-by: Andrew Boie <[email protected]>
Use uintptr_t instead. Fixes some 64-bit issues. Signed-off-by: Andrew Boie <[email protected]>
We don't need this cast, just use %p format code. Signed-off-by: Andrew Boie <[email protected]>
The requests are somewhat larger on 64-bit since we are allocating structs with pointer members. Increase these to a larger multiple of the minimum block size. Signed-off-by: Andrew Boie <[email protected]>
This test has a problem, specifically in the scenario for test_mem_domain_remove_partitions. A low priority thread (10) is created which is expected to produce an exception. Then the following happens: - The thread indeed crashes and ends up in the custom fatal error handler, on the stack used for exceptions - The call to ztest_test_pass() is made - ztest_test_pass() gives the test_end_signal semaphore - We then context switch to the ztest main thread which is higher priority, leaving the thread that crashed context switched out *on the exception stack* - More tests are run, and some of them also produce exceptions - Eventually we do a sleep and the original crashed thread is swapped in again - Since several other exceptions have taken place on the exception stack since then, resuming context results in an unexpected error, causing the test to fail Only seems to affect arches that have a dedicated stack for exceptions, like x86_64. For now, increase the priority of the child thread so it's cleaned up immediately. Longer-term, this all needs to be re-thought in the test case to make this less fragile. Signed-off-by: Andrew Boie <[email protected]>
In addition to not assuming all pointers fit in a u32_t, logic is added to find the privilege mode stack on x86_64 and several error messages now contain more information. Signed-off-by: Andrew Boie <[email protected]>
We should use size_t for memory region sizes everywhere, not u32_t. Signed-off-by: Andrew Boie <[email protected]>
andrewboie
force-pushed
the
64bit-fixes-user
branch
from
0e082e8 to
fa9558c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Development of user mode support for x86_64 revealed several issues in the kernel. Please review individual commit messages for details on each fix.