userspace: proxy: Introduce user worker to service IPC #10433
Conversation
Update variable names to use user_ctx instead of user to improve clarity. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Change lib_manager_start_agent signature to accept a config object instead of component_id. Get core id from the ipc configuration and pass it to the system agent at startup. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
…tition Relocate specific variables to a memory partition accessible from userspace to enable running the system agent in userspace. This change ensures that only the required structures is accessible for userspace modules, while privileged data remains in kernel-only memory. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
softwarecki
requested review from
abonislawski,
serhiy-katsyuba-intel,
tmleman and
wjablon1
The non-privileged modules code should be executed in separate thread executed in userspace mode. This way the code and data owned by such module will be isolated from other non-privileged modules in the system. The implementation creates user work queue thread that will service all IPC's for all such modules. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com> Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>
softwarecki
force-pushed
the
userspace-ipc
branch
from
0b957d0 to
7bfaaae
Compare
softwarecki
requested review from
dbaluta,
jxstelter,
kv2019i,
lbetlej,
lgirdwood,
mmaka1,
plbossart and
ranj063
as code owners
There was a problem hiding this comment.
Pull request overview
This PR introduces a user worker thread to service IPC requests for non-privileged modules, enabling execution of module code in userspace mode for isolation. The implementation creates a shared user work queue thread that handles all IPC operations for userspace modules.
Key changes include:
- Modified
lib_manager_start_agentto accept aconfigobject to extract core ID from IPC configuration - Created user worker thread infrastructure with work queue and event handling for IPC processing
- Relocated system service structures to userspace-accessible memory partitions using
APP_TASK_DATA
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/library_manager/lib_manager.c | Updated function signature to use config object and extract core ID from IPC config |
| src/include/sof/schedule/dp_schedule.h | Added IPC completion event flag for worker synchronization |
| src/include/sof/audio/module_adapter/library/userspace_proxy_user.h | New header defining userspace proxy commands, parameters, and work item structures |
| src/include/sof/audio/module_adapter/library/userspace_proxy.h | Added work item field to userspace context structure |
| src/audio/module_adapter/library/userspace_proxy_user.c | New implementation file containing userspace-executed IPC request handlers |
| src/audio/module_adapter/library/userspace_proxy.c | Refactored to route module operations through user worker with memory domain management |
| src/audio/module_adapter/library/native_system_service.c | Relocated service structure to userspace-accessible memory |
| src/audio/module_adapter/iadk/system_agent.cpp | Relocated system service structure to userspace-accessible memory |
| src/audio/module_adapter/CMakeLists.txt | Added new userspace_proxy_user.c source file to build |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Mostly minor opens and questions.
|
|
||
| static const struct module_interface userspace_proxy_interface; | ||
|
|
||
| struct user_worker { |
There was a problem hiding this comment.
Its probably worth having some comments that describe the transaction flow between threads.
|
|
||
| static int user_worker_create(void) | ||
| { | ||
| if (worker.reference_count) { |
There was a problem hiding this comment.
Is this number of IPC clients per worker thread ?
| return 0; | ||
| } | ||
|
|
||
| worker.stack_ptr = user_stack_allocate(CONFIG_SOF_STACK_SIZE, K_USER); |
There was a problem hiding this comment.
I think this size is OK now, but we could have a separate Kconfig option for user worker thread.
| } | ||
|
|
||
| k_event_init(&worker.event); | ||
| k_work_user_queue_start(&worker.work_queue, worker.stack_ptr, CONFIG_SOF_STACK_SIZE, 0, |
There was a problem hiding this comment.
Ditto for size, these are probably tune-able to save SRAM.
|
|
||
| k_thread_access_grant(worker.thread_id, &worker.event); | ||
|
|
||
| worker.reference_count++; |
There was a problem hiding this comment.
Does this need to be atomic ? i.e. will it be multicore.
| #endif | ||
|
|
||
| struct k_mem_partition *parts_ptr[] = { | ||
| &common_partition, |
| struct module_params *params = user_work_get_params(mod->user_ctx); | ||
| struct k_mem_domain *domain = mod->user_ctx->comp_dom; | ||
| struct k_mem_partition ipc_resp_part = { | ||
| .start = (uintptr_t)ipc_get()->comp_data, |
There was a problem hiding this comment.
do we need to validate this, i.e. is any part of this passed in from host
| const struct module_interface *ops = params->context->interface; | ||
|
|
||
| switch (params->cmd) { | ||
| case MODULE_CMD_AGENT_START: |
There was a problem hiding this comment.
I assume of these ops are mandatory ops, i.e. no need to check them before use ?
| }; | ||
|
|
||
| enum userspace_proxy_cmd { | ||
| MODULE_CMD_AGENT_START, |
There was a problem hiding this comment.
Can we include USER_PROXY somewhere in the enum value names here to align with the usage and enum definition name as the current naming is quite generic and could collide with other legitimate usages.
| int status; | ||
| struct processing_module *mod; | ||
| struct userspace_context *context; | ||
| union { |
There was a problem hiding this comment.
I guess the type we are using is encoded in cmd ?
3 participants
The non-privileged modules code should be executed in separate thread executed in userspace mode. This way the code and data owned by such module will be isolated from other non-privileged modules in the system.
The implementation creates user work queue thread that will service all IPC's for all such modules.
Change
lib_manager_start_agentsignature to accept aconfigobject instead ofcomponent_id. Get core id from the ipc configuration and pass it to the system agent at startup.Relocate specific variables to a memory partition accessible from userspace to enable running the system agent in userspace. This change ensures that only the required structures is accessible for userspace modules, while privileged data remains in kernel-only memory.