-
Notifications
You must be signed in to change notification settings - Fork 349
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
c51277d
2da0845
7fa81f8
761633a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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> | ||||||||||||||||||||||
|
|
@@ -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; | ||||||||||||||||||||||
|
|
@@ -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 | | ||||||||||||||||||||||
| user_get_partition_attr(heap_struct_part.start); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| tr_err(&userspace_proxy_tr, "Heap struct partition %#lx + %zx, attr = %u", | ||||||||||||||||||||||
|
||||||||||||||||||||||
| 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
AI
Mar 23, 2026
There was a problem hiding this comment.
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
AI
Mar 23, 2026
There was a problem hiding this comment.
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.
| 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; | |
| } |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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 */ | ||||||||||
|
|
@@ -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) { | ||||||||||
|
||||||||||
| 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))) { |
There was a problem hiding this comment.
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 withU_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_ROorK_MEM_PARTITION_P_RW_U_RW, depending on the expected access pattern). WithU_NA, userspace likely cannot legally reference thek_heapobject.