[userspace LL] scheduling: LLEXT, DP & multicore - #10945
Conversation
a462fcf to
9f6770d
Compare
|
After multiple PRs got merged we're down from 108 commits to 32 |
|
let me mark it "ready" to have some on-device testing |
There was a problem hiding this comment.
Pull request overview
This PR is a WiP/PoC that extends the SOF Zephyr userspace low-latency (LL) work to support LLEXT and multi-core operation, including new syscalls and userspace IPC/LL thread infrastructure per core.
Changes:
- Adds a userspace-accessible vregion syscall interface and vregion-to-mem-domain mapping support.
- Introduces per-core userspace IPC threads and secondary-core initialization plumbing for userspace LL.
- Refactors DP/LL scheduler interactions for multicore and userspace execution contexts, plus library manager changes to support userspace/module-loading flow.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| zephyr/syscall/vregion.c | Adds Zephyr syscall verifiers/marshalling for vregion operations. |
| zephyr/lib/vregion.c | Implements z_impl_* syscall backends, vregion mapping into a mem domain, and userspace verification. |
| zephyr/lib/userspace_helper.c | Adjusts mailbox partition mapping logic for IPC4 userspace access. |
| zephyr/Kconfig | Adds/selects userspace vregion interface Kconfig option. |
| zephyr/CMakeLists.txt | Registers new syscall headers/sources for vregion and other headers needed by syscalls. |
| uuid-registry.txt | Adds UUID entry used for secondary-core init task. |
| src/schedule/zephyr_ll.c | Updates LL tick path and scheduler data source for userspace LL. |
| src/schedule/zephyr_dp_schedule.h | Removes internal prototypes no longer intended for this header. |
| src/schedule/zephyr_dp_schedule.c | Converts scheduler_dp_ll_tick to a syscall-capable z_impl/z_vrfy API; uses userspace scheduler data. |
| src/schedule/zephyr_dp_schedule_application.c | Updates DP IPC paths and adds syscall verifier for DP internal free. |
| src/schedule/zephyr_domain.c | Adds helper to retrieve the LL domain thread for current core. |
| src/library_manager/llext_manager.c | Changes userspace LL domain add/remove behavior for LLEXT mappings. |
| src/library_manager/lib_manager.c | Refactors module creation to a reusable helper and adds syscall for module free; adjusts userspace/module interactions. |
| src/ipc/ipc4/helper.c | Adds helper to pre-load modules in kernel context for userspace LL flows. |
| src/ipc/ipc4/handler-user.c | Routes more IPC4 messages to per-core userspace handling and adjusts init path for LL vs DP. |
| src/ipc/ipc-common.c | Implements per-core userspace IPC threads, per-core stacks/events/threads, and secondary-core userspace init. |
| src/init/init.c | Hooks secondary-core init to start userspace IPC/LL infrastructure when enabled. |
| src/include/sof/schedule/schedule.h | Adds scheduler_list_get_data() and scheduler_get_user_data() helper for userspace schedulers. |
| src/include/sof/schedule/ll_schedule_domain.h | Exposes zephyr_ll_domain_thread(). |
| src/include/sof/schedule/dp_schedule.h | Exposes scheduler_dp_ll_tick and scheduler_dp_internal_free as syscalls for Zephyr full-app builds. |
| src/include/sof/lib/vregion.h | Exposes vregion syscalls (create_map/get/put/alloc/free/set_interim) and verification helper. |
| src/include/sof/lib_manager.h | Exposes lib_manager_mod_create_priv() and lib_manager_free_module syscall/z_impl mapping. |
| src/include/sof/ipc/common.h | Extends ipc_user struct for per-core threads/events/audio threads; updates ipc_user_forward_cmd signature; declares secondary init helpers. |
| src/include/ipc4/handler.h | Declares ipc4_user_module_load(). |
| src/audio/pipeline/pipeline-graph.c | Adds LL domain include needed by userspace LL changes. |
| src/audio/mux/mux_ipc4.c | Tightens IPC4 mux blob size validation. |
| src/audio/module_adapter/module_adapter.c | Switches DP module heap to vregion_create_map() under userspace constraints. |
| src/audio/buffers/comp_buffer.c | Fixes alloc context free path when vregion-backed allocations are used. |
| app/overlays/ptl/ll_userspace_overlay.conf | Updates overlay config for the evolving userspace LL setup. |
Suppressed comments (2)
zephyr/lib/vregion.c:279
- vregion_unmap(vr) runs after vpage_free(vr->base). This leaves a window where userspace can still access the partition while the backing memory is already freed/reusable, and unmapping may also rely on the region still being valid. Unmap partitions before freeing the backing pages.
LOG_DBG(" lifetime used %zu free count %d", vr->lifetime.used, vr->lifetime.free_count);
vpage_free(vr->base);
vregion_unmap(vr);
rfree(vr);
src/ipc/ipc-common.c:342
- ipc_user_forward_cmd() uses 'core' to index init_needed[] and event[] with no bounds checking. core ultimately comes from IPC fields and can be out of range, leading to out-of-bounds access and memory corruption.
int ipc_user_forward_cmd(uint32_t primary, uint32_t extension, unsigned int core)
{
struct ipc *ipc = ipc_get();
struct ipc_user *pdata = ipc->ipc_user_pdata;
k_spinlock_key_t key;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| struct k_mem_domain *domain = zephyr_ll_mem_domain(); | ||
| struct k_mem_partition part = { | ||
| .start = *vreg_start, | ||
| .size = *vreg_size, | ||
| .attr = K_MEM_PARTITION_P_RW_U_RW | XTENSA_MMU_CACHED_WB, | ||
| }; | ||
| int ret = k_mem_domain_add_partition(domain, &part); | ||
|
|
||
| if (ret < 0) { | ||
| vregion_put(vr); | ||
| return NULL; | ||
| } | ||
|
|
||
| part.start = (uintptr_t)sys_cache_uncached_ptr_get((void *)part.start); | ||
| part.attr = K_MEM_PARTITION_P_RW_U_RW; | ||
|
|
||
| ret = k_mem_domain_add_partition(domain, &part); | ||
| if (ret < 0) { | ||
| vregion_put(vr); | ||
| return NULL; | ||
| } | ||
|
|
||
| vr->domain = domain; |
|
|
||
| size_t vr_size = 0; | ||
| uintptr_t vr_start; | ||
|
|
||
| vregion_mem_info(vr, &vr_size, &vr_start); |
| struct vregion *z_vrfy_vregion_create_map(uintptr_t *vreg_start, size_t *vreg_size) | ||
| { | ||
| K_OOPS(K_SYSCALL_MEMORY_WRITE(vreg_start, sizeof(*vreg_start))); | ||
| K_OOPS(K_SYSCALL_MEMORY_WRITE(vreg_size, sizeof(*vreg_size))); | ||
| return z_impl_vregion_create_map(vreg_start, vreg_size); | ||
| } |
e62b4cd to
69f09d7
Compare
PR 10945: test resultsRun date: 2026-09-01 12:50 UTC Tested commit: 34ad06a7b887f838b9b15df1590facb835519e87 |
Extract a privileged LLEXT-related part from lib_manager_module_create() into a separate function to be called from kernel context. At the same time lib_manager_mod_free_priv() already executes privileged operations; to make it callable in userspace, convert lib_manager_free_module() to a system call. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
If LL runs in userspace, it needs access to loaded LLEXT modules, running in DP more too. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Make scheduling LL thread and synchronisation objects per-core and forward IPCs and scheduling events accordingly. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Checking pointer for non-NULL after dereferencing it makes no sense. Swap the order. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
In case of userspace LL scheduling the (also userspace) IPC thread needs access rights to DP assets like the thread itself and its stack and synchronisation primitives. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
When running in syscall context on behalf of a userspace thread dynamically mapped memory doesn't automatically become accessible. To make it accessible it has to be added to the thread memory domain. This is a problem for loadable modules with executable cold sections. To be able to execute them they have to be mapped to threads with the executable bit set. While for linking that memory has to be mapped writable. To solve the problem we perform linking from the kernel IPC context before forwarding to the userspace IPC thread. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Re-enable LLEXT, DRAM execution and data, the DP scheduler and boot time testing with userspace LL with a small restriction for the latter - userspace allocation failure tests aren't currently working. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
There was a problem hiding this comment.
🟡 Changes recommended
Confirmed issues include unsafe userspace LL domain mapping for DP modules and uninitialized/per-core IPC state leading to potential deadlocks or invalid memory access.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/ipc/ipc-common.c:619
- ipc_user is allocated with sof_heap_alloc() but never zero-initialized; init_needed[] and the per-core thread/event pointers start as uninitialized heap garbage. This can cause ipc_user_forward_cmd() to deadlock (spurious init_needed[core]) or dereference invalid per-core pointers.
struct ipc *ipc = ipc_get();
struct ipc_user *ipc_user = sof_heap_alloc(sof_sys_user_heap_get(),
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT,
sizeof(*ipc_user), 0);
int ret;
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
| @@ -431,13 +434,22 @@ int ipc_user_forward_cmd(uint32_t primary, uint32_t extension) | |||
| pdata->ipc_msg_ext = extension; | |||
| pdata->ipc = ipc; | |||
|
|
|||
| /* | |||
| * Forwarding the first IPC to this core, wait for its userspace IPC | |||
| * thread to start | |||
| */ | |||
| if (pdata->init_needed[core]) { | |||
| pdata->init_needed[core] = false; | |||
| k_sem_take(pdata->sem, K_FOREVER); | |||
| } | |||
| ret = llext_manager_add_mod_domain(mctx, zephyr_ll_mem_domain()); | ||
| if (ret < 0) { | ||
| tr_err(&lib_manager_tr, "failed to add domain: %d", ret); | ||
| goto e_data; | ||
| } |
kv2019i
left a comment
There was a problem hiding this comment.
Looks good, some minor comments and usual nagging about z_vrfy code ...
|
|
||
| static int z_vrfy_lib_manager_free_module(const uint32_t component_id) | ||
| { | ||
| return z_impl_lib_manager_free_module(component_id); |
There was a problem hiding this comment.
@lyakh We should have ability to limit which user-space threads can free modules. E.g. especially with non-LL-user builds, Dp modules should not be able to unload arbitrary components. I know we can mitigate this currently by enforcing signing on all loaded code, but we should at least have a TODO here, but ideally this would be only allowed from the LL thread handling user IPCs.
| #define IPC_TASK_POWERDOWN BIT(3) | ||
|
|
||
| #ifdef CONFIG_CORE_COUNT | ||
| #define CORE_COUNT CONFIG_CORE_COUNT |
There was a problem hiding this comment.
Isn't CORE_COUNT a bit generic (and short) definition in a public header file?
Most places in SOF just use CONFIG_CORE_COUNT (defined in sof/src/platform/Kconfig), so this should be available for all builds. Maybe "ifndef CONFIG_CORE_COUNT" and set it 1 for the failing builds (library, cmocka, ztests?)
| if (!ret) { | ||
| /* Wait for completion */ | ||
| ret = k_sem_take(&dp_sync[cpu_get_id()], DP_THREAD_IPC_TIMEOUT); | ||
| ret = k_sem_take(&dp_sync[core], DP_THREAD_IPC_TIMEOUT); |
There was a problem hiding this comment.
These changes in scheduler_dp_thread_ipc() seem unrelated to the commit....
| * That one calls lib_manager_mod_create_priv(), then | ||
| * lib_manager_allocate_module() and eventually | ||
| * llext_manager_allocate_module() for LLEXT modules. | ||
| */ |
There was a problem hiding this comment.
This comment is a bit hard to follow. What do you mean "IPC handling thead would call"? Would or will? As far as I understand, the user-space IPC thread (maybe mention ipc_user_thread_dispatch() explicitly) will call comp_new_ipc4_user(), and do the steps you describe above. This function is run before that (in kernel space). Right?
| userspace/ksem.c | ||
| ) | ||
| if(CONFIG_USERSPACE AND CONFIG_SOF_USERSPACE_INTERFACE_ALLOC) | ||
| if(CONFIG_USERSPACE AND CONFIG_SOF_USERSPACE_INTERFACE_ALLOC AND NOT CONFIG_SOF_USERSPACE_LL) |
This includes #10558 and my patches on top to enable LLEXT and multicore. Current status: passes simple tests with nocodec with both core 0 and core 1 streaming. 2 streams simultaneously run into a problem when the first of them terminates. WiP.
Update: now that #10558 has mostly been merged, this PR still carries remaining fixes for multicore, LLEXT, cold sections. At its peak this PR contained more than a 100 commits, as of today Aug 28th only 12 patches remain
Update: as of today Sep 2nd only 7 patches remain and this is now considered to be the final step (short of making userspace LL on by default on ACE3+)