zephyr: idc: align the IDC payload to the cache line by lgirdwood · Pull Request #11334 · thesofproject/sof

lgirdwood · 2026-10-22T13:06:34Z

Summary

struct idc_payload lives in coherent (SHARED_DATA) memory and is shared across cores, but it is a uint8_t array with no explicit alignment, allowing the linker to place it on an unaligned address. Handlers that read or write the payload as a wider type (such as the cross-core get_attribute path, which accesses a uint32_t attribute field) take an EXCCAUSE=9 (LoadStoreAlignmentCause) fault when a component runs on a secondary core on ACE 3.0 / PTL.

Align struct idc_payload to DCACHE_LINE_SIZE so that every per-core IDC payload slot is cache-line and word aligned.

Verification

Tested on Panther Lake (AAEON UP Xtreme PTL / ACE 3.0) built with LLVM + HiFi (xtensa-intel_ace30_ptl-zephyr-elf) with ffmpeg_dec running as a DP module on secondary core 1:

Cross-core MOD_BIND / comp_get_attribute_remote() succeeds without alignment faults.
Cross-core comp_prepare_remote() completes synchronously within the 10 ms IDC timeout (~0.35 ms with lazy decoder open on the DP thread).
End-to-end compressed playback (cplay), drain EOS, comp_reset_remote(), MOD_UNBIND, and DELETE_PIPELINE complete cleanly.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open issues

This relies on the sender enforcing get_attr_payload->type == COMP_ATTR_BASE_CONFIG, but the… · New
Using K_FOREVER can permanently stall the caller if the remote core wedges, the work item never… · New
Switching comp_prepare_remote() to IDC_NON_BLOCKING means the caller no longer observes the… · New
ipc4_coherent_copy32() is currently introduced in component_ext.h (audio/component layer), but… · New
ipc4_coherent_copy32() silently truncates if bytes is not a multiple of 4 (words = bytes / 4)… · New

What changed in this PR

Fixes cross-core IDC and remote component lifecycle issues on secondary cores by addressing alignment faults, uncached/coherent memcpy faults, and blocking behavior that can stall or time out during heavy DP cold-start/reset.

Changes:

Aligns struct idc_payload to DCACHE_LINE_SIZE to prevent Xtensa alignment faults when reading payload fields as wider types.
Introduces IDC_BLOCKING_NO_TIMEOUT and implements a K_FOREVER wait path for reset completion.
Reworks remote component prepare to be non-blocking and adds 32-bit word copy helper for coherent (uncached) attribute payload transfers.

File	Description
zephyr/include/rtos/idc.h	Adds `IDC_BLOCKING_NO_TIMEOUT` and aligns IDC payload to cache line to avoid alignment faults.
src/include/sof/audio/component_ext.h	Makes remote prepare non-blocking; adds coherent 32-bit copy helper; uses it for IPC4 get_attribute.
src/idc/zephyr_idc.c	Implements `IDC_BLOCKING_NO_TIMEOUT` behavior using `k_p4wq_wait(..., K_FOREVER)`.
src/idc/idc.c	Updates remote get_attribute handler to use local cacheable struct + 32-bit stores into coherent buffer.
posix/include/rtos/idc.h	Defines `IDC_BLOCKING_NO_TIMEOUT` for parity with Zephyr header.

_{Review effort: Lite}

+	/*
+	 * Let the module fill a local (cacheable) copy, then publish it to the
+	 * caller's coherent buffer with aligned 32-bit stores. Passing the
+	 * coherent buffer straight to the module makes it memcpy() a whole
+	 * struct into the uncached alias, which faults (EXCCAUSE 9) on Xtensa.
+	 * Only COMP_ATTR_BASE_CONFIG is supported remotely (enforced by the
+	 * sender), so local_cfg is always the right size.
+	 */
+	ret = comp_get_attribute(ipc_dev->cd, get_attr_payload->type, &local_cfg);
+	if (ret < 0)
+		return ret;
+
+	ipc4_coherent_copy32(get_attr_payload->value, &local_cfg, sizeof(local_cfg));
+
+	return ret;


