[PATCH v2 0/3] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP)

[PATCH v2 0/3] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP)

[Ajay Kumar Nandam]

On platforms such as Qualcomm Shikra, audio is served by the modem DSP
(mDSP) rather than the ADSP. The mDSP runs in a stage-2 protected context
and cannot use the SMMU, so the PCM buffers it consumes must live in
reserved-memory carveouts that are handed to the consumer VMIDs via a
hypervisor (SCM) memory assignment. This series adds that access model to
the q6apm DAI driver and its binding, alongside the existing stage-1/SMMU
(iommus) path, which is left untouched.

The two access models are mutually exclusive: a node carries either
iommus (stage-1) or qcom,vmids (stage-2), enforced in both the binding
(oneOf) and the driver.

This supersedes all prior postings. The v1 was sent as multiple
independent series (VMID binding + driver, memory-region binding + DTS,
and a standalone carveout patch). Per review feedback from Mark Brown [1]
and Srinivas Kandagatla [2][3] the binding and driver patches are now
consolidated into a single series so they can be reviewed and merged
together.

Prior versions:
v1 (VMID binding + driver + GPR domain):
  https://lore.kernel.org/all/20260609064038.492641-1-ajay.nandam@oss.qualcomm.com/
v1 (memory-region binding + DTS):
  https://lore.kernel.org/all/20260618113509.2025881-1-ajay.nandam@oss.qualcomm.com/
v2 (carveout SCM assignment, standalone):
  https://lore.kernel.org/all/20260624123748.502781-1-ajay.nandam@oss.qualcomm.com/

[1] https://lore.kernel.org/all/97cab297-ef92-48dd-8331-71aac0b51538@sirena.org.uk/
[2] https://lore.kernel.org/all/39e90d95-0bae-4eb8-96d4-6e39aae432b2@kernel.org/
[3] https://lore.kernel.org/all/02eee0a8-8c0f-4b0c-acc2-05dc436e5a85@kernel.org/

Changes since v1:
- Consolidate all three independent series (VMID binding + driver,
  memory-region binding, carveout assignment) into a single 3-patch
  series, as requested by Mark Brown [1] and Srinivas Kandagatla [2][3].
- binding: combine qcom,vmids property, memory-region, oneOf constraint,
  and dependentRequired into one binding patch since all changes go to
  the same YAML file.
- binding: rename DT property from qcom,vmid (singular) to qcom,vmids
  (plural) and expand the description to explain VMIDs, the hardware
  access model, and which platforms require this (Krzysztof [4][5],
  Srinivas [2]).
- binding: make iommus and qcom,vmids mutually exclusive via oneOf
  rather than keeping iommus unconditionally required (Krzysztof [4]).
- binding: correct the qcom,vmids description - SCM assignment replaces
  the buffer's owners with the supplied set, so the driver adds HLOS to
  the destination itself; reword to say so instead of "HLOS is always
  retained".
- binding: add uniqueItems to qcom,vmids.
- binding: add ranges to the reserved-memory example node.
- binding: fix qcom,vmids example from two separate cells to a single
  uint32 array.
- binding: make the qcom,vmids example self-contained by including the
  reserved-memory nodes the memory-region phandles point at.
- binding: document that all listed VMIDs and HLOS receive read-write
  access.
- driver (patch 2): merge SCM VMID assignment and carveout buffer
  allocation into a single bisect-safe patch. Move dest_domain stamping
  from every packet-allocation call site into the send helpers
  (q6apm_send_cmd_sync, audioreach_graph_send_cmd_sync,
  q6prm_send_cmd_sync) and the four async data-path sends. This
  centralizes domain routing at the send layer and avoids threading a
  dest_domain parameter through every alloc helper. Suggested by
  Srinivas Kandagatla.
- driver: rewrite commit messages to clearly state the problem, the
  platform, and why it fails without the fix (Srinivas [3]).
- driver: move SCM assign/unassign to probe/remove lifecycle matching
  upstream patterns (rmtfs_mem, qcom_q6v5_pas) instead of per-stream
  open/close.
- driver: guard the reserved-memory per-stream buffer size against
  size_t underflow when the pool is smaller than
  Q6APM_POOL_MAX_STREAMS * POS_BUFFER_BYTES.
- driver: bounds-check num_carveouts against Q6APM_MAX_CARVEOUTS before
  recording a runtime PCM carveout.
- driver: track SCM carveouts per-PCM and reclaim only the freed PCM's
  region in pcm_free, instead of unassigning every region; this also
  stops num_carveouts growing unbounded across new/free cycles.
- driver: log SCM unassign (reclaim-to-HLOS) failures and keep the
  region marked assigned so DSP-owned memory is never returned to the
  pool.
- driver: reject qcom,vmids and iommus present together at probe.
- driver: of_node_put() the iommus phandle args; checkpatch/style
  cleanups.
- driver: remove dead runtime->dma_bytes assignment in open() for
  managed-buffer path.

[4] https://lore.kernel.org/all/20260610-silver-pelican-of-agility-86fcbc@quoll/
[5] https://lore.kernel.org/all/20260622-whimsical-charming-seriema-dcec81@quoll/

---
Ajay Kumar Nandam (3):
      dt-bindings: sound: qcom,q6apm-dai: add stage-2 (SCM/VMID) access model
      ASoC: qcom: q6apm-dai: add VMID-based SCM assignment for mDSP buffers
      ASoC: qcom: qdsp6: generalize GPR service domain

 .../devicetree/bindings/sound/qcom,q6apm-dai.yaml  |  72 +++-
 sound/soc/qcom/Kconfig                             |   1 +
 sound/soc/qcom/qdsp6/audioreach.c                  |  12 +-
 sound/soc/qcom/qdsp6/audioreach.h                  |  22 +-
 sound/soc/qcom/qdsp6/q6apm-dai.c                   | 373 +++++++++++++++++++--
 sound/soc/qcom/qdsp6/q6apm.c                       |   8 +-
 sound/soc/qcom/qdsp6/q6apm.h                       |   2 +-
 sound/soc/qcom/qdsp6/q6prm.c                       |   2 +
 8 files changed, 455 insertions(+), 37 deletions(-)
---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260825-a2a-shikra-vmid-v5-4975625a554a

Best regards,
--  
Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>

[PATCH v2 1/3] dt-bindings: sound: qcom,q6apm-dai: add stage-2 (SCM/VMID) access model

[Ajay Kumar Nandam]

The APM DAIs expose PCM buffers that the backing DSP must be able to
reach. On existing targets the DSP runs in a stage-1 context and the
buffers are described to the SMMU via the iommus property.

On platforms such as Qualcomm Shikra, where audio is served by the modem
DSP (mDSP) rather than the ADSP, the DSP runs in a stage-2 protected
context and cannot use the SMMU. The buffers are reachable only after
hypervisor (SCM) memory assignment to the fixed set of consumer Virtual
Machine IDs (VMIDs) that own that protected context. Describe those
consumers with a new qcom,vmids property.

