Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions src/audio/module_adapter/library/userspace_proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <stdint.h>

#include <sof/lib_manager.h>
#include <sof/llext_manager.h>
#include <sof/audio/component.h>
#include <sof/schedule/dp_schedule.h>
#include <rtos/userspace_helper.h>
Expand Down Expand Up @@ -163,6 +164,7 @@ static int user_work_item_init(struct userspace_context *user_ctx, struct k_heap
work_item->event = &worker.event;
#endif
work_item->params.context = user_ctx;
work_item->params.mod = NULL;
user_ctx->work_item = work_item;

return 0;
Expand Down Expand Up @@ -274,8 +276,24 @@ static int userspace_proxy_memory_init(struct userspace_context *user_ctx,
tr_dbg(&userspace_proxy_tr, "Heap partition %#lx + %zx, attr = %u",
heap_part.start, heap_part.size, heap_part.attr);

#if !defined(CONFIG_XTENSA_MMU_DOUBLE_MAP) && defined(CONFIG_SOF_ZEPHYR_HEAP_CACHED)
#define HEAP_PART_CACHED
/* When a new memory domain is created, only the "factory" entries from the L2 page
* tables are copied. Memory that was dynamically mapped during firmware execution
* will not be accessible from the new domain. The k_heap structure (drv->user_heap)
* resides in such dynamically mapped memory, so we must explicitly add a partition
* for it to ensure that syscalls can access this structure from the userspace domain.
*/
struct k_mem_partition heap_struct_part;

k_mem_region_align(&heap_struct_part.start, &heap_struct_part.size,
POINTER_TO_UINT(drv->user_heap),
sizeof(*drv->user_heap), CONFIG_MM_DRV_PAGE_SIZE);
heap_struct_part.attr = K_MEM_PARTITION_P_RW_U_NA |
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added partition is intended so that drv->user_heap 'can be referenced by syscalls' from the userspace domain, but it is created with U_NA (no user access). For syscall argument validation and/or user-mode access to succeed, the partition typically needs user permissions (e.g., K_MEM_PARTITION_P_RW_U_RO or K_MEM_PARTITION_P_RW_U_RW, depending on the expected access pattern). With U_NA, userspace likely cannot legally reference the k_heap object.

Suggested change
heap_struct_part.attr = K_MEM_PARTITION_P_RW_U_NA |
heap_struct_part.attr = K_MEM_PARTITION_P_RW_U_RO |

Copilot uses AI. Check for mistakes.
user_get_partition_attr(heap_struct_part.start);

tr_err(&userspace_proxy_tr, "Heap struct partition %#lx + %zx, attr = %u",
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like informational tracing (similar to the surrounding tr_dbg() partition logs) but is emitted as tr_err(). Logging this at error level can create noisy logs and false-positive error signals in production. Consider downgrading this to tr_dbg() (or tr_info() if you want it visible by default).

Suggested change
tr_err(&userspace_proxy_tr, "Heap struct partition %#lx + %zx, attr = %u",
tr_dbg(&userspace_proxy_tr, "Heap struct partition %#lx + %zx, attr = %u",

Copilot uses AI. Check for mistakes.
heap_struct_part.start, heap_struct_part.size, heap_struct_part.attr);

#if defined(CONFIG_SOF_ZEPHYR_HEAP_CACHED)
/* Add cached module private heap to memory partitions */
struct k_mem_partition heap_cached_part = {
.attr = K_MEM_PARTITION_P_RW_U_RW | XTENSA_MMU_CACHED_WB
Expand All @@ -294,10 +312,11 @@ static int userspace_proxy_memory_init(struct userspace_context *user_ctx,
* These include ops structures marked with APP_TASK_DATA.
*/
&common_partition,
#ifdef HEAP_PART_CACHED
#ifdef CONFIG_SOF_ZEPHYR_HEAP_CACHED
&heap_cached_part,
#endif
&heap_part
&heap_part,
&heap_struct_part
};

tr_dbg(&userspace_proxy_tr, "Common partition %#lx + %zx, attr = %u",
Expand Down Expand Up @@ -382,7 +401,7 @@ static int userspace_proxy_start_agent(struct userspace_context *user_ctx,
}

int userspace_proxy_create(struct userspace_context **user_ctx, const struct comp_driver *drv,
const struct sof_man_module *manifest, system_agent_start_fn start_fn,
const struct sof_man_module *manifest, system_agent_start_fn agent_fn,
const struct system_agent_params *agent_params,
const void **agent_interface, const struct module_interface **ops)
{
Expand Down Expand Up @@ -410,15 +429,20 @@ int userspace_proxy_create(struct userspace_context **user_ctx, const struct com
if (ret)
goto error_dom;

ret = userspace_proxy_add_sections(context, agent_params->instance_id, manifest);
if (agent_fn)
ret = userspace_proxy_add_sections(context, agent_params->instance_id, manifest);
else
/* llext modules do not use the system agent. */
ret = llext_manager_add_domain(agent_params->module_id, domain);
Comment on lines +432 to +436
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agent_fn is explicitly allowed to be NULL (llext path), but userspace_proxy_start_agent() is still called unconditionally with agent_fn. This can lead to a NULL function-pointer call unless userspace_proxy_start_agent() internally guards it. Consider skipping userspace_proxy_start_agent() entirely when agent_fn == NULL and ensuring any required agent_interface/ops setup for llext is handled in the llext path.

Copilot uses AI. Check for mistakes.

if (ret)
goto error_dom;

ret = user_work_item_init(context, drv->user_heap);
if (ret)
goto error_dom;

ret = userspace_proxy_start_agent(context, start_fn, agent_params, agent_interface);
ret = userspace_proxy_start_agent(context, agent_fn, agent_params, agent_interface);
if (ret) {
tr_err(&userspace_proxy_tr, "System agent failed with error %d.", ret);
goto error_work_item;
Comment on lines +445 to 448
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agent_fn is explicitly allowed to be NULL (llext path), but userspace_proxy_start_agent() is still called unconditionally with agent_fn. This can lead to a NULL function-pointer call unless userspace_proxy_start_agent() internally guards it. Consider skipping userspace_proxy_start_agent() entirely when agent_fn == NULL and ensuring any required agent_interface/ops setup for llext is handled in the llext path.

Suggested change
ret = userspace_proxy_start_agent(context, agent_fn, agent_params, agent_interface);
if (ret) {
tr_err(&userspace_proxy_tr, "System agent failed with error %d.", ret);
goto error_work_item;
if (agent_fn) {
ret = userspace_proxy_start_agent(context, agent_fn, agent_params, agent_interface);
if (ret) {
tr_err(&userspace_proxy_tr, "System agent failed with error %d.", ret);
goto error_work_item;
}

Copilot uses AI. Check for mistakes.
Expand Down
4 changes: 3 additions & 1 deletion src/include/module/module/llext.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#ifndef MODULE_LLEXT_H
#define MODULE_LLEXT_H

#define SOF_LLEXT_MODULE_MANIFEST(manifest_name, entry, affinity, mod_uuid, instances) \
#define SOF_LLEXT_MODULE_MANIFEST(manifest_name, entry, affinity, mod_uuid, instances, ...) \
{ \
.module = { \
.name = manifest_name, \
Expand All @@ -16,6 +16,8 @@
.type = { \
.load_type = SOF_MAN_MOD_TYPE_LLEXT, \
.domain_ll = 1, \
.user_mode = COND_CODE_0(NUM_VA_ARGS_LESS_1(_, ##__VA_ARGS__), (0), \
(GET_ARG_N(1, __VA_ARGS__))), \
}, \
.affinity_mask = (affinity), \
} \
Expand Down
23 changes: 12 additions & 11 deletions src/library_manager/lib_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -543,12 +543,14 @@ static int lib_manager_start_agent(const struct comp_driver *drv,
return ret;
}
#endif /* CONFIG_SOF_USERSPACE_PROXY */
if (agent) {
ret = agent(&agent_params, agent_interface);
if (ret)
tr_err(&lib_manager_tr, "System agent start failed %d!", ret);
return ret;
}

ret = agent(&agent_params, agent_interface);
if (ret)
tr_err(&lib_manager_tr, "System agent start failed %d!", ret);

return ret;
return 0;
}

enum buildinfo_mod_type { MOD_TYPE_INVALID, MOD_TYPE_IADK, MOD_TYPE_LMDK, MOD_TYPE_LLEXT };
Expand Down Expand Up @@ -654,6 +656,7 @@ static struct comp_dev *lib_manager_module_create(const struct comp_driver *drv,
case MOD_TYPE_LLEXT:
agent = NULL;
ops = (const struct module_interface *)module_entry_point;
agent_iface = NULL;
break;
case MOD_TYPE_LMDK:
agent = &native_system_agent_start;
Expand All @@ -671,12 +674,10 @@ static struct comp_dev *lib_manager_module_create(const struct comp_driver *drv,
}

/* At this point module resources are allocated and it is moved to L2 memory. */
if (agent) {
ret = lib_manager_start_agent(drv, config, mod, args, module_entry_point, agent,
agent_iface, &userspace, &ops);
if (ret)
goto err;
}
ret = lib_manager_start_agent(drv, config, mod, args, module_entry_point, agent,
agent_iface, &userspace, &ops);
if (ret)
goto err;

if (comp_set_adapter_ops(drv, ops) < 0)
goto err;
Expand Down
1 change: 1 addition & 0 deletions tools/rimage/src/manifest.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ static void man_get_section_manifest(struct image *image,
man_module->type.domain_dp = sof_mod->module.type.domain_dp;
man_module->type.domain_ll = sof_mod->module.type.domain_ll;
man_module->type.load_type = sof_mod->module.type.load_type;
man_module->type.user_mode = sof_mod->module.type.user_mode;

/* text segment */
segment = &man_module->segment[SOF_MAN_SEGMENT_TEXT];
Expand Down
10 changes: 7 additions & 3 deletions zephyr/lib/fast-get.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,11 @@ const void *fast_get(struct k_heap *heap, const void *dram_ptr, size_t size)
alloc_align = PLATFORM_DCACHE_ALIGN;
}

if (size > FAST_GET_MAX_COPY_SIZE || !IS_ENABLED(CONFIG_USERSPACE))
if (size > FAST_GET_MAX_COPY_SIZE || !IS_ENABLED(CONFIG_USERSPACE) ||
/* The module driver heap is shared by all instances of a given module.
* Instances can share the allocated buffer.
*/
IS_ENABLED(CONFIG_SOF_USERSPACE_USE_DRIVER_HEAP))
alloc_ptr = dram_ptr;
else
/* When userspace is enabled only share large buffers */
Expand Down Expand Up @@ -183,8 +187,8 @@ const void *fast_get(struct k_heap *heap, const void *dram_ptr, size_t size)
ret = entry->sram_ptr;

#if CONFIG_USERSPACE
/* We only get there for large buffers */
if (k_current_get()->mem_domain_info.mem_domain->num_partitions > 1) {
/* We only get there for large buffers or module driver heap is in use */
if (k_current_get()->base.user_options & K_USER && size > FAST_GET_MAX_COPY_SIZE) {
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states this branch applies to 'large buffers or module driver heap is in use', but the condition only gates on size > FAST_GET_MAX_COPY_SIZE. If the driver-heap configuration is intended to share buffers across userspace instances independent of size, the condition should incorporate CONFIG_SOF_USERSPACE_USE_DRIVER_HEAP (or the comment should be corrected). As written, the driver-heap case is ignored by this userspace-thread path.

Suggested change
if (k_current_get()->base.user_options & K_USER && size > FAST_GET_MAX_COPY_SIZE) {
if ((k_current_get()->base.user_options & K_USER) &&
(size > FAST_GET_MAX_COPY_SIZE ||
IS_ENABLED(CONFIG_SOF_USERSPACE_USE_DRIVER_HEAP))) {

Copilot uses AI. Check for mistakes.
/* A userspace thread makes the request */
if (k_current_get() != entry->thread &&
!fast_get_domain_exists(k_current_get(), ret,
Expand Down
2 changes: 0 additions & 2 deletions zephyr/lib/userspace_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
#include <sof/lib/dai.h>
#include <sof/lib/dma.h>

#define MODULE_DRIVER_HEAP_CACHED CONFIG_SOF_ZEPHYR_HEAP_CACHED

/* Zephyr includes */
#include <zephyr/kernel.h>
#include <zephyr/app_memory/app_memdomain.h>
Expand Down
Loading