[Draft] Fix DP userspace crashes and memory-allocation failures for compressed playback on PTL by ujfalusi · Pull Request #11181 · thesofproject/sof

ujfalusi · 2026-09-09T08:51:25Z

This series fixes a chain of crashes and allocation failures hit when running compressed-audio decode (Cadence codecs: MP3/AAC/FLAC/PCM) as a DP module under CONFIG_SOF_USERSPACE_APPLICATION. Each fix was isolated and tested individually against hardware; they eliminate every crash/allocation failure seen from pipeline setup through steady-state playback and teardown.

Highlights:

Cross-core cache-coherency bug in vregion_create(): the vregion metadata struct was allocated as plain cached memory even though it's written on the DP module's core and read from other cores (e.g. IPC-core buffer_new()), leading to an intermittent "0 bytes free" allocation failure when the reading core saw stale/zeroed data. Fixed by allocating it as coherent memory — the root cause behind most of the flakiness chased in this series.
Several DP-module-specific bugs: native-module LLEXT domain handling, ext_data not surviving the DP userspace thread boundary, a non-const API table crossing a memory-domain boundary, buffer allocations bypassing the module's vregion, an IPC teardown race, a proc-domain check missing on module free, and a buffer lifetime/vregion refcounting gap.
Topology: bumped the decoder's heap_bytes_requirement (in two steps, ending at 196608 bytes) to cover Cadence's persist/scratch/input/output tables plus the DP-boundary ring buffers for all four codecs — FLAC/PCM need considerably more than MP3/AAC.
Also includes an upstream cherry-pick moving End-of-Stream tracking from the pipeline to the component, since DP components can't access pipeline objects directly.
Verified on hardware: MP3, AAC, FLAC, and PCM compressed playback all work end-to-end (start/pause/reset/teardown) with no crashes.

kv2019i · 2026-09-09T08:56:13Z

Most seem valid fixes. The IPC empty list remove was just merged to main #11157

lyakh · 2026-09-09T09:39:45Z

 	unsigned int i;

+	if (!ctx || !ctx->n_mod)
+		return -ENOENT;


all three of these changes are needed? With the below 2 changes, this one might not be needed any more?

only the ctx->n_mod check is needed

lyakh · 2026-09-09T09:46:05Z

+		 * The pointers it contains reference the IPC mailbox, which stays
+		 * valid and is covered by the DP thread's SOF_DP_PART_CFG partition.
+		 */
+		struct module_ext_init_data ext_data;


members here are called after IPCs where they are used, so maybe init_instance would be a better name for this one

OK, renaming it

lyakh · 2026-09-09T09:46:47Z

+		/*
+		 * SOF_IPC4_MOD_INIT_INSTANCE: a by-value copy of the caller's
+		 * ext_data. The original lives on the calling thread's stack,
+		 * which the DP thread's memory domain doesn't grant access to.


"doesn't have access to?"

lyakh · 2026-09-09T09:53:38Z

+		 * accessible memory; the original caller-stack copy is out of reach here.
+		 */
+		pmod->priv.cfg.ext_data = &flat->ext_data;
 		flat->ret = ops->init(pmod);


ah, that's because the only tested until now module in DP mode was SRC and it doesn't access .ext_data in its struct module_interface instance

lyakh · 2026-09-09T09:59:37Z

+	/* Allocate buffer memory for module. sof_ctx_alloc() routes through the
+	 * module's vregion when one is configured (e.g. DP/userspace modules),
+	 * falling back to sof_heap_alloc()/rballoc() otherwise - same as
+	 * mod_alloc_ext(). Calling sof_heap_alloc() directly here would bypass


AI comments are often way too detailed. You don't need to describe all wrong possibility, so no need to mention why sof_heap_alloc() is wrong here

yes, very true.

lyakh · 2026-09-09T10:07:46Z


 #if CONFIG_SOF_USERSPACE_APPLICATION
-	if (dev->task)
+	if (dev->task && dev->ipc_config.proc_domain == COMP_PROCESSING_DOMAIN_DP)


only some very special LL scheduled components have their dev->task pointers set. IIRC only those running on a different core, which we shouldn't have. Could you check where this .task comes from?

Let me quote the detective I have hired for this:

Confirmed — dev->task for an LL component is only ever allocated in idc_prepare() (src/idc/idc.c:186-189), explicitly guarded by "we're running LL on different core, so allocate our own task". So a non-NULL dev->task on an LL component is a direct signal that component is being IPC-controlled cross-core via IDC, exactly matching what you described.

This does happen in this topology [1]: pipe1 has an LL task explicitly scheduled on core 1 (ll_schedule.zephyr_ll_task_schedule_common: ... [core 1]), separate from the IPC/pipeline-connect logic on core 0 — a genuine cross-core LL component, not a constructed edge case. That's the component that was incorrectly routed into the DP-only teardown path before this fix.

[1] sof-sdca-amp-compr.tplg, sof-sdca-jack-compr.tplg

@ujfalusi if that is indeed the case, I very much doubt that any of those topologies currently work. As far as I am concerned, cross-core LL linking was hardly ever intensively tested and has long been mostly forgotten and IMHO should rather never be used - if possible. If you manage to get any of those pipelines running and confirm at run-time that one of the components runs on a different core than the rest of the pipeline - we can take this change. But if it's already broken, I'm not sure fixing one location helps it at all, if anything it creates a (false) impression that it should work.

lyakh · 2026-09-09T10:11:17Z

+		 * (e.g. a neighbour component's list still references it). Matches
+		 * the vregion_put() in comp_buffer_free().
+		 */
+		if (alloc && alloc->vreg)


