diff --git a/src/audio/pipeline/pipeline-graph.c b/src/audio/pipeline/pipeline-graph.c index 915987051275..9254d2468447 100644 --- a/src/audio/pipeline/pipeline-graph.c +++ b/src/audio/pipeline/pipeline-graph.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -29,6 +30,7 @@ #include #include #include +#include LOG_MODULE_REGISTER(pipe, CONFIG_SOF_LOG_LEVEL); @@ -181,19 +183,24 @@ static void buffer_set_comp(struct comp_buffer *buffer, struct comp_dev *comp, } #ifdef CONFIG_SOF_USERSPACE_LL +/* + * User-space LL: callers (IPC handlers) already hold the per-core LL + * lock via user_ll_lock_sched()/ll_block() while modifying pipeline + * connections, which provides mutual exclusion with the LL thread. No + * additional lock is taken here; instead assert that the lock is held. + */ #define PPL_LOCK_DECLARE -#define PPL_LOCK() do { \ - int ret = sys_mutex_lock(&comp->list_mutex, K_FOREVER); \ - assert(ret == 0); \ - } while (0) -#define PPL_UNLOCK() do { \ - int ret = sys_mutex_unlock(&comp->list_mutex); \ - assert(ret == 0); \ - } while (0) +#define PPL_LOCK(x) user_ll_assert_locked(x) +#define PPL_UNLOCK(x) #else +/* + * Kernel-space LL. When modifying pipeline connections, block IRQs + * and prevent LL from running. No locking needed when iterating + * the pipeline in the LL thread. + */ #define PPL_LOCK_DECLARE uint32_t flags -#define PPL_LOCK() irq_local_disable(flags) -#define PPL_UNLOCK() irq_local_enable(flags) +#define PPL_LOCK(x) irq_local_disable(flags) +#define PPL_UNLOCK(x) irq_local_enable(flags) #endif int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer, @@ -208,19 +215,19 @@ int pipeline_connect(struct comp_dev *comp, struct comp_buffer *buffer, else comp_info(comp, "connect buffer %d as source", buf_get_id(buffer)); - PPL_LOCK(); + PPL_LOCK(buffer->core); comp_list = comp_buffer_list(comp, dir); ret = buffer_attach(buffer, comp_list, dir); if (ret < 0) { comp_err(comp, "buffer %d already connected dir %d", buf_get_id(buffer), dir); - PPL_UNLOCK(); + PPL_UNLOCK(buffer->core); return ret; } buffer_set_comp(buffer, comp, dir); - PPL_UNLOCK(); + PPL_UNLOCK(buffer->core); return 0; } @@ -235,13 +242,13 @@ void pipeline_disconnect(struct comp_dev *comp, struct comp_buffer *buffer, int else comp_dbg(comp, "disconnect buffer %d as source", buf_get_id(buffer)); - PPL_LOCK(); + PPL_LOCK(buffer->core); comp_list = comp_buffer_list(comp, dir); buffer_detach(buffer, comp_list, dir); buffer_set_comp(buffer, NULL, dir); - PPL_UNLOCK(); + PPL_UNLOCK(buffer->core); } /* pipelines must be inactive */ diff --git a/src/audio/pipeline/pipeline-stream.c b/src/audio/pipeline/pipeline-stream.c index e4a166554fa2..248481b57a3f 100644 --- a/src/audio/pipeline/pipeline-stream.c +++ b/src/audio/pipeline/pipeline-stream.c @@ -21,6 +21,7 @@ #include #include #include +#include #ifdef CONFIG_IPC_MAJOR_4 #include @@ -145,6 +146,20 @@ static int pipeline_comp_copy(struct comp_dev *current, return err; } +#ifdef CONFIG_SOF_USERSPACE_LL +/* + * User-space LL: pipeline_copy() runs as an LL task, with the per-core + * LL lock already held by the LL thread for the whole tick. Taking the + * lock again here would only add syscall overhead on this hot path, so + * we just assert that the lock is in fact held. + */ +#define PPL_LOCK(x) user_ll_assert_locked(x) +#define PPL_UNLOCK(x) +#else +#define PPL_LOCK(x) +#define PPL_UNLOCK(x) +#endif + /* Copy data across all pipeline components. * For capture pipelines it always starts from source component * and continues downstream and for playback pipelines it first @@ -162,6 +177,8 @@ int pipeline_copy(struct pipeline *p) uint32_t dir; int ret; + PPL_LOCK(p->core); + if (p->source_comp->direction == SOF_IPC_STREAM_PLAYBACK) { dir = PPL_DIR_UPSTREAM; start = p->sink_comp; @@ -178,6 +195,8 @@ int pipeline_copy(struct pipeline *p) pipe_err(p, "ret = %d, start->comp.id = %u, dir = %u", ret, dev_comp_id(start), dir); + PPL_UNLOCK(p->core); + return ret; } diff --git a/src/include/sof/audio/component.h b/src/include/sof/audio/component.h index 13f2524ac909..b08ef224ae1e 100644 --- a/src/include/sof/audio/component.h +++ b/src/include/sof/audio/component.h @@ -680,10 +680,6 @@ struct comp_dev { struct list_item bsource_list; /**< list of source buffers */ struct list_item bsink_list; /**< list of sink buffers */ -#ifdef CONFIG_SOF_USERSPACE_LL - struct sys_mutex list_mutex; /**< protect lists of source/sinks */ -#endif - /* performance data*/ struct comp_perf_data perf_data; /* Input Buffer Size for pin 0, add array for other pins if needed */ @@ -868,9 +864,6 @@ static inline void comp_init(const struct comp_driver *drv, dev->state = COMP_STATE_INIT; list_init(&dev->bsink_list); list_init(&dev->bsource_list); -#ifdef CONFIG_SOF_USERSPACE_LL - sys_mutex_init(&dev->list_mutex); -#endif #ifndef __ZEPHYR__ memcpy_s(&dev->tctx, sizeof(dev->tctx), trace_comp_drv_get_tr_ctx(dev->drv), sizeof(struct tr_ctx)); diff --git a/src/include/sof/schedule/ll_schedule_domain.h b/src/include/sof/schedule/ll_schedule_domain.h index 1367c408899f..10f6af21dbf2 100644 --- a/src/include/sof/schedule/ll_schedule_domain.h +++ b/src/include/sof/schedule/ll_schedule_domain.h @@ -120,6 +120,14 @@ struct task *zephyr_ll_task_alloc(void); struct k_heap *zephyr_ll_user_heap(void); bool zephyr_ll_user_heap_verify(struct k_heap *heap); void zephyr_ll_user_resources_init(void); +void user_ll_lock_sched(int core); +void user_ll_unlock_sched(int core); +#ifdef CONFIG_ASSERT +/* Assert that the calling context already holds the LL lock for 'core'. */ +void user_ll_assert_locked(int core); +#else +static inline void user_ll_assert_locked(int core) { (void)core; } +#endif #endif /* CONFIG_SOF_USERSPACE_LL */ static inline struct ll_schedule_domain *domain_init diff --git a/src/ipc/ipc-helper.c b/src/ipc/ipc-helper.c index 737c3b73370c..1a2fcef4194e 100644 --- a/src/ipc/ipc-helper.c +++ b/src/ipc/ipc-helper.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -336,7 +337,15 @@ __cold int ipc_comp_free(struct ipc *ipc, uint32_t comp_id) return -EINVAL; } + /* Lock buffer lists to prevent racing with the LL scheduler. + * In user-space builds, use the LL scheduler's lock + * (re-entrant, so safe if caller already holds it). + */ +#ifdef CONFIG_SOF_USERSPACE_LL + user_ll_lock_sched(icd->core); +#else irq_local_disable(flags); +#endif comp_dev_for_each_producer_safe(icd->cd, buffer, safe) { comp_buffer_set_sink_component(buffer, NULL); /* This breaks the list, but we anyway delete all buffers */ @@ -349,7 +358,11 @@ __cold int ipc_comp_free(struct ipc *ipc, uint32_t comp_id) comp_buffer_reset_source_list(buffer); } +#ifdef CONFIG_SOF_USERSPACE_LL + user_ll_unlock_sched(icd->core); +#else irq_local_enable(flags); +#endif /* * A completed pipeline stores raw comp_dev pointers in its diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 9e0846f6bc02..7119054e6da9 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -448,10 +448,21 @@ __cold static int ipc_pipeline_module_free(uint32_t pipeline_id) struct ipc *ipc = ipc_get(); struct ipc_comp_dev *icd; int ret; +#ifdef CONFIG_SOF_USERSPACE_LL + int ppl_core; +#endif assert_can_be_cold(); icd = ipc_get_comp_by_ppl_id(ipc, COMP_TYPE_COMPONENT, pipeline_id, IPC_COMP_ALL); + if (!icd) + return IPC4_SUCCESS; + +#ifdef CONFIG_SOF_USERSPACE_LL + ppl_core = icd->core; + user_ll_lock_sched(ppl_core); +#endif + while (icd) { struct comp_buffer *buffer; struct comp_buffer *safe; @@ -481,12 +492,20 @@ __cold static int ipc_pipeline_module_free(uint32_t pipeline_id) else ret = ipc_comp_free(ipc, icd->id); - if (ret) + if (ret) { +#ifdef CONFIG_SOF_USERSPACE_LL + user_ll_unlock_sched(ppl_core); +#endif return IPC4_INVALID_RESOURCE_STATE; + } icd = ipc_get_comp_by_ppl_id(ipc, COMP_TYPE_COMPONENT, pipeline_id, IPC_COMP_ALL); } +#ifdef CONFIG_SOF_USERSPACE_LL + user_ll_unlock_sched(ppl_core); +#endif + return IPC4_SUCCESS; } @@ -560,21 +579,25 @@ __cold static struct comp_buffer *ipc4_create_buffer(struct comp_dev *src, bool * disable any interrupts. */ -#define ll_block(cross_core_bind, flags) \ +#if CONFIG_SOF_USERSPACE_LL +#error "CONFIG_SOF_USERSPACE_LL not compatible with cross-core streams" +#else +#define ll_block(src_core, dst_core, flags) \ do { \ - if (cross_core_bind) \ + if (src_core != dst_core) \ domain_block(sof_get()->platform_timer_domain); \ else \ irq_local_disable(flags); \ } while (0) -#define ll_unblock(cross_core_bind, flags) \ +#define ll_unblock(src_core, dst_core, flags) \ do { \ - if (cross_core_bind) \ + if (src_core != dst_core) \ domain_unblock(sof_get()->platform_timer_domain); \ else \ irq_local_enable(flags); \ } while (0) +#endif /* Calling both ll_block() and ll_wait_finished_on_core() makes sure LL will not start its * next cycle and its current cycle on specified core has finished. @@ -606,8 +629,15 @@ static int ll_wait_finished_on_core(struct comp_dev *dev) #else -#define ll_block(cross_core_bind, flags) irq_local_disable(flags) -#define ll_unblock(cross_core_bind, flags) irq_local_enable(flags) +#if CONFIG_SOF_USERSPACE_LL +/* note: cross-core streams are disabled in the build in this branch, + * so we can safely use 'src_core' only */ +#define ll_block(src_core, dst_core, flags) do { user_ll_lock_sched(src_core); (void)flags; } while(0) +#define ll_unblock(src_core, dst_core, flags) do { user_ll_unlock_sched(src_core) ; (void)flags; } while(0) +#else +#define ll_block(src_core, dst_core, flags) irq_local_disable(flags) +#define ll_unblock(src_core, dst_core, flags) irq_local_enable(flags) +#endif #endif @@ -794,7 +824,7 @@ __cold int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) * blocked on corresponding core(s) to prevent IPC or IDC task getting preempted which * could result in buffers being only half connected when a pipeline task gets executed. */ - ll_block(cross_core_bind, flags); + ll_block(source->ipc_config.core, sink->ipc_config.core, flags); if (cross_core_bind) { #if CONFIG_CROSS_CORE_STREAM @@ -851,7 +881,7 @@ __cold int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) source->direction_set = true; } - ll_unblock(cross_core_bind, flags); + ll_unblock(source->ipc_config.core, sink->ipc_config.core, flags); return IPC4_SUCCESS; @@ -865,7 +895,7 @@ __cold int ipc_comp_connect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) e_sink_connect: pipeline_disconnect(source, buffer, PPL_CONN_DIR_COMP_TO_BUFFER); free: - ll_unblock(cross_core_bind, flags); + ll_unblock(source->ipc_config.core, sink->ipc_config.core, flags); buffer_free(buffer); return IPC4_INVALID_RESOURCE_STATE; } @@ -938,24 +968,24 @@ __cold int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) * IPC or IDC task getting preempted which could result in buffers being only half connected * when a pipeline task gets executed. */ - ll_block(cross_core_unbind, flags); + ll_block(src->ipc_config.core, sink->ipc_config.core, flags); if (cross_core_unbind) { #if CONFIG_CROSS_CORE_STREAM /* Make sure LL has finished on both cores */ if (!cpu_is_me(src->ipc_config.core)) if (ll_wait_finished_on_core(src) < 0) { - ll_unblock(cross_core_unbind, flags); + ll_unblock(src->ipc_config.core, sink->ipc_config.core, flags); return IPC4_FAILURE; } if (!cpu_is_me(sink->ipc_config.core)) if (ll_wait_finished_on_core(sink) < 0) { - ll_unblock(cross_core_unbind, flags); + ll_unblock(src->ipc_config.core, sink->ipc_config.core, flags); return IPC4_FAILURE; } #else tr_err(&ipc_tr, "Cross-core binding is disabled"); - ll_unblock(cross_core_unbind, flags); + ll_unblock(src->ipc_config.core, sink->ipc_config.core, flags); return IPC4_FAILURE; #endif } @@ -972,7 +1002,7 @@ __cold int ipc_comp_disconnect(struct ipc *ipc, ipc_pipe_comp_connect *_connect) unbind_data.source = audio_buffer_get_source(&buffer->audio_buffer); ret1 = comp_unbind(sink, &unbind_data); - ll_unblock(cross_core_unbind, flags); + ll_unblock(src->ipc_config.core, sink->ipc_config.core, flags); buffer_free(buffer); diff --git a/src/schedule/zephyr_ll.c b/src/schedule/zephyr_ll.c index ddae93eee8b7..fb65c83fe19b 100644 --- a/src/schedule/zephyr_ll.c +++ b/src/schedule/zephyr_ll.c @@ -44,10 +44,57 @@ struct zephyr_ll_pdata { struct k_sem sem; }; +#if CONFIG_SOF_USERSPACE_LL +/* + * Mutex pointer in user-accessible partition so user-space threads + * can read the pointer for syscalls. The actual lock object resides + * in kernel memory, there are just handles! + */ +static APP_SYSUSER_BSS struct k_mutex *zephyr_ll_locks[CONFIG_CORE_COUNT]; + +/* + * Shadow ownership tracking for the per-core LL mutex, used only to + * power user_ll_assert_locked(). The kernel k_mutex object is not + * readable from user-space threads, so its owner cannot be inspected + * directly; instead record the owner and recursion depth here, in a + * user-accessible partition. Updated on every lock/unlock of the LL + * mutex, by both the LL thread (zephyr_ll_lock()) and IPC handlers + * (user_ll_lock_sched()). Only ever mutated by the current lock holder + * while the lock is held, so no extra synchronization is required. + */ +#ifdef CONFIG_ASSERT +static APP_SYSUSER_BSS k_tid_t zephyr_ll_lock_owner[CONFIG_CORE_COUNT]; +static APP_SYSUSER_BSS uint32_t zephyr_ll_lock_depth[CONFIG_CORE_COUNT]; + +static inline void zephyr_ll_lock_acquired(int core) +{ + zephyr_ll_lock_owner[core] = k_current_get(); + zephyr_ll_lock_depth[core]++; +} + +static inline void zephyr_ll_lock_releasing(int core) +{ + assert(zephyr_ll_lock_owner[core] == k_current_get()); + if (--zephyr_ll_lock_depth[core] == 0) + zephyr_ll_lock_owner[core] = NULL; +} + +void user_ll_assert_locked(int core) +{ + assert(core < CONFIG_CORE_COUNT && + zephyr_ll_lock_owner[core] == k_current_get()); +} +#else +static inline void zephyr_ll_lock_acquired(int core) { (void)core; } +static inline void zephyr_ll_lock_releasing(int core) { (void)core; } +#endif /* CONFIG_ASSERT */ +#endif + static void zephyr_ll_lock(struct zephyr_ll *sch, uint32_t *flags) { #if CONFIG_SOF_USERSPACE_LL k_mutex_lock(sch->lock, K_FOREVER); + zephyr_ll_lock_acquired(sch->core); #else irq_local_disable(*flags); #endif @@ -56,6 +103,7 @@ static void zephyr_ll_lock(struct zephyr_ll *sch, uint32_t *flags) static void zephyr_ll_unlock(struct zephyr_ll *sch, uint32_t *flags) { #if CONFIG_SOF_USERSPACE_LL + zephyr_ll_lock_releasing(sch->core); k_mutex_unlock(sch->lock); #else irq_local_enable(*flags); @@ -222,7 +270,15 @@ static void zephyr_ll_run(void *data) list_item_del(list); list_item_append(list, &task_head); + /* in user-space LL builds, the LL lock protects LL + * tasks against IPC thread, so the lock must be held + * while running the tasks. + * in kernel LL builds, IPC thread blocks interrupts for + * critical section, so lock can be freed here. + */ +#ifndef CONFIG_SOF_USERSPACE_LL zephyr_ll_unlock(sch, &flags); +#endif /* * task's .run() should only return either @@ -237,7 +293,9 @@ static void zephyr_ll_run(void *data) state = SOF_TASK_STATE_RESCHEDULE; } +#ifndef CONFIG_SOF_USERSPACE_LL zephyr_ll_lock(sch, &flags); +#endif if (pdata->freeing || state == SOF_TASK_STATE_COMPLETED) { zephyr_ll_task_done(sch, task); @@ -561,6 +619,33 @@ struct task *zephyr_ll_task_alloc(void) return task; } + +/** + * Lock the LL scheduler to prevent it from processing tasks. + * + * Uses the LL scheduler's own k_mutex which is re-entrant, so + * schedule_task() calls within the locked section will not deadlock. + * Must be paired with user_ll_unlock_sched(). + */ +void user_ll_lock_sched(int core) +{ + assert(core < CONFIG_CORE_COUNT && zephyr_ll_locks[core] != NULL); + int __maybe_unused ret = k_mutex_lock(zephyr_ll_locks[core], K_FOREVER); + assert(!ret); + zephyr_ll_lock_acquired(core); +} + +/** + * Unlock the LL scheduler after a previous user_ll_lock_sched() call. + */ +void user_ll_unlock_sched(int core) +{ + assert(core < CONFIG_CORE_COUNT && zephyr_ll_locks[core] != NULL); + zephyr_ll_lock_releasing(core); + int __maybe_unused ret = k_mutex_unlock(zephyr_ll_locks[core]); + assert(!ret); +} + #endif /* CONFIG_SOF_USERSPACE_LL */ int zephyr_ll_task_init(struct task *task, @@ -649,6 +734,8 @@ __cold int zephyr_ll_scheduler_init(struct ll_schedule_domain *domain) sof_heap_free(sch->heap, sch); return -ENOMEM; } + assert(core < CONFIG_CORE_COUNT && zephyr_ll_locks[core] == NULL); + zephyr_ll_locks[core] = sch->lock; k_mutex_init(sch->lock); tr_dbg(&ll_tr, "ll-scheduler init done, sch %p sch->lock %p", sch, sch->lock);