Skip to content

Commit 3177564

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 3177564

1 file changed

Lines changed: 20 additions & 3 deletions

File tree

src/idc/zephyr_idc.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,23 +115,38 @@ 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 returned 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

138+
k_mutex_lock(&idc_mutex, K_FOREVER);
139+
140+
if (unlikely(work->thread)) {
141+
/* See comment above the idc_work[] array. */
142+
zmsg++;
143+
work = &zmsg->work;
144+
msg_cp = &zmsg->msg;
145+
}
146+
133147
idc_send_memcpy_err = memcpy_s(msg_cp, sizeof(*msg_cp), msg, sizeof(*msg));
134148
assert(!idc_send_memcpy_err);
149+
135150
/* Same priority as the IPC thread which is an EDF task and under Zephyr */
136151
work->priority = CONFIG_EDF_THREAD_PRIORITY;
137152
work->deadline = 0;
@@ -158,6 +173,8 @@ int idc_send_msg(struct idc_msg *msg, uint32_t mode)
158173

159174
k_p4wq_submit(q_zephyr_idc + target_cpu, work);
160175

176+
k_mutex_unlock(&idc_mutex);
177+
161178
#ifdef CONFIG_SOF_TELEMETRY_IO_PERFORMANCE_MEASUREMENTS
162179
/* Increment performance counters */
163180
io_perf_monitor_update_data(idc->io_perf_out_msg_count, 1);

0 commit comments

Comments
 (0)