Memory attributes to user space dp module by jsarha · Pull Request #10603 · thesofproject/sof

jsarha · 2026-03-05T20:13:03Z

Pass current DP memory attributes to the user-space DP thread.

This implementation does not do anything before we have thesofproject/linux#5537 merged (and before that we should merge #10544).

So quite a few conditions before CI can get any hold of this. I'll mark this as draft for now, even if the implementation is mostly complete. FYI @lyakh , @kv2019i , @lgirdwood.

wjablon1 · 2026-03-06T11:49:21Z

 			return NULL;
 	}
 #endif
+	const struct module_ext_init_data *ext_init =


Minor: You could probably pass only dp_data to module_adapter_mem_alloc instead of the entire structure ext_data.
Since the ext_data is 0-initialized, the dp_data pointer is already set to NULL so you probably wouldn't need the "#if":

dp_data is defined only in ipc4 headers, so I think its easiest to keep it behind #if , and keep the module_adapter_mem_alloc() arguments the way they are.

lgirdwood · 2026-03-30T17:20:02Z

@jsarha ping, any update ?

jsarha · 2026-03-31T21:04:57Z

@jsarha ping, any update ?

@lgirdwood this PR does not do anything and it can not be tested before we merge thesofproject/linux#5537 . So I rather keep this as draft until then.

jsarha · 2026-04-15T21:10:51Z

Rebased and tested against rebased thesofproject/linux#5537 . Ready for review.

Copilot

Pull request overview

This PR propagates IPC4 DP memory requirements (stack and heap sizing) into the SOF userspace DP module initialization path, so the DP thread and per-module heap can be sized based on dp_data from the extended init payload.

Changes:

Allow DP task stack size to be taken from IPC4 dp_data->stack_bytes instead of a fixed default.
Allow the DP module heap allocation size to be derived from IPC4 dp_data interim/lifetime heap requirements.
Thread the decoded extended-init (ext_init) info into the module adapter memory allocation path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`src/audio/pipeline/pipeline-schedule.c`	Uses IPC4 `dp_data->stack_bytes` to size the DP task stack at creation time.
`src/audio/module_adapter/module_adapter.c`	Uses IPC4 `dp_data` to size the per-module DP heap allocation and passes ext-init data into allocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-15T21:14:57Z

+	    mod->priv.cfg.ext_data->dp_data->stack_bytes > 0)
+		stack_size = mod->priv.cfg.ext_data->dp_data->stack_bytes;
+


DP task stack sizing should enforce a minimum (and potentially alignment) rather than blindly accepting any positive dp_data->stack_bytes. If the IPC value is smaller than TASK_DP_STACK_SIZE (or smaller than what Zephyr requires), this can create an undersized stack and lead to runtime crashes; consider clamping with max(TASK_DP_STACK_SIZE, requested) (and/or rejecting too-small values).

Suggested change

mod->priv.cfg.ext_data->dp_data->stack_bytes > 0)

stack_size = mod->priv.cfg.ext_data->dp_data->stack_bytes;

mod->priv.cfg.ext_data->dp_data->stack_bytes > 0) {

size_t requested_stack_size = mod->priv.cfg.ext_data->dp_data->stack_bytes;

if (requested_stack_size > TASK_DP_STACK_SIZE)

stack_size = requested_stack_size;

}

Ack, I think a minimum would be good.

lyakh · 2026-04-16T06:18:21Z

