-
Notifications
You must be signed in to change notification settings - Fork 351
userspace: dp: Add option for executing userspace module IPC in DP thread #10579
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
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 | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -49,6 +49,33 @@ DECLARE_TR_CTX(userspace_proxy_tr, SOF_UUID(userspace_proxy_uuid), LOG_LEVEL_INF | |||||||||||||||||
|
|
||||||||||||||||||
| static const struct module_interface userspace_proxy_interface; | ||||||||||||||||||
|
|
||||||||||||||||||
| #if IS_ENABLED(CONFIG_SOF_USERSPACE_MOD_IPC_BY_DP_THREAD) | ||||||||||||||||||
| #include <sof/audio/module_adapter/iadk/system_agent.h> | ||||||||||||||||||
| #include <sof/schedule/dp_schedule.h> | ||||||||||||||||||
|
|
||||||||||||||||||
| static inline int user_worker_get(void) | ||||||||||||||||||
| { | ||||||||||||||||||
| return 0; | ||||||||||||||||||
| } | ||||||||||||||||||
| static inline void user_worker_put(void) { } | ||||||||||||||||||
|
|
||||||||||||||||||
| struct k_work_user *userspace_proxy_register_ipc_handler(struct processing_module *mod, | ||||||||||||||||||
| struct k_event *event) | ||||||||||||||||||
| { | ||||||||||||||||||
| struct userspace_context * const user_ctx = mod->user_ctx; | ||||||||||||||||||
| if (user_ctx) { | ||||||||||||||||||
| tr_dbg(&userspace_proxy_tr, "Set DP event %p for module %p", | ||||||||||||||||||
| (void *)event, (void *)mod); | ||||||||||||||||||
| user_ctx->dp_event = event; | ||||||||||||||||||
| user_ctx->work_item->event = event; | ||||||||||||||||||
|
|
||||||||||||||||||
| assert(user_ctx->work_item); | ||||||||||||||||||
| return &user_ctx->work_item->work_item; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return NULL; | ||||||||||||||||||
| } | ||||||||||||||||||
| #else | ||||||||||||||||||
| /* IPC requests targeting userspace modules are handled through a user work queue. | ||||||||||||||||||
| * Each userspace module provides its own work item that carries the IPC request parameters. | ||||||||||||||||||
| * The worker thread is switched into the module's memory domain and receives the work item. | ||||||||||||||||||
|
|
@@ -106,6 +133,7 @@ static void user_worker_put(void) | |||||||||||||||||
| user_stack_free(worker.stack_ptr); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| #endif | ||||||||||||||||||
|
|
||||||||||||||||||
| static int user_work_item_init(struct userspace_context *user_ctx, struct k_heap *user_heap) | ||||||||||||||||||
| { | ||||||||||||||||||
|
|
@@ -128,7 +156,9 @@ static int user_work_item_init(struct userspace_context *user_ctx, struct k_heap | |||||||||||||||||
|
|
||||||||||||||||||
| k_work_user_init(&work_item->work_item, userspace_proxy_worker_handler); | ||||||||||||||||||
|
|
||||||||||||||||||
| #if !IS_ENABLED(CONFIG_SOF_USERSPACE_MOD_IPC_BY_DP_THREAD) | ||||||||||||||||||
| work_item->event = &worker.event; | ||||||||||||||||||
| #endif | ||||||||||||||||||
| work_item->params.context = user_ctx; | ||||||||||||||||||
| user_ctx->work_item = work_item; | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -155,14 +185,19 @@ BUILD_ASSERT(IS_ALIGNED(MAILBOX_HOSTBOX_SIZE, CONFIG_MMU_PAGE_SIZE), | |||||||||||||||||
| static int userspace_proxy_invoke(struct userspace_context *user_ctx, uint32_t cmd, | ||||||||||||||||||
| bool ipc_payload_access) | ||||||||||||||||||
| { | ||||||||||||||||||
| #if IS_ENABLED(CONFIG_SOF_USERSPACE_MOD_IPC_BY_DP_THREAD) | ||||||||||||||||||
| struct k_event * const event = user_ctx->dp_event; | ||||||||||||||||||
|
||||||||||||||||||
| struct k_event * const event = user_ctx->dp_event; | |
| struct k_event * const event = user_ctx->dp_event; | |
| if (!event) { | |
| tr_err(&userspace_proxy_tr, | |
| "userspace_proxy_invoke(): DP event not registered"); | |
| return -EINVAL; | |
| } |
Copilot
AI
Feb 25, 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.
With CONFIG_SOF_USERSPACE_MOD_IPC_BY_DP_THREAD enabled, userspace_proxy_start_agent() no longer invokes start_fn and does not populate *agent_interface. This breaks the LMDK path where native_system_agent_start() is what initializes ops (via agent_iface = (const void **)&ops), and ops is used immediately after userspace_proxy_create() returns. Either keep starting the agent here for cases that must produce ops/agent_interface, or provide an alternative initialization path that guarantees ops is valid before returning from userspace_proxy_create().
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1432,23 +1432,24 @@ void module_adapter_free(struct comp_dev *dev) | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| comp_dbg(dev, "start"); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (dev->task) { | ||||||||||||||||||||||||||
| #if CONFIG_SOF_USERSPACE_APPLICATION | ||||||||||||||||||||||||||
| if (dev->task) | ||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||
| * Run DP module's .free() method in its thread context. | ||||||||||||||||||||||||||
| * Unlike with other IPCs we first run module's .free() in | ||||||||||||||||||||||||||
| * thread context, then cancel the thread, and then execute | ||||||||||||||||||||||||||
| * final clean up | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| #if CONFIG_SOF_USERSPACE_APPLICATION | ||||||||||||||||||||||||||
| scheduler_dp_thread_ipc(mod, SOF_IPC4_MOD_DELETE_INSTANCE, NULL); | ||||||||||||||||||||||||||
| #endif | ||||||||||||||||||||||||||
| schedule_task_free(dev->task); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ret = module_free(mod); | ||||||||||||||||||||||||||
| if (ret) | ||||||||||||||||||||||||||
| comp_err(dev, "failed with error: %d", ret); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (dev->task) | ||||||||||||||||||||||||||
| schedule_task_free(dev->task); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
1446
to
+1452
|
||||||||||||||||||||||||||
| ret = module_free(mod); | |
| if (ret) | |
| comp_err(dev, "failed with error: %d", ret); | |
| if (dev->task) | |
| schedule_task_free(dev->task); | |
| if (dev->task) | |
| schedule_task_free(dev->task); | |
| ret = module_free(mod); | |
| if (ret) | |
| comp_err(dev, "failed with error: %d", ret); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,12 +3,14 @@ | |||||||||||||||
| * Copyright(c) 2025 Intel Corporation. All rights reserved. | ||||||||||||||||
| * | ||||||||||||||||
| * Author: Marcin Szkudlinski | ||||||||||||||||
| * Adrian Warecki | ||||||||||||||||
| */ | ||||||||||||||||
|
|
||||||||||||||||
| #include <rtos/task.h> | ||||||||||||||||
|
|
||||||||||||||||
| #include <sof/audio/module_adapter/module/generic.h> | ||||||||||||||||
| #include <sof/audio/module_adapter/library/userspace_proxy.h> | ||||||||||||||||
| #include <sof/audio/module_adapter/library/userspace_proxy_user.h> | ||||||||||||||||
| #include <sof/common.h> | ||||||||||||||||
| #include <sof/list.h> | ||||||||||||||||
| #include <sof/schedule/ll_schedule_domain.h> | ||||||||||||||||
|
|
@@ -115,6 +117,7 @@ void dp_thread_fn(void *p1, void *p2, void *p3) | |||||||||||||||
| unsigned int lock_key; | ||||||||||||||||
| enum task_state state; | ||||||||||||||||
| bool task_stop; | ||||||||||||||||
| uint32_t event; | ||||||||||||||||
|
|
||||||||||||||||
|
Comment on lines
119
to
121
|
||||||||||||||||
| if (!(task->flags & K_USER)) | ||||||||||||||||
| dp_sch = scheduler_get_data(SOF_SCHEDULE_DP); | ||||||||||||||||
|
|
@@ -124,50 +127,62 @@ void dp_thread_fn(void *p1, void *p2, void *p3) | |||||||||||||||
| * the thread is started immediately after creation, it will stop on event. | ||||||||||||||||
| * Event will be signalled once the task is ready to process. | ||||||||||||||||
| */ | ||||||||||||||||
| k_event_wait_safe(task_pdata->event, DP_TASK_EVENT_PROCESS | DP_TASK_EVENT_CANCEL, | ||||||||||||||||
| false, K_FOREVER); | ||||||||||||||||
| event = k_event_wait_safe(task_pdata->event, DP_TASK_EVENT_PROCESS | | ||||||||||||||||
| DP_TASK_EVENT_CANCEL | DP_TASK_EVENT_IPC, false, | ||||||||||||||||
| K_FOREVER); | ||||||||||||||||
|
|
||||||||||||||||
| #if IS_ENABLED(CONFIG_SOF_USERSPACE_MOD_IPC_BY_DP_THREAD) | ||||||||||||||||
| if (event & DP_TASK_EVENT_IPC) { | ||||||||||||||||
| assert(task_pdata->ipc_work_item); | ||||||||||||||||
| userspace_proxy_worker_handler(task_pdata->ipc_work_item); | ||||||||||||||||
| } | ||||||||||||||||
| #endif | ||||||||||||||||
|
|
||||||||||||||||
| if (task->state == SOF_TASK_STATE_RUNNING) | ||||||||||||||||
| state = task_run(task); | ||||||||||||||||
| else | ||||||||||||||||
| if (event & DP_TASK_EVENT_PROCESS) { | ||||||||||||||||
| state = task->state; /* to avoid undefined variable warning */ | ||||||||||||||||
| if (task->state == SOF_TASK_STATE_RUNNING && event & DP_TASK_EVENT_PROCESS) | ||||||||||||||||
| state = task_run(task); | ||||||||||||||||
|
|
||||||||||||||||
| lock_key = scheduler_dp_lock(task->core); | ||||||||||||||||
| /* | ||||||||||||||||
| * check if task is still running, may have been canceled by external call | ||||||||||||||||
| * if not, set the state returned by run procedure | ||||||||||||||||
| */ | ||||||||||||||||
| if (task->state == SOF_TASK_STATE_RUNNING) { | ||||||||||||||||
| task->state = state; | ||||||||||||||||
| switch (state) { | ||||||||||||||||
| case SOF_TASK_STATE_RESCHEDULE: | ||||||||||||||||
| /* mark to reschedule, schedule time is already calculated */ | ||||||||||||||||
| task->state = SOF_TASK_STATE_QUEUED; | ||||||||||||||||
| break; | ||||||||||||||||
|
|
||||||||||||||||
| case SOF_TASK_STATE_CANCEL: | ||||||||||||||||
| case SOF_TASK_STATE_COMPLETED: | ||||||||||||||||
| /* remove from scheduling */ | ||||||||||||||||
| list_item_del(&task->list); | ||||||||||||||||
| break; | ||||||||||||||||
|
|
||||||||||||||||
| default: | ||||||||||||||||
| /* illegal state, serious defect, won't happen */ | ||||||||||||||||
| k_panic(); | ||||||||||||||||
| lock_key = scheduler_dp_lock(task->core); | ||||||||||||||||
| /* | ||||||||||||||||
| * check if task is still running, may have been canceled by external call | ||||||||||||||||
| * if not, set the state returned by run procedure | ||||||||||||||||
| */ | ||||||||||||||||
| if (task->state == SOF_TASK_STATE_RUNNING) { | ||||||||||||||||
| task->state = state; | ||||||||||||||||
| switch (state) { | ||||||||||||||||
| case SOF_TASK_STATE_RESCHEDULE: | ||||||||||||||||
| /* mark to reschedule, schedule time is already calculated */ | ||||||||||||||||
| task->state = SOF_TASK_STATE_QUEUED; | ||||||||||||||||
| break; | ||||||||||||||||
|
|
||||||||||||||||
| case SOF_TASK_STATE_CANCEL: | ||||||||||||||||
| case SOF_TASK_STATE_COMPLETED: | ||||||||||||||||
| /* remove from scheduling */ | ||||||||||||||||
| list_item_del(&task->list); | ||||||||||||||||
| break; | ||||||||||||||||
|
|
||||||||||||||||
| default: | ||||||||||||||||
| /* illegal state, serious defect, won't happen */ | ||||||||||||||||
| k_panic(); | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /* if true exit the while loop, terminate the thread */ | ||||||||||||||||
| task_stop = task->state == SOF_TASK_STATE_COMPLETED || | ||||||||||||||||
| task->state == SOF_TASK_STATE_CANCEL; | ||||||||||||||||
| /* recalculate all DP tasks readiness and deadlines | ||||||||||||||||
| * TODO: it should be for all tasks, for all cores | ||||||||||||||||
| * currently its limited to current core only | ||||||||||||||||
| */ | ||||||||||||||||
| if (dp_sch) | ||||||||||||||||
| scheduler_dp_recalculate(dp_sch, false); | ||||||||||||||||
| /* if true exit the while loop, terminate the thread */ | ||||||||||||||||
| task_stop = task->state == SOF_TASK_STATE_COMPLETED || | ||||||||||||||||
| task->state == SOF_TASK_STATE_CANCEL; | ||||||||||||||||
| /* recalculate all DP tasks readiness and deadlines | ||||||||||||||||
| * TODO: it should be for all tasks, for all cores | ||||||||||||||||
| * currently its limited to current core only | ||||||||||||||||
| */ | ||||||||||||||||
| if (dp_sch) | ||||||||||||||||
| scheduler_dp_recalculate(dp_sch, false); | ||||||||||||||||
|
|
||||||||||||||||
| scheduler_dp_unlock(lock_key); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| scheduler_dp_unlock(lock_key); | ||||||||||||||||
| if (event & DP_TASK_EVENT_CANCEL) | ||||||||||||||||
| task_stop = true; | ||||||||||||||||
| } while (!task_stop); | ||||||||||||||||
|
|
||||||||||||||||
| /* call task_complete */ | ||||||||||||||||
|
|
@@ -294,6 +309,10 @@ int scheduler_dp_task_init(struct task **task, | |||||||||||||||
| k_event_init(pdata->event); | ||||||||||||||||
| k_thread_start(pdata->thread_id); | ||||||||||||||||
|
|
||||||||||||||||
| #if IS_ENABLED(CONFIG_SOF_USERSPACE_MOD_IPC_BY_DP_THREAD) | ||||||||||||||||
| pdata->ipc_work_item = userspace_proxy_register_ipc_handler(mod, pdata->event); | ||||||||||||||||
|
||||||||||||||||
| pdata->ipc_work_item = userspace_proxy_register_ipc_handler(mod, pdata->event); | |
| pdata->ipc_work_item = userspace_proxy_register_ipc_handler(mod, pdata->event); | |
| if (!pdata->ipc_work_item) { | |
| tr_err(&dp_tr, "userspace_proxy_register_ipc_handler() failed"); | |
| ret = -ENOMEM; | |
| goto e_thread; | |
| } |
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.
userspace_proxy_register_ipc_handler()dereferencesuser_ctx->work_itembefore validating it (user_ctx->work_item->event = event;happens beforeassert(user_ctx->work_item)). This can NULL-deref if work item allocation/init failed. Assert/checkuser_ctx->work_itembefore using it and return NULL/error if missing.