+	case IDC_BLOCKING_NO_TIMEOUT:
+		/* wait for completion; the handler is bounded but may exceed
+		 * CONFIG_IDC_TIMEOUT_US (see IDC_BLOCKING_NO_TIMEOUT) */
+		ret = k_p4wq_wait(work, K_FOREVER);
+		if (!ret)
+			ret = idc_msg_status_get(msg->core);
+		break;


+	 * Non-blocking: a remote component's prepare may be expensive (e.g. a DP
+	 * decoder's one-time codec open building large trig tables -- measured
+	 * ~580 ms for mp3 on a secondary core). Waiting for it here stalls the
+	 * initiator core's IPC/EDF thread for that whole time, and with the
+	 * default CONFIG_IDC_TIMEOUT_US the k_p4wq_wait() would instead time out
+	 * mid-prepare (-EAGAIN) and race the pipeline teardown into a crash.
+	 *
+	 * Ordering is preserved without the wait: each core drains its IDC work
+	 * through a single in-order p4wq worker, so this prepare completes before
+	 * the subsequent cross-core trigger (START) that is queued behind it. For
+	 * a compress stream the START IPC also arrives only after the app's first
+	 * write, giving the remote prepare time to finish off the critical path.
+	 */
+	return idc_send_msg(&msg, IDC_NON_BLOCKING);


+	if (ret < 0)
+		return ret;
+
+	ipc4_coherent_copy32(get_attr_payload->value, &local_cfg, sizeof(local_cfg));


+static inline void ipc4_coherent_copy32(void *dst, const void *src, size_t bytes)
+{
+	volatile uint32_t *d = dst;
+	const volatile uint32_t *s = src;
+	size_t words = bytes / sizeof(uint32_t);
+	size_t i;
+
+	for (i = 0; i < words; i++)
+		d[i] = s[i];
+}


lgirdwood

Revisit IDC update afer single core is upstream. Needs to be generic.

lgirdwood · 2026-10-22T13:29:50Z

+	size_t words = bytes / sizeof(uint32_t);
+	size_t i;
+
+	for (i = 0; i < words; i++)


we can use a vectorized version here, there is a global call that can be reused.

lgirdwood · 2026-10-22T13:33:01Z

@@ -129,7 +132,21 @@ static int idc_get_attribute(uint32_t comp_id)
 	idc_payload = idc_payload_get(*idc_get(), cpu_get_id());
 	get_attr_payload = (struct get_attribute_remote_payload *)idc_payload;

-	return comp_get_attribute(ipc_dev->cd, get_attr_payload->type, get_attr_payload->value);
+	/*
+	 * Let the module fill a local (cacheable) copy, then publish it to the


I suspect the copy is fixing a race we have somewhere else - revisit after ffmpeg only parts are merged.

lgirdwood · 2026-10-22T13:34:01Z

@@ -242,7 +242,21 @@ static inline int comp_prepare_remote(struct comp_dev *dev)
 	struct idc_msg msg = { IDC_MSG_PREPARE,
 		IDC_MSG_PREPARE_EXT(dev->ipc_config.id), dev->ipc_config.core, };

-	return idc_send_msg(&msg, IDC_BLOCKING);
+	/*


revisit after ffmpeg merge

intel-sofci · 2026-10-22T13:53:14Z

PR 11334: test results

Run date: 2026-11-03 16:42 UTC

Tested commit: 141d71515cb4171bc6e4b5ef415d268213c1016f

Copilot

🟡 Changes recommended

Asynchronous remote prepare can corrupt shared requests, overwrite queued work, discard failure statuses, and cause trigger timeouts, allowing failed pipelines to be marked prepared.

9 open issues

PRE_START can time out while waiting for remote prepare · New
Prepare completion overwrites a queued PRE_START command · New
Nonblocking prepare drops remote errors and advances pipeline · New
Switching comp_prepare_remote() to IDC_NON_BLOCKING means the caller no longer observes the…
Using K_FOREVER can permanently stall the caller if the remote core wedges, the work item never…
This relies on the sender enforcing get_attr_payload->type == COMP_ATTR_BASE_CONFIG, but the…
Third prepare request overwrites a queued IDC work descriptor · New
ipc4_coherent_copy32() silently truncates if bytes is not a multiple of 4 (words = bytes / 4)…
ipc4_coherent_copy32() is currently introduced in component_ext.h (audio/component layer), but…

_{Review effort: Balanced}

+	 * the subsequent cross-core trigger (START) that is queued behind it. For
+	 * a compress stream the START IPC also arrives only after the app's first
+	 * write, giving the remote prepare time to finish off the critical path.


+	 * a compress stream the START IPC also arrives only after the app's first
+	 * write, giving the remote prepare time to finish off the critical path.
+	 */
+	return idc_send_msg(&msg, IDC_NON_BLOCKING);


