Skip to content

Commit 6014386

Browse files
committed
ipc: userspace: fix IPC serialization with multiple cores
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 Additional complication is that there is no hard requirement for host to send an IPC destinated to a particular core x, just after MOD_SET_DX was sent to power up this specific core. This is the normal sequence, but FW needs to at least gracefully handle alternative sequences. Fix the serialization issue by moving IPC thread startup synchronization to MOD_SET_DX handling. When a secondary core is booted up the first time after last primary core boot, additional setup steps are done. Reset "init_needed[]" after each primary core boot, and make MOD_SET_DX synchronous when a new secondary core is booted up, not sending a response back to host until the secondary core is booted up and the one-time initialization is done. Note that after this, secondary cores may be powered down and up many times, but the initialization (and related synchronization) is no longer needed. With these changes, there is no longer need to synchronize with secondary core when forwarding IPC messages (in ipc_user_forward_cmd()). It is now guaranteed the target core is running and set up correctly. If any failures happen, these are reported already at MOD_SET_DX. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
1 parent 22c33a6 commit 6014386

2 files changed

Lines changed: 24 additions & 13 deletions

File tree

src/ipc/ipc-common.c

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -446,15 +446,6 @@ int ipc_user_forward_cmd(uint32_t primary, uint32_t extension, unsigned int core
446446
pdata->ipc_msg_ext = extension;
447447
pdata->ipc = ipc;
448448

449-
/*
450-
* Forwarding the first IPC to this core, wait for its userspace IPC
451-
* thread to start
452-
*/
453-
if (pdata->init_needed[core]) {
454-
pdata->init_needed[core] = false;
455-
k_sem_take(pdata->sem, K_FOREVER);
456-
}
457-
458449
/* Prevent host completion until user thread finishes */
459450
key = k_spin_lock(&ipc->lock);
460451
ipc->task_mask |= IPC_TASK_IN_THREAD;
@@ -607,9 +598,6 @@ __cold int ipc_user_init_secondary(unsigned int core)
607598
}
608599

609600
k_thread_access_grant(ipc_user->thread[core], ipc_user->audio_thread[core]);
610-
ipc_user->init_needed[core] = true;
611-
612-
/* Wait for user thread startup — consumes the initial k_sem_give from thread */
613601
return 0;
614602
}
615603

@@ -628,7 +616,7 @@ __cold static void ipc_user_init(void)
628616
struct ipc_user *ipc_user = sof_heap_alloc(sof_sys_user_heap_get(),
629617
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT,
630618
sizeof(*ipc_user), 0);
631-
int ret;
619+
int ret, core;
632620

633621
if (!ipc_user) {
634622
LOG_ERR("user IPC pdata alloc failed");
@@ -637,6 +625,11 @@ __cold static void ipc_user_init(void)
637625

638626
assert_can_be_cold();
639627

628+
for (core = 0; core < CONFIG_CORE_COUNT; core++) {
629+
if (core != PLATFORM_PRIMARY_CORE_ID)
630+
ipc_user->init_needed[core] = true;
631+
}
632+
640633
ipc_user->sem = k_object_alloc(K_OBJ_SEM);
641634
if (!ipc_user->sem) {
642635
LOG_ERR("user IPC sem alloc failed");

src/ipc/ipc4/handler-kernel.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,23 @@ __cold static int ipc4_module_process_d0ix(struct ipc4_message_request *ipc4)
328328
return 0;
329329
}
330330

331+
/* block until core has powered-up (in user-ll builds) */
332+
__cold static void ipc_sec_core_sync_boot(uint32_t core_id)
333+
{
334+
#ifdef CONFIG_SOF_USERSPACE_LL
335+
struct ipc *ipc = ipc_get();
336+
struct ipc_user *ipc_user = ipc->ipc_user_pdata;
337+
338+
assert(core_id != PLATFORM_PRIMARY_CORE_ID);
339+
340+
if (ipc_user->init_needed[core_id]) {
341+
/* wait for IPC thread (ipc_user_thread_fn()) */
342+
k_sem_take(ipc_user->sem, K_FOREVER);
343+
ipc_user->init_needed[core_id] = false;
344+
}
345+
#endif
346+
}
347+
331348
/* enable/disable cores according to the state mask */
332349
__cold static int ipc4_module_process_dx(struct ipc4_message_request *ipc4)
333350
{
@@ -385,6 +402,7 @@ __cold static int ipc4_module_process_dx(struct ipc4_message_request *ipc4)
385402
ipc_cmd_err(&ipc_tr, "failed to enable core %d", core_id);
386403
return IPC4_FAILURE;
387404
}
405+
ipc_sec_core_sync_boot(core_id);
388406
} else {
389407
cpu_disable_core(core_id);
390408
if (cpu_is_core_enabled(core_id)) {

0 commit comments

Comments
 (0)