idc: fix cross-core component attribute access, prepare, and reset

posix/include/rtos/idc.h

@@ -28,6 +28,9 @@
 /** \brief IDC send non-blocking flag. */
 #define IDC_NON_BLOCKING	1
 
+/** \brief IDC send blocking flag, wait for completion without timing out. */
+#define IDC_BLOCKING_NO_TIMEOUT	4
+
 /** \brief IDC send core power up flag. */
 #define IDC_POWER_UP		2
 

src/idc/idc.c

@@ -30,6 +30,7 @@
 #include <ipc/header.h>
 #include <ipc/stream.h>
 #include <ipc/topology.h>
+#include <ipc4/base-config.h>
 #include <errno.h>
 #include <stdint.h>
 #include <sof/lib/ams.h>
@@ -121,6 +122,8 @@ static int idc_get_attribute(uint32_t comp_id)
 	struct ipc_comp_dev *ipc_dev;
 	struct idc_payload *idc_payload;
 	struct get_attribute_remote_payload *get_attr_payload;
+	struct ipc4_base_module_cfg local_cfg = { 0 };
+	int ret;
 
 	ipc_dev = ipc_get_comp_by_id(ipc_get(), comp_id);
 	if (!ipc_dev)
@@ -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
+	 * 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;
 }
 
 /**

src/idc/zephyr_idc.c

@@ -157,7 +157,7 @@ int idc_send_msg(struct idc_msg *msg, uint32_t mode)
 	work->priority = CONFIG_EDF_THREAD_PRIORITY;
 	work->deadline = 0;
 	work->handler = idc_handler;
-	work->sync = mode == IDC_BLOCKING;
+	work->sync = mode == IDC_BLOCKING || mode == IDC_BLOCKING_NO_TIMEOUT;
 
 	if (msg->payload) {
 		idc_send_memcpy_err = memcpy_s(payload->data, sizeof(payload->data),
@@ -189,6 +189,13 @@ int idc_send_msg(struct idc_msg *msg, uint32_t mode)
 			/* message was sent and executed successfully, get status code */
 			ret = idc_msg_status_get(msg->core);
 		break;
+	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;
 	case IDC_POWER_UP:
 	case IDC_NON_BLOCKING:
 	default:

src/include/sof/audio/component_ext.h

@@ -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);
+	/*
+	 * 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);
 }
 
 /**
@@ -275,6 +289,29 @@ struct get_attribute_remote_payload {
 	void *value;   /**< Pointer to attribute value storage */
 };
 
+/*
+ * Copy to/from a coherent (uncached) buffer using aligned 32-bit word accesses.
+ *
+ * A whole-struct memcpy() straight into (or out of) the uncached alias faults
+ * on Xtensa with EXCCAUSE 9: the compiler lowers the struct copy to wide/HiFi
+ * accesses (e.g. AE_L32X2 / 64-bit loads) that the uncached window does not
+ * support. The IDC status word (a single aligned uint32_t) already round-trips
+ * through this same coherent region without faulting, so restricting the copy
+ * to volatile 32-bit word transactions is safe. @bytes must be a multiple of 4
+ * and both pointers 32-bit aligned (guaranteed here: ipc4_base_module_cfg is
+ * aligned(4) and rzalloc() returns >=4-byte-aligned memory).
+ */
+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];
+}
+
 /**
  * \brief Retrieve component attribute from a remote core (IPC4).
  * \param[in,out] dev Component device.
@@ -305,9 +342,12 @@ static inline int comp_ipc4_get_attribute_remote(struct comp_dev *dev, uint32_t
 
 	ret = idc_send_msg(&msg, IDC_BLOCKING);
 
+	/* base_cfg is coherent (uncached) -- read it back with 32-bit word
+	 * accesses, not a struct memcpy that would fault on the uncached alias.
+	 */
 	if (ret == 0)
-		memcpy_s(value, sizeof(struct ipc4_base_module_cfg),
-			 base_cfg, sizeof(struct ipc4_base_module_cfg));
+		ipc4_coherent_copy32(value, base_cfg,
+				     sizeof(struct ipc4_base_module_cfg));
 
 	rfree(base_cfg);
 	return ret;
@@ -365,7 +405,22 @@ static inline int comp_reset_remote(struct comp_dev *dev)
 	struct idc_msg msg = { IDC_MSG_RESET,
 		IDC_MSG_RESET_EXT(dev->ipc_config.id), dev->ipc_config.core, };
 
-	return idc_send_msg(&msg, IDC_BLOCKING);
+	/*
+	 * Blocking, and waiting for actual completion (no timeout): the caller
+	 * (pipeline_reset -> host) depends on the reset being finished before it
+	 * frees the component and powers down the remote core. A DP decoder's
+	 * reset runs schedule_task_cancel() -> k_thread_join() on that core, which
+	 * only returns once the DP thread finishes its in-flight process() call.
+	 * That call is bounded but can be long during codec cold-start (a deferred
+	 * avcodec_open2 table build, ~1.4 s for aac, or the first decode after it),
+	 * exceeding CONFIG_IDC_TIMEOUT_US. A plain IDC_BLOCKING would give up at
+	 * that timeout (-EAGAIN) while the reset is still running remotely, leaving
+	 * the component un-reset; pipeline_reset() then fails and the subsequent
+	 * free is refused (invalid state) / the core cannot be disabled, wedging
+	 * every teardown until reboot. A normal end-of-stream teardown joins an
+	 * idle DP thread and returns immediately, so this only waits when it must.
+	 */
+	return idc_send_msg(&msg, IDC_BLOCKING_NO_TIMEOUT);
 }
 
 /**

zephyr/include/rtos/idc.h

@@ -28,6 +28,15 @@
 /** \brief IDC send non-blocking flag. */
 #define IDC_NON_BLOCKING	1
 
+/** \brief IDC send blocking flag, wait for completion without timing out.
+ * Like IDC_BLOCKING but waits (K_FOREVER) for the handler to finish rather
+ * than only CONFIG_IDC_TIMEOUT_US. For remote ops whose completion the
+ * caller depends on and which are bounded but can exceed the IDC timeout
+ * (e.g. a DP component reset that joins a decoder thread mid codec
+ * cold-start).
+ */
+#define IDC_BLOCKING_NO_TIMEOUT	4
+
 /** \brief IDC send core power up flag. */
 #define IDC_POWER_UP		2
 
@@ -149,10 +158,20 @@
 /** \brief IDC free function flags */
 #define IDC_FREE_IRQ_ONLY	BIT(0)	/**< disable only irqs */
 
-/** \brief IDC message payload. */
+/**
+ * \brief IDC message payload.
+ *
+ * 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).
+ */
 struct idc_payload {
 	uint8_t data[IDC_MAX_PAYLOAD_SIZE];
-};
+} __aligned(DCACHE_LINE_SIZE);
 
 /** \brief IDC message. */
 struct idc_msg {