Skip to content

userspace: proxy: Add support for llext modules #10643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
softwarecki wants to merge 4 commits into
base: main
Choose a base branch
from
+54 −24
Open

userspace: proxy: Add support for llext modules #10643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c51277d
manifest: rimage: llext: Add user_mode param to SOF_LLEXT_MODULE_MANI…
softwarecki Mar 23, 2026
2da0845
userspace: proxy: Add support for llext modules
softwarecki Mar 3, 2026
7fa81f8
fast_get: Enable buffer sharing when using module driver heap
softwarecki Mar 20, 2026
761633a
userspace: proxy: Add partition for k_heap struct
softwarecki Mar 19, 2026
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
View file
Open in desktop
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.
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Mar 24, 2026

Choose a reason for hiding this comment

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

not sure I understand the comment correctly. "syscalls can access" - syscalls execute in kernel mode, so they can access everything?

Copy link
Copy Markdown
Collaborator Author

@softwarecki softwarecki Mar 24, 2026

Choose a reason for hiding this comment

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

Yes, syscalls executes in kernel mode but it still use userspace module memory domain

Copy link
Copy Markdown
Collaborator

@lyakh lyakh Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, syscalls executes in kernel mode but it still use userspace module memory domain

@softwarecki do you mean that when a syscall is executed, it inherits page tables from the userspace context?

Copy link
Copy Markdown
Collaborator Author

@softwarecki softwarecki Mar 27, 2026

Choose a reason for hiding this comment

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

@lyakh Exactly, yes. That is why, when a new memory domain is created, the L2 page tables are copied. This provides access for kernel to selected memory regions.

@dcpleung Can you confirm?

Copy link
Copy Markdown
Contributor

@dcpleung dcpleung Mar 27, 2026

Choose a reason for hiding this comment

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

A new memory domain is created by copying the kernel pagetables. So during syscall, the memory can be accessed if it is accessible via kernel threads. If you have already mapped the heap area at boot as readable by kernel threads, syscall can use it.

Copy link
Copy Markdown
Collaborator Author

@softwarecki softwarecki Mar 27, 2026

Choose a reason for hiding this comment

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

However, if the memory was mapped dynamically at runtime, it will not be present in the new domain and must be explicitly mapped there as well.

Copy link
Copy Markdown
Contributor

@dcpleung dcpleung Mar 27, 2026

Choose a reason for hiding this comment

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

True, if you allocate memory, you will need to explicitly add it via memory partition to a memory domain before domain associated user threads can access them. Kernel threads and syscalls depend on whether that memory block has been mapped at boot or via k_mem_map(). If not, you definitely need to add it to the domain or else it cannot be accessed in kernel mode.

*/
struct k_mem_partition heap_struct_part;
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Mar 25, 2026

Choose a reason for hiding this comment

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

this creates an uninitialised object on stack. Below all its fields are set individually, but if the structure changes and fields are added to it, they will end up uninitialised here. Doing something like

struct k_mem_partition heap_struct_part = {
    .attr = K_MEM_PARTITION_P_RW_U_NA | user_get_partition_attr(drv->user_heap),
};

would be safer.


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);
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Mar 24, 2026

Choose a reason for hiding this comment

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

does this mean that heap_struct_part can be unaligned? If so, wouldn't this then make some random adjacent memory user-accessible?

Copy link
Copy Markdown
Collaborator Author

@softwarecki softwarecki Mar 24, 2026

Choose a reason for hiding this comment

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

Its grants access for kernel mode only.

heap_struct_part.attr = K_MEM_PARTITION_P_RW_U_NA |
user_get_partition_attr(heap_struct_part.start);
Copy link
Copy Markdown
Contributor

@wjablon1 wjablon1 Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

How about: user_get_partition_attr -> user_get_cache_attr OR user_get_cache_part_attr?


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;
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Mar 24, 2026

Choose a reason for hiding this comment

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

unrelated to this PR, but you might want to revisit your memory domain life cycle. Currently memory domain destruction isn't really supported by the Zephyr version, that SOF is linking to. Recently 3032b58f52d776296a6e7e9c1a9c44b87f3cf019 has been merged into Zephyr, which makes it possible, but even with that domains have to be freed explicitly


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);
Copy link
Copy Markdown
Contributor

@wjablon1 wjablon1 Mar 25, 2026

Choose a reason for hiding this comment

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

does llext_manager_add_domain really add a domain to something (thread i guess?)? If not, I encourage you to change the function name to *prep_domain or *init_domain. The previous use case for llext_manager_add_domain in scheduler_dp_task_init is straightforward whereas this use case is much harder to understand. A better name would greatly improve readability.

Copy link
Copy Markdown
Collaborator Author

@softwarecki softwarecki Mar 27, 2026

Choose a reason for hiding this comment

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

The llext_manager_add_domain() adds a llext module memory partitions to a given domain (second argument).

Copy link
Copy Markdown
Contributor

@wjablon1 wjablon1 Mar 27, 2026

Choose a reason for hiding this comment

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

So it doesn't add a domain itself, i.e. a domain remains unassigned. This caused my confusion and I started to wonder why the domain isn't added for the non-llext case. I suspected a bug, but then I figured out that this function only adds partitions, but the domain itself is assigned to the thread in another place.

Copy link
Copy Markdown
Collaborator Author

@softwarecki softwarecki Mar 27, 2026

Choose a reason for hiding this comment

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

Exactly, the memory domain is a separate entity. Memory partitions and threads are added to a domain. Multiple threads can be assigned to the same memory domain using the k_mem_domain_add_thread() and share its mappings.


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;
Expand Down
4 changes: 3 additions & 1 deletion src/include/module/module/llext.h
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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