[PATCH v1 0/3] ASoC: qcom: add q6apm VMID support and qdsp6 GPR destination-domain routing

[PATCH v1 0/3] ASoC: qcom: add q6apm VMID support and qdsp6 GPR destination-domain routing

[Ajay Kumar Nandam]

This series adds two related pieces for Qualcomm audio paths that are not
always ADSP-targeted.

1. Add optional qcom,vmid support in q6apm-dai so fixed PCM DMA buffers can
   be shared with secure/non-HLOS VMIDs via SCM assignment when required.
2. Generalize qdsp6 AudioReach/APM/PRM packet destination-domain routing so
   packet allocation follows the active GPR device domain instead of assuming
   ADSP, while preserving ADSP fallback for legacy/default paths.

This is needed for platforms such as Shikra where audio may execute on mDSP
and buffer ownership must be explicitly shared across VMIDs.

Ajay Kumar Nandam (3):
  dt-bindings: sound: qcom,q6apm-dai: add optional qcom,vmid
  ASoC: qcom: q6apm-dai: add VMID-based SCM assignment
  ASoC: qcom: qdsp6: generalize GPR service domain

 .../bindings/sound/qcom,q6apm-dai.yaml        |  10 +
 sound/soc/qcom/Kconfig                        |   1 +
 sound/soc/qcom/qdsp6/audioreach.c             | 112 ++++++++---
 sound/soc/qcom/qdsp6/audioreach.h             |  23 ++-
 sound/soc/qcom/qdsp6/q6apm-dai.c              | 178 +++++++++++++++++-
 sound/soc/qcom/qdsp6/q6apm.c                  |  48 +++--
 sound/soc/qcom/qdsp6/q6prm.c                  |  19 +-
 7 files changed, 318 insertions(+), 73 deletions(-)

-- 
2.34.1

[PATCH v1 1/3] dt-bindings: sound: qcom,q6apm-dai: add optional qcom,vmid

[Ajay Kumar Nandam]

Add an optional qcom,vmid property for q6apm-dais nodes to describe
additional VMIDs that should be granted access to PCM DMA buffers.

The property is optional and bounded, and driver-side validation will
reject invalid values such as including HLOS in the list or VMIDs that
exceed 31, matching current SCM source-permission mask handling.

Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
 .../devicetree/bindings/sound/qcom,q6apm-dai.yaml      | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
index 9e5b30d9c6e6..b767625985a7 100644
--- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
@@ -20,6 +20,16 @@ properties:
     minItems: 1
     maxItems: 2
 
+  qcom,vmid:
+    description: Optional list of destination VMIDs to share PCM DMA buffers with.
+      HLOS retains RW access as source owner and must not be listed.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      minimum: 1
+      maximum: 31
+    minItems: 1
+    maxItems: 8
+
 required:
   - compatible
   - iommus
-- 
2.34.1

[PATCH v1 2/3] ASoC: qcom: q6apm-dai: add VMID-based SCM assignment

[Ajay Kumar Nandam]

Add optional VMID-based SCM memory assignment for q6apm fixed PCM DMA
buffers when qcom,vmid is provided in DT.

On platforms where audio processing runs outside HLOS, LPASS PCM DMA
buffers must be shared with secure/non-HLOS VMIDs so both endpoints can
access the same payload memory.

On platforms where audio runs on mDSP and buffers are backed by
secure/stage-2 managed memory, HLOS-only ownership is not sufficient for
DSP access. Optional VMID-based SCM assignment provides the required
shared ownership model for those systems.

Keep HLOS as an RW owner and grant configured destination VMIDs RW
access so both playback and capture data paths remain functional.

Assign and unassign SCM permissions at fixed-buffer lifetime boundaries:
assign in pcm_new() after fixed buffer allocation/map, and restore HLOS
ownership in pcm_free() before unmap.

This avoids reassigning the same fixed buffer in stream-lifecycle paths
and aligns ownership transitions with the fixed DMA pool lifetime.

Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
 sound/soc/qcom/Kconfig           |   1 +
 sound/soc/qcom/qdsp6/q6apm-dai.c | 178 +++++++++++++++++++++++++++++--
 2 files changed, 170 insertions(+), 9 deletions(-)

diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index e6e24f3b9922..82f496e53acb 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -101,6 +101,7 @@ config SND_SOC_QDSP6_ASM_DAI
 
 config SND_SOC_QDSP6_APM_DAI
 	tristate
+	select QCOM_SCM
 	select SND_SOC_COMPRESS
 
 config SND_SOC_QDSP6_APM_LPASS_DAI
diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index bf1f872a09f4..ae7bcf980236 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -14,6 +14,7 @@
 #include <asm/div64.h>
 #include <asm/dma.h>
 #include <linux/dma-mapping.h>
+#include <linux/firmware/qcom/qcom_scm.h>
 #include <sound/pcm_params.h>
 #include "q6apm.h"
 
@@ -34,6 +35,8 @@
 #define COMPR_PLAYBACK_MAX_NUM_FRAGMENTS (16 * 4)
 #define COMPR_PLAYBACK_MIN_FRAGMENT_SIZE (8 * 1024)
 #define COMPR_PLAYBACK_MIN_NUM_FRAGMENTS (4)