The two access models are mutually exclusive: a node carries either
iommus (stage-1/SMMU) or qcom,vmids (stage-2/SCM), expressed with a
oneOf constraint. iommus is therefore no longer unconditionally required.
SCM assignment replaces the buffer's current owners with the supplied
destination set, so the driver always adds HLOS to that set itself to
retain host access; HLOS must not be listed in qcom,vmids.

When qcom,vmids is present the PCM buffers must reside in reserved-memory
carveouts that are SCM-assigned to the consumer VMIDs. Add an optional
memory-region property listing those carveouts. The first entry is the
control-path buffer and subsequent entries are data-path buffers.
memory-region is only meaningful alongside qcom,vmids, enforced via
dependentRequired.

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

diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
index 9e5b30d9c6e6..d9a6bc4a14f2 100644
--- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
@@ -10,7 +10,15 @@ maintainers:
   - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
 
 description: |
-  This binding describes the Qualcomm APM DAIs in DSP
+  This binding describes the Qualcomm APM DAIs in DSP.
+
+  The DSP that runs the APM must be able to reach the PCM buffers. On
+  existing targets the DSP runs in a stage-1 context and the buffers are
+  described to the SMMU via iommus. On targets whose APM DSP runs in a
+  stage-2 protected context that cannot use the SMMU, the buffers are
+  reachable only after hypervisor (SCM) memory assignment to the fixed
+  set of consumer VMIDs that own that protected context, described by
+  qcom,vmids. The two access models are mutually exclusive.
 
 properties:
   compatible:
@@ -20,9 +28,42 @@ properties:
     minItems: 1
     maxItems: 2
 
+  memory-region:
+    description:
+      Reserved-memory carveouts (shared-dma-pool, no-map) that hold the
+      PCM buffers and must be SCM-assigned to the VMIDs in qcom,vmids.
+      The first entry is the control-path buffer; the second entry is
+      the data-path buffer. Only used together with qcom,vmids.
+    minItems: 1
+    maxItems: 2
+
+  qcom,vmids:
+    description:
+      Virtual Machine IDs (VMIDs) of the processors that consume the PCM
+      buffers and therefore must be granted access through SCM memory
+      assignment. Required on targets whose APM DSP runs in a stage-2
+      protected context and cannot use the SMMU; mutually exclusive with
+      iommus. SCM assignment replaces the buffer's current owners with the
+      supplied set, so the driver always adds HLOS to the destination list
+      itself to retain host access; HLOS must not be listed here. All
+      listed VMIDs and HLOS receive read-write access.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 2
+    uniqueItems: true
+
 required:
   - compatible
-  - iommus
+
+oneOf:
+  - required:
+      - iommus
+  - required:
+      - qcom,vmids
+
+dependentRequired:
+  memory-region:
+    - qcom,vmids
 
 additionalProperties: false
 
@@ -32,3 +73,30 @@ examples:
         compatible = "qcom,q6apm-dais";
         iommus = <&apps_smmu 0x1801 0x0>;
     };
+  - |
+    #include <dt-bindings/firmware/qcom,scm.h>
+
+    reserved-memory {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        ranges;
+
+        audio_heap_mem: audio-heap@a1000000 {
+            compatible = "shared-dma-pool";
+            reg = <0x0 0xa1000000 0x0 0x100000>;
+            no-map;
+        };
+
+        audio_mdsp_carveout_mem: audio-carveout@a1100000 {
+            compatible = "shared-dma-pool";
+            reg = <0x0 0xa1100000 0x0 0x400000>;
+            no-map;
+        };
+    };
+
+    dais {
+        compatible = "qcom,q6apm-dais";
+        /* index 0: control path, index 1: data path */
+        memory-region = <&audio_heap_mem>, <&audio_mdsp_carveout_mem>;
+        qcom,vmids = <QCOM_SCM_VMID_LPASS QCOM_SCM_VMID_MSS_MSA>;
+    };

-- 
2.34.1

[PATCH v2 2/3] ASoC: qcom: q6apm-dai: add VMID-based SCM assignment for mDSP buffers

[Ajay Kumar Nandam]

On platforms such as Qualcomm Shikra, audio is served by the modem DSP
(mDSP) which runs in a stage-2 protected context. Unlike ADSP targets
where SMMU-mapped system RAM is directly accessible, the mDSP cannot
reach the PCM buffers unless they are explicitly SCM-assigned to the
appropriate Virtual Machine IDs (VMIDs). Without this assignment, audio
does not function on these platforms.

Parse the qcom,vmids DT property and, when present, SCM-assign all
memory regions to HLOS (retained as source owner, RW) plus the listed
consumer VMIDs (all RW). The two access models are mutually exclusive:
a node uses the SMMU path (iommus) or the SCM path (qcom,vmids), never
both.

Memory assignment follows the upstream pattern used by rmtfs_mem and
qcom_q6v5_pas: reserved-memory carveouts are SCM-assigned once at
probe() time and restored to HLOS-only ownership via
devm_add_action_or_reset() at device removal. For the non-reserved-mem
path (qcom,vmids without memory-region), the fixed DMA buffer is
assigned at pcm_new() time since its address is only known after
allocation.

When memory-region is present (has_reserved_mem), the data-path carveout
is attached as a DMA pool via of_reserved_mem_device_init_by_idx() and
PCM buffers allocate directly from the carveout instead of system RAM.
Buffer constraints are capped at reserved_buf_size and
snd_pcm_set_managed_buffer_all() replaces snd_pcm_set_fixed_buffer_all().

All new code paths are gated on use_scm_assign (false when qcom,vmids is
absent), ensuring existing ADSP/iommus targets are completely unaffected.

Co-developed-by: Mohit Sharma <mohit.sharma@oss.qualcomm.com>
Signed-off-by: Mohit Sharma <mohit.sharma@oss.qualcomm.com>
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
 sound/soc/qcom/Kconfig           |   1 +
 sound/soc/qcom/qdsp6/q6apm-dai.c | 373 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 351 insertions(+), 23 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..19a5c62cc4e1 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -1,20 +1,22 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2021, Linaro Limited
 
-#include <linux/init.h>
+#include <linux/dma-mapping.h>
 #include <linux/err.h>
+#include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/init.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <sound/soc.h>
-#include <sound/soc-dapm.h>
 #include <linux/spinlock.h>
-#include <sound/pcm.h>
 #include <asm/div64.h>
 #include <asm/dma.h>
-#include <linux/dma-mapping.h>
+#include <sound/pcm.h>
 #include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
 #include "q6apm.h"
 
 #define DRV_NAME "q6apm-dai"
@@ -34,6 +36,9 @@
 #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		2
