From ea8b7cc24798dc123bb03f42b6c7bb79da48daa8 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Mon, 15 Dec 2025 00:28:43 +0200 Subject: [PATCH 1/9] audio: module_adapter: Use %zu in size_t prints in module_ext_init_decode() Use %zu in size_t prints in module_ext_init_decode(). Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module_adapter_ipc4.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/audio/module_adapter/module_adapter_ipc4.c b/src/audio/module_adapter/module_adapter_ipc4.c index 54ee3dd94f5e..e87474dad1db 100644 --- a/src/audio/module_adapter/module_adapter_ipc4.c +++ b/src/audio/module_adapter/module_adapter_ipc4.c @@ -35,7 +35,7 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, const struct ipc4_module_init_ext_object *obj; if (*size < sizeof(ext_init)) { - comp_err(dev, "Size too small for ext init %u < %u", + comp_err(dev, "Size too small for ext init %zu < %zu", *size, sizeof(ext_init)); return NULL; } @@ -47,7 +47,7 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, /* Check if there is space for the object header */ if ((unsigned char *)(obj + 1) - data > *size) { - comp_err(dev, "ext init obj overflow, %u > %u", + comp_err(dev, "ext init obj overflow, %u > %zu", (unsigned char *)(obj + 1) - data, *size); return NULL; } @@ -55,7 +55,7 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, next_obj = (const struct ipc4_module_init_ext_object *) (((uint32_t *) (obj + 1)) + obj->object_words); if ((unsigned char *)next_obj - data > *size) { - comp_err(dev, "ext init object array overflow, %u > %u", + comp_err(dev, "ext init object array overflow, %u > %zu", (unsigned char *)obj - data, *size); return NULL; } @@ -67,7 +67,7 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, (const struct ipc4_module_init_ext_obj_dp_data *)(obj + 1); if (obj->object_words * sizeof(uint32_t) < sizeof(*dp_data)) { - comp_err(dev, "dp_data object too small %u < %u", + comp_err(dev, "dp_data object too small %zu < %zu", obj->object_words * sizeof(uint32_t), sizeof(*dp_data)); return NULL; } From 49de8af17898b00b995349b40c2b9b1ed5f3ddaf Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Tue, 7 Oct 2025 23:57:24 +0300 Subject: [PATCH 2/9] topology2: Add lifetime and shared -bytes_requirement widget attributes 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 --- .../topology2/include/common/tokens.conf | 4 +++- .../include/components/widget-common.conf | 23 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/tools/topology/topology2/include/common/tokens.conf b/tools/topology/topology2/include/common/tokens.conf index d1b986ab01d4..7c4f31b319ab 100644 --- a/tools/topology/topology2/include/common/tokens.conf +++ b/tools/topology/topology2/include/common/tokens.conf @@ -28,7 +28,9 @@ Object.Base.VendorToken { scheduler_domain 418 domain_id 419 stack_bytes_requirement 420 - heap_bytes_requirement 421 + interim_heap_bytes_requirement 421 + lifetime_heap_bytes_requirement 422 + shared_bytes_requirement 423 } "2" { diff --git a/tools/topology/topology2/include/components/widget-common.conf b/tools/topology/topology2/include/components/widget-common.conf index 851e78ae1721..aed814f3be0d 100644 --- a/tools/topology/topology2/include/components/widget-common.conf +++ b/tools/topology/topology2/include/components/widget-common.conf @@ -149,8 +149,27 @@ DefineAttribute."stack_bytes_requirement" { token_ref "comp.word" } -## Heap size requirement in bytes for this component. Zero indicates default heap size. -DefineAttribute."heap_bytes_requirement" { +## Interim Heap size requirement in bytes for this component. +## Used for resources that may change in size or be freed during the lifetime of the component. +## In practice this means anything allocated outside module's init() call-back. +DefineAttribute."interim_heap_bytes_requirement" { + # Token set reference name and type + token_ref "comp.word" +} + +## Lifetime heap memory allocation requirement in bytes for this component. +## This token's value indicates the amount of allocated memory needed at component init phase +## and used over the lifetime of the component until the component is destroyed. +## In practice this means anything allocated in init() call-back. +DefineAttribute."lifetime_heap_bytes_requirement" { + # Token set reference name and type + token_ref "comp.word" +} + +## Shared memory allocation requirement in bytes for this component. +## This token's value indicates the amount of shared memory the component may need to allocated. +## Shared memory may be shared to other components. +DefineAttribute."shared_bytes_requirement" { # Token set reference name and type token_ref "comp.word" } From 65a57f04722ed58f6ae9b69c4fdd38742a3df5e2 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Tue, 23 Sep 2025 23:28:38 +0300 Subject: [PATCH 3/9] ipc4: Add payload structs definitions to ipc4_pipeline_create msg 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 --- src/include/ipc4/pipeline.h | 38 ++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/src/include/ipc4/pipeline.h b/src/include/ipc4/pipeline.h index 219c36a89a3d..198918cf8577 100644 --- a/src/include/ipc4/pipeline.h +++ b/src/include/ipc4/pipeline.h @@ -69,6 +69,41 @@ enum ipc4_pipeline_state { SOF_IPC4_PIPELINE_STATE_SAVED }; +/* IDs for all pipeline ext data types in struct ipc4_pipeline_init_ext_object */ +enum ipc4_pipeline_ext_obj_id { + IPC4_GLB_PIPE_EXT_OBJ_ID_INVALID = 0, + IPC4_GLB_PIPE_EXT_OBJ_ID_MEM_DATA = 1, + IPC4_GLB_PIPE_EXT_OBJ_ID_MAX = IPC4_GLB_PIPE_EXT_OBJ_ID_MEM_DATA, +}; + +/* data object for vendor bespoke data with ABI growth and backwards compat */ +struct ipc4_pipeline_ext_object { + uint32_t last_object : 1; /* object is last in array if 1 else object follows. */ + uint32_t object_id : 15; /* unique ID for this object or globally */ + uint32_t object_words : 16; /* size in dwords (excluding this structure) */ +} __packed __aligned(4); +/* the object data will be placed in memory here and will have size "object_words" */ + +/* Ext array data object for pipeline instance's memory requirements */ +struct ipc4_pipeline_ext_obj_mem_data { + uint32_t domain_id; /* userspace domain ID */ + uint32_t stack_bytes; /* required stack size in bytes */ + uint32_t interim_heap_bytes; /* required interim heap size in bytes */ + uint32_t lifetime_heap_bytes; /* required lifetime heap size in bytes */ + uint32_t shared_bytes; /* required shared memory in bytes */ +} __packed __aligned(4); + +/* + * Host Driver sends this message to create a new pipeline instance. + */ +struct ipc4_pipeline_ext_payload { + uint32_t payload_words : 24; /* size in dwords (excluding this structure) */ + uint32_t data_obj_array : 1; /* struct ipc4_pipeline_ext_object data */ + uint32_t rsvd0 : 7; + uint32_t rsvd1; + uint32_t rsvd2; +} __packed __aligned(4); + /*! * lp - indicates whether the pipeline should be kept on running in low power * mode. On BXT the driver should set this flag to 1 for WoV pipeline. @@ -106,7 +141,8 @@ struct ipc4_pipeline_create { uint32_t rsvd1 : 3; uint32_t attributes : 16; uint32_t core_id : 4; - uint32_t rsvd2 : 6; + uint32_t rsvd2 : 5; + uint32_t payload : 1; uint32_t _reserved_2 : 2; } r; } extension; From 95b5898483d211ab4eedb66a241a18ac23887ded Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Thu, 25 Sep 2025 19:58:27 +0300 Subject: [PATCH 4/9] ipc4: helper: Add ipc4_create_pipeline_payload_decode() and call it 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 SQUASH ipc4: Handler: Add ipc4_create_pipeline_payload_decode() and call it --- src/ipc/ipc4/helper.c | 117 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index bdfac278bbc9..48cb9705e5bd 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -229,6 +229,98 @@ struct ipc_comp_dev *ipc_get_comp_by_ppl_id(struct ipc *ipc, uint16_t type, return NULL; } +/* + * This function currently only decodes the payload and prints out + * data it finds, but it does not store it anywhere. + */ +__cold static int ipc4_create_pipeline_payload_decode(char *data) +{ + const struct ipc4_pipeline_ext_payload *hdr = + (struct ipc4_pipeline_ext_payload *)data; + const struct ipc4_pipeline_ext_object *obj; +#ifdef CONFIG_DCACHE_LINE_SIZE + size_t cache_line_size = CONFIG_DCACHE_LINE_SIZE; + size_t hdr_cache_size = ALIGN_UP(sizeof(*hdr), cache_line_size); +#endif + bool last_object; + size_t size; + +#ifdef CONFIG_DCACHE_LINE_SIZE + if (!IS_ENABLED(CONFIG_LIBRARY)) + sys_cache_data_invd_range((__sparse_force void __sparse_cache *)data, + hdr_cache_size); +#endif + size = hdr->payload_words * sizeof(uint32_t); + last_object = !hdr->data_obj_array; + + if (size < sizeof(*hdr)) { + tr_err(&ipc_tr, "Payload size too small: %u : %#x", hdr->payload_words, + *((uint32_t *)hdr)); + return -EINVAL; + } + + tr_info(&ipc_tr, "payload size %u array %u: %#x", hdr->payload_words, hdr->data_obj_array, + *((uint32_t *)hdr)); + +#ifdef CONFIG_DCACHE_LINE_SIZE + if (!IS_ENABLED(CONFIG_LIBRARY) && ALIGN_UP(size, cache_line_size) > hdr_cache_size) + sys_cache_data_invd_range((__sparse_force void __sparse_cache *) + ((char *)data + hdr_cache_size), + ALIGN_UP(size, cache_line_size) - hdr_cache_size); +#endif + + obj = (const struct ipc4_pipeline_ext_object *)(hdr + 1); + while (!last_object) { + const struct ipc4_pipeline_ext_object *next_obj; + + /* Check if there is space for the object header */ + if ((char *)(obj + 1) - data > size) { + tr_err(&ipc_tr, "obj header overflow, %u > %zu", + (char *)(obj + 1) - data, size); + return -EINVAL; + } + + /* Calculate would be next object position and check if current object fits */ + next_obj = (const struct ipc4_pipeline_ext_object *) + (((const uint32_t *)(obj + 1)) + obj->object_words); + if ((char *)next_obj - data > size) { + tr_err(&ipc_tr, "object size overflow, %u > %zu", + (char *)next_obj - data, size); + return -EINVAL; + } + + switch (obj->object_id) { + case IPC4_GLB_PIPE_EXT_OBJ_ID_MEM_DATA: + { + /* Get mem_data struct that follows the obj struct */ + const struct ipc4_pipeline_ext_obj_mem_data *mem_data = + (const struct ipc4_pipeline_ext_obj_mem_data *)(obj + 1); + + if (obj->object_words * sizeof(uint32_t) < sizeof(*mem_data)) { + tr_err(&ipc_tr, "mem_data object does not fit %zu < %zu", + obj->object_words * sizeof(uint32_t), sizeof(*mem_data)); + break; + } + tr_info(&ipc_tr, + "init_ext_obj_mem_data domain %u stack %u interim %u lifetime %u shared %u", + mem_data->domain_id, mem_data->stack_bytes, + mem_data->interim_heap_bytes, mem_data->lifetime_heap_bytes, + mem_data->shared_bytes); + break; + } + default: + tr_warn(&ipc_tr, "Unknown ext init object id %u of %u words", + obj->object_id, obj->object_words); + } + /* Read the last object flag from obj header */ + last_object = obj->last_object; + /* Move to next object */ + obj = next_obj; + } + + return 0; +} + __cold static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) { struct ipc_comp_dev *ipc_pipe; @@ -280,6 +372,23 @@ __cold static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) return IPC4_SUCCESS; } +#if CONFIG_LIBRARY +static inline char *ipc4_get_pipe_create_data(void) +{ + struct ipc *ipc = ipc_get(); + char *data = (char *)ipc->comp_data + sizeof(struct ipc4_pipeline_create); + + return data; +} +#else +__cold static inline char *ipc4_get_pipe_create_data(void) +{ + assert_can_be_cold(); + + return (char *)MAILBOX_HOSTBOX_BASE; +} +#endif + /* Only called from ipc4_new_pipeline(), which is __cold */ __cold int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) { @@ -293,6 +402,14 @@ __cold int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) if (!cpu_is_me(pipe_desc->extension.r.core_id)) return ipc4_process_on_core(pipe_desc->extension.r.core_id, false); + if (pipe_desc->extension.r.payload) { + char *data; + + data = ipc4_get_pipe_create_data(); + + ipc4_create_pipeline_payload_decode(data); + } + return ipc4_create_pipeline(pipe_desc); } From 521c2a0a78057545a0c4795f19b38ea7338bbca5 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Thu, 2 Oct 2025 20:04:05 +0300 Subject: [PATCH 5/9] ipc4: helper: Do not invalidate cache of whole mailbox for module init Invalidate cache for only the module init payload size not the whole mailbox. Signed-off-by: Jyri Sarha --- src/ipc/ipc4/helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 48cb9705e5bd..a61d7568042d 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -139,10 +139,10 @@ __cold struct comp_dev *comp_new_ipc4(struct ipc4_module_init_instance *module_i ipc_config.ipc_config_size = module_init->extension.r.param_block_size * sizeof(uint32_t); ipc_config.ipc_extended_init = module_init->extension.r.extended_init; - dcache_invalidate_region((__sparse_force void __sparse_cache *)MAILBOX_HOSTBOX_BASE, - MAILBOX_HOSTBOX_SIZE); - data = ipc4_get_comp_new_data(); + if (!IS_ENABLED(CONFIG_LIBRARY)) + sys_cache_data_invd_range((__sparse_force void __sparse_cache *)data, + ipc_config.ipc_config_size); #if CONFIG_LIBRARY ipc_config.ipc_config_size -= sizeof(struct sof_uuid); From 6ef3023ea544a69b879cac1f7f0244a6735dd6b2 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Tue, 2 Dec 2025 00:03:41 +0200 Subject: [PATCH 6/9] pipeline: Pass create pipeline payload parameters down to pipeline_new() 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 --- src/audio/pipeline/pipeline-graph.c | 3 ++- src/include/sof/audio/pipeline.h | 11 +++++++++- src/ipc/ipc3/helper.c | 2 +- src/ipc/ipc4/helper.c | 20 +++++++++++++------ test/cmocka/src/audio/pipeline/pipeline_new.c | 3 ++- 5 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index 678b8095289f..05c89dc85742 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -108,7 +108,8 @@ void pipeline_posn_init(struct sof *sof) } /* create new pipeline - returns pipeline id or negative error */ -struct pipeline *pipeline_new(uint32_t pipeline_id, uint32_t priority, uint32_t comp_id) +struct pipeline *pipeline_new(uint32_t pipeline_id, uint32_t priority, uint32_t comp_id, + struct create_pipeline_params *pparams) { struct sof_ipc_stream_posn posn; struct pipeline *p; diff --git a/src/include/sof/audio/pipeline.h b/src/include/sof/audio/pipeline.h index 5221d330e0f1..c242865a7cf3 100644 --- a/src/include/sof/audio/pipeline.h +++ b/src/include/sof/audio/pipeline.h @@ -134,6 +134,13 @@ struct pipeline_task { struct comp_dev *sched_comp; /**< pipeline scheduling component */ }; +struct ipc4_pipeline_ext_obj_mem_data; + +/** \brief For storing IPC payload data. */ +struct create_pipeline_params { + const struct ipc4_pipeline_ext_obj_mem_data *mem_data; +}; + #define pipeline_task_get(t) container_of(t, struct pipeline_task, task) /* @@ -148,9 +155,11 @@ struct pipeline_task { * \param[in] pipeline_id Pipeline ID number. * \param[in] priority Pipeline scheduling priority. * \param[in] comp_id Pipeline component ID number. + * \param[in] pparams Pipeline parameters from IPC payload, maybe NULL. * \return New pipeline pointer or NULL. */ -struct pipeline *pipeline_new(uint32_t pipeline_id, uint32_t priority, uint32_t comp_id); +struct pipeline *pipeline_new(uint32_t pipeline_id, uint32_t priority, uint32_t comp_id, + struct create_pipeline_params *pparams); /** * \brief Free's a pipeline. diff --git a/src/ipc/ipc3/helper.c b/src/ipc/ipc3/helper.c index 658ceaa26784..52978dcaa34f 100644 --- a/src/ipc/ipc3/helper.c +++ b/src/ipc/ipc3/helper.c @@ -396,7 +396,7 @@ int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) /* create the pipeline */ pipe = pipeline_new(pipe_desc->pipeline_id, pipe_desc->priority, - pipe_desc->comp_id); + pipe_desc->comp_id, NULL); if (!pipe) { tr_err(&ipc_tr, "pipeline_new() failed"); return -ENOMEM; diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index a61d7568042d..46c204368ee6 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -233,7 +233,8 @@ struct ipc_comp_dev *ipc_get_comp_by_ppl_id(struct ipc *ipc, uint16_t type, * This function currently only decodes the payload and prints out * data it finds, but it does not store it anywhere. */ -__cold static int ipc4_create_pipeline_payload_decode(char *data) +__cold static int ipc4_create_pipeline_payload_decode(char *data, + struct create_pipeline_params *pparams) { const struct ipc4_pipeline_ext_payload *hdr = (struct ipc4_pipeline_ext_payload *)data; @@ -301,6 +302,7 @@ __cold static int ipc4_create_pipeline_payload_decode(char *data) obj->object_words * sizeof(uint32_t), sizeof(*mem_data)); break; } + pparams->mem_data = mem_data; tr_info(&ipc_tr, "init_ext_obj_mem_data domain %u stack %u interim %u lifetime %u shared %u", mem_data->domain_id, mem_data->stack_bytes, @@ -321,7 +323,8 @@ __cold static int ipc4_create_pipeline_payload_decode(char *data) return 0; } -__cold static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) +__cold static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc, + struct create_pipeline_params *pparams) { struct ipc_comp_dev *ipc_pipe; struct pipeline *pipe; @@ -338,7 +341,8 @@ __cold static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) } /* create the pipeline */ - pipe = pipeline_new(pipe_desc->primary.r.instance_id, pipe_desc->primary.r.ppl_priority, 0); + pipe = pipeline_new(pipe_desc->primary.r.instance_id, pipe_desc->primary.r.ppl_priority, 0, + pparams); if (!pipe) { tr_err(&ipc_tr, "ipc: pipeline_new() failed"); return IPC4_OUT_OF_MEMORY; @@ -393,7 +397,8 @@ __cold static inline char *ipc4_get_pipe_create_data(void) __cold int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) { struct ipc4_pipeline_create *pipe_desc = ipc_from_pipe_new(_pipe_desc); - + struct create_pipeline_params pparams = { 0 }; + bool valid_pparams = false; assert_can_be_cold(); tr_dbg(&ipc_tr, "ipc: pipeline id = %u", (uint32_t)pipe_desc->primary.r.instance_id); @@ -404,13 +409,16 @@ __cold int ipc_pipeline_new(struct ipc *ipc, ipc_pipe_new *_pipe_desc) if (pipe_desc->extension.r.payload) { char *data; + int ret; data = ipc4_get_pipe_create_data(); - ipc4_create_pipeline_payload_decode(data); + ret = ipc4_create_pipeline_payload_decode(data, &pparams); + if (ret == 0) + valid_pparams = true; } - return ipc4_create_pipeline(pipe_desc); + return ipc4_create_pipeline(pipe_desc, valid_pparams ? &pparams : NULL); } __cold static inline int ipc_comp_free_remote(struct comp_dev *dev) diff --git a/test/cmocka/src/audio/pipeline/pipeline_new.c b/test/cmocka/src/audio/pipeline/pipeline_new.c index c08e499a5a05..fd5145ef6d99 100644 --- a/test/cmocka/src/audio/pipeline/pipeline_new.c +++ b/test/cmocka/src/audio/pipeline/pipeline_new.c @@ -51,7 +51,8 @@ static void test_audio_pipeline_pipeline_new_creation(void **state) /*Testing component*/ struct pipeline *result = pipeline_new(test_data->pipe_id, test_data->priority, - test_data->comp_id); + test_data->comp_id, + NULL); /*Pipeline should have been created so pointer can't be null*/ assert_non_null(result); From c3bf97f038ced3f09e87703e49f284f5519864ce Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Fri, 7 Nov 2025 15:42:27 +0200 Subject: [PATCH 7/9] topology2: Widget defaults for stack, lifetime, interim and shared memory Add default values for stack, lifetime heap, interim heap, and shared memory sizes 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 --- .../topology/topology2/include/components/widget-common.conf | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/topology/topology2/include/components/widget-common.conf b/tools/topology/topology2/include/components/widget-common.conf index aed814f3be0d..43ac5691c778 100644 --- a/tools/topology/topology2/include/components/widget-common.conf +++ b/tools/topology/topology2/include/components/widget-common.conf @@ -174,3 +174,8 @@ DefineAttribute."shared_bytes_requirement" { token_ref "comp.word" } +# These default values are here until we have measured module specific numbers +stack_bytes_requirement 8192 +interim_heap_bytes_requirement 4096 +lifetime_heap_bytes_requirement 16384 +shared_bytes_requirement 4096 \ No newline at end of file From 039175e16991905d14258313961c3a25f7ef9421 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Sun, 14 Dec 2025 22:06:03 +0200 Subject: [PATCH 8/9] component: module_adapter: Update struct ipc4_module_init_ext_obj_dp_data Update struct ipc4_module_init_ext_obj_dp_data to match what is required for latest user space features. This is a tricky change as it chenges already part of ABI. However, as the structure was not really used for anything before, changing it should be safe. That is with one exception. The case where an earlier SOF driver sends us the old smaller struct identifier with IPC4_MOD_INIT_DATA_ID_DP_DATA, then we should not fail on that, but only ignore the struct. This is why "return" is changed to "break" in case IPC4_MOD_INIT_DATA_ID_DP_DATA. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module_adapter_ipc4.c | 17 +++++++++++------ src/include/ipc4/module.h | 8 +++++--- src/include/module/module/base.h | 8 +++++--- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/audio/module_adapter/module_adapter_ipc4.c b/src/audio/module_adapter/module_adapter_ipc4.c index e87474dad1db..e399a8bff2cc 100644 --- a/src/audio/module_adapter/module_adapter_ipc4.c +++ b/src/audio/module_adapter/module_adapter_ipc4.c @@ -67,15 +67,20 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, (const struct ipc4_module_init_ext_obj_dp_data *)(obj + 1); if (obj->object_words * sizeof(uint32_t) < sizeof(*dp_data)) { - comp_err(dev, "dp_data object too small %zu < %zu", - obj->object_words * sizeof(uint32_t), sizeof(*dp_data)); - return NULL; + comp_warn(dev, "dp_data object too small %zu < %zu", + obj->object_words * sizeof(uint32_t), sizeof(*dp_data)); + break; } dst->domain_id = dp_data->domain_id; dst->stack_bytes = dp_data->stack_bytes; - dst->heap_bytes = dp_data->heap_bytes; - comp_info(dev, "init_ext_obj_dp_data domain %u stack %u heap %u", - dp_data->domain_id, dp_data->stack_bytes, dp_data->heap_bytes); + dst->interim_heap_bytes = dp_data->interim_heap_bytes; + dst->lifetime_heap_bytes = dp_data->interim_heap_bytes; + dst->shared_bytes = dp_data->shared_bytes; + comp_info(dev, + "init_ext_obj_dp_data domain %u stack %u interim %u lifetime %u shared %u", + dp_data->domain_id, dp_data->stack_bytes, + dp_data->interim_heap_bytes, dp_data->lifetime_heap_bytes, + dp_data->shared_bytes); break; } default: diff --git a/src/include/ipc4/module.h b/src/include/ipc4/module.h index a73ede0b0bd5..906cbd3e8dda 100644 --- a/src/include/ipc4/module.h +++ b/src/include/ipc4/module.h @@ -90,9 +90,11 @@ struct ipc4_module_init_ext_object { /* Ext init array data object for Data Processing module memory requirements */ struct ipc4_module_init_ext_obj_dp_data { - 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 */ + uint32_t domain_id; /* userspace domain ID */ + uint32_t stack_bytes; /* required stack size in bytes */ + uint32_t interim_heap_bytes; /* required interim heap size in bytes */ + uint32_t lifetime_heap_bytes; /* required lifetime heap size in bytes */ + uint32_t shared_bytes; /* required shared memory size in bytes */ } __attribute__((packed, aligned(4))); /* diff --git a/src/include/module/module/base.h b/src/include/module/module/base.h index 6d7dea657314..86b1e2b9016d 100644 --- a/src/include/module/module/base.h +++ b/src/include/module/module/base.h @@ -38,9 +38,11 @@ struct module_config { uint8_t nb_output_pins; struct ipc4_input_pin_format *input_pins; struct ipc4_output_pin_format *output_pins; - uint32_t domain_id; /* userspace domain ID */ - uint32_t stack_bytes; /* stack size in bytes, 0 means default value */ - uint32_t heap_bytes; /* max heap size in bytes, 0 means default value */ + uint32_t domain_id; /* userspace domain ID */ + uint32_t stack_bytes; /* stack size in bytes */ + uint32_t interim_heap_bytes; /* interim heap size in bytes */ + uint32_t lifetime_heap_bytes; /* lifetime heap size in bytes */ + uint32_t shared_bytes; /* shared size in bytes */ #endif }; From ae1aa2f2ec28491af139ec8cc9ceab8e55f3b709 Mon Sep 17 00:00:00 2001 From: Jyri Sarha Date: Mon, 15 Dec 2025 22:40:36 +0200 Subject: [PATCH 9/9] component: module_adapter: Move module_ext_init_decode()-call earlier Move module_ext_init_decode()-call earlier in call chain so that struct ipc4_module_init_ext_obj_dp_data is available when module_adapter_mem_alloc() is called. That is, if the struct ipc4_module_init_ext_object is present in the struct ipc4_module_init_ext_init payload. To accomplish this I needed to redesign how module_ext_init_decode() works and how its called a bit. Signed-off-by: Jyri Sarha --- src/audio/module_adapter/module_adapter.c | 38 ++++++++-- .../module_adapter/module_adapter_ipc4.c | 70 +++++++++---------- src/include/module/module/base.h | 18 +++-- .../sof/audio/module_adapter/module/generic.h | 13 +++- 4 files changed, 90 insertions(+), 49 deletions(-) diff --git a/src/audio/module_adapter/module_adapter.c b/src/audio/module_adapter/module_adapter.c index 7edc018dabb4..9e6ad0850fec 100644 --- a/src/audio/module_adapter/module_adapter.c +++ b/src/audio/module_adapter/module_adapter.c @@ -158,7 +158,7 @@ static void module_adapter_mem_free(struct processing_module *mod) * \brief Create a module adapter component. * \param[in] drv - component driver pointer. * \param[in] config - component ipc descriptor pointer. - * \param[in] spec - passdowned data from driver. + * \param[in] const_spec - passdowned data from driver. * \param[in] mod_priv - Pointer to private data for processing module. * * \return: a pointer to newly created module adapter component on success. NULL on error. @@ -167,13 +167,18 @@ static void module_adapter_mem_free(struct processing_module *mod) * the create method. */ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv, - const struct comp_ipc_config *config, const void *spec, - void *mod_priv, struct userspace_context *user_ctx) + const struct comp_ipc_config *config, + const void *const_spec, void *mod_priv, + struct userspace_context *user_ctx) { int ret; struct module_config *dst; const struct module_interface *const interface = drv->adapter_ops; - + struct ipc_config_process spec = + *((const struct ipc_config_process *) const_spec); +#if CONFIG_IPC_MAJOR_4 + struct module_ext_init_data ext_data = { 0 }; +#endif comp_cl_dbg(drv, "start"); if (!config) { @@ -181,6 +186,13 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv, (size_t)drv, (size_t)config); return NULL; } +#if CONFIG_IPC_MAJOR_4 + if (config->ipc_extended_init) { + ret = module_ext_init_decode(drv, &ext_data, &spec); + if (ret != 0) + return NULL; + } +#endif struct processing_module *mod = module_adapter_mem_alloc(drv, config); @@ -204,7 +216,17 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv, #endif /* CONFIG_USERSPACE */ dst = &mod->priv.cfg; - ret = module_adapter_init_data(dev, dst, config, spec); + /* + * NOTE: dst->ext_data points to stack variable and contains + * pointers to IPC payload mailbox, so its only valid in + * functions that called from this function. This why + * the pointer is set NULL before the this function + * exits. + */ +#if CONFIG_IPC_MAJOR_4 + dst->ext_data = &ext_data; +#endif + ret = module_adapter_init_data(dev, dst, config, &spec); if (ret) { comp_err(dev, "%d: module init data failed", ret); @@ -270,6 +292,9 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv, component_set_nearest_period_frames(dev, params.rate); #endif +#if CONFIG_IPC_MAJOR_4 + dst->ext_data = NULL; +#endif comp_dbg(dev, "done"); return dev; @@ -279,6 +304,9 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv, schedule_task_free(dev->task); #endif module_adapter_mem_free(mod); +#if CONFIG_IPC_MAJOR_4 + dst->ext_data = NULL; +#endif return NULL; } diff --git a/src/audio/module_adapter/module_adapter_ipc4.c b/src/audio/module_adapter/module_adapter_ipc4.c index e399a8bff2cc..f29c18cda92e 100644 --- a/src/audio/module_adapter/module_adapter_ipc4.c +++ b/src/audio/module_adapter/module_adapter_ipc4.c @@ -25,19 +25,19 @@ LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL); -static const struct ipc4_base_module_extended_cfg * -module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, - const unsigned char *data, size_t *size) +int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data, + struct ipc_config_process *spec) { const struct ipc4_module_init_ext_init *ext_init = - (const struct ipc4_module_init_ext_init *)data; + (const struct ipc4_module_init_ext_init *)spec->data; bool last_object = !ext_init->data_obj_array; const struct ipc4_module_init_ext_object *obj; - if (*size < sizeof(ext_init)) { - comp_err(dev, "Size too small for ext init %zu < %zu", - *size, sizeof(ext_init)); - return NULL; + assert(drv->type == SOF_COMP_MODULE_ADAPTER); + if (spec->size < sizeof(ext_init)) { + comp_cl_err(drv, "Size too small for ext init %zu < %zu", + spec->size, sizeof(ext_init)); + return -EINVAL; } /* TODO: Handle ext_init->gna_used and ext_init->rtos_domain here */ /* Get the first obj struct right after ext_init struct */ @@ -46,18 +46,18 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, const struct ipc4_module_init_ext_object *next_obj; /* Check if there is space for the object header */ - if ((unsigned char *)(obj + 1) - data > *size) { - comp_err(dev, "ext init obj overflow, %u > %zu", - (unsigned char *)(obj + 1) - data, *size); - return NULL; + if ((unsigned char *)(obj + 1) - spec->data > spec->size) { + comp_cl_err(drv, "ext init obj overflow, %u > %zu", + (unsigned char *)(obj + 1) - spec->data, spec->size); + return -EINVAL; } /* Calculate would be next object position and check if current object fits */ next_obj = (const struct ipc4_module_init_ext_object *) (((uint32_t *) (obj + 1)) + obj->object_words); - if ((unsigned char *)next_obj - data > *size) { - comp_err(dev, "ext init object array overflow, %u > %zu", - (unsigned char *)obj - data, *size); - return NULL; + if ((unsigned char *)next_obj - spec->data > spec->size) { + comp_cl_err(drv, "ext init object array overflow, %u > %zu", + (unsigned char *)obj - spec->data, spec->size); + return -EINVAL; } switch (obj->object_id) { case IPC4_MOD_INIT_DATA_ID_DP_DATA: @@ -67,25 +67,22 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, (const struct ipc4_module_init_ext_obj_dp_data *)(obj + 1); if (obj->object_words * sizeof(uint32_t) < sizeof(*dp_data)) { - comp_warn(dev, "dp_data object too small %zu < %zu", - obj->object_words * sizeof(uint32_t), sizeof(*dp_data)); + comp_cl_warn(drv, "dp_data object too small %zu < %zu", + obj->object_words * sizeof(uint32_t), + sizeof(*dp_data)); break; } - dst->domain_id = dp_data->domain_id; - dst->stack_bytes = dp_data->stack_bytes; - dst->interim_heap_bytes = dp_data->interim_heap_bytes; - dst->lifetime_heap_bytes = dp_data->interim_heap_bytes; - dst->shared_bytes = dp_data->shared_bytes; - comp_info(dev, - "init_ext_obj_dp_data domain %u stack %u interim %u lifetime %u shared %u", - dp_data->domain_id, dp_data->stack_bytes, - dp_data->interim_heap_bytes, dp_data->lifetime_heap_bytes, - dp_data->shared_bytes); + ext_data->dp_data = dp_data; + comp_cl_info(drv, + "init_ext_obj_dp_data domain %u stack %u interim %u lifetime %u shared %u", + dp_data->domain_id, dp_data->stack_bytes, + dp_data->interim_heap_bytes, dp_data->lifetime_heap_bytes, + dp_data->shared_bytes); break; } default: - comp_info(dev, "Unknown ext init object id %u of %u words", - obj->object_id, obj->object_words); + comp_cl_info(drv, "Unknown ext init object id %u of %u words", + obj->object_id, obj->object_words); } /* Read the last object flag from obj header */ last_object = obj->last_object; @@ -93,11 +90,11 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst, obj = next_obj; } - /* Remove decoded ext_init payload from the size */ - *size -= (unsigned char *) obj - data; + /* Remove decoded ext_init payload from spec */ + spec->size -= (unsigned char *)obj - spec->data; + spec->data = (const unsigned char *)obj; - /* return remaining payload */ - return (const struct ipc4_base_module_extended_cfg *)obj; + return 0; } /* @@ -119,10 +116,7 @@ int module_adapter_init_data(struct comp_dev *dev, size_t cfgsz = args->size; assert(dev->drv->type == SOF_COMP_MODULE_ADAPTER); - if (config->ipc_extended_init) - cfg = module_ext_init_decode(dev, dst, args->data, &cfgsz); - else - cfg = (const struct ipc4_base_module_extended_cfg *)args->data; + cfg = (const struct ipc4_base_module_extended_cfg *)args->data; if (cfg == NULL) return -EINVAL; diff --git a/src/include/module/module/base.h b/src/include/module/module/base.h index 86b1e2b9016d..f6d1172ddf41 100644 --- a/src/include/module/module/base.h +++ b/src/include/module/module/base.h @@ -23,6 +23,18 @@ #define module_get_private_data(mod) ((mod)->priv.private) #define module_set_private_data(mod, data) ((mod)->priv.private = data) +/** + * \struct module_ext_init_data + * \brief Container for found ext init object pointers + * This struct contains pointers that point to IPC payload directly. The + * module should store what it needs in its init() callback as the data + * is not valid after that. + */ +struct ipc4_module_init_ext_obj_dp_data; +struct module_ext_init_data { + const struct ipc4_module_init_ext_obj_dp_data *dp_data; +}; + /** * \struct module_config * \brief Module config container, used for both config types. @@ -38,11 +50,7 @@ struct module_config { uint8_t nb_output_pins; struct ipc4_input_pin_format *input_pins; struct ipc4_output_pin_format *output_pins; - uint32_t domain_id; /* userspace domain ID */ - uint32_t stack_bytes; /* stack size in bytes */ - uint32_t interim_heap_bytes; /* interim heap size in bytes */ - uint32_t lifetime_heap_bytes; /* lifetime heap size in bytes */ - uint32_t shared_bytes; /* shared size in bytes */ + struct module_ext_init_data *ext_data; /**< IPC payload pointers, NULL after init() */ #endif }; diff --git a/src/include/sof/audio/module_adapter/module/generic.h b/src/include/sof/audio/module_adapter/module/generic.h index f339ac158ab0..ad65cf73cefe 100644 --- a/src/include/sof/audio/module_adapter/module/generic.h +++ b/src/include/sof/audio/module_adapter/module/generic.h @@ -412,7 +412,18 @@ int module_adapter_ts_get_op(struct comp_dev *dev, struct timestamp_data *tsd); void module_update_buffer_position(struct input_stream_buffer *input_buffers, struct output_stream_buffer *output_buffers, uint32_t frames); - +struct module_ext_init_data; +#if CONFIG_IPC_MAJOR_4 +int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data, + struct ipc_config_process *spec); +#else +static inline +int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data, + struct ipc_config_process *spec) +{ + return 0; +} +#endif int module_adapter_init_data(struct comp_dev *dev, struct module_config *dst, const struct comp_ipc_config *config,