Re: [PATCH v6 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ALSA: q6apm-dai: Uncapped 8MB allocation for compressed streams exhausts carveout
- [Medium] ALSA: q6apm-dai: Missing buffer size hardware constraint for capture streams
--

commit 0f7913cfca3d367699e852961892b2259896b6d2
Author: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>

ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms

This commit adds SCM buffer assignment for mDSP platforms where audio buffers
must be explicitly assigned to specific VMIDs for the DSP to access them.
It implements support for both control-path and data-path memory carveouts
assigned at probe time.

> diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
> index 033197bdd8705..b0b10ba51c99d 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
[ ... ]
> @@ -409,8 +518,11 @@ static int q6apm_dai_open(struct snd_soc_component *component,
>  	}
>  
>  	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		size_t buf_max = pdata->has_reserved_mem ? pdata->reserved_buf_size :
> +							   BUFFER_BYTES_MAX;
> +
>  		ret = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_BYTES,
> -						   BUFFER_BYTES_MIN, BUFFER_BYTES_MAX);
> +						   BUFFER_BYTES_MIN, buf_max);

[Severity: Medium]
What happens when a user opens a capture stream on a DAI using the restricted
memory pool (has_reserved_mem is true)?

Since the dynamic buf_max constraint is applied exclusively to playback
streams, will capture streams be left unguarded, potentially advertising
larger buffer capabilities than physically allocated in the carveout pool and
causing allocation failures during ALSA hw_params negotiation?

>  		if (ret < 0) {
>  			dev_err(dev, "constraint for buffer bytes min max ret = %d\n", ret);
>  			goto err;
[ ... ]
> @@ -683,6 +830,17 @@ static int q6apm_dai_compr_open(struct snd_soc_component *component,
>  	if (ret)
>  		return ret;
>  
> +	if (pdata->use_scm_assign && !pdata->has_reserved_mem) {
> +		ret = q6apm_dai_scm_assign(pdata, prtd->dma_buffer.addr,
> +					   ALIGN(size, PAGE_SIZE));

[Severity: High]
Does this code risk exhausting the carveout pool when opening a compressed
stream?

In q6apm_dai_compr_open(), just before this snippet, the buffer size is
statically set to 8MB (COMPR_PLAYBACK_MAX_FRAGMENT_SIZE *
COMPR_PLAYBACK_MAX_NUM_FRAGMENTS) and allocated via snd_dma_alloc_pages()
using the device DMA context.

Since the device is bound to a small restricted memory carveout pool via
of_reserved_mem_device_init_by_idx() when has_reserved_mem is true, wouldn't
this unbounded allocation instantly fail or exhaust the pool?

Should the allocation size be capped by reserved_buf_size, similar to the
implementation for standard PCM streams?

> +		if (ret) {
> +			dev_err(dev, "SCM assign compr buffer failed: %d\n",
> +				ret);
> +			snd_dma_free_pages(&prtd->dma_buffer);
> +			return ret;
> +		}
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-vmid-v4-v6-0-3d61a0319b1c@oss.qualcomm.com?part=4