Code to decode pipeline create messages payload #10265

jsarha · 2025-09-26T11:52:33Z

Code to decode pipline create messages payload. E.g. this code is written to test this PR: thesofproject/linux#5537

lyakh · 2025-09-29T06:20:21Z

src/include/ipc4/pipeline.h

+	uint32_t domain_id;	/* userspace domain ID */
+	uint32_t stack_bytes;	/* required stack size in bytes, 0 means default size */
+	uint32_t heap_bytes;	/* required heap size in bytes, 0 means default size */
+} __packed __aligned(4);


on the one hand bytes are simple to understand and work with. OTOH if we use a larger unit (words, or cache lines, or pages), then we (1) can potentially use a smaller integer, (2) avoid the need to check and enforce alignment. No strong preference, just a thought

bytes fine here since these are uint32_t - we should have enough space.

lgirdwood

LGTM, we can followup with the allocations per pipeline or domain later.

lgirdwood · 2025-09-30T16:30:32Z

src/include/ipc4/pipeline.h

+	uint32_t domain_id;	/* userspace domain ID */
+	uint32_t stack_bytes;	/* required stack size in bytes, 0 means default size */
+	uint32_t heap_bytes;	/* required heap size in bytes, 0 means default size */
+} __packed __aligned(4);


bytes fine here since these are uint32_t - we should have enough space.

lyakh · 2025-10-01T07:36:10Z

src/ipc/ipc4/helper.c

+		char *data;
+
+		dcache_invalidate_region((__sparse_force void __sparse_cache *)MAILBOX_HOSTBOX_BASE,
+					 MAILBOX_HOSTBOX_SIZE);


this is currently 8KiB on PTL... if there's a reasonable maximum size of data here, would be good to use it

In case of pipeline create, that would mean two calls, first just the payload header and then the rest, as there was no space for payload size in the msg itself. But the piece of code is from module init message handling, and there the size of the payload is encoded in the msg extension bits and could be used in the cache invalidation call.

I can do the both and unify the code a bit... actually there would be be quite a bit of cleaning work to be done in the IPC cocde...

Now, after picking this up again, it makes me wonder what is the harm in invalidating the whole mailbox? The operation time is hardly proportional to the range of memory the cache is being invalidated? And if there are some saved cache lines still present form some earlier IPC message, wouldn't it actually be good to mark those invalid. They should be of no use to anyone anymore.

well, you'll have a loop over 8KiB taking 64 bytes at a time, which makes 128 iterations. We should expect, that most addresses won't have cache lines associated with them, so their invalidation hopefully should be a NOP. Cache operations are relatively expensive, especially writing back, but if most addresses have no cache lines to them, their inivalidation shouldn't take too long. So, as I wrote - if some reasonable maximum size is easily available, it would be good to use it, if not - we can use this to begin with,

If the whole mailbox is always invalidated, there should not be too many active lines there, only the amount left from previus IPC message. One could even think that it would be beneficial to free up these lines as they are certainly not of any use any more.

they will be freed automatically, the hardware shouldn't need our help for that :-)

lyakh · 2025-10-06T06:21:53Z

src/ipc/ipc4/helper.c

+	size_t size;
+
+	dcache_invalidate_region((__sparse_force void __sparse_cache *)MAILBOX_HOSTBOX_BASE,
+				 hdr_cache_size);


I think we're trying now to use sys_cache_data_invd_range() directly

lyakh · 2025-10-06T06:22:54Z

src/ipc/ipc4/helper.c

+		dcache_invalidate_region((__sparse_force void __sparse_cache *)
+					 ((char *)MAILBOX_HOSTBOX_BASE + hdr_cache_size),
+					 ALIGN_UP(size, CONFIG_DCACHE_LINE_SIZE) -
+					 hdr_cache_size);


it should be safe to just use whatever pointers and sizes you have, cache management functions adjust for cache lines internally

Yes, but to avoid invalidating the first cache line twice, and to avoid the second call all together if its not needed, I need the complex calculations.

lyakh · 2025-11-14T11:37:30Z

src/ipc/ipc4/helper.c

