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
50 changes: 34 additions & 16 deletions src/audio/dai-zephyr.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <sof/lib/notifier.h>
#include <sof/lib/uuid.h>
#include <sof/lib/dma.h>
#include <sof/schedule/ll_schedule_domain.h> /* zephyr_ll_user_heap() */
#include <sof/list.h>
#include <rtos/spinlock.h>
#include <rtos/string.h>
Expand Down Expand Up @@ -529,6 +530,17 @@ __cold int dai_common_new(struct dai_data *dd, struct comp_dev *dev,
dma_sg_init(&dd->config.elem_array);
dd->xrun = 0;

#ifdef CONFIG_SOF_USERSPACE_LL
/*
* copier_dai_create() uses mod_zalloc() to allocate
* the 'dd' dai data object and does not set dd->alloc_ctx.
* If LL is run in user-space, assign the 'heap' here.
*/
dd->alloc_ctx.heap = zephyr_ll_user_heap();
#else
dd->alloc_ctx.heap = NULL;
#endif

/* I/O performance init, keep it last so the function does not reach this in case
* of return on error, so that we do not waste a slot
*/
Expand Down Expand Up @@ -581,6 +593,7 @@ __cold static struct comp_dev *dai_new(const struct comp_driver *drv,
struct comp_dev *dev;
const struct ipc_config_dai *dai_cfg = spec;
struct dai_data *dd;
struct k_heap *heap = NULL;
int ret;

assert_can_be_cold();
Expand All @@ -593,10 +606,12 @@ __cold static struct comp_dev *dai_new(const struct comp_driver *drv,

dev->ipc_config = *config;

dd = rzalloc(SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, sizeof(*dd));
dd = sof_heap_alloc(heap, SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, sizeof(*dd), 0);
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.

just NULL

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.

I went back and forth between "heap" variable and just putting NULL, but in the end I think this is more readable and makes it explicit same value needs to be passed to alloc and free in this function. Compilation output should be the same in both cases.

if (!dd)
goto e_data;

memset(dd, 0, sizeof(*dd));

comp_set_drvdata(dev, dd);

ret = dai_common_new(dd, dev, dai_cfg);
Expand All @@ -610,7 +625,7 @@ __cold static struct comp_dev *dai_new(const struct comp_driver *drv,
return dev;

error:
rfree(dd);
sof_heap_free(heap, dd);
e_data:
comp_free_device(dev);
return NULL;
Expand Down Expand Up @@ -638,7 +653,7 @@ __cold void dai_common_free(struct dai_data *dd)

dai_put(dd->dai);

rfree(dd->dai_spec_config);
sof_heap_free(dd->alloc_ctx.heap, dd->dai_spec_config);
}

__cold static void dai_free(struct comp_dev *dev)
Expand All @@ -652,7 +667,8 @@ __cold static void dai_free(struct comp_dev *dev)

dai_common_free(dd);

rfree(dd);
/* heap is NULL to match what is passed in dai_new() */
sof_heap_free(NULL, dd);
comp_free_device(dev);
}

Expand Down Expand Up @@ -847,7 +863,7 @@ static int dai_set_sg_config(struct dai_data *dd, struct comp_dev *dev, uint32_t
} while (--max_block_count > 0);
}

err = dma_sg_alloc(NULL, &config->elem_array, SOF_MEM_FLAG_USER,
err = dma_sg_alloc(dd->alloc_ctx.heap, &config->elem_array, SOF_MEM_FLAG_USER,
config->direction,
period_count,
period_bytes,
Expand All @@ -873,8 +889,9 @@ static int dai_set_dma_config(struct dai_data *dd, struct comp_dev *dev)

comp_dbg(dev, "entry");

dma_cfg = rmalloc(SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT | SOF_MEM_FLAG_DMA,
sizeof(struct dma_config));
dma_cfg = sof_heap_alloc(dd->alloc_ctx.heap,
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT | SOF_MEM_FLAG_DMA,
sizeof(struct dma_config), 0);
if (!dma_cfg) {
comp_err(dev, "dma_cfg allocation failed");
return -ENOMEM;
Expand Down Expand Up @@ -903,10 +920,11 @@ static int dai_set_dma_config(struct dai_data *dd, struct comp_dev *dev)
else
dma_cfg->dma_slot = config->src_dev;

dma_block_cfg = rballoc(SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT | SOF_MEM_FLAG_DMA,
sizeof(struct dma_block_config) * dma_cfg->block_count);
dma_block_cfg = sof_heap_alloc(dd->alloc_ctx.heap,
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT | SOF_MEM_FLAG_DMA,
sizeof(struct dma_block_config) * dma_cfg->block_count, 0);
if (!dma_block_cfg) {
rfree(dma_cfg);
sof_heap_free(dd->alloc_ctx.heap, dma_cfg);
comp_err(dev, "dma_block_config allocation failed");
return -ENOMEM;
}
Expand Down Expand Up @@ -1040,7 +1058,7 @@ static int dai_set_dma_buffer(struct dai_data *dd, struct comp_dev *dev,
return err;
}
} else {
dd->dma_buffer = buffer_alloc_range(NULL, buffer_size_preferred, buffer_size,
dd->dma_buffer = buffer_alloc_range(&dd->alloc_ctx, buffer_size_preferred, buffer_size,
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_DMA,
addr_align, BUFFER_USAGE_NOT_SHARED);
if (!dd->dma_buffer) {
Expand Down Expand Up @@ -1128,8 +1146,8 @@ int dai_common_params(struct dai_data *dd, struct comp_dev *dev,
if (err < 0) {
buffer_free(dd->dma_buffer);
dd->dma_buffer = NULL;
dma_sg_free(NULL, &config->elem_array);
rfree(dd->z_config);
dma_sg_free(dd->alloc_ctx.heap, &config->elem_array);
Copy link
Copy Markdown
Collaborator Author

@kv2019i kv2019i May 22, 2026

Choose a reason for hiding this comment

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

This is a valid problem, but problem exists in existing code, so I prefer to handle this case in a follow-up PR. If I need to revise this series, I will add a new commit to address this.
UPDATE: actually, this is not a real leak. Only error case where dd->z_config is allocated and non-zero, is the case where dai_set_dma_config() fails. But if that fails, nothing is allocated to dd->z_config->head_block either. So current code in main is safe.

sof_heap_free(dd->alloc_ctx.heap, dd->z_config);
dd->z_config = NULL;
}

Expand Down Expand Up @@ -1255,10 +1273,10 @@ void dai_common_reset(struct dai_data *dd, struct comp_dev *dev)
if (!dd->delayed_dma_stop)
dai_dma_release(dd, dev);

dma_sg_free(NULL, &config->elem_array);
dma_sg_free(dd->alloc_ctx.heap, &config->elem_array);
if (dd->z_config) {
rfree(dd->z_config->head_block);
rfree(dd->z_config);
sof_heap_free(dd->alloc_ctx.heap, dd->z_config->head_block);
sof_heap_free(dd->alloc_ctx.heap, dd->z_config);
dd->z_config = NULL;
}

Expand Down
18 changes: 10 additions & 8 deletions src/include/sof/audio/component.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include <rtos/idc.h>
#include <rtos/mutex.h>
#include <rtos/userspace_helper.h>
#include <sof/lib/dai.h>
#include <sof/schedule/schedule.h>
#include <ipc/control.h>
#include <sof/ipc/topology.h>
Expand All @@ -37,6 +36,16 @@ struct dai_hw_params;
struct timestamp_data;
struct dai_ts_data;

struct k_heap;
struct vregion;
struct mod_alloc_ctx {
struct k_heap *heap;
struct vregion *vreg;
};

/* dai.h requires definition for mod_alloc_ctx */
#include <sof/lib/dai.h>

/** \addtogroup component_api Component API
* @{
*/
Expand Down Expand Up @@ -579,13 +588,6 @@ struct comp_ops {
uint64_t (*get_total_data_processed)(struct comp_dev *dev, uint32_t stream_no, bool input);
};

struct k_heap;
struct vregion;
struct mod_alloc_ctx {
struct k_heap *heap;
struct vregion *vreg;
};

/**
* Audio component base driver "class"
* - used by all other component types.
Expand Down
3 changes: 3 additions & 0 deletions src/include/sof/lib/dai-zephyr.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <sof/ipc/topology.h>
#include <sof/audio/pcm_converter.h>
#include <sof/audio/ipc-config.h>
#include <sof/audio/component.h>
#include <ipc/dai.h>
#include <errno.h>
#include <stddef.h>
Expand Down Expand Up @@ -168,6 +169,8 @@ struct dai_data {
#endif
/* Copier gain params */
struct copier_gain_params *gain_data;

struct mod_alloc_ctx alloc_ctx;
};

/* these 3 are here to satisfy clk.c and ssp.h interconnection, will be removed leter */
Expand Down
6 changes: 4 additions & 2 deletions src/ipc/ipc4/dai.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,15 +387,17 @@ __cold int dai_config(struct dai_data *dd, struct comp_dev *dev,
/* allocated dai_config if not yet */
if (!dd->dai_spec_config) {
size = sizeof(*copier_cfg);
dd->dai_spec_config = rzalloc(SOF_MEM_FLAG_USER, size);
dd->dai_spec_config = sof_heap_alloc(dd->alloc_ctx.heap, SOF_MEM_FLAG_USER, size, 0);
if (!dd->dai_spec_config) {
comp_err(dev, "No memory for size %d", size);
return -ENOMEM;
}

memset(dd->dai_spec_config, 0, size);

ret = memcpy_s(dd->dai_spec_config, size, copier_cfg, size);
if (ret < 0) {
rfree(dd->dai_spec_config);
sof_heap_free(dd->alloc_ctx.heap, dd->dai_spec_config);
dd->dai_spec_config = NULL;
return -EINVAL;
}
Expand Down
19 changes: 16 additions & 3 deletions src/lib/dai.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <sof/lib/uuid.h>
#include <rtos/spinlock.h>
#include <sof/trace/trace.h>
#include <sof/schedule/ll_schedule_domain.h> /* for zephyr_ll_user_heap() */
#include <ipc/topology.h>
#include <ipc/dai.h>
#include <user/trace.h>
Expand Down Expand Up @@ -311,6 +312,11 @@ struct dai *dai_get(uint32_t type, uint32_t index, uint32_t flags)
{
const struct device *dev;
struct dai *d;
struct k_heap *heap = NULL;

#ifdef CONFIG_SOF_USERSPACE_LL
heap = zephyr_ll_user_heap();
#endif

dev = dai_get_device(type, index);
if (!dev) {
Expand All @@ -319,10 +325,12 @@ struct dai *dai_get(uint32_t type, uint32_t index, uint32_t flags)
return NULL;
}

d = rzalloc(SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, sizeof(struct dai));
d = sof_heap_alloc(heap, SOF_MEM_FLAG_USER | SOF_MEM_FLAG_COHERENT, sizeof(struct dai), 0);
if (!d)
return NULL;

memset(d, 0, sizeof(struct dai));

d->index = index;
d->type = type;
d->dev = dev;
Expand All @@ -332,7 +340,7 @@ struct dai *dai_get(uint32_t type, uint32_t index, uint32_t flags)
if (dai_probe(d->dev)) {
tr_err(&dai_tr, "dai_get: failed to probe dai with index %d type %d",
index, type);
rfree(d);
sof_heap_free(heap, d);
return NULL;
}

Expand All @@ -343,14 +351,19 @@ struct dai *dai_get(uint32_t type, uint32_t index, uint32_t flags)
void dai_put(struct dai *dai)
{
int ret;
struct k_heap *heap = NULL;

#ifdef CONFIG_SOF_USERSPACE_LL
heap = zephyr_ll_user_heap();
#endif

ret = dai_remove(dai->dev);
if (ret < 0) {
tr_err(&dai_tr, "index %d failed ret = %d",
dai->index, ret);
}

rfree(dai);
sof_heap_free(heap, dai);
}
#else
static inline const struct dai_type_info *dai_find_type(uint32_t type)
Expand Down
19 changes: 14 additions & 5 deletions zephyr/syscall/sof_dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,19 +109,28 @@ static inline void z_vrfy_sof_dma_release_channel(struct sof_dma *dma,
*/
static inline struct dma_block_config *deep_copy_dma_blk_cfg_list(struct dma_config *cfg)
{
struct dma_block_config *kern_cfg =
rmalloc(0, sizeof(*kern_cfg) * cfg->block_count);
struct dma_block_config *kern_cfg;
struct dma_block_config *kern_prev = NULL, *kern_next, *user_next;
int i = 0;

if (!cfg->block_count)
return NULL;

kern_cfg = rmalloc(0, sizeof(*kern_cfg) * cfg->block_count);
if (!kern_cfg)
return NULL;

for (user_next = cfg->head_block, kern_next = kern_cfg;
user_next;
user_next = user_next->next_block, kern_next++) {
if (++i > cfg->block_count)
goto err;
user_next = user_next->next_block, kern_next++, i++) {
if (i == cfg->block_count) {
/* last block can point to first one */
if (user_next != cfg->head_block)
goto err;

kern_prev->next_block = kern_cfg;
break;
}

if (k_usermode_from_copy(kern_next, user_next, sizeof(*kern_next)))
goto err;
Expand Down
Loading