-static struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv,
-							  const struct comp_ipc_config *config)
+static struct processing_module *
+module_adapter_mem_alloc(const struct comp_driver *drv,


is this needed? Would the added third argument otherwise exceed 100 characters?

If I put "static struct processing_module * module_adapter_mem_alloc(" on the same line and, arguments on top of each other, and align them to ( , then "const struct module_ext_init_data *ext_init" goes over 100 characters. I'll put "static" on separate line, that should be enough.

kv2019i

A few notes inline, please check.

kv2019i · 2026-04-16T09:59:24Z

+	    mod->priv.cfg.ext_data->dp_data->stack_bytes > 0)
+		stack_size = mod->priv.cfg.ext_data->dp_data->stack_bytes;
+


Ack, I think a minimum would be good.

jsarha · 2026-04-20T18:57:50Z

@kv2019i I addressed your comment here.

lgirdwood · 2026-04-21T16:16:31Z

 	/* src-lite with 8 channels has been seen allocating 14k in one go */
 	/* FIXME: the size will be derived from configuration */
-	const size_t buf_size = 20 * 1024;
+	size_t buf_size = 20 * 1024;


can we kconfig this atm.

lgirdwood · 2026-04-21T16:18:11Z


+	if (mod->priv.cfg.ext_data && mod->priv.cfg.ext_data->dp_data &&
+	    mod->priv.cfg.ext_data->dp_data->stack_bytes > 0)
+		stack_size = MAX(mod->priv.cfg.ext_data->dp_data->stack_bytes, 2048);


I would Kconfig and log this size too.

kv2019i

One more q inline, please check.

lyakh · 2026-04-29T06:38:21Z

+	LOG_INF("Allocating %zu byte heap (default %zu) for %#x", buf_size,
+		CONFIG_SOF_USERSPACE_DP_DEFAULT_HEAP_SIZE, config->id);
 	/* Keep uncached to match the default SOF heap! */
+	/* Note that 'align' argumet is ignored by vmh backend */


well, kind of... I see an assertion there. And maybe the idea is, that "nobody" would ever want to have alignment > size. As long as alignment <= size, the VMH will obey that implicitly because allocations are done in (respectively aligned) buckets. But a larger alignment will fail. But I agree, that that assumption doesn't look perfectly reasonable. I might well need to allocate 100 bytes at a page boundary.

The comment was there to explain why I simply won't put buf_size = ALIGN_UP(buf_size, PAGE_SZ) here, instead of making sure the buffer can at least fit the heap prefix.

But now that you mentioned the assert() (I new it was there, but forgot about it), I think I should remove that alignment requirement, since its likely that the asset() fires if a heap smaller than a page is requested.

lyakh · 2026-04-29T06:39:26Z

 	 *       pointers to IPC payload mailbox, so its only valid in
-	 *       functions that called from this function. This why
-	 *       the pointer is set NULL before this function exits.
+	 *       functions that called from this function. This is why


while at it you could also fix "that are called..."

lyakh · 2026-04-30T06:20:31Z

 	/* Keep uncached to match the default SOF heap! */
-	uint8_t *mod_heap_mem = rballoc_align(SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT,
-					      buf_size, PAGE_SZ);
+	uint8_t *mod_heap_mem = rballoc(SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, buf_size);


good, that this is removed by 9d97d23 in #10702

jsarha · 2026-05-08T06:03:26Z

Rebased. Now that vregions are in and they actually obey the memory parameters coming in IPC, there are also some non DP repated topology updates that are needed, before wa can merge thesofproject/linux#5537 . This PR contains at least part of those if not maybe all.

lyakh · 2026-05-08T07:18:01Z

+	if (lifetime_size == 0)
+		lifetime_size = 1;
+	if (buf_size <= lifetime_size)
+		buf_size = lifetime_size + 1;


sorry, not sure I understand. Requesting 1 byte doesn't seem very helpful to me. Do you have any use-cases in mind where this would be useful? This function is only called for valid userspace ("application" style) DP modules. Maybe make both 1 page large at least. This will fail for any reasonable module, but at least it makes some semantic sense. Or use some reasonable defaults like the current 7 pages total or fail this function completely.

Argh... I thought that I fixed that. This is untouched claude code.

lyakh · 2026-05-08T07:21:48Z

+	if (mod->priv.cfg.ext_data && mod->priv.cfg.ext_data->dp_data &&
+	    mod->priv.cfg.ext_data->dp_data->stack_bytes > 0) {
+		stack_size = MAX(mod->priv.cfg.ext_data->dp_data->stack_bytes,
+				 CONFIG_ZEPHYR_DP_SCHEDULER_MIN_STACK_SIZE);


this doesn't seem very logical to me. The default is TASK_DP_STACK_SIZE (which is CONFIG_SOF_STACK_SIZE). But if stack_bytes > 0 then the stack size is the maximum of that value and CONFIG_ZEPHYR_DP_SCHEDULER_MIN_STACK_SIZE. Maybe change the default to CONFIG_ZEPHYR_DP_SCHEDULER_MIN_STACK_SIZE too?

I can of course change this, but to me it looks quite logical. If the stack size is not set, e.g. dp_data->stack_bytes = 0, use the default which is TASK_DP_STACK_SIZE. Then if somebody is deliberately trying to cause some specific crash by sending a ridiculously small stack size, then that is prevented by forcing the minimum to 512.

@lgirdwood what did you mean exactly by this comment: #10603 (comment) ?

ps. Sorry, something wrong with github webpage today. It does not show all the comments in the commits section, and then its impossible to comment to them in review mode ̣.

I mean we have a Kconfig for any hard coded stack sizes or memory sizes that today we assume to be correct.

lyakh · 2026-05-08T07:26:22Z

+						domain_id 0
+						stack_bytes_requirement		2048
+						interim_heap_bytes_requirement	20480
+						lifetime_heap_bytes_requirement	4096


Let's keep the same values as set by 260ce13

Isn't it setting the size to 28+4=32k now, while the above commit was setting it to 28k total? Is this on purpose or an oversight?

@jsarha can you check ^^

Yes, this is on purpose. As you @lyakh suggested, I allocated a full page of lifetime memory, despite SRC only allocating sizeof(struct comp_data) from it. The significant amount of memory is allocated from interim memory, so if we want to add something on top of 24kb of memory for safety, this is how it looks like.

Now that I think again how the vregions work, e.g. everything in the init phase is allocated from lifetime memory, and then after the init all allocations go to interim heap. Can't we make vregions work in such a way, that once the first alloc from interim memory comes, we use what ever is left of the lifetime memory to create the interim heap? All lifetime allocations after that will either fail or be directed to interim heap. How does this sound @lgirdwood , @lyakh ?

@jsarha to 24+4 / 28+4 k: did you see any tests failing with the 24+4 allocation? Originally my local tests were passing with 20+4. So I think 20+4 is actually enough to pass our tests. But I went one page up "for safety" and did it 24+4. Now you're adding yet another page. Did you see any tests failing with 24+4?

To dynamic interim: in principle this can be done. You would basically postpone Zephyr struct k_heap initialisation until you get the first "interim" allocation request. Before doing that I'd do a simple test - add a print to that first interim allocation to show how much lifetime RAM has been allocated by then. And then also print any lifetime allocations after that to see how much of that we would be allocating as interim instead. So, it's a possible optimisation, but I'd first collect a bit of information about how allocations are currently done

Yes. That is what I intend to do, and am actually doing ATM. But for the time being I think its better to put this PR and thesofproject/linux#5537 to hold. We do not want to go too much back and forth with the IPC and the topology properties, especially on the Linux side.

Hmmm, actually this PR does not matter. The lifetime/interrim/shared properties are there in the topology and IPC FW side already.

Yes. That is what I intend to do, and am actually doing ATM.

@jsarha but why? Every change should have an explanation. What is wrong with the current values?

kv2019i

One comment/question from Guennadi in last commit, but otherwise series seems good to me and my concerns addressed.

kv2019i · 2026-05-11T10:26:40Z

+						domain_id 0
+						stack_bytes_requirement		2048
+						interim_heap_bytes_requirement	20480
+						lifetime_heap_bytes_requirement	4096


@jsarha can you check ^^

lgirdwood · 2026-05-13T12:23:06Z

@jsarha can you check CI and rebase. Thanks !

softwarecki · 2026-05-14T13:23:14Z

+	if (config->ipc_extended_init && ext_init && ext_init->dp_data &&
+	    (ext_init->dp_data->lifetime_heap_bytes > 0 ||
+	     ext_init->dp_data->interim_heap_bytes > 0)) {
+		if (ext_init->dp_data->lifetime_heap_bytes > 64*1024*1024 ||


softwarecki · 2026-05-14T13:35:20Z


+config ZEPHYR_DP_SCHEDULER_MIN_STACK_SIZE
+       int "Minimum stack size for DP processing thread"
+       default 512


What is source of this value? Can stack be smaller than the page size?

@softwarecki , That is just from my head when addressing this:
#10603 (comment)
and this
#10603 (comment)

Would you like to change this safetylimit value? Perhaps increase it?

softwarecki · 2026-05-14T13:39:02Z

-	/* FIXME: the size will be derived from configuration */
-	const size_t buf_size = 28 * 1024;
+	size_t buf_size = CONFIG_SOF_USERSPACE_DP_DEFAULT_HEAP_SIZE;
+	size_t lifetime_size = 4096;


Can we move this value to Kconfig instead of hardcoding it? Is this value related to memory page size?

The separate lifetime heap size is not needed anymore now that #10863 is merged.

lgirdwood · 2026-06-19T15:39:54Z

@jsarha ping

jsarha

A new rebased version after merging #10863 will be pushed soon.

jsarha · 2026-06-22T19:43:40Z

-	/* FIXME: the size will be derived from configuration */
-	const size_t buf_size = 28 * 1024;
+	size_t buf_size = CONFIG_SOF_USERSPACE_DP_DEFAULT_HEAP_SIZE;
+	size_t lifetime_size = 4096;


The separate lifetime heap size is not needed anymore now that #10863 is merged.

jsarha · 2026-06-22T21:24:02Z

It seems our main branch does not produce a booting binary ATM, so I could not test this rebased version. That is why I added [DNM] tag in the headline.

Move pipeline_comp_dp_task_init() call after module private data initializations so that the struct module_config is available already at pipeline_comp_dp_task_init() init time. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Use IPC module init extended data (the dp_data) to determine DP module heap size when available. Add Kconfig option SOF_USERSPACE_DP_DEFAULT_HEAP_SIZE (default 20480) as fallback when extended init data is not present or does not provide heap sizes. Sanity-check the requested sizes (reject values above 64 MB) and log the allocated heap size. Also pass ext_init through module_adapter_mem_alloc() to module_adapter_dp_heap_new() and fix a minor comment typo. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

Use IPC module init extended data (dp_data stack_bytes) to set the DP processing thread stack size when available. Fall back to TASK_DP_STACK_SIZE when ext init data is not present or does not provide a stack size. Add Kconfig option ZEPHYR_DP_SCHEDULER_MIN_STACK_SIZE (default 2048) to enforce a minimum stack size regardless of what the IPC payload requests. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>

jsarha · 2026-06-23T09:43:03Z

Now this works after rebasing on a version from last week,

wjablon1 reviewed Mar 6, 2026

View reviewed changes

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from 43000f8 to 3d9b5c4 Compare April 15, 2026 21:00

jsarha marked this pull request as ready for review April 15, 2026 21:11

Copilot AI review requested due to automatic review settings April 15, 2026 21:11

jsarha requested review from dbaluta, kv2019i, lbetlej, lgirdwood, mmaka1, plbossart and ranj063 as code owners April 15, 2026 21:11

jsarha mentioned this pull request Apr 15, 2026

New memory domain logic thesofproject/linux#5537

Open

Copilot started reviewing on behalf of jsarha April 15, 2026 21:12 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

lyakh reviewed Apr 16, 2026

View reviewed changes

kv2019i requested changes Apr 16, 2026

View reviewed changes

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch 2 times, most recently from d2578b5 to 8b70073 Compare April 16, 2026 20:56

jsarha mentioned this pull request Apr 17, 2026

userspace: DP: application: switch to vregion #10702

Merged

lgirdwood reviewed Apr 21, 2026

View reviewed changes

kv2019i reviewed Apr 22, 2026

View reviewed changes

Comment thread src/audio/module_adapter/module_adapter.c Outdated

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from 8b70073 to 6bd9aad Compare April 28, 2026 18:46

jsarha requested review from abonislawski and iuliana-prodan as code owners April 28, 2026 18:46

lyakh reviewed Apr 29, 2026

View reviewed changes

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from 6bd9aad to 34a2c67 Compare April 29, 2026 15:57

lyakh reviewed Apr 30, 2026

View reviewed changes

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from 34a2c67 to a103d37 Compare May 7, 2026 21:20

jsarha requested review from LaurentiuM1234, marcinszkudlinski and pblaszko as code owners May 7, 2026 21:20

lyakh reviewed May 8, 2026

View reviewed changes

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from a103d37 to 4b50cac Compare May 8, 2026 12:10

kv2019i approved these changes May 11, 2026

View reviewed changes

jsarha requested a review from singalsu as a code owner May 12, 2026 15:21

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch 2 times, most recently from 77ba618 to 362854c Compare May 12, 2026 20:04

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from 362854c to 0bb25e7 Compare May 13, 2026 16:33

softwarecki reviewed May 14, 2026

View reviewed changes

jsarha mentioned this pull request May 28, 2026

[DNM] vregions phase II: Actually use the lifetime allocations and make the memory division between lifetime and interim dynamic #10783

Draft

jsarha commented Jun 22, 2026

View reviewed changes

jsarha changed the title ~~Memory attributes to user space dp module~~ [DNM] Memory attributes to user space dp module Jun 22, 2026

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from 0bb25e7 to 6f74d9b Compare June 22, 2026 21:22

jsarha added the DNM Do Not Merge tag label Jun 22, 2026

Jyri Sarha added 3 commits June 23, 2026 12:34

jsarha force-pushed the memory_attributes_to_user_space_dp_module branch from 6f74d9b to a8ccee9 Compare June 23, 2026 09:41

jsarha changed the title ~~[DNM] Memory attributes to user space dp module~~ Memory attributes to user space dp module Jun 23, 2026

jsarha removed the DNM Do Not Merge tag label Jun 23, 2026

		mod->priv.cfg.ext_data->dp_data->stack_bytes > 0)
		stack_size = mod->priv.cfg.ext_data->dp_data->stack_bytes;

Conversation

jsarha commented Mar 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsarha Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Mar 30, 2026

Uh oh!

jsarha commented Mar 31, 2026

Uh oh!

jsarha commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 15, 2026

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!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsarha commented Apr 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsarha Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsarha commented May 8, 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

jsarha Apr 16, 2026 •

edited

Loading

jsarha Apr 29, 2026 •

edited

Loading

jsarha May 12, 2026 •

edited

Loading

jsarha Jun 22, 2026 •

edited

Loading