Skip to content

Commit

Permalink
WIP: Example code for downstream type conversion
Browse files Browse the repository at this point in the history
ONLY COMPILE/FUZZ TESTED

Given we cannot trust our enum field in the IPC as communicator may lie
about the component on the other side and the module adapter is
obscuring the nature of many of the components we need to move the type
casting into the components directly. Here is a breakdown of the general
steps to achieve this.

1. report ipc config size for all versions
2. remove type casting in the IPC3 layer
3. on non module adapter components cast the config in a compile time
   selected shim
4. in the module adapter ipc3 layer, remove type casting again
5. in module adapter components realloc init data and cast over in a
   compile time enabled shim

Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
  • Loading branch information
cujomalainey committed Oct 8, 2024
1 parent 2bbdf0a commit 43352f0
Show file tree
Hide file tree
Showing 12 changed files with 235 additions and 213 deletions.
29 changes: 28 additions & 1 deletion src/audio/asrc/asrc.c
Original file line number Diff line number Diff line change
Expand Up @@ -860,8 +860,35 @@ static int asrc_reset(struct processing_module *mod)
return 0;
}

#if CONFIG_IPC_MAJOR_3
static int asrc_init_shim(struct processing_module *mod)
{
struct ipc_config_asrc *ipc_asrc;
struct module_data *mod_data = &mod->priv;
const struct sof_ipc_comp_asrc *asrc =
(const struct sof_ipc_comp_asrc *)mod_data->cfg.init_data;

if (IPC_TAIL_IS_SIZE_INVALID(*asrc))
return -EBADMSG;

ipc_asrc = rballoc(0, SOF_MEM_CAPS_RAM, sizeof(*ipc_asrc));
if (!ipc_asrc)
return -ENOMEM;

ipc_asrc->source_rate = asrc->source_rate;
ipc_asrc->sink_rate = asrc->sink_rate;
ipc_asrc->asynchronous_mode = asrc->asynchronous_mode;
ipc_asrc->operation_mode = asrc->operation_mode;

rfree(mod_data->cfg.data);
mod_data->cfg.init_data = ipc_asrc;
mod_data->cfg.data = ipc_asrc;
return asrc_init(mod);
}
#endif

