Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/audio/buffers/comp_buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <sof/audio/sink_api.h>
#include <sof/audio/source_api.h>
#include <sof/audio/sink_source_utils.h>
#include <sof/audio/module_adapter/module/generic.h>
#include <rtos/userspace_helper.h>
#include <sof/common.h>
#include <rtos/interrupt.h>
Expand Down Expand Up @@ -165,8 +166,7 @@ static void comp_buffer_free(struct sof_audio_buffer *audio_buffer)

if (alloc && alloc->vreg) {
vregion_free(alloc->vreg, buffer);
if (!vregion_put(alloc->vreg))
rfree(alloc);
module_adapter_vreg_free(alloc);
} else {
sof_heap_free(alloc ? alloc->heap : NULL, buffer);
}
Expand Down
116 changes: 108 additions & 8 deletions src/audio/module_adapter/module_adapter.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,83 @@ struct comp_dev *module_adapter_new(const struct comp_driver *drv,
return module_adapter_new_ext(drv, config, spec, NULL, NULL, NULL);
}

static struct vregion *module_adapter_dp_heap_new(const struct comp_ipc_config *config,
size_t *heap_size)
struct vregion *z_impl_module_adapter_vreg_new(uintptr_t *vreg_start, size_t *vreg_size)
{
/* src-lite with 8 channels has been seen allocating 14k in one go */
/* FIXME: the size will be derived from configuration */
const size_t buf_size = 28 * 1024;
struct vregion *vr = vregion_create(buf_size);

return vregion_create(buf_size);
if (!vr)
return NULL;

#ifdef CONFIG_SOF_USERSPACE_LL
vregion_mem_info(vr, vreg_size, vreg_start);

/*
* In the userspace LL case allocations are also performed by the
* userspace IPC thread, which is also the one, executing this syscall
*/
struct k_mem_partition cached_part = {
.start = *vreg_start,
.size = *vreg_size,
.attr = K_MEM_PARTITION_P_RW_U_RW | XTENSA_MMU_CACHED_WB,
};
int ret = k_mem_domain_add_partition(zephyr_ll_mem_domain(), &cached_part);

if (ret < 0) {
vregion_put(vr);
return NULL;
}

struct k_mem_partition uncached_part = {
.start = (uintptr_t)sys_cache_uncached_ptr_get((void *)cached_part.start),
.size = cached_part.size,
.attr = K_MEM_PARTITION_P_RW_U_RW,
};

ret = k_mem_domain_add_partition(zephyr_ll_mem_domain(), &uncached_part);
if (ret < 0) {
k_mem_domain_remove_partition(zephyr_ll_mem_domain(), &cached_part);
vregion_put(vr);
return NULL;
}
#else
(void)vreg_start;
(void)vreg_size;
#endif

return vr;
}

void z_impl_module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc)
{
#ifdef CONFIG_SOF_USERSPACE_LL
struct k_mem_partition part = {
.attr = K_MEM_PARTITION_P_RW_U_RW | XTENSA_MMU_CACHED_WB,
};

vregion_mem_info(alloc->vreg, &part.size, &part.start);

k_mem_domain_remove_partition(zephyr_ll_mem_domain(), &part);

part.start = (uintptr_t)sys_cache_uncached_ptr_get((void *)part.start);
part.attr = K_MEM_PARTITION_P_RW_U_RW;

k_mem_domain_remove_partition(zephyr_ll_mem_domain(), &part);
#else
(void)alloc;
#endif
}

void module_adapter_vreg_free(struct mod_alloc_ctx *alloc)
{
if (vregion_put(alloc->vreg))
return;

module_adapter_vreg_unmap(alloc);

sof_heap_free(alloc->heap, alloc);
}

