ipc: userspace: fix IPC serialization with multiple cores - #11186
Open
kv2019i wants to merge 1 commit into
Open
Conversation
kv2019i
requested review from
bardliao,
pblaszko and
tmleman
as code owners
September 10, 2026 10:42
Collaborator
Author
|
FYI, this is easily hit on sof-ptl-nocodec.tplg with PR11164 (see comment #11164 (review) ). |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The updated logic still relies on a cross-core init_needed[] flag that is accessed non-atomically, which can allow stale reads and reintroduce the serialization race on weakly ordered systems.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes an IPC serialization race in multi-core scenarios by ensuring the per-core “userspace IPC thread not yet ready” flag is set before the secondary-core IPC userspace thread is started, so the primary core correctly blocks on the initial startup semaphore signal rather than misinterpreting it as command completion.
Changes:
- Move
ipc_user->init_needed[core] = true;ahead ofk_thread_start()inipc_user_init_secondary()to close the startup ordering race on secondary cores.
File summaries
| File | Description |
|---|---|
| src/ipc/ipc-common.c | Adjusts secondary-core IPC userspace thread startup ordering to prevent premature host replies and broken IPC serialization. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+598
to
+600
| k_thread_access_grant(ipc_user->thread[core], ipc_user->audio_thread[core]); | ||
| ipc_user->init_needed[core] = true; | ||
| k_thread_start(ipc_user->thread[core]); |
PR 11186: test resultsRun date: 2026-09-10 19:40 UTC Tested commit: 5db4397f18199d1188896fab381f61aeb23080c3
|
kv2019i
marked this pull request as draft
September 10, 2026 12:13
Collaborator
Author
|
Back to draft, still fails, V2 coming |
kv2019i
force-pushed
the
202609-ipc-ordering-fix-multicore
branch
from
September 10, 2026 19:07
fe491d8 to
66120c3
Compare
kv2019i
marked this pull request as ready for review
September 10, 2026 19:07
kv2019i
requested review from
dbaluta,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
September 10, 2026 19:07
Collaborator
Author
|
V2:
|
Fix a race in IPC serialization with multi-core. One sequence observed: - MOD_SET_DX IPC to power up core 1 - IPC reply to host - CREATE_PIPELINE IPC (routed via core 0 to core 1) - core 1 IPC thread starts, signals ipc_user->sem semaphore - core 0 does NOT wait for thread as ipc_user->init_needed is set late - ipc_user->sem signal for thread start is handled as indication that IPC is handled (this is wrong) - IPC reply to host (before CREATE_PIPELINE is handled) - INIT_INSTANCE IPC (routed via core 0 to core 1) - core 0 sees init_needed, but it is already signaled so execution continues -> DSP panic as IPC mailbox is modified while still in use Using a semaphore is not ideal to synchronize with the secondary core IPC threads. Proper execution requires the host to send the correct IPC sequence (e.g. first to send MOD_SET_DX for core x, and then follow-up with IPC messages for same core x). Additionally the start-up is different for first power-up of a secondary core (context is initialized in memory), and subsequent power-ups (IPC thread is never terminated and restarted, it just resumes when core is powered up again). To handle all cases, replace the sem based synchronization with a simple per-core bitmask to indicate whether IPC thread has been created. Before forwarding IPC messages from ipc_user_forward_cmd(), use the bitmask to check core status, and sleep if necessary. This leaves ipc_user->sem dedicated to signal completion of IPC handling in a user thread, and this will no longer get mixed with IPC thread boot signaling. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
force-pushed
the
202609-ipc-ordering-fix-multicore
branch
from
September 10, 2026 19:10
66120c3 to
5db4397
Compare
Collaborator
Author
|
V3:
|
lyakh
requested changes
Sep 11, 2026
lyakh
left a comment
lyakh
left a comment
Collaborator
There was a problem hiding this comment.
this looks strange... Would a very simple fix: set init_needed on core 0 before booting core 1 fix the problem too?
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
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.
Fix a race in IPC serialization with multi-core. The sequence observed:
The race is caused by a bug in setting "init_needed" and starting the IPC user thread on a secondary core (core 1 above). Due to the race, the CREATE_PIPELINE does not wait for core 1 to signal readiness and instead replies to host early, breaking IPC serialization.
Fix the problem by moving set of "init_needed" before the IPC thread is started on a new core. This will ensure CREATE_PIPELINE in above sequence will be not forwarded to core 1 until the IPC thread is ready on the new core.