+	 * a compress stream the START IPC also arrives only after the app's first
+	 * write, giving the remote prepare time to finish off the critical path.
+	 */
+	return idc_send_msg(&msg, IDC_NON_BLOCKING);


+	 * a compress stream the START IPC also arrives only after the app's first
+	 * write, giving the remote prepare time to finish off the critical path.
+	 */
+	return idc_send_msg(&msg, IDC_NON_BLOCKING);


lgirdwood · 2026-11-03T11:56:54Z

+ * Placed in coherent (SHARED_DATA) memory and shared across cores. The struct
+ * is only a byte array, so without an explicit alignment the linker may drop
+ * it (and every per-core slot) on an odd address. Handlers that read the
+ * payload as wider types -- e.g. the cross-core get_attribute path reads a
+ * uint32 'type' field -- then take an EXCCAUSE 9 (load/store alignment) fault.
+ * Align to the cache line, which is the size this payload is already built
+ * around (IDC_MAX_PAYLOAD_SIZE == DCACHE_LINE_SIZE * 2).


Minimize this to just the 1st sentence in the comment.

lgirdwood · 2026-11-03T12:06:23Z

+	/*
+	 * Let the module fill a local (cacheable) copy, then publish it to the
+	 * caller's coherent buffer with aligned 32-bit stores. Passing the
+	 * coherent buffer straight to the module makes it memcpy() a whole
+	 * struct into the uncached alias, which faults (EXCCAUSE 9) on Xtensa.
+	 * Only COMP_ATTR_BASE_CONFIG is supported remotely (enforced by the
+	 * sender), so local_cfg is always the right size.
+	 */
+	ret = comp_get_attribute(ipc_dev->cd, get_attr_payload->type, &local_cfg);
+	if (ret < 0)
+		return ret;
+
+	ipc4_coherent_copy32(get_attr_payload->value, &local_cfg, sizeof(local_cfg));
+
+	return ret;


I dont think this is needed, maybe a side effect of an earlier bug, lets re-test with this reverted.

lgirdwood · 2026-11-03T12:07:28Z

+	/*
+	 * Non-blocking: a remote component's prepare may be expensive (e.g. a DP
+	 * decoder's one-time codec open building large trig tables -- measured
+	 * ~580 ms for mp3 on a secondary core). Waiting for it here stalls the
+	 * initiator core's IPC/EDF thread for that whole time, and with the
+	 * default CONFIG_IDC_TIMEOUT_US the k_p4wq_wait() would instead time out
+	 * mid-prepare (-EAGAIN) and race the pipeline teardown into a crash.
+	 *
+	 * Ordering is preserved without the wait: each core drains its IDC work
+	 * through a single in-order p4wq worker, so this prepare completes before
+	 * the subsequent cross-core trigger (START) that is queued behind it. For
+	 * a compress stream the START IPC also arrives only after the app's first
+	 * write, giving the remote prepare time to finish off the critical path.
+	 */
+	return idc_send_msg(&msg, IDC_NON_BLOCKING);


Is this still needed after optimization of mp3 init ? lets revert and retest.

lgirdwood · 2026-11-03T12:09:20Z

 			/* message was sent and executed successfully, get status code */
 			ret = idc_msg_status_get(msg->core);
 		break;
