From 3177564c52d561d6bd817821c25e736589fa6181 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 26 Mar 2026 15:34:08 +0100 Subject: [PATCH 1/2] idc: fix a race Under Zephyr SOF IDC uses Zephyr's P4WQ for message passing. In it k_p4wq_submit() checks that the new work item isn't already on the queue, and it does that by checking its .thread member. That pointer is set to NULL in p4wq_loop() after calling the work handler with no locks held. So, the work handler could've completed handling an IPC, a response could have been sent and a new IPC could arrive, while p4wq_loop() still hasn't cleared the .thread pointer. To fix this use a mutex in idc_send_msg() to protect the work items. Signed-off-by: Guennadi Liakhovetski --- src/idc/zephyr_idc.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/idc/zephyr_idc.c b/src/idc/zephyr_idc.c index f1af9d53a85b..2f5c048f82a4 100644 --- a/src/idc/zephyr_idc.c +++ b/src/idc/zephyr_idc.c @@ -115,23 +115,38 @@ static void idc_handler(struct k_p4wq_work *work) /* * Used for *target* CPUs, since the initiator (usually core 0) can launch - * several IDC messages at once + * several IDC messages at once. Also we need 2 work items per target core, + * because the p4wq thread might just have returned from the work handler, but + * hasn't released the work buffer yet (hasn't set thread pointer to NULL). + * Then submitting the same work item again can result in an assertion failure. */ -static struct zephyr_idc_msg idc_work[CONFIG_CORE_COUNT]; +static struct zephyr_idc_msg idc_work[CONFIG_CORE_COUNT * 2]; +/* Protect the above array */ +static K_MUTEX_DEFINE(idc_mutex); int idc_send_msg(struct idc_msg *msg, uint32_t mode) { struct idc *idc = *idc_get(); struct idc_payload *payload = idc_payload_get(idc, msg->core); unsigned int target_cpu = msg->core; - struct zephyr_idc_msg *zmsg = idc_work + target_cpu; + struct zephyr_idc_msg *zmsg = idc_work + target_cpu * 2; struct idc_msg *msg_cp = &zmsg->msg; struct k_p4wq_work *work = &zmsg->work; int ret; int idc_send_memcpy_err __unused; + k_mutex_lock(&idc_mutex, K_FOREVER); + + if (unlikely(work->thread)) { + /* See comment above the idc_work[] array. */ + zmsg++; + work = &zmsg->work; + msg_cp = &zmsg->msg; + } + idc_send_memcpy_err = memcpy_s(msg_cp, sizeof(*msg_cp), msg, sizeof(*msg)); assert(!idc_send_memcpy_err); + /* Same priority as the IPC thread which is an EDF task and under Zephyr */ work->priority = CONFIG_EDF_THREAD_PRIORITY; work->deadline = 0; @@ -158,6 +173,8 @@ int idc_send_msg(struct idc_msg *msg, uint32_t mode) k_p4wq_submit(q_zephyr_idc + target_cpu, work); + k_mutex_unlock(&idc_mutex); + #ifdef CONFIG_SOF_TELEMETRY_IO_PERFORMANCE_MEASUREMENTS /* Increment performance counters */ io_perf_monitor_update_data(idc->io_perf_out_msg_count, 1); From e6c73bfa6fa2cd6c62e17a69d89ff3cfe6899221 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 26 Mar 2026 15:39:48 +0100 Subject: [PATCH 2/2] dai: (cosmetic) fix an open-coded notifier call Use NOTIFIER_TARGET_CORE_LOCAL instead of open coding it. Signed-off-by: Guennadi Liakhovetski --- src/audio/dai-zephyr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/audio/dai-zephyr.c b/src/audio/dai-zephyr.c index c383557556ef..3c3da63f3662 100644 --- a/src/audio/dai-zephyr.c +++ b/src/audio/dai-zephyr.c @@ -1455,7 +1455,7 @@ int dai_common_trigger(struct dai_data *dd, struct comp_dev *dev, int cmd) irq_local_disable(irq_flags); notifier_event(group, NOTIFIER_ID_DAI_TRIGGER, - BIT(cpu_get_id()), NULL, 0); + NOTIFIER_TARGET_CORE_LOCAL, NULL, 0); irq_local_enable(irq_flags); /* return error of last trigger */