+#define Q6APM_MAX_CARVEOUTS	2
+#define Q6APM_POOL_MAX_STREAMS	8
 #define SID_MASK_DEFAULT	0xF
 
 static const struct snd_compr_codec_caps q6apm_compr_caps = {
@@ -57,6 +62,13 @@ enum stream_state {
 	Q6APM_STREAM_RUNNING,
 };
 
+struct q6apm_scm_region {
+	phys_addr_t addr;
+	size_t size;
+	u64 src_perms;
+	bool assigned;
+};
+
 struct q6apm_dai_rtd {
 	struct snd_pcm_substream *substream;
 	struct snd_compr_stream *cstream;
@@ -84,9 +96,123 @@ struct q6apm_dai_rtd {
 };
 
 struct q6apm_dai_data {
+	struct device *dev;
 	long long sid;
+	int num_vmids;
+	u32 vmids[Q6APM_MAX_VMIDS];
+	bool use_scm_assign;
+	bool has_reserved_mem;
+	size_t reserved_buf_size;
+	struct q6apm_scm_region carveout_regions[Q6APM_MAX_CARVEOUTS];
+	int num_carveouts;
 };
 
+static int q6apm_dai_scm_assign(struct q6apm_scm_region *region,
+				const struct q6apm_dai_data *pdata)
+{
+	struct qcom_scm_vmperm *dst;
+	int dst_count = 0;
+	int ret, i;
+
+	if (region->assigned)
+		return 0;
+
+	dst = kcalloc(pdata->num_vmids + 1, sizeof(*dst), GFP_KERNEL);
+	if (!dst)
+		return -ENOMEM;
+
+	dst[dst_count].vmid = QCOM_SCM_VMID_HLOS;
+	dst[dst_count].perm = QCOM_SCM_PERM_RW;
+	dst_count++;
+
+	for (i = 0; i < pdata->num_vmids; i++) {
+		if (WARN_ON_ONCE(pdata->vmids[i] == QCOM_SCM_VMID_HLOS))
+			continue;
+		dst[dst_count].vmid = pdata->vmids[i];
+		dst[dst_count].perm = QCOM_SCM_PERM_RW;
+		dst_count++;
+	}
+
+	if (dst_count == 1) {
+		kfree(dst);
+		return 0;
+	}
+
+	ret = qcom_scm_assign_mem(region->addr, region->size,
+				  &region->src_perms, dst, dst_count);
+	kfree(dst);
+	if (!ret)
+		region->assigned = true;
+
+	return ret;
+}
+
+static void q6apm_dai_scm_unassign(struct q6apm_scm_region *region,
+				   const struct q6apm_dai_data *pdata)
+{
+	struct qcom_scm_vmperm hlos = {
+		.vmid = QCOM_SCM_VMID_HLOS,
+		.perm = QCOM_SCM_PERM_RW,
+	};
+	int ret;
+
+	if (!region->assigned)
+		return;
+
+	ret = qcom_scm_assign_mem(region->addr, region->size,
+				  &region->src_perms, &hlos, 1);
+	if (ret) {
+		/*
+		 * A failed reclaim leaves the buffer owned by the DSP VMIDs
+		 * instead of HLOS: it must not be reused. Warn loudly and keep
+		 * it marked assigned so it is never handed back to the pool.
+		 */
+		dev_err(pdata->dev,
+			"SCM unassign failed for %pa (size %zu): %d\n",
+			&region->addr, region->size, ret);
+		return;
+	}
+
+	region->assigned = false;
+	region->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
+}
+
+static void q6apm_dai_unassign_all(void *data)
+{
+	struct q6apm_dai_data *pdata = data;
+	int i;
+
+	for (i = 0; i < pdata->num_carveouts; i++)
+		q6apm_dai_scm_unassign(&pdata->carveout_regions[i], pdata);
+}
+
+/*
+ * Reclaim and drop the runtime carveout tracking the buffer at @addr.
+ * Only per-PCM buffers (allocated in pcm_new when there is no reserved
+ * memory pool) are removed this way, so tearing one PCM down never
+ * disturbs regions still owned by other live PCMs.
+ */
+static void q6apm_dai_scm_unassign_addr(struct q6apm_dai_data *pdata,
+					phys_addr_t addr)
+{
+	int i;
+
+	for (i = 0; i < pdata->num_carveouts; i++) {
+		if (pdata->carveout_regions[i].addr != addr)
+			continue;
+
+		q6apm_dai_scm_unassign(&pdata->carveout_regions[i], pdata);
+		if (pdata->carveout_regions[i].assigned)
+			return;	/* reclaim failed: keep tracking, warned already */
+
+		/* swap-remove the (now HLOS-owned) slot */
+		pdata->num_carveouts--;
+		pdata->carveout_regions[i] =
+			pdata->carveout_regions[pdata->num_carveouts];
+		return;
+	}
+}
+
 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 |
@@ -409,8 +535,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);
 		if (ret < 0) {
 			dev_err(dev, "constraint for buffer bytes min max ret = %d\n", ret);
 			goto err;
@@ -431,17 +560,18 @@ static int q6apm_dai_open(struct snd_soc_component *component,
 	}
 
 	runtime->private_data = prtd;
-	runtime->dma_bytes = BUFFER_BYTES_MAX;
 	if (pdata->sid < 0)
 		prtd->phys = substream->dma_buffer.addr;
 	else
 		prtd->phys = substream->dma_buffer.addr | (pdata->sid << 32);
 
 	if (q6apm_is_graph_in_push_pull_mode(prtd->graph)) {
+		size_t buf_sz = pdata->has_reserved_mem ?
+				pdata->reserved_buf_size : BUFFER_BYTES_MAX;
 		void *pos_buffer;
 
-		prtd->pos_phys = prtd->phys + BUFFER_BYTES_MAX;
-		pos_buffer = (void *)(substream->dma_buffer.area + BUFFER_BYTES_MAX);
+		prtd->pos_phys = prtd->phys + buf_sz;
+		pos_buffer = (void *)(substream->dma_buffer.area + buf_sz);
 		prtd->pos_buffer = (struct sh_mem_pull_push_mode_position_buffer *)(pos_buffer);
 	}
 
@@ -529,6 +659,9 @@ static int q6apm_dai_hw_params(struct snd_soc_component *component,
 	return 0;
 }
 
+static void q6apm_dai_memory_unmap(struct snd_soc_component *component,
+				   struct snd_pcm_substream *substream);
+
 static int q6apm_dai_memory_map(struct snd_soc_component *component,
 				struct snd_pcm_substream *substream,
 				int graph_id, bool is_push_pull)
@@ -549,15 +682,21 @@ static int q6apm_dai_memory_map(struct snd_soc_component *component,
 	else
 		phys = substream->dma_buffer.addr | (pdata->sid << 32);
 
-	ret = q6apm_map_memory_fixed_region(dev, graph_id, phys, BUFFER_BYTES_MAX);
+	ret = q6apm_map_memory_fixed_region(dev, graph_id, phys,
+					    pdata->has_reserved_mem ?
+					    pdata->reserved_buf_size :
+					    BUFFER_BYTES_MAX);
 	if (ret < 0)
 		dev_err(dev, "Audio Start: Buffer Allocation failed rc = %d\n",	ret);
 
 	if (is_push_pull) {
+		size_t buf_sz = pdata->has_reserved_mem ?
+				pdata->reserved_buf_size : BUFFER_BYTES_MAX;
+
 		if (pdata->sid < 0)
-			phys = substream->dma_buffer.addr + BUFFER_BYTES_MAX;
+			phys = substream->dma_buffer.addr + buf_sz;
 		else
-			phys = (substream->dma_buffer.addr + BUFFER_BYTES_MAX) | (pdata->sid << 32);
+			phys = (substream->dma_buffer.addr + buf_sz) | (pdata->sid << 32);
 
 		ret = q6apm_map_pos_buffer(dev, graph_id, phys, POS_BUFFER_BYTES);
 		if (ret < 0)
@@ -572,6 +711,7 @@ static int q6apm_dai_memory_map(struct snd_soc_component *component,
 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 +723,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,
@@ -599,13 +741,46 @@ static int q6apm_dai_pcm_new(struct snd_soc_component *component, struct snd_soc
 		if (is_push_pull)
 			size += POS_BUFFER_BYTES;
 
-		ret = snd_pcm_set_fixed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV, component->dev, size);
+		if (pdata->has_reserved_mem)
+			ret = snd_pcm_set_managed_buffer_all(pcm,
+							     SNDRV_DMA_TYPE_DEV, component->dev,
+							     pdata->reserved_buf_size,
+							     pdata->reserved_buf_size);
+		else
+			ret = snd_pcm_set_fixed_buffer_all(pcm,
+							   SNDRV_DMA_TYPE_DEV, component->dev,
+							   size);
 		if (ret)
 			return ret;
 
 		ret = q6apm_dai_memory_map(component, substream, graph_id, is_push_pull);
 		if (ret)
 			return ret;
+
+		if (pdata->use_scm_assign && !pdata->has_reserved_mem) {
+			struct q6apm_scm_region *r;
+
+			if (pdata->num_carveouts >= Q6APM_MAX_CARVEOUTS) {
+				dev_err(component->dev,
+					"too many SCM carveouts (max %d)\n",
+					Q6APM_MAX_CARVEOUTS);
+				q6apm_dai_memory_unmap(component, substream);
+				return -ENOSPC;
+			}
+
+			r = &pdata->carveout_regions[pdata->num_carveouts];
+			r->addr = substream->dma_buffer.addr;
+			r->size = ALIGN(size, PAGE_SIZE);
+			r->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
+			ret = q6apm_dai_scm_assign(r, pdata);
+			if (ret) {
+				dev_err(component->dev,
+					"SCM assign DMA buffer failed: %d\n", ret);
+				q6apm_dai_memory_unmap(component, substream);
+				return ret;
+			}
+			pdata->num_carveouts++;
+		}
 	}
 
 	return 0;
@@ -635,15 +810,29 @@ 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;
+	int i;
 
-	substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
-	if (substream)
-		q6apm_dai_memory_unmap(component, substream);
+	if (!pdata)
+		return;
+
+	/*
+	 * Reverse of pcm_new: unmap the buffer from the DSP first, then
+	 * reclaim only this PCM's carveout(s) back to HLOS. Regions owned
+	 * by other live PCMs, and the reserved-memory pool, are untouched.
+	 */
+	for (i = 0; i < SNDRV_PCM_STREAM_LAST + 1; i++) {
+		substream = pcm->streams[i].substream;
+		if (!substream)
+			continue;
 
-	substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
-	if (substream)
 		q6apm_dai_memory_unmap(component, substream);
+
+		if (pdata->use_scm_assign && !pdata->has_reserved_mem)
+			q6apm_dai_scm_unassign_addr(pdata,
+						    substream->dma_buffer.addr);
+	}
 }
 
 static int q6apm_dai_compr_open(struct snd_soc_component *component,
@@ -1017,23 +1206,161 @@ static const struct snd_soc_component_driver q6apm_fe_dai_component = {
 	.remove_order   = SND_SOC_COMP_ORDER_EARLY,
 };
 
+static void q6apm_dai_reserved_mem_release(void *data)
+{
+	of_reserved_mem_device_release(data);
+}
+
 static int q6apm_dai_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *node = dev->of_node;
 	struct q6apm_dai_data *pdata;
 	struct of_phandle_args args;
-	int rc;
+	int rc, vmids, i;
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
 
+	pdata->dev = dev;
+
 	rc = of_parse_phandle_with_fixed_args(node, "iommus", 1, 0, &args);
-	if (rc < 0)
+	if (rc < 0) {
 		pdata->sid = -1;
-	else
+	} else {
 		pdata->sid = args.args[0] & SID_MASK_DEFAULT;
+		of_node_put(args.np);
+	}
+
+	vmids = of_property_count_u32_elems(node, "qcom,vmids");
+	if (vmids == -EINVAL) {
+		/* no qcom,vmids: SCM assignment not used on this target */
+		pdata->use_scm_assign = false;
+	} else if (vmids < 0) {
+		return vmids;
+	} else if (vmids == 0 || vmids > Q6APM_MAX_VMIDS) {
+		dev_err(dev, "qcom,vmids: invalid count %d (need 1..%d)\n",
+			vmids, Q6APM_MAX_VMIDS);
+		return -EINVAL;
+	} else if (pdata->sid >= 0) {
+		/*
+		 * iommus (SMMU translation) and qcom,vmids (SCM VMID
+		 * assignment) are mutually exclusive buffer-protection
+		 * schemes; the binding forbids both, reject them here too.
+		 */
+		dev_err(dev, "qcom,vmids and iommus are mutually exclusive\n");
+		return -EINVAL;
+	}
+
+	if (vmids > 0) {
+		rc = of_property_read_u32_array(node, "qcom,vmids",
+						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,vmids must not include HLOS\n");
+				return -EINVAL;
+			}
+		}
+		pdata->num_vmids = vmids;
+		pdata->use_scm_assign = true;
+	}
+
+	if (pdata->use_scm_assign) {
+		struct device_node *mem_node;
+		struct reserved_mem *rmem;
+
+		if (!qcom_scm_is_available())
+			return -EPROBE_DEFER;
+
+		mem_node = of_parse_phandle(node, "memory-region", 0);
+		if (mem_node) {
+			rmem = of_reserved_mem_lookup(mem_node);
+			of_node_put(mem_node);
+			if (rmem) {
+				struct q6apm_scm_region *r;
+
+				r = &pdata->carveout_regions[pdata->num_carveouts++];
+				r->addr = rmem->base;
+				r->size = ALIGN(rmem->size, PAGE_SIZE);
+				r->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
+			}
+		}
+
+		mem_node = of_parse_phandle(node, "memory-region", 1);
+		if (mem_node) {
+			rmem = of_reserved_mem_lookup(mem_node);
+			of_node_put(mem_node);
+			if (rmem) {
+				struct q6apm_scm_region *r;
+				size_t per_stream;
+
+				/*
+				 * The data-path carveout is a shared DMA pool
+				 * from which all PCM substreams pre-allocate.
+				 * Divide evenly so concurrent streams fit,
+				 * subtracting per-stream position-buffer
+				 * overhead. Reject a pool too small to give
+				 * each stream a usable buffer (unsigned
+				 * arithmetic would otherwise wrap).
+				 */
+				per_stream = rmem->size / Q6APM_POOL_MAX_STREAMS;
+				if (per_stream <= POS_BUFFER_BYTES) {
+					dev_err(dev,
+						"reserved-memory pool too small: %llu bytes\n",
+						(u64)rmem->size);
+					return -EINVAL;
+				}
+
+				rc = of_reserved_mem_device_init_by_idx(dev, node, 1);
+				if (rc) {
+					dev_err(dev,
+						"reserved-memory pool init failed: %d\n",
+						rc);
+					return rc;
+				}
+				rc = devm_add_action_or_reset(dev,
+							      q6apm_dai_reserved_mem_release,
+							      dev);
+				if (rc)
+					return rc;
+
+				/* never exceed the per-substream buffer cap */
+				pdata->reserved_buf_size =
+					min_t(size_t,
+					      per_stream - POS_BUFFER_BYTES,
+					      BUFFER_BYTES_MAX);
+				pdata->has_reserved_mem = true;
+
+				r = &pdata->carveout_regions[pdata->num_carveouts++];
+				r->addr = rmem->base;
+				r->size = ALIGN(rmem->size, PAGE_SIZE);
+				r->src_perms = BIT_ULL(QCOM_SCM_VMID_HLOS);
+			}
+		}
+
+		if (pdata->num_carveouts) {
+			for (i = 0; i < pdata->num_carveouts; i++) {
+				rc = q6apm_dai_scm_assign(&pdata->carveout_regions[i],
+							  pdata);
+				if (rc) {
+					dev_err(dev,
+						"SCM assign carveout[%d] failed: %d\n",
+						i, rc);
+					return rc;
+				}
+			}
+			rc = devm_add_action_or_reset(dev,
+						      q6apm_dai_unassign_all,
+						      pdata);
+			if (rc)
+				return rc;
+		}
+	}
 
 	dev_set_drvdata(dev, pdata);
 

-- 
2.34.1

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

[Ajay Kumar Nandam]

AudioReach builds APM and PRM command packets with the GPR destination
domain hardcoded to GPR_DOMAIN_ID_ADSP. This assumes audio is always
served by the ADSP, which is true for all currently supported targets.

On platforms such as Qualcomm Shikra, audio is served by the modem DSP
(mDSP) instead. The GPR node in DT already describes which DSP backs
the service via its qcom,domain property (e.g. GPR_DOMAIN_ID_MODEM),
and the GPR core exposes it as gdev->domain_id. But the AudioReach
packet builders ignore this and always target the ADSP, so every
APM/PRM command is routed to the wrong DSP on mDSP targets and audio
does not function.

Fix this by reading the GPR destination domain from gdev->domain_id
and stamping it in the send helpers (q6apm_send_cmd_sync,
audioreach_graph_send_cmd_sync, q6prm_send_cmd_sync) just before
dispatch. This centralizes the domain decision at the send layer
rather than threading it through every packet-allocation call site.
For the small number of async data-path sends that bypass the sync
helpers (write, read, compr, EOS), the domain is stamped inline
before gpr_send_port_pkt(). When no domain is available the helper
falls back to GPR_DOMAIN_ID_ADSP, so all existing ADSP targets
remain unchanged.

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 | 12 +++++++++---
 sound/soc/qcom/qdsp6/audioreach.h | 22 +++++++++++++++-------
 sound/soc/qcom/qdsp6/q6apm.c      |  8 +++++++-
 sound/soc/qcom/qdsp6/q6apm.h      |  2 +-
 sound/soc/qcom/qdsp6/q6prm.c      |  2 ++
 5 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index e6e9eb2e85aa..f7ae6d0db7e7 100644
--- a/sound/soc/qcom/qdsp6/audioreach.c
+++ b/sound/soc/qcom/qdsp6/audioreach.c
@@ -579,10 +579,10 @@ EXPORT_SYMBOL_GPL(audioreach_alloc_graph_pkt);
 int audioreach_send_cmd_sync(struct device *dev, gpr_device_t *gdev,
 			     struct gpr_ibasic_rsp_result_t *result, struct mutex *cmd_lock,
 			     gpr_port_t *port, wait_queue_head_t *cmd_wait,
-			     const struct gpr_pkt *pkt, uint32_t rsp_opcode)
+			     struct gpr_pkt *pkt, uint32_t rsp_opcode)
 {
 
-	const struct gpr_hdr *hdr = &pkt->hdr;
+	struct gpr_hdr *hdr = &pkt->hdr;
 	int rc;
 
 	mutex_lock(cmd_lock);
@@ -622,10 +622,12 @@ int audioreach_send_cmd_sync(struct device *dev, gpr_device_t *gdev,
 }
 EXPORT_SYMBOL_GPL(audioreach_send_cmd_sync);
 
-int audioreach_graph_send_cmd_sync(struct q6apm_graph *graph, const struct gpr_pkt *pkt,
+int audioreach_graph_send_cmd_sync(struct q6apm_graph *graph, struct gpr_pkt *pkt,
 				   uint32_t rsp_opcode)
 {
 
+	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+
 	return audioreach_send_cmd_sync(graph->dev, NULL,  &graph->result, &graph->lock,
 					graph->port, &graph->cmd_wait, pkt, rsp_opcode);
 }
@@ -970,6 +972,8 @@ int audioreach_compr_set_param(struct q6apm_graph *graph,
 	if (rc)
 		return rc;
 
+	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+
 	return gpr_send_port_pkt(graph->port, pkt);
 }
 EXPORT_SYMBOL_GPL(audioreach_compr_set_param);
@@ -1489,6 +1493,8 @@ int audioreach_shared_memory_send_eos(struct q6apm_graph *graph)
 
 	eos->policy = WR_SH_MEM_EP_EOS_POLICY_LAST;
 
+	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+
 	return gpr_send_port_pkt(graph->port, pkt);
 }
 EXPORT_SYMBOL_GPL(audioreach_shared_memory_send_eos);
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index 62a2fd79bbcb..2ae7b402a137 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -912,14 +912,19 @@ 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, uint32_t opcode,
+				   uint32_t token);
 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);
+			       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);
@@ -930,10 +935,13 @@ int audioreach_tplg_init(struct snd_soc_component *component);
 
 /* Module specific */
 void audioreach_graph_free_buf(struct q6apm_graph *graph);