+		*((uint32_t *)hdr));
+
+	obj = (const struct ipc4_pipeline_ext_object *)(hdr + 1);
+	while (!last_object) {


for (obj = (const struct ipc4_pipeline_ext_object *)(hdr + 1); !last_object; obj = next_obj)

I like while() better, especially as its now a better match to module_ext_init_decode() implementation

lyakh · 2025-11-14T11:41:11Z

src/ipc/ipc4/helper.c

+
+		/* Check if there is space for the object header */
+		if ((char *)(obj + 1) - data > size) {
+			tr_err(&ipc_tr, "obj header overflow, %u > %u",


shouldn't the format be %zu for size here and below?

lyakh · 2025-11-14T11:42:31Z

src/ipc/ipc4/helper.c

+			break;
+		}
+		default:
+			tr_info(&ipc_tr, "Unknown ext init object id %u of %u words",


tr_warn() or tr_dbg()?

What do you have agains info 😄? If I must change this I'd go with warn, as if somebody is indeed reading the logs, it could be useful to know that the firmware is not able to use all info from the pipeline create payload.

supposedly we have all the different logging levels for a reason, so we might as well use them ;-) It's just that a message, beginning with "Unknown" but with a neutral status is a bit unclear to me. Is it bad or not? If it isn't bad - do I need to know about it? If not - maybe make it dbg(). If it does indeed indicate a potential problem, then a warn()

Add lifetime_bytes_requirement and shared_bytes_requirement widget attributes. This token's value indicates the amount of heap memory needed at component initialization phase. Signed-off-by: Jyri Sarha <[email protected]>

Adds structs and definitions for adding a payload to struct ipc4_pipeline_create message. The structure of the payload is very similar to struct ipc4_module_init_ext_init payload for struct ipc4_module_init_instance. Signed-off-by: Jyri Sarha <[email protected]>

Add ipc4_create_pipeline_payload_decode() and call it if struct ipc4_pipeline_create's extension.r.payload bit is set. The function decodes the message payload, logs the contents, but does not store the information anywhere. Signed-off-by: Jyri Sarha <[email protected]>

Invalidate cache for only the create pipeline payload size not whole mailbox. Signed-off-by: Jyri Sarha <[email protected]>

Invalidate cache for only the module init payload size not the whole mailbox. Signed-off-by: Jyri Sarha <[email protected]>

Add default values for stack, lifetime, and heap memory size for all widgets, e.g. module instances. These values were tested with vpages branch and all cases that I tried worked with them. Eventually each module should be added with more accurate values, based on measurements. Signed-off-by: Jyri Sarha <[email protected]>

Pass create pipeline payload parameters down to pipeline_new() by using struct pipeline_params. That is the place where the data will be used. Signed-off-by: Jyri Sarha <[email protected]>

jsarha · 2025-12-03T19:41:39Z

@lgirdwood this is now updated so that it should be almost exact match to #10281 , vpages code only needs to use the params passed to pipeline_new(). But to use this the topology should be built from this PR and you need my linux driver PR #10281 too to pass down the topology info.

jsarha mentioned this pull request Sep 26, 2025

New memory domain logic thesofproject/linux#5537

Open

lyakh reviewed Sep 29, 2025

View reviewed changes

lgirdwood reviewed Sep 30, 2025

View reviewed changes

jsarha force-pushed the pipeline_memory_attributes branch from ac3bf18 to 5c3c095 Compare September 30, 2025 20:59

lyakh reviewed Oct 1, 2025

View reviewed changes

lyakh changed the title ~~Code to decode pipline create messages payload~~ Code to decode pipeline create messages payload Oct 2, 2025

lyakh reviewed Oct 6, 2025

View reviewed changes

jsarha force-pushed the pipeline_memory_attributes branch from e43cb7b to 9f5d981 Compare October 7, 2025 21:37

jsarha force-pushed the pipeline_memory_attributes branch 2 times, most recently from 2dc8ac4 to 7e4bb9a Compare November 11, 2025 18:06

lyakh reviewed Nov 14, 2025

View reviewed changes

Jyri Sarha added 7 commits December 3, 2025 19:40

ipc4: helper: Do not invalidate cache of whole mailbox for pipe create

35f67ed

Invalidate cache for only the create pipeline payload size not whole mailbox. Signed-off-by: Jyri Sarha <[email protected]>

ipc4: helper: Do not invalidate cache of whole mailbox for module init

73710ca

Invalidate cache for only the module init payload size not the whole mailbox. Signed-off-by: Jyri Sarha <[email protected]>

pipeline: Pass create pipeline payload parameters down to pipeline_new()

d9de565

Pass create pipeline payload parameters down to pipeline_new() by using struct pipeline_params. That is the place where the data will be used. Signed-off-by: Jyri Sarha <[email protected]>

jsarha force-pushed the pipeline_memory_attributes branch from 7e4bb9a to d9de565 Compare December 3, 2025 19:17

Code to decode pipeline create messages payload #10265

Are you sure you want to change the base?

Code to decode pipeline create messages payload #10265

Uh oh!

Conversation

jsarha commented Sep 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lyakh Oct 1, 2025 • 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!

jsarha Oct 1, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

jsarha Dec 3, 2025 • 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!

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!

jsarha commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lyakh Oct 1, 2025 •

edited

Loading

jsarha Oct 1, 2025 •

edited

Loading

jsarha Dec 3, 2025 •

edited

Loading