+#define Q6APM_MAX_VMIDS 8
+#define Q6APM_SCM_MAX_VMID 31
 #define SID_MASK_DEFAULT	0xF
 
 static const struct snd_compr_codec_caps q6apm_compr_caps = {
@@ -83,10 +86,106 @@ struct q6apm_dai_rtd {
 	bool notify_on_drain;
 };
 
+struct q6apm_scm_region {
+	phys_addr_t dma_addr;
+	unsigned int size;
+	u64 src_perms;
+	bool assigned;
+};
+
 struct q6apm_dai_data {
 	long long sid;
+	int num_vmids;
+	u32 vmids[Q6APM_MAX_VMIDS];
+	bool use_scm_assign;
+	struct q6apm_scm_region scm_regions[SNDRV_PCM_STREAM_LAST + 1];
 };
 
+static int q6apm_dai_assign_memory(struct snd_pcm_substream *substream,
+				   struct q6apm_dai_data *pdata)
+{
+	struct q6apm_scm_region *scm_region = &pdata->scm_regions[substream->stream];
+	struct qcom_scm_vmperm *dst_vmids;
+	int dst_count = 0;
+	int ret;
+	int i;
+
+	if (!pdata->use_scm_assign || pdata->num_vmids <= 0 || scm_region->assigned)
+		return 0;
+
+	if (!substream->dma_buffer.addr)
+		return -ENOMEM;
+
+	dst_vmids = kcalloc(pdata->num_vmids + 1, sizeof(*dst_vmids), GFP_KERNEL);
+	if (!dst_vmids)
+		return -ENOMEM;
+
+	/* Always keep HLOS RW so CPU can continue buffer access. */
+	dst_vmids[dst_count].vmid = QCOM_SCM_VMID_HLOS;
+	dst_vmids[dst_count].perm = QCOM_SCM_PERM_RW;
+	dst_count++;
+
+	for (i = 0; i < pdata->num_vmids; i++) {
+		/*
+		 * Probe-time validation rejects HLOS in qcom,vmid, so this is
+		 * only a defensive check for future non-DT vmids[] population.
+		 */
+		if (WARN_ON_ONCE(pdata->vmids[i] == QCOM_SCM_VMID_HLOS))
+			continue;
+
+		dst_vmids[dst_count].vmid = pdata->vmids[i];
+		dst_vmids[dst_count].perm = QCOM_SCM_PERM_RW;
+		dst_count++;
+	}
+
+	/* Nothing to assign beyond HLOS access. */
+	if (dst_count == 1) {
+		kfree(dst_vmids);
+		return 0;
+	}
+
+	scm_region->dma_addr = substream->dma_buffer.addr;
+	scm_region->size = ALIGN(BUFFER_BYTES_MAX, PAGE_SIZE);
+	scm_region->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
+
+	ret = qcom_scm_assign_mem(scm_region->dma_addr, scm_region->size,
+				  &scm_region->src_perms, dst_vmids, dst_count);
+	kfree(dst_vmids);
+	if (ret)
+		return ret;
+
+	scm_region->assigned = true;
+	return 0;
+}
+
+static int q6apm_dai_unassign_memory(struct snd_soc_component *component,
+				     struct snd_pcm_substream *substream,
+				     struct q6apm_dai_data *pdata)
+{
+	struct q6apm_scm_region *scm_region = &pdata->scm_regions[substream->stream];
+	struct qcom_scm_vmperm hlos = {
+		.vmid = QCOM_SCM_VMID_HLOS,
+		.perm = QCOM_SCM_PERM_RW,
+	};
+	struct device *dev = component->dev;
+	int ret;
+
+	if (!pdata->use_scm_assign || !scm_region->assigned)
+		return 0;
+
+	ret = qcom_scm_assign_mem(scm_region->dma_addr, scm_region->size,
+				  &scm_region->src_perms, &hlos, 1);
+	if (!ret) {
+		scm_region->assigned = false;
+		scm_region->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
+	} else {
+		dev_err(dev, "Failed to unassign DMA buffer %pa from VMIDs: %d\n",
+			&scm_region->dma_addr, ret);
+	}
+
+	return ret;
+}
+
 static const struct snd_pcm_hardware q6apm_dai_hardware_capture = {
 	.info =                 (SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_BLOCK_TRANSFER |
 				 SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_INTERLEAVED |
@@ -218,13 +317,8 @@ static int q6apm_dai_prepare(struct snd_soc_component *component,
 	struct q6apm_dai_rtd *prtd = runtime->private_data;
 	struct audioreach_module_config cfg;
 	struct device *dev = component->dev;
-	struct q6apm_dai_data *pdata;
 	int ret;
 
-	pdata = snd_soc_component_get_drvdata(component);
-	if (!pdata)
-		return -EINVAL;
-
 	if (!prtd || !prtd->graph) {
 		dev_err(dev, "%s: private data null or audio client freed\n", __func__);
 		return -EINVAL;
@@ -569,9 +663,13 @@ static int q6apm_dai_memory_map(struct snd_soc_component *component,
 	return ret;
 }
 
+static void q6apm_dai_memory_unmap(struct snd_soc_component *component,
+				   struct snd_pcm_substream *substream);
+
 static int q6apm_dai_pcm_new(struct snd_soc_component *component, struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+	struct q6apm_dai_data *pdata = snd_soc_component_get_drvdata(component);
 	struct snd_pcm *pcm = rtd->pcm;
 	/*
 	 * Allocate one extra page as a workaround for a DSP bug where 32-bit
@@ -583,15 +681,17 @@ static int q6apm_dai_pcm_new(struct snd_soc_component *component, struct snd_soc
 	bool is_push_pull;
 	struct snd_pcm_substream *substream = NULL;
 
+	if (!pdata)
+		return -EINVAL;
+
 	graph_id = cpu_dai->driver->id;
 
 	/* Note: DSP backend dais are uni-directional ONLY(either playback or capture) */
 	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream)
 		substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
-	else  if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream)
+	else if (pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream)
 		substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
 
-
 	if (substream) {
 		is_push_pull = q6apm_is_graph_in_push_pull_mode_from_id(component->dev,
 									graph_id,
@@ -606,6 +706,14 @@ static int q6apm_dai_pcm_new(struct snd_soc_component *component, struct snd_soc
 		ret = q6apm_dai_memory_map(component, substream, graph_id, is_push_pull);
 		if (ret)
 			return ret;
+
+		if (pdata->use_scm_assign) {
+			ret = q6apm_dai_assign_memory(substream, pdata);
+			if (ret) {
+				q6apm_dai_memory_unmap(component, substream);
+				return ret;
+			}
+		}
 	}
 
 	return 0;
@@ -635,15 +743,25 @@ static void q6apm_dai_memory_unmap(struct snd_soc_component *component,
 
 static void q6apm_dai_pcm_free(struct snd_soc_component *component, struct snd_pcm *pcm)
 {
+	struct q6apm_dai_data *pdata = snd_soc_component_get_drvdata(component);
 	struct snd_pcm_substream *substream;
 
+	if (!pdata)
+		return;
+
 	substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
-	if (substream)
+	if (substream) {
+		if (pdata->use_scm_assign)
+			q6apm_dai_unassign_memory(component, substream, pdata);
 		q6apm_dai_memory_unmap(component, substream);
+	}
 
 	substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
-	if (substream)
+	if (substream) {
+		if (pdata->use_scm_assign)
+			q6apm_dai_unassign_memory(component, substream, pdata);
 		q6apm_dai_memory_unmap(component, substream);
+	}
 }
 
 static int q6apm_dai_compr_open(struct snd_soc_component *component,
@@ -1023,6 +1141,7 @@ static int q6apm_dai_probe(struct platform_device *pdev)
 	struct device_node *node = dev->of_node;
 	struct q6apm_dai_data *pdata;
 	struct of_phandle_args args;
+	int vmids;
 	int rc;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
@@ -1035,6 +1154,47 @@ static int q6apm_dai_probe(struct platform_device *pdev)
 	else
 		pdata->sid = args.args[0] & SID_MASK_DEFAULT;
 
+	vmids = of_property_count_u32_elems(node, "qcom,vmid");
+	if (vmids == -EINVAL) {
+		pdata->num_vmids = 0;
+		pdata->use_scm_assign = false;
+	} else if (vmids < 0) {
+		return vmids;
+	} else if (vmids == 0) {
+		dev_err(dev, "qcom,vmid must contain at least one VMID\n");
+		return -EINVAL;
+	} else if (vmids > Q6APM_MAX_VMIDS) {
+		dev_err(dev, "qcom,vmid: %d VMIDs exceeds maximum of %d\n",
+			vmids, Q6APM_MAX_VMIDS);
+		return -EINVAL;
+	}
+
+	if (vmids > 0) {
+		int i;
+
+		rc = of_property_read_u32_array(node, "qcom,vmid",
+						pdata->vmids, vmids);
+		if (rc)
+			return rc;
+		for (i = 0; i < vmids; i++) {
+			if (pdata->vmids[i] == QCOM_SCM_VMID_HLOS) {
+				dev_err(dev, "qcom,vmid must not include HLOS VMID (%u)\n",
+					QCOM_SCM_VMID_HLOS);
+				return -EINVAL;
+			}
+			if (pdata->vmids[i] > Q6APM_SCM_MAX_VMID) {
+				dev_err(dev, "qcom,vmid[%d]=%u exceeds SCM max VMID %u\n",
+					i, pdata->vmids[i], Q6APM_SCM_MAX_VMID);
+				return -EINVAL;
+			}
+		}
+		pdata->num_vmids = vmids;
+		pdata->use_scm_assign = true;
+	}
+
+	if (pdata->use_scm_assign && !qcom_scm_is_available())
+		return -EPROBE_DEFER;
+
 	dev_set_drvdata(dev, pdata);
 
 	return devm_snd_soc_register_component(dev, &q6apm_fe_dai_component, NULL, 0);
-- 
2.34.1

[PATCH v1 3/3] ASoC: qcom: qdsp6: generalize GPR service domain

[Ajay Kumar Nandam]

Use each GPR domain advertised by the qcom APR/GPR layer when building
APM/PRM packets and add explicit dest_domain parameters to all AudioReach
alloc helpers.
Legacy helpers fall back to ADSP while graphs, media-format, and
shared-memory packets reuse gdev->domain_id for modem targets.

Co-developed-by: Pratyush Meduri <mpratyus@qti.qualcomm.com>
Signed-off-by: Pratyush Meduri <mpratyus@qti.qualcomm.com>
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
 sound/soc/qcom/qdsp6/audioreach.c | 112 +++++++++++++++++++++---------
 sound/soc/qcom/qdsp6/audioreach.h |  23 +++---
 sound/soc/qcom/qdsp6/q6apm.c      |  48 +++++++++----
 sound/soc/qcom/qdsp6/q6prm.c      |  19 ++---
 4 files changed, 138 insertions(+), 64 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index e6e9eb2e85aa..8090181ac7ce 100644
--- a/sound/soc/qcom/qdsp6/audioreach.c
+++ b/sound/soc/qcom/qdsp6/audioreach.c
@@ -227,8 +227,9 @@ struct apm_module_sp_vi_channel_map_cfg {
 				sizeof(struct apm_module_sp_vi_channel_map_cfg) + \
 				(ch) * sizeof(uint32_t), 8)
 
-static void *__audioreach_alloc_pkt(int payload_size, uint32_t opcode, uint32_t token,
-				    uint32_t src_port, uint32_t dest_port, bool has_cmd_hdr)
+static void *__audioreach_alloc_pkt(int payload_size, u32 opcode, u32 token,
+				    u32 src_port, u32 dest_port, u16 dest_domain,
+				    bool has_cmd_hdr)
 {
 	struct gpr_pkt *pkt;
 	void *p;
@@ -248,7 +249,10 @@ static void *__audioreach_alloc_pkt(int payload_size, uint32_t opcode, uint32_t
 	pkt->hdr.dest_port = dest_port;
 	pkt->hdr.src_port = src_port;
 
-	pkt->hdr.dest_domain = GPR_DOMAIN_ID_ADSP;
+	if (!dest_domain)
+		dest_domain = GPR_DOMAIN_ID_ADSP;
+
+	pkt->hdr.dest_domain = dest_domain;
 	pkt->hdr.src_domain = GPR_DOMAIN_ID_APPS;
 	pkt->hdr.token = token;
 	pkt->hdr.opcode = opcode;
@@ -264,31 +268,35 @@ static void *__audioreach_alloc_pkt(int payload_size, uint32_t opcode, uint32_t
 	return pkt;
 }
 
-void *audioreach_alloc_pkt(int payload_size, uint32_t opcode, uint32_t token,
-			   uint32_t src_port, uint32_t dest_port)
+void *audioreach_alloc_pkt(int payload_size, u32 opcode, u32 token,
+			   u32 src_port, u32 dest_port, u16 dest_domain)
 {
-	return __audioreach_alloc_pkt(payload_size, opcode, token, src_port, dest_port, false);
+	return __audioreach_alloc_pkt(payload_size, opcode, token, src_port, dest_port,
+				      dest_domain, false);
 }
 EXPORT_SYMBOL_GPL(audioreach_alloc_pkt);
 
-void *audioreach_alloc_apm_pkt(int pkt_size, uint32_t opcode, uint32_t token, uint32_t src_port)
+void *audioreach_alloc_apm_pkt(int pkt_size, u32 opcode, u32 token,
+			       u32 src_port, u16 dest_domain)
 {
 	return __audioreach_alloc_pkt(pkt_size, opcode, token, src_port, APM_MODULE_INSTANCE_ID,
-				      false);
+		  dest_domain, false);
 }
 EXPORT_SYMBOL_GPL(audioreach_alloc_apm_pkt);
 
-void *audioreach_alloc_cmd_pkt(int payload_size, uint32_t opcode, uint32_t token,
-			       uint32_t src_port, uint32_t dest_port)
+void *audioreach_alloc_cmd_pkt(int payload_size, u32 opcode, u32 token,
+			       u32 src_port, u32 dest_port, u16 dest_domain)
 {
-	return __audioreach_alloc_pkt(payload_size, opcode, token, src_port, dest_port, true);
+	return __audioreach_alloc_pkt(payload_size, opcode, token, src_port, dest_port,
+		  dest_domain, true);
 }
 EXPORT_SYMBOL_GPL(audioreach_alloc_cmd_pkt);
 
-void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t token)
+void *audioreach_alloc_apm_cmd_pkt(int pkt_size, u32 opcode, u32 token,
+				   u16 dest_domain)
 {
 	return __audioreach_alloc_pkt(pkt_size, opcode, token, GPR_APM_MODULE_IID,
-				       APM_MODULE_INSTANCE_ID, true);
+		  APM_MODULE_INSTANCE_ID, dest_domain, true);
 }
 EXPORT_SYMBOL_GPL(audioreach_alloc_apm_cmd_pkt);
 
@@ -485,6 +493,7 @@ void *audioreach_alloc_graph_pkt(struct q6apm *apm,
 	int num_sub_graphs = 0;
 	int num_modules = 0;
 	int num_modules_list;
+	u16 dest_domain = audioreach_gpr_dest_domain(apm->gdev);
 	struct gpr_pkt *pkt;
 	void *p;
 
@@ -519,7 +528,8 @@ void *audioreach_alloc_graph_pkt(struct q6apm *apm,
 	mc_sz =	APM_MOD_CONN_PSIZE(mcon, num_connections);
 
 	payload_size = sg_sz + cont_sz + ml_sz + mp_sz + mc_sz;
-	pkt = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_GRAPH_OPEN, 0);
+	pkt = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_GRAPH_OPEN, 0,
+					   dest_domain);
 	if (IS_ERR(pkt))
 		return pkt;
 
@@ -645,7 +655,10 @@ static int audioreach_display_port_set_media_format(struct q6apm_graph *graph,
 	int size = ic_sz + ep_sz + fs_sz;
 	void *p;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -704,7 +717,10 @@ static int audioreach_codec_dma_set_media_format(struct q6apm_graph *graph,
 	int size = ic_sz + ep_sz + fs_sz + pm_sz;
 	void *p;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -764,7 +780,10 @@ int audioreach_send_u32_param(struct q6apm_graph *graph,
 	int payload_size = sizeof(uint32_t) + APM_MODULE_PARAM_DATA_SIZE;
 	void *p;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return -ENOMEM;
 
@@ -820,7 +839,10 @@ static int audioreach_set_module_config(struct q6apm_graph *graph,
 	int size = le32_to_cpu(module->data->size);
 	void *p;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -843,7 +865,10 @@ static int audioreach_mfc_set_media_format(struct q6apm_graph *graph,
 	int i;
 	void *p;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -958,9 +983,12 @@ int audioreach_compr_set_param(struct q6apm_graph *graph,
 	int iid = graph->shm_iid;
 	int payload_size = sizeof(struct apm_sh_module_media_fmt_cmd);
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_cmd_pkt(payload_size,
-					DATA_CMD_WR_SH_MEM_EP_MEDIA_FORMAT,
-					0, graph->port->id, iid);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_cmd_pkt(payload_size,
+					 DATA_CMD_WR_SH_MEM_EP_MEDIA_FORMAT,
+						0, graph->port->id, iid,
+					 dest_domain);
 	if (IS_ERR(pkt))
 		return -ENOMEM;
 
@@ -988,7 +1016,10 @@ static int audioreach_i2s_set_media_format(struct q6apm_graph *graph,
 	int size = ic_sz + ep_sz + fs_sz;
 	void *p;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1050,7 +1081,10 @@ static int audioreach_logging_set_media_format(struct q6apm_graph *graph,
 	int size = sizeof(*cfg) + APM_MODULE_PARAM_DATA_SIZE;
 	void *p;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1088,8 +1122,10 @@ static int audioreach_pcm_set_media_format(struct q6apm_graph *graph,
 
 	payload_size = APM_PCM_MODULE_FMT_CMD_PSIZE(num_channels);
 
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
 	struct gpr_pkt *pkt __free(kfree) =
-		audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
+		audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1172,9 +1208,11 @@ static int audioreach_shmem_set_media_format(struct q6apm_graph *graph,
 
 	payload_size = APM_SHMEM_FMT_CFG_PSIZE(num_channels) + APM_MODULE_PARAM_DATA_SIZE;
 
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
 	struct gpr_pkt *pkt __free(kfree) =
 		audioreach_alloc_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
-					 graph->port->id, module->instance_id);
+					 graph->port->id, module->instance_id,
+					 dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1219,7 +1257,10 @@ int audioreach_gain_set_vol_ctrl(struct q6apm *apm,
 	struct apm_module_param_data *param_data;
 	int size = sizeof(*cfg) + APM_MODULE_PARAM_DATA_SIZE;
 	void *p;
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1244,7 +1285,10 @@ static int audioreach_gain_set(struct q6apm_graph *graph,
 	struct apm_module_param_data *param_data;
 	struct apm_gain_module_cfg *cfg;
 	int size = APM_GAIN_CFG_PSIZE;
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1280,6 +1324,7 @@ static int audioreach_speaker_protection_vi(struct q6apm_graph *graph,
 	int op_sz, cm_sz, ex_sz;
 	struct apm_module_param_data *param_data;
 	int rc, i, payload_size;
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
 	struct gpr_pkt *pkt;
 	void *p;
 
@@ -1295,7 +1340,8 @@ static int audioreach_speaker_protection_vi(struct q6apm_graph *graph,
 
 	payload_size = op_sz + cm_sz + ex_sz;
 
-	pkt = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);
+	pkt = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0,
+					   dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -1480,8 +1526,12 @@ int audioreach_shared_memory_send_eos(struct q6apm_graph *graph)
 {
 	struct data_cmd_wr_sh_mem_ep_eos *eos;
 	int iid = graph->shm_iid;
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_cmd_pkt(sizeof(*eos),
-					DATA_CMD_WR_SH_MEM_EP_EOS, 0, graph->port->id, iid);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_cmd_pkt(sizeof(*eos),
+					 DATA_CMD_WR_SH_MEM_EP_EOS, 0,
+					 graph->port->id, iid,
+					 dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index 62a2fd79bbcb..d07fcd3a8fa7 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -912,17 +912,20 @@ struct audioreach_module_config {
 };
 
 /* Packet Allocation routines */
-void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t
-				    token);
+static inline u16 audioreach_gpr_dest_domain(gpr_device_t *gdev)
+{
+	return gdev && gdev->domain_id ? gdev->domain_id : GPR_DOMAIN_ID_ADSP;
+}
+
+void *audioreach_alloc_apm_cmd_pkt(int pkt_size, u32 opcode, u32 token,
+				   u16 dest_domain);
 void audioreach_set_default_channel_mapping(u8 *ch_map, int num_channels);
-void *audioreach_alloc_cmd_pkt(int payload_size, uint32_t opcode,
-			       uint32_t token, uint32_t src_port,
-			       uint32_t dest_port);
-void *audioreach_alloc_apm_pkt(int pkt_size, uint32_t opcode, uint32_t token,
-				uint32_t src_port);
-void *audioreach_alloc_pkt(int payload_size, uint32_t opcode,
-			   uint32_t token, uint32_t src_port,
-			   uint32_t dest_port);
+void *audioreach_alloc_cmd_pkt(int payload_size, u32 opcode, u32 token,
+			       u32 src_port, u32 dest_port, u16 dest_domain);
+void *audioreach_alloc_apm_pkt(int pkt_size, u32 opcode, u32 token,
+			       u32 src_port, u16 dest_domain);
+void *audioreach_alloc_pkt(int payload_size, u32 opcode, u32 token,
+			   u32 src_port, u32 dest_port, u16 dest_domain);
 void *audioreach_alloc_graph_pkt(struct q6apm *apm,
 				 const struct audioreach_graph_info *info);
 /* Topology specific */
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index 2e5b25b8d00f..12ca9fd0fbe8 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -102,7 +102,10 @@ static int audioreach_graph_mgmt_cmd(struct audioreach_graph *graph, uint32_t op
 	struct q6apm *apm = graph->apm;
 	int i = 0, payload_size = APM_GRAPH_MGMT_PSIZE(mgmt_cmd, num_sub_graphs);
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(payload_size, opcode, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(payload_size, opcode, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -142,8 +145,10 @@ static void q6apm_put_audioreach_graph(struct kref *ref)
 
 static int q6apm_get_apm_state(struct q6apm *apm)
 {
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(0,
-								APM_CMD_GET_SPF_STATE, 0);
+	u16 dest_domain = audioreach_gpr_dest_domain(apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(0, APM_CMD_GET_SPF_STATE, 0,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -216,9 +221,11 @@ static int __q6apm_map_memory_fixed_region(struct device *dev, unsigned int grap
 	uint32_t buf_sz;
 	void *p;
 	uint32_t pos_mask = is_pos_buf ? APM_MMAP_TOKEN_MAP_TYPE_POS_BUF : 0;
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(payload_size,
-					APM_CMD_SHARED_MEM_MAP_REGIONS, (graph_id | pos_mask));
-
+	u16 dest_domain = audioreach_gpr_dest_domain(apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(payload_size,
+					     APM_CMD_SHARED_MEM_MAP_REGIONS,
+					     graph_id | pos_mask, dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -326,8 +333,11 @@ static int __q6apm_unmap_memory_fixed_region(struct device *dev, unsigned int gr
 	struct q6apm *apm = dev_get_drvdata(dev->parent);
 	struct audioreach_graph_info *info;
 	uint32_t mem_map_handle;
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(sizeof(*cmd),
-						APM_CMD_SHARED_MEM_UNMAP_REGIONS, graph_id);
+	u16 dest_domain = audioreach_gpr_dest_domain(apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_apm_cmd_pkt(sizeof(*cmd),
+					     APM_CMD_SHARED_MEM_UNMAP_REGIONS, graph_id,
+					     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -475,10 +485,14 @@ int q6apm_write_async(struct q6apm_graph *graph, uint32_t len, uint32_t msw_ts,
 	struct apm_data_cmd_wr_sh_mem_ep_data_buffer_v2 *write_buffer;
 	struct audio_buffer *ab;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_pkt(sizeof(*write_buffer),
-					DATA_CMD_WR_SH_MEM_EP_DATA_BUFFER_V2,
-					graph->rx_data.dsp_buf | (len << APM_WRITE_TOKEN_LEN_SHIFT),
-					graph->port->id, graph->shm_iid);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_pkt(sizeof(*write_buffer),
+				     DATA_CMD_WR_SH_MEM_EP_DATA_BUFFER_V2,
+				     graph->rx_data.dsp_buf |
+				     (len << APM_WRITE_TOKEN_LEN_SHIFT),
+				     graph->port->id, graph->shm_iid,
+				     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -512,9 +526,13 @@ int q6apm_read(struct q6apm_graph *graph)
 	struct audioreach_graph_data *port;
 	struct audio_buffer *ab;
 
-	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_pkt(sizeof(*read_buffer),
-					DATA_CMD_RD_SH_MEM_EP_DATA_BUFFER_V2,
-					graph->tx_data.dsp_buf, graph->port->id, graph->shm_iid);
+	u16 dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+	struct gpr_pkt *pkt __free(kfree) =
+		audioreach_alloc_pkt(sizeof(*read_buffer),
+				     DATA_CMD_RD_SH_MEM_EP_DATA_BUFFER_V2,
+				     graph->tx_data.dsp_buf,
+				     graph->port->id, graph->shm_iid,
+				     dest_domain);
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
diff --git a/sound/soc/qcom/qdsp6/q6prm.c b/sound/soc/qcom/qdsp6/q6prm.c
index 04892fb4423f..15ca51ce33b8 100644
--- a/sound/soc/qcom/qdsp6/q6prm.c
+++ b/sound/soc/qcom/qdsp6/q6prm.c
@@ -71,8 +71,9 @@ static int q6prm_set_hw_core_req(struct device *dev, uint32_t hw_block_id, bool
 		rsp_opcode = PRM_CMD_RSP_RELEASE_HW_RSC;
 	}
 
-	struct gpr_pkt *pkt __free(kfree) =
-		audioreach_alloc_cmd_pkt(sizeof(*req), opcode, 0, gdev->svc.id, GPR_PRM_MODULE_IID);
+	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_cmd_pkt(sizeof(*req),
+					    opcode, 0, gdev->svc.id, GPR_PRM_MODULE_IID,
+					    audioreach_gpr_dest_domain(gdev));
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -112,9 +113,10 @@ static int q6prm_request_lpass_clock(struct device *dev, int clk_id, int clk_att
 	struct prm_cmd_request_rsc *req;
 	gpr_device_t *gdev = prm->gdev;
 
-	struct gpr_pkt *pkt __free(kfree) =
-		audioreach_alloc_cmd_pkt(sizeof(*req), PRM_CMD_REQUEST_HW_RSC, 0,
-					 gdev->svc.id, GPR_PRM_MODULE_IID);
+	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_cmd_pkt(sizeof(*req),
+					    PRM_CMD_REQUEST_HW_RSC, 0, gdev->svc.id,
+					    GPR_PRM_MODULE_IID,
+					    audioreach_gpr_dest_domain(gdev));
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
@@ -144,9 +146,10 @@ static int q6prm_release_lpass_clock(struct device *dev, int clk_id, int clk_att
 	struct prm_cmd_release_rsc *rel;
 	gpr_device_t *gdev = prm->gdev;
 
-	struct gpr_pkt *pkt __free(kfree) =
-		audioreach_alloc_cmd_pkt(sizeof(*rel), PRM_CMD_RELEASE_HW_RSC, 0,
-					 gdev->svc.id, GPR_PRM_MODULE_IID);
+	struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_cmd_pkt(sizeof(*rel),
+					    PRM_CMD_RELEASE_HW_RSC, 0, gdev->svc.id,
+					    GPR_PRM_MODULE_IID,
+					    audioreach_gpr_dest_domain(gdev));
 	if (IS_ERR(pkt))
 		return PTR_ERR(pkt);
 
-- 
2.34.1

Re: [PATCH v1 1/3] dt-bindings: sound: qcom,q6apm-dai: add optional qcom,vmid

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The device tree binding arbitrarily restricts `qcom,vmid` array elements to a maximum of 31, encoding an OS driver bug as a hardware constraint.
--

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

dt-bindings: sound: qcom,q6apm-dai: add optional qcom,vmid

This commit adds an optional qcom,vmid property to the q6apm-dais node to
describe additional VMIDs that should be granted access to PCM DMA buffers.
It introduces a maximum value of 31 for the VMIDs to match the current SCM
source-permission mask handling.

> diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
> index 9e5b30d9c6e62..b767625985a74 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
> @@ -20,6 +20,16 @@ properties:
>      minItems: 1
>      maxItems: 2
>  
> +  qcom,vmid:
> +    description: Optional list of destination VMIDs to share PCM DMA buffers with.
> +      HLOS retains RW access as source owner and must not be listed.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    items:
> +      minimum: 1
> +      maximum: 31

[Severity: Medium]
Does this artificially limit the hardware description based on a driver
implementation issue?

The commit message notes this matches current SCM source-permission mask
handling, but include/dt-bindings/firmware/qcom,scm.h defines valid VMIDs
that exceed 31 (e.g., QCOM_SCM_VMID_ADSP_HEAP = 37,
QCOM_SCM_VMID_CP_ADSP_SHARED = 51).

The limit of 31 seems to stem from the Linux qcom_scm_assign_mem() function
where it uses a 32-bit mask to track the source VMID mask. Should the device
tree binding allow higher VMIDs since bindings are meant to describe the
hardware capabilities rather than software limitations?

> +    minItems: 1
> +    maxItems: 8

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064038.492641-1-ajay.nandam@oss.qualcomm.com?part=1

Re: [PATCH v1 2/3] ASoC: qcom: q6apm-dai: add VMID-based SCM assignment

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Passing a DMA address (IOVA) to TrustZone instead of a physical address causes arbitrary memory permission corruption.
- [Critical] If SCM unassign fails, the physical memory is freed back to the kernel while the DSP retains RW permissions, creating a severe memory corruption vulnerability.
- [High] The SCM memory assignment size truncates the actually allocated buffer size, omitting the required DSP bug workaround page and the push-pull position buffer.
- [High] Compress audio streams are completely excluded from VMID SCM assignment, causing playback/capture faults.
- [High] Component-wide SCM assignment state breaks support for multiple DAIs, leading to SMMU faults and active memory unassignment.
- [High] Double unmap of DSP memory on pcm_new failure due to manual cleanup overriding the ALSA core pcm_free sequence.
--

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

ASoC: qcom: q6apm-dai: add VMID-based SCM assignment

This commit adds optional VMID-based SCM memory assignment for q6apm
fixed PCM DMA buffers. It introduces a use_scm_assign flag and
assign/unassign handlers to grant the DSP shared ownership of the DMA
buffers when the qcom,vmid DT property is present.

> diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
> index bf1f872a09f45..ae7bcf9802367 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
[ ... ]
> @@ -83,10 +86,106 @@ struct q6apm_dai_rtd {
>  	bool notify_on_drain;
>  };
>  
> +struct q6apm_scm_region {
> +	phys_addr_t dma_addr;
> +	unsigned int size;
> +	u64 src_perms;
> +	bool assigned;
> +};
> +
>  struct q6apm_dai_data {
>  	long long sid;
> +	int num_vmids;
> +	u32 vmids[Q6APM_MAX_VMIDS];
> +	bool use_scm_assign;
> +	struct q6apm_scm_region scm_regions[SNDRV_PCM_STREAM_LAST + 1];
>  };

[Severity: High]
Is it safe to store the SCM assignment state globally per component?

If multiple playback DAIs are opened, q6apm_dai_assign_memory() might
see scm_region->assigned as true from the first DAI and silently skip
assigning the second DAI's buffer. If the first DAI is subsequently closed,
the memory could be unassigned while the second DAI is still actively
using its buffer. Should this state be tracked per-substream or per-DAI
instead?

[Severity: High]
Are compress audio streams intentionally excluded from VMID SCM assignment?

The VMID assignment appears to be integrated exclusively into the standard
PCM path. Compress audio streams allocate their own DMA buffers in
q6apm_dai_compr_open() using snd_dma_alloc_pages(), but they never seem
to get assigned to the configured VMIDs. Could this trigger a secure Stage-2
memory protection fault on platforms requiring VMID assignment?

[ ... ]
> +static int q6apm_dai_assign_memory(struct snd_pcm_substream *substream,
> +				   struct q6apm_dai_data *pdata)
> +{
[ ... ]
> +	scm_region->dma_addr = substream->dma_buffer.addr;
> +	scm_region->size = ALIGN(BUFFER_BYTES_MAX, PAGE_SIZE);
> +	scm_region->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
> +
> +	ret = qcom_scm_assign_mem(scm_region->dma_addr, scm_region->size,
> +				  &scm_region->src_perms, dst_vmids, dst_count);

[Severity: Critical]
Does substream->dma_buffer.addr contain an IOVA rather than a physical address?

If an IOMMU is present, passing an IOVA to qcom_scm_assign_mem() can cause
issues since TrustZone modifies Stage-2 translation tables, which strictly
operate on physical addresses. Passing an IOVA could instruct the hypervisor
to restrict physical memory that numerically matches the IOVA, leading to
memory permission corruption of arbitrary pages.

[Severity: High]
Does this hardcoded assignment size omit the extra workaround page and the
push-pull position buffer?

In q6apm_dai_pcm_new(), the allocated buffer size is calculated as
BUFFER_BYTES_MAX plus an extra PAGE_SIZE, and an additional POS_BUFFER_BYTES
if push-pull mode is active. Since the SCM assigned size is hardcoded to
ALIGN(BUFFER_BYTES_MAX, PAGE_SIZE), it seems the extra padding page and
position buffer might remain without correct SCM permissions.

[ ... ]
> +static int q6apm_dai_unassign_memory(struct snd_soc_component *component,
> +				     struct snd_pcm_substream *substream,
> +				     struct q6apm_dai_data *pdata)
> +{
[ ... ]
> +	ret = qcom_scm_assign_mem(scm_region->dma_addr, scm_region->size,
> +				  &scm_region->src_perms, &hlos, 1);
> +	if (!ret) {
> +		scm_region->assigned = false;
> +		scm_region->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
> +	} else {
> +		dev_err(dev, "Failed to unassign DMA buffer %pa from VMIDs: %d\n",
> +			&scm_region->dma_addr, ret);
> +	}
> +
> +	return ret;
> +}

[Severity: Critical]
If qcom_scm_assign_mem() fails, does returning the error code allow the ALSA
core to subsequently free the physical DMA buffer back to the kernel?

If the unassign fails, the code merely logs an error and returns. Since
q6apm_dai_pcm_free() ignores this return value, the ALSA core could free
the buffer while the DSP retains architectural RW permissions to the freed
memory. Could this allow the DSP to arbitrarily corrupt kernel or userspace
memory that later reuses these pages? Should the memory be intentionally
leaked instead if it cannot be successfully unassigned?

[ ... ]
> @@ -606,6 +706,14 @@ static int q6apm_dai_pcm_new(struct snd_soc_component *component, struct snd_soc
>  		ret = q6apm_dai_memory_map(component, substream, graph_id, is_push_pull);
>  		if (ret)
>  			return ret;
> +
> +		if (pdata->use_scm_assign) {
> +			ret = q6apm_dai_assign_memory(substream, pdata);
> +			if (ret) {
> +				q6apm_dai_memory_unmap(component, substream);
> +				return ret;
> +			}
> +		}
>  	}

[Severity: High]
Does this manual call to q6apm_dai_memory_unmap() cause a double-free on the
DSP?

If q6apm_dai_assign_memory() fails, the driver manually calls
q6apm_dai_memory_unmap(). However, the ALSA framework inherently handles
pcm_new failures by tearing down the sound card components, which invokes
q6apm_dai_pcm_free(). This callback unconditionally calls
q6apm_dai_memory_unmap() again, which might result in a duplicate unmap
command sent to the DSP firmware.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064038.492641-1-ajay.nandam@oss.qualcomm.com?part=2

Re: [PATCH v1 3/3] ASoC: qcom: qdsp6: generalize GPR service domain

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch adds a new `dest_domain` parameter to allocation helpers but fails to update all caller sites, causing 'too few arguments to function' compilation errors.
--

commit 7837af8c42df151e433f6c30da08674cc4716b78
Author: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>

ASoC: qcom: qdsp6: generalize GPR service domain

This commit updates the AudioReach allocation helpers to accept an
explicit dest_domain parameter. It also updates various callers to pass
the appropriate destination domain ID.

> diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
> index 62a2fd79bbcb9..d07fcd3a8fa73 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.h
> +++ b/sound/soc/qcom/qdsp6/audioreach.h
[ ... ]
> -void *audioreach_alloc_apm_cmd_pkt(int pkt_size, uint32_t opcode, uint32_t
> -				    token);
> +void *audioreach_alloc_apm_cmd_pkt(int pkt_size, u32 opcode, u32 token,
> +				   u16 dest_domain);
>  void audioreach_set_default_channel_mapping(u8 *ch_map, int num_channels);
> -void *audioreach_alloc_cmd_pkt(int payload_size, uint32_t opcode,
> -			       uint32_t token, uint32_t src_port,
> -			       uint32_t dest_port);
> -void *audioreach_alloc_apm_pkt(int pkt_size, uint32_t opcode, uint32_t token,
> -				uint32_t src_port);
> -void *audioreach_alloc_pkt(int payload_size, uint32_t opcode,
> -			   uint32_t token, uint32_t src_port,
> -			   uint32_t dest_port);
> +void *audioreach_alloc_cmd_pkt(int payload_size, u32 opcode, u32 token,
> +			       u32 src_port, u32 dest_port, u16 dest_domain);

[Severity: Low]
Were some callers missed when updating these function signatures?

The new dest_domain parameter is required, but it appears some existing
callers in sound/soc/qcom/qdsp6/audioreach.c were not updated and are
still using the old signatures.

For example, in audioreach_shmem_register_event():

    pkt = audioreach_alloc_cmd_pkt(payload_size,
                                   APM_CMD_REGISTER_MODULE_EVENTS, 0,
                                   graph->port->id, graph->shm_iid);

And in audioreach_setup_push_pull():

    pkt = audioreach_alloc_apm_cmd_pkt(payload_size, APM_CMD_SET_CFG, 0);

Does this cause compilation errors due to too few arguments being passed
to these helpers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064038.492641-1-ajay.nandam@oss.qualcomm.com?part=3