no need to check alloc->vreg, NULL is allowed.

lyakh · 2026-09-09T10:17:40Z

+		 * the vregion_put() in comp_buffer_free().
+		 */
+		if (alloc && alloc->vreg)
+			vregion_get(alloc->vreg);


I think this isn't quite correct. With this ipc4_comp_connect() would refcount the buffer twice. Can you check on which path this one is called? I don't see any path leading here, where refcount isn't taken yet.

right, agreed, dropping this patch

lyakh · 2026-09-09T10:20:22Z

+	 * so it must be coherent - plain cached memory left writes from the
+	 * creating core invisible to other cores without an explicit flush.
+	 */
+	vr = rmalloc(SOF_MEM_FLAG_KERNEL | SOF_MEM_FLAG_COHERENT, sizeof(*vr));


until now all accesses to vregion metadata were taking place on the same core. Do you have a DP module on a core different, than the rest of the pipeline?

it looks like we do — the decoder/encoder consistently ends up on core 1 while the rest of the pipeline (and the IPC thread connecting buffers to it) runs on core 0.

hm, @jsarha was saying that it didn't work for him IIRC?

lyakh · 2026-09-09T10:21:07Z

 	# allocates from dp->mod->priv.resources.alloc), so it must cover the
 	# codec's own memory tables plus every connected buffer, not just the codec.
-	heap_bytes_requirement		131072
+	heap_bytes_requirement		196608


should be squashed with an earlier one

ujfalusi · 2026-09-09T12:18:16Z

Changes since v1 (addressing review feedback):