static const struct module_interface asrc_interface = {
.init = asrc_init,
.init = IPC3_SHIM(asrc_init),
.prepare = asrc_prepare,
.process_audio_stream = asrc_process,
.trigger = asrc_trigger,
Expand Down
21 changes: 20 additions & 1 deletion src/audio/dai-legacy.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,25 @@ static struct comp_dev *dai_new(const struct comp_driver *drv,
return NULL;
}

#if CONFIG_IPC_MAJOR_3
static struct comp_dev *dai_new_shim(const struct comp_driver *drv,
const struct comp_ipc_config *config,
const void *spec)
{
const struct sof_ipc_comp_dai *comp = spec;
struct ipc_config_dai dai;

if (IPC_TAIL_IS_SIZE_INVALID(*comp))
return NULL;

dai.dai_index = comp->dai_index;
dai.direction = comp->direction;
dai.type = comp->type;

return dai_new(drv, config, &dai);
}
#endif

void dai_common_free(struct dai_data *dd)
{
if (dd->group)
Expand Down Expand Up @@ -1109,7 +1128,7 @@ static const struct comp_driver comp_dai = {
.uid = SOF_RT_UUID(dai_uuid),
.tctx = &dai_comp_tr,
.ops = {
.create = dai_new,
.create = IPC3_SHIM(dai_new),
.free = dai_free,
.params = dai_params,
.dai_get_hw_params = dai_comp_get_hw_params,
Expand Down
21 changes: 20 additions & 1 deletion src/audio/dai-zephyr.c
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,25 @@ static struct comp_dev *dai_new(const struct comp_driver *drv,
return NULL;
}

#if CONFIG_IPC_MAJOR_3
static struct comp_dev *dai_new_shim(const struct comp_driver *drv,
const struct comp_ipc_config *config,
const void *spec)
{
const struct sof_ipc_comp_dai *comp = spec;
struct ipc_config_dai dai;

if (IPC_TAIL_IS_SIZE_INVALID(*comp))
return NULL;

dai.dai_index = comp->dai_index;
dai.direction = comp->direction;
dai.type = comp->type;

return dai_new(drv, config, &dai);
}
#endif

void dai_common_free(struct dai_data *dd)
{
#ifdef CONFIG_SOF_TELEMETRY_IO_PERFORMANCE_MEASUREMENTS
Expand Down Expand Up @@ -1919,7 +1938,7 @@ static const struct comp_driver comp_dai = {
.uid = SOF_RT_UUID(dai_uuid),
.tctx = &dai_comp_tr,
.ops = {
.create = dai_new,
.create = IPC3_SHIM(dai_new),
.free = dai_free,
.params = dai_params,
.dai_get_hw_params = dai_comp_get_hw_params,
Expand Down
21 changes: 20 additions & 1 deletion src/audio/host-legacy.c
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,25 @@ static struct comp_dev *host_new(const struct comp_driver *drv,
return NULL;
}

#if CONFIG_IPC_MAJOR_3
static struct comp_dev *host_new_shim(const struct comp_driver *drv,
const struct comp_ipc_config *config,
const void *spec)
{
const struct sof_ipc_comp_host *comp = spec;
struct ipc_config_host host;

if (IPC_TAIL_IS_SIZE_INVALID(*comp))
return NULL;

host.direction = comp->direction;
host.no_irq = comp->no_irq;
host.dmac_config = comp->dmac_config;

return host_new(drv, config, &host);
}
#endif

void host_common_free(struct host_data *hd)
{
dma_put(hd->dma);
Expand Down Expand Up @@ -1004,7 +1023,7 @@ static const struct comp_driver comp_host = {
.uid = SOF_RT_UUID(host_uuid),
.tctx = &host_tr,
.ops = {
.create = host_new,
.create = IPC3_SHIM(host_new),
.free = host_free,
.params = host_params,
.reset = host_reset,
Expand Down
60 changes: 2 additions & 58 deletions src/audio/module_adapter/module_adapter_ipc3.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,65 +39,9 @@ int module_adapter_init_data(struct comp_dev *dev,
const void *spec)
{
int ret;

const unsigned char *data = NULL;
uint32_t size = 0;

switch (config->type) {
case SOF_COMP_VOLUME:
{
const struct ipc_config_volume *ipc_volume = spec;

size = sizeof(*ipc_volume);
data = spec;
break;
}
case SOF_COMP_SRC:
{
const struct ipc_config_src *ipc_src = spec;

size = sizeof(*ipc_src);
data = spec;
break;
}
case SOF_COMP_ASRC:
{
const struct ipc_config_asrc *ipc_asrc = spec;

size = sizeof(*ipc_asrc);
data = spec;
break;
}
case SOF_COMP_MIXER:
break;
case SOF_COMP_EQ_IIR:
case SOF_COMP_EQ_FIR:
case SOF_COMP_KEYWORD_DETECT:
case SOF_COMP_KPB:
case SOF_COMP_SELECTOR:
case SOF_COMP_DEMUX:
case SOF_COMP_MUX:
case SOF_COMP_DCBLOCK:
case SOF_COMP_SMART_AMP:
case SOF_COMP_MODULE_ADAPTER:
case SOF_COMP_FILEREAD:
case SOF_COMP_FILEWRITE:
case SOF_COMP_NONE:
{
const struct ipc_config_process *ipc_module_adapter = spec;

size = ipc_module_adapter->size;
data = ipc_module_adapter->data;
break;
}
default:
comp_err(dev, "module_adapter_init_data() unsupported comp type %d", config->type);
return -EINVAL;
}

/* Copy initial config */
if (size) {
ret = module_load_config(dev, data, size);
if (config->ipc_config_size) {
ret = module_load_config(dev, spec, config->ipc_config_size);
if (ret < 0) {
comp_err(dev, "module_adapter_init_data() error %d: config loading has failed.",
ret);
Expand Down
16 changes: 15 additions & 1 deletion src/audio/smart_amp/smart_amp.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,20 @@ static struct comp_dev *smart_amp_new(const struct comp_driver *drv,
return NULL;
}

#if CONFIG_IPC_MAJOR_3
static struct comp_dev *smart_amp_new_shim(const struct comp_driver *drv,
const struct comp_ipc_config *config,
const void *spec)
{
struct ipc_config_process proc;

if (comp_sof_process_to_ipc_process(config->ipc_config_size, spec, &proc) < 0)
return NULL;

return smart_amp_new(drv, config, &proc);
}
#endif

static int smart_amp_set_config(struct comp_dev *dev,
struct sof_ipc_ctrl_data *cdata)
{
Expand Down Expand Up @@ -813,7 +827,7 @@ static const struct comp_driver comp_smart_amp = {
.uid = SOF_RT_UUID(UUID_SYM),
.tctx = &smart_amp_comp_tr,
.ops = {
.create = smart_amp_new,
.create = IPC3_SHIM(smart_amp_new),
.free = smart_amp_free,
.params = smart_amp_params,
.prepare = smart_amp_prepare,
Expand Down
28 changes: 27 additions & 1 deletion src/audio/src/src.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,34 @@ static int src_prepare(struct processing_module *mod,
return src_prepare_general(mod, sources[0], sinks[0]);
}

#if CONFIG_IPC_MAJOR_3
static int src_init_shim(struct processing_module *mod)
{
struct ipc_config_src *ipc_src;
struct module_data *mod_data = &mod->priv;
const struct sof_ipc_comp_src *src =
(const struct sof_ipc_comp_src *)mod_data->cfg.init_data;

if (IPC_TAIL_IS_SIZE_INVALID(*src))
return -EBADMSG;

ipc_src = rballoc(0, SOF_MEM_CAPS_RAM, sizeof(*ipc_src));
if (!ipc_src)
return -ENOMEM;

ipc_src->source_rate = src->source_rate;
ipc_src->sink_rate = src->sink_rate;
ipc_src->rate_mask = src->rate_mask;

rfree(mod_data->cfg.data);
mod_data->cfg.init_data = ipc_src;
mod_data->cfg.data = ipc_src;
return src_init(mod);
}
#endif

static const struct module_interface src_interface = {
.init = src_init,
.init = IPC3_SHIM(src_init),
.prepare = src_prepare,
.process = src_process,
.is_ready_to_process = src_is_ready_to_process,
Expand Down
27 changes: 26 additions & 1 deletion src/audio/tone.c
Original file line number Diff line number Diff line change
Expand Up @@ -706,12 +706,37 @@ static int tone_reset(struct comp_dev *dev)
return 0;
}

#if CONFIG_IPC_MAJOR_3
static struct comp_dev *tone_new_shim(const struct comp_driver *drv,
const struct comp_ipc_config *config,
const void *spec)
{
const struct sof_ipc_comp_tone *comp = spec;
struct ipc_config_tone tone;

if (IPC_TAIL_IS_SIZE_INVALID(*comp))
return NULL;

tone.ampl_mult = comp->ampl_mult;
tone.amplitude = comp->amplitude;
tone.freq_mult = comp->freq_mult;
tone.frequency = comp->frequency;
tone.length = comp->length;
tone.period = comp->period;
tone.ramp_step = comp->ramp_step;
tone.repeats = comp->repeats;
tone.sample_rate = comp->sample_rate;

return tone_new(drv, config, &tone);
}
#endif

static const struct comp_driver comp_tone = {
.type = SOF_COMP_TONE,
.uid = SOF_RT_UUID(tone_uuid),
.tctx = &tone_tr,
.ops = {
.create = tone_new,
.create = IPC3_SHIM(tone_new),
.free = tone_free,
.params = tone_params,
#if CONFIG_IPC_MAJOR_3
Expand Down
32 changes: 30 additions & 2 deletions src/audio/volume/volume.c
Original file line number Diff line number Diff line change
Expand Up @@ -775,8 +775,36 @@ static int volume_reset(struct processing_module *mod)
return 0;
}

#if CONFIG_IPC_MAJOR_3
static int volume_init_shim(struct processing_module *mod)
{
struct ipc_config_volume *ipc_volume;
struct module_data *mod_data = &mod->priv;
const struct sof_ipc_comp_volume *volume =
(const struct sof_ipc_comp_volume *)mod_data->cfg.init_data;

if (IPC_TAIL_IS_SIZE_INVALID(*volume))
return -EBADMSG;

ipc_volume = rballoc(0, SOF_MEM_CAPS_RAM, sizeof(*ipc_volume));
if (!ipc_volume)
return -ENOMEM;

ipc_volume->channels = volume->channels;
ipc_volume->initial_ramp = volume->initial_ramp;
ipc_volume->max_value = volume->max_value;
ipc_volume->min_value = volume->min_value;
ipc_volume->ramp = volume->ramp;

rfree(mod_data->cfg.data);
mod_data->cfg.init_data = ipc_volume;
mod_data->cfg.data = ipc_volume;
return volume_init(mod);
}
#endif

static const struct module_interface volume_interface = {
.init = volume_init,
.init = IPC3_SHIM(volume_init),
.prepare = volume_prepare,
.process_audio_stream = volume_process,
.set_configuration = volume_set_config,
Expand All @@ -790,7 +818,7 @@ SOF_MODULE_INIT(volume, sys_comp_module_volume_interface_init);

#if CONFIG_COMP_GAIN
static const struct module_interface gain_interface = {
.init = volume_init,
.init = IPC3_SHIM(volume_init),
.prepare = volume_prepare,
.process_audio_stream = volume_process,
.set_configuration = volume_set_config,
Expand Down
17 changes: 17 additions & 0 deletions src/include/sof/audio/component.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,23 @@ enum {
#define IPC3_SHIM(init) init
#endif

/** \brief Helper to convert sof_ipc_comp_process to ipc_config_process */
static inline int comp_sof_process_to_ipc_process(size_t size,
const struct sof_ipc_comp_process *comp,
struct ipc_config_process *proc)
{
if (size < sizeof(*comp) + comp->size || size + comp->size > SOF_IPC_MSG_MAX_SIZE)
return -EBADMSG;
proc->type = comp->type;
proc->size = comp->size;
#if CONFIG_LIBRARY || UNIT_TEST
proc->data = comp->data + comp->comp.ext_data_length;
#else
proc->data = comp->data;
#endif
return 0;
}

/** \brief Type of endpoint this component is connected to in a pipeline */
enum comp_endpoint_type {
COMP_ENDPOINT_HOST, /**< Connected to host dma */
Expand Down
Loading

0 comments on commit 43352f0

Please sign in to comment.