kernel: introduce k_smp_cpu_start() and k_smp_cpu_resume()
#64755
Conversation
dcpleung
force-pushed
the
kernel/custom_smp_start
branch
3 times, most recently
from
851de5e to
478101f
Compare
|
The following west manifest projects have been modified in this Pull Request:
Note: This message is automatically posted and updated by the Manifest GitHub Action. |
zephyrbot
added
manifest
manifest-sof
DNM
dcpleung
requested review from
andyross,
nashif,
carlescufi,
galak,
MaureenHelm and
mbolivar-ampere
as code owners
zephyrbot
added
area: Kernel
area: Audio
platform: Intel ADSP
zephyrbot
requested review from
ceolin,
cfriedt,
jhedberg,
laurenmurphyx64,
lyakh,
npitre,
peter-mitsis and
tbursztyka
|
@kv2019i zephyrproject-rtos/sof#32 is my crude attempt at changing the SOF secondary core power up code. I am not totally sure on the power management side. Please take a look and see if there are any issues. |
dcpleung
force-pushed
the
kernel/custom_smp_start
branch
from
14a1658 to
e273354
Compare
For some reason, unrelated code change triggered compiler warning about this function returns even though it is marked nonreturn. So add CODE_UNREACHABLE to silence the warning, possibly to catch any errors. Signed-off-by: Daniel Leung <[email protected]>
qemu_x86_64 has default of 2 CPUs but the device tree only has 1. For correctness, add another CPU node to the tree. Signed-off-by: Daniel Leung <[email protected]>
This extends the wording so that not only architecture code can start secondary CPUs at a later time. Also adds a missing 'to'. Signed-off-by: Daniel Leung <[email protected]>
This makes the test a bit more generic so it can run on other SMP enabled platforms. Though, we don't need CI running this on every SMP platforms as this is not testing the mechanism of bringing up a CPU. That is being tested on the other SMP test. Therefore, only enable qemu_x86_64 to be testing in CI so that this feature will still be tested on CI. Signed-off-by: Daniel Leung <[email protected]>
This renames z_smp_cpu_start() to k_smp_cpu_start(). This effectively promotes z_smp_cpu_start() into a public API which allows out of tree usage. Since this is a new API, we can afford to change it signature, where it allows an additional initialization steps to be done before a newly powered up CPU starts participating in scheduling threads to run. Signed-off-by: Daniel Leung <[email protected]>
Adds some comments to the SMP code to, hopefully, make it a bit more clear to future readers. Signed-off-by: Daniel Leung <[email protected]>
This provides a path to resume a previously suspended CPU. This differs from k_smp_cpu_start() where per-CPU kernel structs are not initialized such that execution context can be saved during suspend and restored during resume. Though the actual context saving and restoring are platform specific. Signed-off-by: Daniel Leung <[email protected]>
This extends the smp_boot_delay test to test the newly introduced function k_smp_cpu_custom_start(). Signed-off-by: Daniel Leung <[email protected]>
This updates the SOF to use the newly introduced k_smp_cpu_custom_start() for secondary core power up. This removes the need to mirror part of the SMP power up code from Zephyr kernel into SOF tree. Also removes the need to expose private kernel code to SOF. Signed-off-by: Daniel Leung <[email protected]>
This removes z_smp_thread_init() and z_smp_thread_swap() as SOF has been updated to use k_smp_cpu_custom_start() instead. This removes the need for SOF to mirror part of the SMP power up code, and thus these two functions are no longer needed. Signed-off-by: Daniel Leung <[email protected]>
z_init_cpu() is a private kernel API so move it under kernel/include. Signed-off-by: Daniel Leung <[email protected]>
When exiting power gated state, call the CPU start function passed to arch_start_cpu(). Signed-off-by: Rander Wang <[email protected]> Signed-off-by: Daniel Leung <[email protected]>
dcpleung
force-pushed
the
kernel/custom_smp_start
branch
from
e273354 to
b235db2
Compare
|
SOF PR merged. Updated the manifest to use the commit SHA. |
|
@kv2019i this PR was merged, so we need another PR to switch api in sof. I will submit it if the thesofproject/sof#8747 is merged |
|
@RanderWang please prepare a SOF PR that has your changes and updates the Zephyr version as well. We have a SOF regression with Zephyr unfortunately now so thesofproject/sof#8747 might never be merged (in its current form) and we may need to jump to a newer Zephyr (if a fix is found on Zephyr side) and that will then require the k_smp changes, so better to prepare that PR now (even if some SOF CI fails are expected for it). |
| z_dummy_thread_init(&dummy_thread); | ||
| #ifdef CONFIG_SYS_CLOCK_EXISTS | ||
| smp_timer_init(); | ||
| #endif | ||
|
|
||
| /* Do additional initialization steps if needed. */ | ||
| if ((csc != NULL) && (csc->fn != NULL)) { | ||
| csc->fn(csc->arg); |
There was a problem hiding this comment.
@dcpleung isn't this racy in general? On all CPUs csc points to cpu_start_fn, right? And the CPU gets here after signalling the initiator CPU, the one that started this one. So the initiator CPU can go ahead and start yet another CPU potentially with different parameters. So far that shouldn't be a problem for SOF, because there .fn is the same for all and .arg is always NULL, but in general this seems wrong?
There was a problem hiding this comment.
Yes, there is potential for conflicts here. So the caller must take precautions to avoid that situation. Or we can implement a per-CPU data struct (but this is going to take up some space).
| /* Secondary core is resumed by set_dx */ | ||
| if (arch_proc_id()) { | ||
| mp_resume_entry(); | ||
| } |
There was a problem hiding this comment.
This has been causing regressions and test failures (thesofproject/sof#8818). Tentative removal of this part in:
z_smp_start_cpu()to public API ask_smp_cpu_start()so it can be used out of tree.k_smp_cpu_start()to do additional initialization before handing off to scheduler to run threads if desired.k_smp_cpu_resume()to address the need to power up a CPU without re-initialize per-CPU kernel structs.z_smp_thread_init()andz_smp_thread_swap()exposed ininclude/zephyr/kernel/internal/smp.hspecifically for SOF.