arch: arm: Introduce SMP support for Cortex-A/R #61206
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
|
I added |
|
You are not on the reviewer list, but I think you must be interested in it. Please have a look when you have time. @povergoing, @ithinuel |
|
Thank you! |
|
@ibirnbaum FYI |
andyross
reviewed
Aug 7, 2023
arch/arm/core/cortex_a_r/thread.c
Outdated
| #if defined(CONFIG_USE_SWITCH) | ||
| extern void z_arm_exit_exc(void); | ||
| thread->switch_handle = thread; | ||
| thread->callee_saved.lr = (uint32_t)z_arm_exc_exit; |
There was a problem hiding this comment.
Naming: calling that value "LR" is a minor abuse; that register is for the return address. As seen here it's actually being faked as the entry (!) address of the exception return path. Which is fine as far as implementation goes, but IMHO it would be clearer to call this "PC", since it's the address at which at execution will resume. Calling it "LR" is just an implementation detail from how you implemented the cooperative switch case.
There was a problem hiding this comment.
Calling it "LR" can tell the user that we save the LR register into it, I'm worried that the user may get confused if we call it "PC".
| static ALWAYS_INLINE void | ||
| arch_thread_return_value_set(struct k_thread *thread, unsigned int value) | ||
| { | ||
| thread->arch.swap_return_value = value; | ||
| } | ||
|
|
||
| #else | ||
|
|
||
| static inline void arch_switch(void *switch_to, void **switched_from) |
There was a problem hiding this comment.
You want ALWAYS_INLINE here. This gets expanded in just a few hot paths in the kernel; we know we want it inlined, don't give the toolchain the option to mess it up.
| /* save old thread into switch handle which is required by | ||
| * z_sched_switch_spin() | ||
| */ | ||
| str r1, [r1, #___thread_t_switch_handle_OFFSET] |
There was a problem hiding this comment.
FWIW: this must be the very last thing done with the old thread context. Since someone will be coming back here to add FPU support later, it might be nice for the comment to warn about that and/or mention the docs in arch_interface.h that explain the requirement.
Also, at least on SMP Cortex-A you need a write barrier immediately before this store, so that no other CPU can possibly see the earlier stores cross with it and miss some of the saved data.
| ldr r2, =_thread_offset_to_callee_saved | ||
| add r2, r1, r2 | ||
|
|
||
| stm r2, {r4-r11, sp, lr} |
There was a problem hiding this comment.
This is pleasingly simple, but FWIW making it a function call is an extra set of instructions that you really don't need in the cooperative switch case It wouldn't be much more complicated to make arch_switch() an inline assembly expansion that does mostly the same stuff, adds a clobber for r0-3 and r12 [1], and stores the address of the following instruction into the thread context via a simple assembly symbol.
[1] Probably. I'm winging my knowledge of the ARM ABI based on general intuition and a copy of the AAPCS spec I just googled in another window. Apply salt.
There was a problem hiding this comment.
I'm reconsidering this now.
7 tasks
SgrrZhf
force-pushed
the
topics/huizha01/cortex-ar/introduce-smp
branch
from
325f358
to
e80aee1
Compare
SgrrZhf
requested review from
stephanosio,
MaureenHelm,
galak,
ioannisg,
tejlmand,
nashif and
bbolen
as code owners
zephyrbot
added
area: ARM
zephyrbot
requested review from
57300,
carlocaione,
jeremybettis,
microbuilder,
nordicjm and
povergoing
MMU or MPU unit need to be initialized by its own CPU. - Primary core initialize MMU or MPU unit in z_arm_prep_c. - Secondary core initialize MMU or MPU unit in z_arm_secondary_start. Signed-off-by: Huifeng Zhang <Huifeng.Zhang@arm.com>
Replace the TLS base address pointer from TPIDRURO to TPIDRURW. The difference between them is that TPIDRURO is read-only in user mode but TPIDRURW isn't. So TPIDRURO is much more suitable for store the address of _kernel.CPU[n]. For this reason, this commit replaces the base pointer of the TLS area. Signed-off-by: Huifeng Zhang <Huifeng.Zhang@arm.com>
Store the current CPU's struct _cpu instance into TPIDRURO, so that the CPU core can get its struct _cpu instance by reading TPIDRURO. This is useful in the SMP system. Signed-off-by: Huifeng Zhang <Huifeng.Zhang@arm.com>
This commit introduce 'USE_SWITCH' feature into cortex-A/R(aarch32) architecture For introducing USE_SWITCH, the exception entry and exit are unified via `z_arm_cortex_ar_enter_exc` and `z_arm_cortex_ar_exit_exc`. All exceptions including ISR are using this way to enter and exit exception handler. Differentiate exception depth and interrupt depth. Allow doing context switch when exception depth greater than 1 but not allow doing this when interrupt depth greater than 1. Currently, USE_SWITCH doesn't support FPU_SHARING and USERSPACE. Signed-off-by: Huifeng Zhang <Huifeng.Zhang@arm.com>
This commit introduces SMP support into Cortex-A/R aarch32 architecture. For now, this only supports multiple core start together and only allow one CPU initialize system as primary core, others loop at the beginning as the secondary cores and wait for wake up. Signed-off-by: Huifeng Zhang <Huifeng.Zhang@arm.com>
This commit add a new board named 'fvp_baser_aemv8r_aarch32_smp' Signed-off-by: Huifeng Zhang <Huifeng.Zhang@arm.com>
SgrrZhf
force-pushed
the
topics/huizha01/cortex-ar/introduce-smp
branch
from
e80aee1
to
c9f2ef9
Compare
|
Now, |
povergoing
approved these changes
Nov 2, 2023
carlocaione
approved these changes
Nov 3, 2023
microbuilder
requested changes
Nov 3, 2023
andyross
approved these changes
Nov 6, 2023
@SgrrZhf Why don't we do a new documentation PR with the release notes update there instead, and maybe see if anything in |
microbuilder
approved these changes
Nov 6, 2023
Ok, I‘ll update the release notes in another PR later. |
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.
This PR do mainly things below.
USE_SWITCH_SUPPORTfor Cortex-A/R (AArch32).z_arm_cortex_ar_enter_excz_arm_cortex_ar_exit_excisr_wrappercode. Don't allow context switch when interrupt depth is greater than 1.fvp_baser_aemv8r_aarch32_smpplatform.Note:
fvp_baser_aemv8r_aarch32_smpplatform enabledUSE_SWITCH_SUPPORT.