+	case IDC_BLOCKING_NO_TIMEOUT:


if needed this patch should probably be squashed with teh previous patch.

struct idc_payload lives in coherent (SHARED_DATA) memory and is shared across cores, but it is only a byte array with no explicit alignment, so the linker was free to place it (and every per-core slot) on an odd address. Handlers that read the payload as a wider type -- e.g. the cross-core get_attribute path, which reads a uint32 type field -- then take an EXCCAUSE 9 (load/store alignment) fault when a component runs on a secondary core (ptl/ace30). Align the payload to the cache line. That is the granularity the payload is already built around (IDC_MAX_PAYLOAD_SIZE == DCACHE_LINE_SIZE * 2) and it guarantees every per-core slot starts word-aligned. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>

lgirdwood · 2026-11-03T14:01:24Z

Updated and retested on PTL hardware (UP Xtreme PTL / ACE 3.0, built with LLVM + HiFi and ffmpeg_dec pinned to secondary Core 1):

zephyr/include/rtos/idc.h: Shortened the comment above struct idc_payload to the first sentence as requested.
src/idc/idc.c (idc_Copy32): Dropped — confirmed on PTL hardware that cache-line aligning struct idc_payload is sufficient and memcpy_s() works cleanly across cores.
src/include/sof/audio/component_ext.h (non-blocking comp_prepare_remote): Dropped — with avcodec_open2() deferred to the DP thread on the first process() cycle, ffmpeg_dec_prepare() on Core 1 completes synchronously in ~0.35 ms (well within IDC_TIMEOUT).
src/idc/zephyr_idc.c (blocking comp_reset_remote): Dropped — verified full stream playback, drain EOS, comp_reset_remote(), MOD_UNBIND, and DELETE_PIPELINE complete cleanly without blocking reset.

kv2019i

Ok. A bit puzzled by the described test sequence as this specific sequence is currently not working with DP modules assigned to non-zero cores unless you disable the os_linux_overlay.conf. But not really related to this specific PR.

kv2019i · 2026-11-03T14:17:43Z

 struct idc_payload {
 	uint8_t data[IDC_MAX_PAYLOAD_SIZE];
-};
+} __aligned(DCACHE_LINE_SIZE);


It's a bit funky to have this in in the type definition, but seems good otherwise.

This is still WIP btw, using latest clang with hifi.

Clang linker would optimize on a byte boundary for size/perf reasons. We will get this on the userspace tests and linux configs next.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 13:06

lgirdwood requested review from abonislawski, dbaluta, iuliana-prodan, kv2019i, lbetlej, lyakh, mmaka1 and plbossart as code owners October 22, 2026 13:06

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Copilot stopped reviewing on behalf of lgirdwood due to an error October 22, 2026 13:34
The job was not acquired by a runner even after multiple attempts.

lgirdwood commented Oct 22, 2026

View reviewed changes

lgirdwood marked this pull request as draft October 22, 2026 13:36

lgirdwood force-pushed the topic/idc-remote-fixes branch from 035cf09 to 7411f40 Compare October 22, 2026 14:09

lgirdwood requested a balanced review from Copilot November 2, 2026 17:18

Copilot started reviewing on behalf of lgirdwood November 2, 2026 17:18

View session

Copilot AI reviewed Nov 2, 2026

View reviewed changes

lgirdwood commented Nov 3, 2026

View reviewed changes

lgirdwood force-pushed the topic/idc-remote-fixes branch from 7411f40 to 141d715 Compare November 3, 2026 14:01

lgirdwood changed the title ~~idc: fix cross-core component attribute access, prepare, and reset~~ zephyr: idc: align the IDC payload to the cache line Nov 3, 2026

kv2019i approved these changes Nov 3, 2026

View reviewed changes

lgirdwood marked this pull request as ready for review November 3, 2026 16:20

Conversation

lgirdwood commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

intel-sofci commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR 11334: test results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Nov 3, 2026

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lgirdwood commented Oct 22, 2026 •

edited

Loading

intel-sofci commented Oct 22, 2026 •

edited

Loading