Skip to content

Commit d9aaf8e

Browse files
committed
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 <guennadi.liakhovetski@linux.intel.com>
1 parent 1f18231 commit d9aaf8e

1 file changed

Lines changed: 17 additions & 3 deletions

File tree

src/idc/zephyr_idc.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,23 +115,35 @@ static void idc_handler(struct k_p4wq_work *work)
115115

116116
/*
117117
* Used for *target* CPUs, since the initiator (usually core 0) can launch
118-
* several IDC messages at once
118+
* several IDC messages at once. Also we need 2 work items per target core,
119+
* because the p4wq thread might just have return from the work handler, but
120+
* hasn't released the work buffer yet (hasn't set thread pointer to NULL).
121+
* Then submitting the same work item again can result in an assertion failure.
119122
*/
120-
static struct zephyr_idc_msg idc_work[CONFIG_CORE_COUNT];
123+
static struct zephyr_idc_msg idc_work[CONFIG_CORE_COUNT * 2];
124+
/* Protect the above array */
125+
static K_MUTEX_DEFINE(idc_mutex);
121126

122127
int idc_send_msg(struct idc_msg *msg, uint32_t mode)
123128
{
124129
struct idc *idc = *idc_get();
125130
struct idc_payload *payload = idc_payload_get(idc, msg->core);
126131
unsigned int target_cpu = msg->core;
127-
struct zephyr_idc_msg *zmsg = idc_work + target_cpu;
132+
struct zephyr_idc_msg *zmsg = idc_work + target_cpu * 2;
128133
struct idc_msg *msg_cp = &zmsg->msg;
129134
struct k_p4wq_work *work = &zmsg->work;
130135
int ret;
131136
int idc_send_memcpy_err __unused;
132137

133138
idc_send_memcpy_err = memcpy_s(msg_cp, sizeof(*msg_cp), msg, sizeof(*msg));
134139
assert(!idc_send_memcpy_err);
140+
141+
k_mutex_lock(&idc_mutex, K_FOREVER);
142+
143+
if (work->thread)
144+
/* See comment above the idc_work[] array. */
145+
work++;
146+
135147
/* Same priority as the IPC thread which is an EDF task and under Zephyr */
136148
work->priority = CONFIG_EDF_THREAD_PRIORITY;
137149
work->deadline = 0;
@@ -158,6 +170,8 @@ int idc_send_msg(struct idc_msg *msg, uint32_t mode)
158170

159171
k_p4wq_submit(q_zephyr_idc + target_cpu, work);
160172

173+
k_mutex_unlock(&idc_mutex);
174+
161175
#ifdef CONFIG_SOF_TELEMETRY_IO_PERFORMANCE_MEASUREMENTS
162176
/* Increment performance counters */
163177
io_perf_monitor_update_data(idc->io_perf_out_msg_count, 1);

0 commit comments

Comments
 (0)