diff --git a/src/schedule/zephyr_dp_schedule_application.c b/src/schedule/zephyr_dp_schedule_application.c index 1ce105392c2f..6c48aa6b5159 100644 --- a/src/schedule/zephyr_dp_schedule_application.c +++ b/src/schedule/zephyr_dp_schedule_application.c @@ -147,6 +147,58 @@ struct scheduler_dp_task_memory { struct ipc4_flat flat; }; +/* + * Kernel-only record of the kernel objects owned by a DP task. + * + * struct scheduler_dp_task_memory lives in the module's heap, which is mapped + * into the DP thread's memory domain, and is also writable by the LL and IPC + * user threads. Freeing must therefore never use the pointers stored there: + * scheduler_dp_internal_free() takes them from this record instead. The task + * pointer is only used as a lookup key. + */ +struct scheduler_dp_task_kobj { + struct list_item list; + struct task *task; + struct processing_module *mod; + struct k_event *event; + struct k_thread *thread; + struct k_mem_domain *mdom; + struct k_mem_partition mpart[SOF_DP_PART_TYPE_COUNT]; + uint32_t comp_id; +}; + +static struct list_item dp_task_kobj_list = LIST_INIT(dp_task_kobj_list); +static struct k_spinlock dp_task_kobj_lock; + +static void dp_task_kobj_add(struct scheduler_dp_task_kobj *kobj) +{ + k_spinlock_key_t key = k_spin_lock(&dp_task_kobj_lock); + + list_item_append(&kobj->list, &dp_task_kobj_list); + k_spin_unlock(&dp_task_kobj_lock, key); +} + +/* Find the record for @task and unlink it, so it can only be freed once */ +static struct scheduler_dp_task_kobj *dp_task_kobj_take(const struct task *task) +{ + struct scheduler_dp_task_kobj *kobj, *found = NULL; + k_spinlock_key_t key = k_spin_lock(&dp_task_kobj_lock); + struct list_item *item; + + list_for_item(item, &dp_task_kobj_list) { + kobj = container_of(item, struct scheduler_dp_task_kobj, list); + if (kobj->task == task) { + list_item_del(&kobj->list); + found = kobj; + break; + } + } + + k_spin_unlock(&dp_task_kobj_lock, key); + + return found; +} + /* Unpack IPC data and execute a callback */ static void ipc_thread_unflatten_run(struct processing_module *pmod, struct ipc4_flat *flat) { @@ -441,17 +493,15 @@ void dp_thread_fn(void *p1, void *p2, void *p3) * Safe to call with partial successful initialisation, * k_mem_domain_remove_partition() then just returns -ENOENT */ -static void scheduler_dp_domain_free(struct task_dp_pdata *pdata) +static void scheduler_dp_domain_free(struct processing_module *pmod, struct k_mem_domain *mdom, + struct k_mem_partition *mpart, uint32_t comp_id) { - struct processing_module *pmod = pdata->mod; - struct k_mem_domain *mdom = pmod->mdom; - - llext_manager_rm_domain(pmod->dev->ipc_config.id, mdom); + llext_manager_rm_domain(comp_id, mdom); - k_mem_domain_remove_partition(mdom, pdata->mpart + SOF_DP_PART_HEAP); - k_mem_domain_remove_partition(mdom, pdata->mpart + SOF_DP_PART_HEAP_CACHE); - k_mem_domain_remove_partition(mdom, pdata->mpart + SOF_DP_PART_CFG); - k_mem_domain_remove_partition(mdom, pdata->mpart + SOF_DP_PART_CFG_CACHE); + k_mem_domain_remove_partition(mdom, mpart + SOF_DP_PART_HEAP); + k_mem_domain_remove_partition(mdom, mpart + SOF_DP_PART_HEAP_CACHE); + k_mem_domain_remove_partition(mdom, mpart + SOF_DP_PART_CFG); + k_mem_domain_remove_partition(mdom, mpart + SOF_DP_PART_CFG_CACHE); if (mdom->num_partitions) { /* Violation: all partitions should be freed by now */ @@ -475,15 +525,34 @@ static void scheduler_dp_domain_free(struct task_dp_pdata *pdata) objpool_free(&dp_mdom_head, mdom); } -void z_impl_scheduler_dp_internal_free(struct task *task) +/* + * Free the kernel objects of @task, using only the kernel-only record. + * + * @return false if @task has no record (unknown task or already freed) + */ +static bool scheduler_dp_internal_free_kobj(struct task *task) { - struct task_dp_pdata *pdata = task->priv_data; + struct scheduler_dp_task_kobj *kobj = dp_task_kobj_take(task); - k_object_free(pdata->event); - k_object_free(pdata->thread); - scheduler_dp_domain_free(pdata); + if (!kobj) + return false; - mod_free(pdata->mod, container_of(task, struct scheduler_dp_task_memory, task)); + /* normally already terminated by scheduler_dp_task_free(), no-op then */ + k_thread_abort(kobj->thread); + k_object_free(kobj->event); + k_object_free(kobj->thread); + scheduler_dp_domain_free(kobj->mod, kobj->mdom, kobj->mpart, kobj->comp_id); + + mod_free(kobj->mod, container_of(task, struct scheduler_dp_task_memory, task)); + rfree(kobj); + + return true; +} + +void z_impl_scheduler_dp_internal_free(struct task *task) +{ + if (!scheduler_dp_internal_free_kobj(task)) + tr_err(&dp_tr, "unknown DP task %p", task); } #ifdef CONFIG_THREAD_NAME @@ -553,6 +622,14 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid, } struct task_dp_pdata *pdata = &task_memory->pdata; + struct scheduler_dp_task_kobj *kobj = rmalloc(SOF_MEM_FLAG_KERNEL | SOF_MEM_FLAG_COHERENT, + sizeof(*kobj)); + + if (!kobj) { + tr_err(&dp_tr, "task record alloc failed"); + ret = -ENOMEM; + goto e_stack; + } if (mod->dev->drv->adapter_ops->get_configuration) { pdata->ipc_config_data = mod_alloc_ext(mod, SOF_MEM_FLAG_USER, SOF_IPC_MSG_MAX_SIZE, @@ -560,7 +637,7 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid, if (!pdata->ipc_config_data) { tr_err(&dp_tr, "Failed to allocate memory for get-config"); ret = -ENOMEM; - goto e_stack; + goto e_kobj; } } else { pdata->ipc_config_data = NULL; @@ -688,6 +765,16 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid, goto e_dom; } + /* record the objects to free, before the thread can touch task_memory */ + kobj->task = ptask; + kobj->mod = mod; + kobj->event = pdata->event; + kobj->thread = pdata->thread; + kobj->mdom = mdom; + memcpy(kobj->mpart, pdata->mpart, sizeof(kobj->mpart)); + kobj->comp_id = mod->dev->ipc_config.id; + dp_task_kobj_add(kobj); + /* start the thread, it should immediately stop at the semaphore */ k_event_init(pdata->event); k_thread_start(pdata->thread_id); @@ -698,7 +785,7 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid, return 0; e_dom: - scheduler_dp_domain_free(pdata); + scheduler_dp_domain_free(mod, mdom, pdata->mpart, mod->dev->ipc_config.id); on_pool = false; e_thread: if (on_pool) @@ -709,6 +796,8 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid, k_object_free(pdata->event); e_config: mod_free(mod, pdata->ipc_config_data); +e_kobj: + rfree(kobj); e_stack: user_stack_free(p_stack); e_tmem: @@ -721,43 +810,13 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid, #ifdef CONFIG_USERSPACE #include -static void scheduler_dp_mod_vrfy(struct processing_module *mod) -{ - K_OOPS(K_SYSCALL_MEMORY_WRITE(mod, sizeof(*mod))); - K_OOPS(K_SYSCALL_MEMORY_WRITE(mod->dev, sizeof(*mod->dev))); - K_OOPS(K_SYSCALL_MEMORY_READ(mod->dev->drv, sizeof(*mod->dev->drv))); - - struct mod_alloc_ctx *alloc = mod->priv.resources.alloc; - - K_OOPS(K_SYSCALL_MEMORY_WRITE(alloc, sizeof(*alloc))); - if (alloc->heap) { - size_t h_size = 0; - uintptr_t h_start; - - mod_heap_info(mod, &h_size, &h_start); - if (h_size) - K_OOPS(K_SYSCALL_MEMORY_WRITE(h_start, h_size)); - } - if (alloc->vreg) { - size_t vr_size = 0; - uintptr_t vr_start; - - vregion_mem_info(alloc->vreg, &vr_size, &vr_start); - K_OOPS(K_SYSCALL_MEMORY_WRITE(vr_start, vr_size)); - } -} - void z_vrfy_scheduler_dp_internal_free(struct task *task) { - K_OOPS(K_SYSCALL_MEMORY_WRITE(task, sizeof(*task))); - - struct task_dp_pdata *pdata = task->priv_data; - - K_OOPS(K_SYSCALL_MEMORY_WRITE(pdata, sizeof(*pdata))); - K_OOPS(K_SYSCALL_OBJ(pdata->event, K_OBJ_EVENT)); - K_OOPS(K_SYSCALL_OBJ_INIT(pdata->thread, K_OBJ_THREAD)); - scheduler_dp_mod_vrfy(pdata->mod); - return z_impl_scheduler_dp_internal_free(task); + /* + * Do not read task->priv_data: everything needed comes from the + * kernel-only record, and an unknown task pointer is rejected. + */ + K_OOPS(!scheduler_dp_internal_free_kobj(task)); } #include #endif /* CONFIG_USERSPACE */