-
Notifications
You must be signed in to change notification settings - Fork 350
userspace: proxy: Introduce user worker to service IPC #10433
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?
Conversation
0b957d0 to
7bfaaae
Compare
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.
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
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.
Mostly minor opens and questions.
src/include/sof/audio/module_adapter/library/userspace_proxy_user.h
Outdated
Show resolved
Hide resolved
tmleman
left a comment
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.
In my opinion, the code looks good. After addressing all comments and resolving CI issues, it will be ready to merge.
7bfaaae to
4b1124b
Compare
1199ee5 to
1f3de00
Compare
1f3de00 to
b975511
Compare
tmleman
left a comment
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.
My comments have been addressed, I don't have any new ones.
Only the issue of failing CI.
lgirdwood
left a comment
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.
Some opens, main one is about how we do mailbox IO for user DP.
kv2019i
left a comment
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.
A few questions inline, but no showstoppers.
| struct module_large_cfg_set_params set_conf; | ||
| struct module_large_cfg_get_params get_conf; | ||
| struct module_processing_mode_params proc_mode; | ||
| struct module_process_params proc; |
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.
This is now specific to IPC4 (while e.g. sof/src/include/sof/audio/module_adapter/module/generic.h still has support for both IPC3 and IPC4). Do you have some Kconfig depenceny or other dependency to ensure this does not interfere with IPC3 builds? And maybe some note this is IPC4 only until someone adds the missing bits.
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.
Honestly, I don't understand the question. struct module_interface does not change depending on ipc4 or ipc3. These structures are only used to pass parameters to functions exposed by the module via its module_interface.
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>
Add function user_get_partition_attr() that returns the appropriate memory partition attribute based on the address. The function returns attribute XTENSA_MMU_CACHED_WB for cacheable addresses so that user space and kernel space threads access memory in a consistent way. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
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>
Add include that contains the definition of k_tid_t used by the user_access_to_mailbox function's parameter. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
b975511 to
0670719
Compare
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.