static struct processing_module *module_adapter_mem_alloc(const struct comp_driver *drv,
Expand All @@ -77,11 +146,12 @@ static struct processing_module *module_adapter_mem_alloc(const struct comp_driv
*/
uint32_t flags = config->proc_domain == COMP_PROCESSING_DOMAIN_DP ?
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT : SOF_MEM_FLAG_USER;
size_t heap_size;
size_t vreg_size;
uintptr_t vreg_start;

if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP && IS_ENABLED(CONFIG_SOF_VREGIONS) &&
IS_ENABLED(CONFIG_USERSPACE) && !IS_ENABLED(CONFIG_SOF_USERSPACE_USE_DRIVER_HEAP)) {
mod_vreg = module_adapter_dp_heap_new(config, &heap_size);
mod_vreg = module_adapter_vreg_new(&vreg_start, &vreg_size);
if (!mod_vreg) {
comp_cl_err(drv, "Failed to allocate DP module heap / vregion");
return NULL;
Expand All @@ -98,7 +168,8 @@ static struct processing_module *module_adapter_mem_alloc(const struct comp_driv
#else
mod_heap = drv->user_heap;
#endif
heap_size = 0;
vreg_size = 0;
vreg_start = 0;
mod_vreg = NULL;
}

Expand Down Expand Up @@ -162,6 +233,32 @@ static struct processing_module *module_adapter_mem_alloc(const struct comp_driv
return NULL;
}

#ifdef CONFIG_USERSPACE
#include <zephyr/internal/syscall_handler.h>
struct vregion *z_vrfy_module_adapter_vreg_new(uintptr_t *vreg_start, size_t *vreg_size)
{
K_OOPS(K_SYSCALL_MEMORY_WRITE(vreg_start, sizeof(*vreg_start)));
K_OOPS(K_SYSCALL_MEMORY_WRITE(vreg_size, sizeof(*vreg_size)));
return z_impl_module_adapter_vreg_new(vreg_start, vreg_size);
}
#include <zephyr/syscalls/module_adapter_vreg_new_mrsh.c>
void z_vrfy_module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc)
{
/*
* Protect against rogue module attempting to unmap another module's
* vregion. It could pass an "alloc" pointer, belonging to another
* module (assuming it somehow could get it), but then it wouldn't have
* access rights to it. And with the "alloc" pointer to which it has
* access rights, it can only unmap a vregion, of which the pointer is
* the owner.
*/
K_OOPS(K_SYSCALL_MEMORY_READ(alloc, sizeof(*alloc)));
K_OOPS(vregion_owner_get(alloc->vreg) != alloc);
z_impl_module_adapter_vreg_unmap(alloc);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this is not safe, user-space can pass arbitrary alloc->vreg and this is passed unchecked to kernel code.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i added an owner, please re-check

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better, but as "alloc" is untrusted memory, "alloc->vreg" can point to a made-up vreg object (with owner set to point to alloc by the user-space thread). I think another layer is needed to look up "vreg" is an actual kernel vreg object, and not a pointer to some random user created object.

}
#include <zephyr/syscalls/module_adapter_vreg_unmap_mrsh.c>
#endif

static void module_adapter_mem_free(struct processing_module *mod)
{
struct mod_alloc_ctx *alloc = mod->priv.resources.alloc;
Expand All @@ -179,8 +276,7 @@ static void module_adapter_mem_free(struct processing_module *mod)

vregion_free(mod_vreg, mod->dev);
vregion_free(mod_vreg, mod);
if (!vregion_put(mod_vreg))
sof_heap_free(alloc->heap, alloc);
module_adapter_vreg_free(alloc);
} else {
sof_heap_free(mod_heap, mod->dev);
sof_heap_free(mod_heap, mod);
Expand All @@ -199,6 +295,7 @@ static void module_adapter_mem_free(struct processing_module *mod)
*
* Note: Use the ext version if you need to set the module's private data before calling
* the create method.
* Note 2: ATM runs in privileged / kernel mode for DP modules
*/
struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
const struct comp_ipc_config *config,
Expand Down Expand Up @@ -244,12 +341,15 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
#if CONFIG_ZEPHYR_DP_SCHEDULER
/* create a task for DP processing */
if (config->proc_domain == COMP_PROCESSING_DOMAIN_DP) {
struct mod_alloc_ctx *alloc = mod->priv.resources.alloc;

/* All data allocated, create a thread */
ret = pipeline_comp_dp_task_init(dev);
if (ret) {
comp_cl_err(drv, "DP task creation failed with error %d.", ret);
goto err;
}
vregion_owner_set(alloc->vreg, alloc);
}
#endif /* CONFIG_ZEPHYR_DP_SCHEDULER */

Expand Down
7 changes: 7 additions & 0 deletions src/include/sof/audio/module_adapter/module/generic.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,23 @@ void *z_impl_mod_balloc_align(struct processing_module *mod, size_t size, size_t
#endif
void mod_resource_init(struct processing_module *mod);
void mod_heap_info(struct processing_module *mod, size_t *size, uintptr_t *start);
void module_adapter_vreg_free(struct mod_alloc_ctx *alloc);
#if defined(__ZEPHYR__) && defined(CONFIG_SOF_FULL_ZEPHYR_APPLICATION)
__syscall struct vregion *module_adapter_vreg_new(uintptr_t *vreg_start, size_t *vreg_size);
__syscall void module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc);
__syscall void *mod_alloc_ext(struct processing_module *mod, uint32_t flags, size_t size,
size_t alignment);
__syscall int mod_free(struct processing_module *mod, const void *ptr);
__syscall void mod_free_all(struct processing_module *mod);
#else
struct vregion *z_impl_module_adapter_vreg_new(uintptr_t *vreg_start, size_t *vreg_size);
void z_impl_module_adapter_vreg_unmap(const struct mod_alloc_ctx *alloc);
void *z_impl_mod_alloc_ext(struct processing_module *mod, uint32_t flags, size_t size,
size_t alignment);
int z_impl_mod_free(struct processing_module *mod, const void *ptr);
void z_impl_mod_free_all(struct processing_module *mod);
#define module_adapter_vreg_new z_impl_module_adapter_vreg_new
#define module_adapter_vreg_unmap z_impl_module_adapter_vreg_unmap
#define mod_alloc_ext z_impl_mod_alloc_ext
#define mod_free z_impl_mod_free
#define mod_free_all z_impl_mod_free_all
Expand Down
37 changes: 29 additions & 8 deletions src/include/sof/lib/vregion.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#define __SOF_LIB_VREGION_H__

#include <stddef.h>
#include <stdint.h>
#include <sof/compiler_attributes.h>

Comment thread
lyakh marked this conversation as resolved.
#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -49,7 +51,7 @@ struct vregion *vregion_create(size_t memsize);
*
* @param[in] vr Pointer to the virtual region instance.
*/
void vregion_set_interim(struct vregion *vr);
__syscall void vregion_set_interim(struct vregion *vr);

/**
* @brief Increment virtual region's user count.
Expand All @@ -60,7 +62,7 @@ void vregion_set_interim(struct vregion *vr);
* @param[in] vr Pointer to the virtual region instance to release.
* @return struct vregion* Pointer to the virtual region instance.
*/
struct vregion *vregion_get(struct vregion *vr);
__syscall struct vregion *vregion_get(struct vregion *vr);

/**
* @brief Decrement virtual region's user count or destroy it.
Expand All @@ -71,7 +73,7 @@ struct vregion *vregion_get(struct vregion *vr);
* @param[in] vr Pointer to the virtual region instance to release.
* @return struct vregion* Pointer to the virtual region instance or NULL if it has been destroyed.
*/
struct vregion *vregion_put(struct vregion *vr);
__syscall struct vregion *vregion_put(struct vregion *vr);

/**
* @brief Allocate memory from the specified virtual region.
Expand All @@ -80,12 +82,16 @@ struct vregion *vregion_put(struct vregion *vr);
* @param[in] size Size of memory to allocate in bytes.
* @return void* Pointer to the allocated memory, or NULL on failure.
*/
void *vregion_alloc(struct vregion *vr, size_t size);
__syscall void *vregion_alloc(struct vregion *vr, size_t size);

void *z_impl_vregion_alloc(struct vregion *vr, size_t size);

/**
* @brief like vregion_alloc() but allocates coherent memory
*/
void *vregion_alloc_coherent(struct vregion *vr, size_t size);
__syscall void *vregion_alloc_coherent(struct vregion *vr, size_t size);

void *z_impl_vregion_alloc_coherent(struct vregion *vr, size_t size);

/**
* @brief Allocate aligned memory from the specified virtual region.
Expand All @@ -98,12 +104,16 @@ void *vregion_alloc_coherent(struct vregion *vr, size_t size);
* @param[in] alignment Alignment of memory to allocate in bytes.
* @return void* Pointer to the allocated memory, or NULL on failure.
*/
void *vregion_alloc_align(struct vregion *vr, size_t size, size_t alignment);
__syscall void *vregion_alloc_align(struct vregion *vr, size_t size, size_t alignment);

void *z_impl_vregion_alloc_align(struct vregion *vr, size_t size, size_t alignment);

/**
* @brief like vregion_alloc_align() but allocates coherent memory
*/
void *vregion_alloc_coherent_align(struct vregion *vr, size_t size, size_t alignment);
__syscall void *vregion_alloc_coherent_align(struct vregion *vr, size_t size, size_t alignment);

void *z_impl_vregion_alloc_coherent_align(struct vregion *vr, size_t size, size_t alignment);

/**
* @brief Free memory allocated from the specified virtual region.
Expand All @@ -113,7 +123,9 @@ void *vregion_alloc_coherent_align(struct vregion *vr, size_t size, size_t align
* @param[in] vr Pointer to the virtual region instance.
* @param[in] ptr Pointer to the memory to free.
*/
void vregion_free(struct vregion *vr, void *ptr);
__syscall void vregion_free(struct vregion *vr, void *ptr);

void z_impl_vregion_free(struct vregion *vr, void *ptr);

/**
* @brief Log virtual region memory usage.
Expand All @@ -131,6 +143,12 @@ void vregion_info(struct vregion *vr);
*/
void vregion_mem_info(struct vregion *vr, size_t *size, uintptr_t *start);

void vregion_owner_set(struct vregion *vr, void *owner);
void *vregion_owner_get(struct vregion *vr);
bool vregion_verify(struct vregion *vr);

#include <zephyr/syscalls/vregion.h>

#else /* CONFIG_SOF_VREGIONS */

struct vregion {
Expand Down Expand Up @@ -174,6 +192,9 @@ static inline void vregion_mem_info(struct vregion *vr, size_t *size, uintptr_t
if (size)
*size = 0;
}
static inline void vregion_owner_set(struct vregion *vr, void *owner) {}
static inline void *vregion_owner_get(struct vregion *vr) {return NULL;}
static inline bool vregion_verify(struct vregion *vr) {return false;}

#endif /* CONFIG_SOF_VREGIONS */

Expand Down
3 changes: 2 additions & 1 deletion src/schedule/zephyr_dp_schedule_application.c
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ static void scheduler_dp_thread_name_set(k_tid_t thread_id, struct processing_mo
#define scheduler_dp_thread_name_set(x, y)
#endif

/* Called only in IPC context */
/* Called only in IPC context in kernel mode (this can change) */
int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid,
const struct task_ops *ops, struct processing_module *mod,
uint16_t core, size_t stack_size, uint32_t options)
Expand All @@ -437,6 +437,7 @@ int scheduler_dp_task_init(struct task **task, const struct sof_uuid_entry *uid,

/* must be called on the same core the task will be bound to */
assert(cpu_get_id() == core);
assert(!k_is_user_context());

/*
* allocate memory
Expand Down
1 change: 1 addition & 0 deletions test/cmocka/src/audio/volume/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ add_library(audio_for_volume STATIC
${PROJECT_SOURCE_DIR}/src/audio/pipeline/pipeline-stream.c
${PROJECT_SOURCE_DIR}/src/audio/pipeline/pipeline-xrun.c
${PROJECT_SOURCE_DIR}/src/audio/component.c
${PROJECT_SOURCE_DIR}/src/audio/data_blob.c
${PROJECT_SOURCE_DIR}/src/math/numbers.c
)
sof_append_relative_path_definitions(audio_for_volume)
Expand Down
5 changes: 5 additions & 0 deletions test/cmocka/src/common_mocks.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ void WEAK sof_heap_free(struct k_heap *heap, void *addr)
free(addr);
}

void WEAK module_adapter_vreg_free(struct mod_alloc_ctx *alloc)
{
(void)alloc;
}

int WEAK memcpy_s(void *dest, size_t dest_size,
const void *src, size_t count)
{
Expand Down
2 changes: 2 additions & 0 deletions zephyr/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,8 @@ zephyr_syscall_header(${SOF_SRC_PATH}/include/sof/schedule/ll_schedule_domain.h)
zephyr_syscall_header(${SOF_SRC_PATH}/include/ipc4/handler.h)
zephyr_syscall_header(include/rtos/alloc.h)
zephyr_library_sources_ifdef(CONFIG_SOF_USERSPACE_INTERFACE_ALLOC syscall/alloc.c)
zephyr_syscall_header(${SOF_SRC_PATH}/include/sof/lib/vregion.h)
zephyr_library_sources_ifdef(CONFIG_SOF_USERSPACE_INTERFACE_VREGION syscall/vregion.c)
zephyr_syscall_header(${SOF_SRC_PATH}/include/sof/lib/dai-zephyr.h)
zephyr_library_sources_ifdef(CONFIG_USERSPACE syscall/dai.c)

Expand Down
9 changes: 9 additions & 0 deletions zephyr/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,20 @@ config SOF_USERSPACE_INTERFACE_ALLOC
Allow user-space threads to use sof_heap_alloc/sof_heap_free
as Zephyr system calls.

config SOF_USERSPACE_INTERFACE_VREGION
bool "Enable SOF vregion interface to userspace threads"
depends on USERSPACE
depends on SOF_VREGIONS
help
Allow user-space threads to use vregion_alloc/vregion_free
and their variants as Zephyr system calls.

config SOF_USERSPACE_LL
bool "Run Low-Latency pipelines in userspace threads"
depends on USERSPACE
select SOF_USERSPACE_INTERFACE_ALLOC
select SOF_USERSPACE_INTERFACE_DMA
select SOF_USERSPACE_INTERFACE_VREGION if SOF_VREGIONS
help
Run Low-Latency (LL) pipelines in userspace threads. This adds
memory protection between operating system resources and
Expand Down
Loading
Loading