-int audioreach_send_cmd_sync(struct device *dev, gpr_device_t *gdev, struct gpr_ibasic_rsp_result_t *result,
-			     struct mutex *cmd_lock, gpr_port_t *port, wait_queue_head_t *cmd_wait,
-			     const struct gpr_pkt *pkt, uint32_t rsp_opcode);
-int audioreach_graph_send_cmd_sync(struct q6apm_graph *graph, const struct gpr_pkt *pkt,
+int audioreach_send_cmd_sync(struct device *dev, gpr_device_t *gdev,
+			     struct gpr_ibasic_rsp_result_t *result,
+			     struct mutex *cmd_lock, gpr_port_t *port,
+			     wait_queue_head_t *cmd_wait,
+			     struct gpr_pkt *pkt, uint32_t rsp_opcode);
+int audioreach_graph_send_cmd_sync(struct q6apm_graph *graph,
+				   struct gpr_pkt *pkt,
 				   uint32_t rsp_opcode);
 int audioreach_set_media_format(struct q6apm_graph *graph,
 				const struct audioreach_module *module,
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index 641d6d243229..e5563014fa5b 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -29,11 +29,13 @@ struct apm_graph_mgmt_cmd {
 
 static struct q6apm *g_apm;
 
-int q6apm_send_cmd_sync(struct q6apm *apm, const struct gpr_pkt *pkt,
+int q6apm_send_cmd_sync(struct q6apm *apm, struct gpr_pkt *pkt,
 			uint32_t rsp_opcode)
 {
 	gpr_device_t *gdev = apm->gdev;
 
+	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(gdev);
+
 	return audioreach_send_cmd_sync(&gdev->dev, gdev, &apm->result, &apm->lock,
 					NULL, &apm->wait, pkt, rsp_opcode);
 }
@@ -502,6 +504,8 @@ int q6apm_write_async(struct q6apm_graph *graph, uint32_t len, uint32_t msw_ts,
 
 	mutex_unlock(&graph->lock);
 
+	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+
 	return gpr_send_port_pkt(graph->port, pkt);
 }
 EXPORT_SYMBOL_GPL(q6apm_write_async);
@@ -536,6 +540,8 @@ int q6apm_read(struct q6apm_graph *graph)
 
 	mutex_unlock(&graph->lock);
 
+	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
+
 	return gpr_send_port_pkt(graph->port, pkt);
 }
 EXPORT_SYMBOL_GPL(q6apm_read);
diff --git a/sound/soc/qcom/qdsp6/q6apm.h b/sound/soc/qcom/qdsp6/q6apm.h
index 5cb51ca491dc..9092359ccf90 100644
--- a/sound/soc/qcom/qdsp6/q6apm.h
+++ b/sound/soc/qcom/qdsp6/q6apm.h
@@ -147,7 +147,7 @@ int q6apm_alloc_fragments(struct q6apm_graph *graph,
 int q6apm_free_fragments(struct q6apm_graph *graph, unsigned int dir);
 int q6apm_unmap_memory_fixed_region(struct device *dev, unsigned int graph_id);
 /* Helpers */
-int q6apm_send_cmd_sync(struct q6apm *apm, const struct gpr_pkt *pkt,
+int q6apm_send_cmd_sync(struct q6apm *apm, struct gpr_pkt *pkt,
 			uint32_t rsp_opcode);
 
 /* Callback for graph specific */
diff --git a/sound/soc/qcom/qdsp6/q6prm.c b/sound/soc/qcom/qdsp6/q6prm.c
index 04892fb4423f..f93383078eb1 100644
--- a/sound/soc/qcom/qdsp6/q6prm.c
+++ b/sound/soc/qcom/qdsp6/q6prm.c
@@ -51,6 +51,8 @@ struct prm_cmd_release_rsc {
 
 static int q6prm_send_cmd_sync(struct q6prm *prm, struct gpr_pkt *pkt, uint32_t rsp_opcode)
 {
+	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(prm->gdev);
+
 	return audioreach_send_cmd_sync(prm->dev, prm->gdev, &prm->result, &prm->lock,
 					NULL, &prm->wait, pkt, rsp_opcode);
 }

-- 
2.34.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Global pointer `g_apm` is not cleared on driver removal, leading to a Use-After-Free.
- [High] Functions acquire a mutex and allocate memory with `GFP_KERNEL` while holding a spinlock with IRQs disabled.
--

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

ASoC: qcom: qdsp6: generalize GPR service domain

This commit generalizes the GPR destination domain in AudioReach to support 
targets where audio is served by the modem DSP instead of the ADSP. It 
updates the packet builders to dynamically stamp the correct domain from 
the device, rather than hardcoding it.

> diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
> index f167b9dae3fa1..3ec3c5a18c7a2 100644
> --- a/sound/soc/qcom/qdsp6/q6apm.c
> +++ b/sound/soc/qcom/qdsp6/q6apm.c
> @@ -29,11 +29,13 @@ struct apm_graph_mgmt_cmd {
>  
>  static struct q6apm *g_apm;

[Severity: High]
This is a pre-existing issue, but does this code leave a dangling pointer
on driver removal?

The global pointer g_apm is assigned dynamically allocated memory during 
apm_probe(), which is bound to the device's lifecycle. However, apm_remove() 
fails to clear g_apm:

sound/soc/qcom/qdsp6/q6apm.c:apm_remove() {
	of_platform_depopulate(&gdev->dev);
	snd_soc_unregister_component(&gdev->dev);
}

The devres framework will free the apm structure automatically after removal. 
If another subsystem subsequently calls q6apm_is_adsp_ready(), it will 
dereference the freed pointer:

sound/soc/qcom/qdsp6/q6apm.c:q6apm_is_adsp_ready() {
	if (g_apm)
		return q6apm_get_apm_state(g_apm);
}

>  
> -int q6apm_send_cmd_sync(struct q6apm *apm, const struct gpr_pkt *pkt,
> +int q6apm_send_cmd_sync(struct q6apm *apm, struct gpr_pkt *pkt,
>  			uint32_t rsp_opcode)
>  {

[ ... ]

> @@ -502,6 +504,8 @@ int q6apm_write_async(struct q6apm_graph *graph, uint32_t len, uint32_t msw_ts,
>  
>  	mutex_unlock(&graph->lock);
>  
> +	pkt->hdr.dest_domain = audioreach_gpr_dest_domain(graph->apm->gdev);
> +

[Severity: High]
This is a pre-existing issue, but does calling this function result in a 
sleep-in-atomic bug?

Earlier in q6apm_write_async(), it calls audioreach_alloc_pkt(), which 
allocates memory with GFP_KERNEL. It also acquires a sleepable mutex via 
mutex_lock(&graph->lock).

However, callers in sound/soc/qcom/qdsp6/q6apm-dai.c such as 
event_handler_compr() and q6apm_compr_copy() call this while holding an 
IRQ-disabled spinlock:

sound/soc/qcom/qdsp6/q6apm-dai.c:event_handler_compr() {
	...
	guard(spinlock_irqsave)(&prtd->lock);
	...
	q6apm_write_async(prtd->graph, bytes_to_write, 0, 0, wflags);
}

Could this lead to schedule-while-atomic panics or deadlocks during compressed 
audio playback?

>  	return gpr_send_port_pkt(graph->port, pkt);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-a2a-shikra-vmid-v5-v2-0-c3dc62354eee@oss.qualcomm.com?part=3

Re: [PATCH v2 2/3] ASoC: qcom: q6apm-dai: add VMID-based SCM assignment for mDSP buffers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] The DMA buffer allocation strictly limits the size to `reserved_buf_size`, entirely omitting the required `POS_BUFFER_BYTES` for push-pull mode streams. The position buffer calculation then points out-of-bounds, corrupting adjacent allocations in the DMA pool.
- [High] Compressed audio streams bypass SCM memory assignment completely, causing Stage-2 faults on protected targets.
- [High] Hardcoded Q6APM_MAX_CARVEOUTS limit breaks sound card probe for topologies with more than 2 DAIs.
- [High] A failed SCM unassign in the non-reserved memory path allows the Stage-2 protected memory to be returned to the system page allocator, leading to kernel panics upon reallocation.
- [Medium] SCM VMID assignment leak on probe error path permanently locks out HLOS memory access.
- [Medium] Missing runtime buffer size constraint for capture streams when using reserved memory.
--

commit 57510541fbc0081167c27ca31e55e5b8fd89730c
Author: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
Subject: ASoC: qcom: q6apm-dai: add VMID-based SCM assignment for mDSP buffers

This commit adds VMID-based SCM assignment for memory buffers on platforms
with an mDSP. It parses the qcom,vmids property and assigns memory to HLOS
and consumer VMIDs to allow the DSP to access PCM buffers.

diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -84,9 +96,123 @@ struct q6apm_dai_rtd {
 };
[ ... ]
> +static void q6apm_dai_scm_unassign_addr(struct q6apm_dai_data *pdata,
> +					phys_addr_t addr)
> +{
> +	int i;
> +
> +	for (i = 0; i < pdata->num_carveouts; i++) {
> +		if (pdata->carveout_regions[i].addr != addr)
> +			continue;
> +
> +		q6apm_dai_scm_unassign(&pdata->carveout_regions[i], pdata);
> +		if (pdata->carveout_regions[i].assigned)
> +			return;	/* reclaim failed: keep tracking, warned already */

[Severity: High]
If restoring HLOS permissions fails in q6apm_dai_scm_unassign_addr(), it returns
early while leaving the buffer marked as assigned to the DSP.

However, looking at q6apm_dai_pcm_free(), which calls this function during PCM
destruction, the failure is ignored since the callback returns void.
Won't the ALSA core then proceed to blindly free this Stage-2 protected memory
back to the system page allocator? This could lead to a kernel panic when the
kernel later attempts to reuse the memory.

[ ... ]
> @@ -409,8 +535,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]
This constraint restricts the buffer size to the reserved memory limit, but it
appears to be gated entirely behind a check for playback streams.

Are capture streams also supposed to be constrained when using reserved memory?
Without it, userspace might request a capture buffer larger than the reserved
DMA pool slice, leading to unexpected -ENOMEM errors.

[ ... ]
> @@ -431,17 +560,18 @@ static int q6apm_dai_open(struct snd_soc_component *component,
>  	}
>  
>  	runtime->private_data = prtd;
> -	runtime->dma_bytes = BUFFER_BYTES_MAX;
>  	if (pdata->sid < 0)
>  		prtd->phys = substream->dma_buffer.addr;
>  	else
>  		prtd->phys = substream->dma_buffer.addr | (pdata->sid << 32);
>  
>  	if (q6apm_is_graph_in_push_pull_mode(prtd->graph)) {
> +		size_t buf_sz = pdata->has_reserved_mem ?
> +				pdata->reserved_buf_size : BUFFER_BYTES_MAX;
>  		void *pos_buffer;
>  
> -		prtd->pos_phys = prtd->phys + BUFFER_BYTES_MAX;
> -		pos_buffer = (void *)(substream->dma_buffer.area + BUFFER_BYTES_MAX);
> +		prtd->pos_phys = prtd->phys + buf_sz;
> +		pos_buffer = (void *)(substream->dma_buffer.area + buf_sz);
>  		prtd->pos_buffer = (struct sh_mem_pull_push_mode_position_buffer *)(pos_buffer);
>  	}

[ ... ]
> @@ -599,13 +741,46 @@ static int q6apm_dai_pcm_new(struct snd_soc_component *component, struct snd_soc
>  		if (is_push_pull)
>  			size += POS_BUFFER_BYTES;
>  
> -		ret = snd_pcm_set_fixed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV, component->dev, size);
> +		if (pdata->has_reserved_mem)
> +			ret = snd_pcm_set_managed_buffer_all(pcm,
> +							     SNDRV_DMA_TYPE_DEV, component->dev,
> +							     pdata->reserved_buf_size,
> +							     pdata->reserved_buf_size);
> +		else
> +			ret = snd_pcm_set_fixed_buffer_all(pcm,
> +							   SNDRV_DMA_TYPE_DEV, component->dev,
> +							   size);

[Severity: High]
When using reserved memory for push-pull streams, the managed buffer size is set
strictly to pdata->reserved_buf_size, completely omitting the extra overhead
required for POS_BUFFER_BYTES.

However, looking back at q6apm_dai_open(), the position buffer is mapped to:
pos_buffer = (void *)(substream->dma_buffer.area + pdata->reserved_buf_size);

Since the allocation doesn't account for this extra size, does this pointer
now access out-of-bounds virtual memory immediately following the allocated
chunk? This seems like it could corrupt adjacent allocations in the DMA pool.

[ ... ]
> +		if (pdata->use_scm_assign && !pdata->has_reserved_mem) {
> +			struct q6apm_scm_region *r;
> +
> +			if (pdata->num_carveouts >= Q6APM_MAX_CARVEOUTS) {
> +				dev_err(component->dev,
> +					"too many SCM carveouts (max %d)\n",
> +					Q6APM_MAX_CARVEOUTS);
> +				q6apm_dai_memory_unmap(component, substream);
> +				return -ENOSPC;
> +			}

[Severity: High]
This dynamically adds a carveout for every PCM substream but rigidly caps the
limit at Q6APM_MAX_CARVEOUTS (which is defined as 2).

Does this arbitrarily restrict the entire sound card to only 2 PCM streams?
Standard configurations often instantiate multiple DAIs with both playback and
capture streams. Would a third PCM stream creation fail probe with -ENOSPC?

[ ... ]
> @@ -1017,23 +1206,161 @@ static const struct snd_soc_component_driver q6apm_fe_dai_component = {
[ ... ]
> +		if (pdata->num_carveouts) {
> +			for (i = 0; i < pdata->num_carveouts; i++) {
> +				rc = q6apm_dai_scm_assign(&pdata->carveout_regions[i],
> +							  pdata);
> +				if (rc) {
> +					dev_err(dev,
> +						"SCM assign carveout[%d] failed: %d\n",
> +						i, rc);
> +					return rc;
> +				}
> +			}
> +			rc = devm_add_action_or_reset(dev,
> +						      q6apm_dai_unassign_all,
> +						      pdata);

[Severity: Medium]
If q6apm_dai_scm_assign() succeeds for the first carveout but fails during a
subsequent iteration, does this early return leak the earlier assignment?

The cleanup action (devm_add_action_or_reset) hasn't been registered yet,
so we might permanently lose HLOS access to that reserved memory region if
probe fails partway through this loop.

[ ... ]

Looking at q6apm_dai_compr_open() which handles compressed audio streams:

static int q6apm_dai_compr_open(struct snd_soc_component *component,
				struct snd_compr_stream *stream)
{
[ ... ]
	size = COMPR_PLAYBACK_MAX_FRAGMENT_SIZE * COMPR_PLAYBACK_MAX_NUM_FRAGMENTS;
	ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size, &prtd->dma_buffer);
[ ... ]
}

[Severity: High]
For compressed streams, the DMA buffer is allocated from system memory via
snd_dma_alloc_pages() but never appears to be assigned via q6apm_dai_scm_assign().

Since the DSP on these platforms requires explicit VMID assignment to access
memory, will this omission cause a Stage-2 permission fault or DSP crash when
attempting to play compressed audio?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-a2a-shikra-vmid-v5-v2-0-c3dc62354eee@oss.qualcomm.com?part=2

Re: [PATCH v2 1/3] dt-bindings: sound: qcom,q6apm-dai: add stage-2 (SCM/VMID) access model

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Wed, Aug 26, 2026 at 12:10:12AM +0530, Ajay Kumar Nandam wrote:
> The APM DAIs expose PCM buffers that the backing DSP must be able to
> reach. On existing targets the DSP runs in a stage-1 context and the
> buffers are described to the SMMU via the iommus property.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] ASoC: qcom: q6apm-dai: add VMID-based SCM assignment for mDSP buffers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 910 bytes --]

On Wed, Aug 26, 2026 at 12:10:13AM +0530, Ajay Kumar Nandam wrote:
> On platforms such as Qualcomm Shikra, audio is served by the modem DSP
> (mDSP) which runs in a stage-2 protected context. Unlike ADSP targets
> where SMMU-mapped system RAM is directly accessible, the mDSP cannot
> reach the PCM buffers unless they are explicitly SCM-assigned to the
> appropriate Virtual Machine IDs (VMIDs). Without this assignment, audio
> does not function on these platforms.

> -		ret = snd_pcm_set_fixed_buffer_all(pcm, SNDRV_DMA_TYPE_DEV, component->dev, size);
> +		if (pdata->has_reserved_mem)
> +			ret = snd_pcm_set_managed_buffer_all(pcm,
> +							     SNDRV_DMA_TYPE_DEV, component->dev,
> +							     pdata->reserved_buf_size,
> +							     pdata->reserved_buf_size);

Does this do the right thing if the ALSA core preallocate_dma module
param is set to 0 for some reason?  I'm not clear why you would...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/3] dt-bindings: sound: qcom,q6apm-dai: add stage-2 (SCM/VMID) access model

[Krzysztof Kozlowski]

On 25/08/2026 20:40, Ajay Kumar Nandam wrote:
> The APM DAIs expose PCM buffers that the backing DSP must be able to
> reach. On existing targets the DSP runs in a stage-1 context and the
> buffers are described to the SMMU via the iommus property.
> 
> On platforms such as Qualcomm Shikra, where audio is served by the modem
> DSP (mDSP) rather than the ADSP, the DSP runs in a stage-2 protected
> context and cannot use the SMMU. The buffers are reachable only after
> hypervisor (SCM) memory assignment to the fixed set of consumer Virtual
> Machine IDs (VMIDs) that own that protected context. Describe those
> consumers with a new qcom,vmids property.
> 
> The two access models are mutually exclusive: a node carries either
> iommus (stage-1/SMMU) or qcom,vmids (stage-2/SCM), expressed with a
> oneOf constraint. iommus is therefore no longer unconditionally required.
> SCM assignment replaces the buffer's current owners with the supplied
> destination set, so the driver always adds HLOS to that set itself to
> retain host access; HLOS must not be listed in qcom,vmids.
> 
> When qcom,vmids is present the PCM buffers must reside in reserved-memory
> carveouts that are SCM-assigned to the consumer VMIDs. Add an optional
> memory-region property listing those carveouts. The first entry is the
> control-path buffer and subsequent entries are data-path buffers.
> memory-region is only meaningful alongside qcom,vmids, enforced via
> dependentRequired.
> 
> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/sound/qcom,q6apm-dai.yaml  | 72 +++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
> index 9e5b30d9c6e6..d9a6bc4a14f2 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml
> @@ -10,7 +10,15 @@ maintainers:
>    - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>  
>  description: |
> -  This binding describes the Qualcomm APM DAIs in DSP
> +  This binding describes the Qualcomm APM DAIs in DSP.
> +
> +  The DSP that runs the APM must be able to reach the PCM buffers. On
> +  existing targets the DSP runs in a stage-1 context and the buffers are
> +  described to the SMMU via iommus. On targets whose APM DSP runs in a
> +  stage-2 protected context that cannot use the SMMU, the buffers are
> +  reachable only after hypervisor (SCM) memory assignment to the fixed
> +  set of consumer VMIDs that own that protected context, described by
> +  qcom,vmids. The two access models are mutually exclusive.
>  
>  properties:
>    compatible:
> @@ -20,9 +28,42 @@ properties:
>      minItems: 1
>      maxItems: 2

As I said last time, different hardware, different programming
interface, different features, so a different compatible.


>  
> +  memory-region:
> +    description:
> +      Reserved-memory carveouts (shared-dma-pool, no-map) that hold the
> +      PCM buffers and must be SCM-assigned to the VMIDs in qcom,vmids.
> +      The first entry is the control-path buffer; the second entry is
> +      the data-path buffer. Only used together with qcom,vmids.
> +    minItems: 1
> +    maxItems: 2

List the items with description, isntead of free form text

items:
  - description:
  - description:

> +
> +  qcom,vmids:
> +    description:
> +      Virtual Machine IDs (VMIDs) of the processors that consume the PCM
> +      buffers and therefore must be granted access through SCM memory
> +      assignment. Required on targets whose APM DSP runs in a stage-2
> +      protected context and cannot use the SMMU; mutually exclusive with
> +      iommus. SCM assignment replaces the buffer's current owners with the
> +      supplied set, so the driver always adds HLOS to the destination list
> +      itself to retain host access; HLOS must not be listed here. All
> +      listed VMIDs and HLOS receive read-write access.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 2
> +    uniqueItems: true
> +
>  required:
>    - compatible
> -  - iommus
> +
> +oneOf:
> +  - required:
> +      - iommus
> +  - required:
> +      - qcom,vmids
> +
> +dependentRequired:
> +  memory-region:
> +    - qcom,vmids
>  
>  additionalProperties: false
>  
> @@ -32,3 +73,30 @@ examples:
>          compatible = "qcom,q6apm-dais";
>          iommus = <&apps_smmu 0x1801 0x0>;
>      };
> +  - |
> +    #include <dt-bindings/firmware/qcom,scm.h>
> +
> +    reserved-memory {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        ranges;
> +
> +        audio_heap_mem: audio-heap@a1000000 {
> +            compatible = "shared-dma-pool";
> +            reg = <0x0 0xa1000000 0x0 0x100000>;
> +            no-map;
> +        };
> +
> +        audio_mdsp_carveout_mem: audio-carveout@a1100000 {
> +            compatible = "shared-dma-pool";
> +            reg = <0x0 0xa1100000 0x0 0x400000>;
> +            no-map;
> +        };
> +    };

Drop entire node, not really relevant.

> +
> +    dais {
> +        compatible = "qcom,q6apm-dais";
> +        /* index 0: control path, index 1: data path */
> +        memory-region = <&audio_heap_mem>, <&audio_mdsp_carveout_mem>;
> +        qcom,vmids = <QCOM_SCM_VMID_LPASS QCOM_SCM_VMID_MSS_MSA>;
> +    };
> 


Best regards,
Krzysztof