Skip to content

Commit 66120c3

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 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>
1 parent 158518d commit 66120c3

2 files changed

Lines changed: 20 additions & 12 deletions

File tree

src/include/sof/ipc/common.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ struct ipc_user {
8282
struct k_thread *audio_thread[IPC_DSP_CORE_COUNT];
8383
/** @brief Original kernel driver pointer for restoring dev->drv after create */
8484
const struct comp_driver *init_drv;
85-
bool init_needed[IPC_DSP_CORE_COUNT];
85+
atomic_t thread_ready;
8686
/**
8787
* @brief User-accessible copy of comp_driver + tr_ctx for create().
8888
*

src/ipc/ipc-common.c

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,8 @@ void ipc_schedule_process(struct ipc *ipc)
403403
#define IPC_USER_EVENT_CMD BIT(0)
404404
#define IPC_USER_EVENT_STOP BIT(1)
405405

406+
#define IPC_USER_THREAD_TIMEOUT 100
407+
406408
SOF_DEFINE_REG_UUID(sec_core_init);
407409

408410
static K_THREAD_STACK_ARRAY_DEFINE(ipc_user_stack, CONFIG_CORE_COUNT,
@@ -437,13 +439,15 @@ int ipc_user_forward_cmd(uint32_t primary, uint32_t extension, unsigned int core
437439
pdata->ipc_msg_ext = extension;
438440
pdata->ipc = ipc;
439441

440-
/*
441-
* Forwarding the first IPC to this core, wait for its userspace IPC
442-
* thread to start
443-
*/
444-
if (pdata->init_needed[core]) {
445-
pdata->init_needed[core] = false;
446-
k_sem_take(pdata->sem, K_FOREVER);
442+
if (core != PLATFORM_PRIMARY_CORE_ID) {
443+
/* Secondary cores: must ensure thread and matching pdata->event[core] is ready */
444+
for (int cnt = 0; !atomic_test_bit(&pdata->thread_ready, core); ++cnt) {
445+
k_msleep(1);
446+
if (cnt == IPC_USER_THREAD_TIMEOUT) {
447+
LOG_ERR("Unable to start core %u for IPC", core);
448+
return -ETIME;
449+
}
450+
}
447451
}
448452

449453
/* Prevent host completion until user thread finishes */
@@ -496,7 +500,12 @@ static void ipc_user_thread_fn(void *p1, void *p2, void *p3)
496500
__ASSERT(k_is_user_context(), "expected user context");
497501

498502
/* Signal startup complete — unblocks init waiting on semaphore */
499-
k_sem_give(ipc_user->sem);
503+
if (core == PLATFORM_PRIMARY_CORE_ID)
504+
k_sem_give(ipc_user->sem);
505+
else
506+
/* allow IPCs to sent to this thread */
507+
atomic_set_bit(&ipc_user->thread_ready, core);
508+
500509
LOG_INF("IPC user-space thread started");
501510

502511
for (;;) {
@@ -598,9 +607,6 @@ __cold int ipc_user_init_secondary(unsigned int core)
598607
}
599608

600609
k_thread_access_grant(ipc_user->thread[core], ipc_user->audio_thread[core]);
601-
ipc_user->init_needed[core] = true;
602-
603-
/* Wait for user thread startup — consumes the initial k_sem_give from thread */
604610
return 0;
605611
}
606612

@@ -628,6 +634,8 @@ __cold static void ipc_user_init(void)
628634

629635
assert_can_be_cold();
630636

637+
atomic_set(&ipc_user->thread_ready, 0);
638+
631639
ipc_user->sem = k_object_alloc(K_OBJ_SEM);
632640
if (!ipc_user->sem) {
633641
LOG_ERR("user IPC sem alloc failed");

0 commit comments

Comments
 (0)