Dropped 2 patches that turned out to be unnecessary:
- ipc-helper.c (buffer_new vregion_get) — was double-counting the vregion refcount; ipc4_comp_connect()/comp_buffer_free() already correctly balance get/put.
- ipc-common.c (tolerate freeing a never-queued message) — duplicates Kai's already-merged fix (4a5e37d).
llext_manager.c: kept only the ctx->n_mod check in llext_manager_mod_find(), dropped the redundant !ctx check (guarded elsewhere by the caller's lib_id == 0 early-returns).
zephyr_dp_schedule_application.c: renamed the ext_data union member to init_instance to match the naming convention of its siblings (bind, pipeline_state); fixed a comment typo.
generic.c: trimmed an overly verbose comment on the mod_balloc_align() fix; fixed indentation.
Squashed the two topology heap-bump commits into one (heap_bytes_requirement 131072 → 196608, verified against FLAC/PCM, not just MP3/AAC) and moved it to the top of the series.

Net: 11 commits → 8. All four codecs (MP3, AAC, FLAC, PCM) re-verified working on hardware after these changes.

The generic widget-common.conf defaults (24576 bytes heap, 8192 bytes stack) are sized for the general case, but Cadence codecs running as DP modules under CONFIG_SOF_USERSPACE_APPLICATION need more of both: - The codec's own memory tables (persist/scratch/input/output) can exceed the default heap budget on their own. On top of that, every buffer connecting to a DP module is also carved out of that same module's vregion (ipc4_comp_connect() allocates from dp->mod->priv.resources.alloc), so the requirement has to cover the codec's memory tables plus all of its connected buffers, not just the codec. - Codec init (e.g. xa_aac_dec, xa_mp3_enc) can need more stack than the default when run inside the DP userspace thread. Bump stack_bytes_requirement to 16384 for both decoder and encoder widgets. Bump heap_bytes_requirement to 196608 for the decoder: AAC and MP3 fit within 131072, but FLAC's own persist/scratch/input/output tables plus its DP-boundary ring buffers measure ~152KB, so 131072 isn't enough once FLAC or PCM are exercised. Encoder is left at 131072, unverified against a codec that needs more. Verified on hardware: AAC, MP3, FLAC and PCM decode all work. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

llext_manager_add_domain()/llext_manager_rm_domain() crashed (llext_manager_mod_find() dereferencing an invalid ctx) when called for a native (non-LLEXT) module scheduled on the DP+userspace-application path. lib_id == 0 is reserved and never populated by a loaded library, but was not guarded against. Skip the LLEXT domain add/remove entirely when LIB_MANAGER_GET_LIB_ID(module_id) == 0. llext_manager_mod_find() also needs its own guard against ctx->n_mod == 0: that's reachable for a genuine (non-NULL) library context, e.g. before its module segments are registered, and without the check the i == 0 case in the loop below reads ctx->mod[-1], out of bounds. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

cfg->ext_data points to a stack variable in module_adapter_new_ext(), valid only synchronously - but SOF_IPC4_MOD_INIT_INSTANCE runs asynchronously in the DP userspace thread, so ops->init() ended up reading a stale pointer into a different thread's stack (privilege violation, not a simple NULL deref). Add ext_data to union scheduler_dp_thread_ipc_param, flatten it by value in ipc_thread_flatten() (copying it while still on the calling thread, before the pointer goes stale), and repoint pmod->priv.cfg.ext_data at the flattened copy in ipc_thread_unflatten_run() before calling ops->init(), clearing it again afterwards to match the existing "not valid outside init()" convention. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

cadence_api_table[] is a compile-time-constant lookup table (codec id -> function pointer), populated once at link time and never written to at runtime, but declared without const - placing it in .data. Zephyr's Xtensa MMU marks .text/.rodata XTENSA_MMU_MAP_SHARED (globally accessible from every memory domain) but not .data, so the DP userspace thread's restricted domain couldn't reach it, causing a privilege-violation crash on the very first codec dispatch. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

z_impl_mod_balloc_align() called sof_heap_alloc(res->alloc->heap, ...) directly, bypassing the module's vregion entirely - unlike mod_alloc_ext(), which correctly checks vreg first via sof_ctx_alloc(). For DP/vregion-scoped modules res->alloc->heap is NULL, so this silently fell through to the global heap/rballoc(), handing back memory outside the DP thread's memory domain. This is what made cd->self (a codec's own library object, allocated via mod_balloc()) unreachable from inside the DP userspace thread. Route through sof_ctx_alloc(res->alloc, ...) like mod_alloc_ext() already does. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

DP components cannot access pipeline objects. Move the End Of Stream flag to the component type. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

struct vregion is created on the DP module's own core (via vregion_create(), invoked from module_adapter_dp_heap_new()) but is read and written from other cores too - e.g. buffer_new() on the IPC core dereferences vr->lifetime.{base,size,used,ptr} for every buffer connecting to that DP module. The struct was allocated with plain rmalloc(0, ...), i.e. ordinary cached memory. Writes made by the creating core can stay in that core's D-cache indefinitely, invisible to any other core reading the same physical address, since nothing ever flushes them out. In practice this showed up as buffer_new() on the IPC core intermittently seeing an all-zero vregion (base/size/used all 0), so any allocation failed as "lifetime alloc failed ... free 0" even though the vregion had over 100KB genuinely free - the metadata core 0 read simply hadn't propagated from core 1's cache yet. Allocate the vregion metadata with SOF_MEM_FLAG_KERNEL | SOF_MEM_FLAG_COHERENT instead, matching the existing pattern used for other cross-core shared structures in this codebase (e.g. vpage.c, regions_mm.c). This returns an uncached alias, so every core sees writes immediately without needing explicit cache maintenance. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

ujfalusi · 2026-09-09T13:36:49Z

Changes since v2:

drop 'module_adapter: free: only dispatch DP delete-instance IPC for DP modules' patch, it is no longer needed or it was not needed at all.

lyakh reviewed Sep 9, 2026

View reviewed changes

ujfalusi force-pushed the peter/pr/compress-ptl-os_linux-gotcha branch from ae006ba to 6031e32 Compare September 9, 2026 12:16

ujfalusi force-pushed the peter/pr/compress-ptl-os_linux-gotcha branch from 6031e32 to 73c24d7 Compare September 9, 2026 13:35

ujfalusi and others added 7 commits September 9, 2026 16:35

pipeline: move End Of Stream to component

f22e91b

DP components cannot access pipeline objects. Move the End Of Stream flag to the component type. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

Uh oh!

Conversation

ujfalusi commented Sep 9, 2026

Uh oh!

kv2019i commented Sep 9, 2026

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ujfalusi commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ujfalusi commented Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ujfalusi commented Sep 9, 2026 •

edited

Loading