* [PATCH v5 0/4] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP)
@ 2026-09-21 19:08 Ajay Kumar Nandam
2026-09-21 19:08 ` [PATCH v5 1/4] ASoC: qcom: q6apm: clear g_apm on driver removal Ajay Kumar Nandam
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Ajay Kumar Nandam @ 2026-09-21 19:08 UTC (permalink / raw)
To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Pierre-Louis Bossart, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: ajay.nandam, linux-sound, linux-arm-msm, linux-kernel, devicetree,
Pratyush Meduri, Mohit Sharma
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 driver detects the mDSP target at runtime from the GPR domain_id
(GPR_DOMAIN_ID_MODEM) and enables the SCM assignment path automatically.
VMIDs are hardcoded in the driver (HLOS + MSS_MSA + LPASS) rather than
read from DT, following the consensus from v2 review discussion with
Krzysztof Kozlowski and Srinivas Kandagatla.
Tested on Qualcomm Shikra with mDSP audio playback and capture.
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:
https://lore.kernel.org/all/20260826-a2a-shikra-vmid-v5-v2-0-c3dc62354eee@oss.qualcomm.com/
v3:
https://lore.kernel.org/all/20260918-vmid-v3-v3-0-f1cbf47bf173@oss.qualcomm.com/
v4:
https://lore.kernel.org/all/20260922-vmid-v4-v4-0-5fe21b1365a1@oss.qualcomm.com/
Changes since v4:
- Resend — v4 was partially delivered due to SMTP failure, no code
changes.
Changes since v3:
- Split DT binding changes into a separate patch (3/4) from the driver
implementation (4/4). (Rob Herring)
- SCM-assign the data-path pool (memory-region[1]) as a single whole-pool
operation at probe instead of per-stream in pcm_new()/compr_open().
- SCM-assign compressed audio stream buffers in compr_open() and
unassign in compr_free() when no data-path pool is present. v3 only
covered PCM streams.
- Fix error path in pcm_new: unassign SCM region if memory_map fails
after a successful SCM assign, preventing a resource leak.
- Account for PAGE_SIZE padding in reserved-memory pool budget
calculation. v3 under-reserved by PAGE_SIZE per stream, which could
overflow the pool at maximum concurrent stream count.
Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
---
Ajay Kumar Nandam (4):
ASoC: qcom: q6apm: clear g_apm on driver removal
ASoC: qcom: qdsp6: generalize GPR service domain
dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus
ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms
.../devicetree/bindings/sound/qcom,q6apm-dai.yaml | 12 +-
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 | 300 +++++++++++++++++++--
sound/soc/qcom/qdsp6/q6apm.c | 9 +-
sound/soc/qcom/qdsp6/q6apm.h | 2 +-
sound/soc/qcom/qdsp6/q6prm.c | 2 +
8 files changed, 326 insertions(+), 34 deletions(-)
---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260921-vmid-v4-0116efe5937b
Best regards,
--
Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v5 1/4] ASoC: qcom: q6apm: clear g_apm on driver removal 2026-09-21 19:08 [PATCH v5 0/4] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP) Ajay Kumar Nandam @ 2026-09-21 19:08 ` Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 2/4] ASoC: qcom: qdsp6: generalize GPR service domain Ajay Kumar Nandam ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-21 19:08 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: ajay.nandam, linux-sound, linux-arm-msm, linux-kernel, devicetree The global g_apm pointer is set during apm_probe() but never cleared in apm_remove(). After the driver is removed the devm-managed struct q6apm is freed, leaving g_apm dangling. A subsequent call to q6apm_is_adsp_ready() dereferences the freed pointer. Clear g_apm in apm_remove() before the component is unregistered so that q6apm_is_adsp_ready() returns false instead of triggering a use-after-free. Fixes: 5477518b8a0e ("ASoC: qdsp6: audioreach: add q6apm support") Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com> --- sound/soc/qcom/qdsp6/q6apm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c index 641d6d243229..12c6dfe4c58e 100644 --- a/sound/soc/qcom/qdsp6/q6apm.c +++ b/sound/soc/qcom/qdsp6/q6apm.c @@ -894,6 +894,7 @@ static int apm_probe(gpr_device_t *gdev) static void apm_remove(gpr_device_t *gdev) { + g_apm = NULL; of_platform_depopulate(&gdev->dev); snd_soc_unregister_component(&gdev->dev); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 2/4] ASoC: qcom: qdsp6: generalize GPR service domain 2026-09-21 19:08 [PATCH v5 0/4] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP) Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 1/4] ASoC: qcom: q6apm: clear g_apm on driver removal Ajay Kumar Nandam @ 2026-09-21 19:08 ` Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms Ajay Kumar Nandam 3 siblings, 0 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-21 19:08 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: ajay.nandam, linux-sound, linux-arm-msm, linux-kernel, devicetree, Pratyush Meduri 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 12c6dfe4c58e..1845eb7b5739 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-21 19:08 [PATCH v5 0/4] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP) Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 1/4] ASoC: qcom: q6apm: clear g_apm on driver removal Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 2/4] ASoC: qcom: qdsp6: generalize GPR service domain Ajay Kumar Nandam @ 2026-09-21 19:08 ` Ajay Kumar Nandam 2026-09-21 19:17 ` sashiko-bot ` (3 more replies) 2026-09-21 19:08 ` [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms Ajay Kumar Nandam 3 siblings, 4 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-21 19:08 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: ajay.nandam, linux-sound, linux-arm-msm, linux-kernel, devicetree On modem-DSP (mDSP) platforms the DSP runs in a stage-2 protected context and cannot use the SMMU. PCM buffers must live in reserved-memory carveouts that are SCM-assigned to the appropriate VMIDs. Add an optional memory-region property (1-2 entries) for the control-path and data-path carveouts, and remove iommus from the required list since mDSP platforms have neither IOMMU nor VMID DT properties. Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com> --- Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml index 9e5b30d9c6e6..7d0415f2463a 100644 --- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml @@ -20,9 +20,19 @@ properties: minItems: 1 maxItems: 2 + memory-region: + minItems: 1 + maxItems: 2 + items: + - description: + Control-path buffer (audio_heap_mem) for platforms where the DSP + runs in a stage-2 protected context (e.g. modem-DSP). + - description: + Data-path buffer pool from which per-stream DMA buffers are + allocated. + required: - compatible - - iommus additionalProperties: false -- 2.34.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-21 19:08 ` [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus Ajay Kumar Nandam @ 2026-09-21 19:17 ` sashiko-bot 2026-09-22 12:21 ` Mark Brown ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: sashiko-bot @ 2026-09-21 19:17 UTC (permalink / raw) To: Ajay Kumar Nandam; +Cc: conor+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dt-bindings: sound: q6apm-dai: schema allows omitting both iommus and memory-region -- commit 0ba7ab9d36bf919ab318abe00f3ad28279a11313 Author: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com> dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus This commit adds an optional memory-region property for control-path and data-path carveouts on modem-DSP platforms. It also removes iommus from the required list because mDSP platforms lack IOMMU or VMID device tree properties. > diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml > index 9e5b30d9c6e62..7d0415f2463a5 100644 > --- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml > +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml > @@ -20,9 +20,19 @@ properties: > minItems: 1 > maxItems: 2 > > + memory-region: > + minItems: 1 > + maxItems: 2 > + items: > + - description: > + Control-path buffer (audio_heap_mem) for platforms where the DSP > + runs in a stage-2 protected context (e.g. modem-DSP). > + - description: > + Data-path buffer pool from which per-stream DMA buffers are > + allocated. > + > required: > - compatible > - - iommus [Severity: Medium] Does removing iommus here without adding a constraint like oneOf or anyOf allow the schema to silently pass if both properties are omitted? If a device tree is written without either iommus or memory-region, it looks like it will incorrectly pass schema validation. At runtime, the driver would then allocate normal memory, and when the DSP attempts to access it, it could trigger an SMMU or stage-2 translation fault and crash the DSP/subsystem. Should we add a structural constraint to enforce that at least one of the memory protection mechanisms is specified? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260922-vmid-v4-v5-0-e79cfd7af5b3@oss.qualcomm.com?part=3 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-21 19:08 ` [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus Ajay Kumar Nandam 2026-09-21 19:17 ` sashiko-bot @ 2026-09-22 12:21 ` Mark Brown 2026-09-23 11:08 ` Ajay Kumar Nandam 2026-09-22 12:46 ` Rob Herring (Arm) 2026-09-23 11:57 ` Krzysztof Kozlowski 3 siblings, 1 reply; 15+ messages in thread From: Mark Brown @ 2026-09-22 12:21 UTC (permalink / raw) To: Ajay Kumar Nandam Cc: Srinivas Kandagatla, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-sound, linux-arm-msm, linux-kernel, devicetree [-- Attachment #1: Type: text/plain, Size: 602 bytes --] On Tue, Sep 22, 2026 at 12:38:11AM +0530, Ajay Kumar Nandam wrote: > On modem-DSP (mDSP) platforms the DSP runs in a stage-2 protected > context and cannot use the SMMU. PCM buffers must live in reserved-memory > carveouts that are SCM-assigned to the appropriate VMIDs. 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 --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-22 12:21 ` Mark Brown @ 2026-09-23 11:08 ` Ajay Kumar Nandam 0 siblings, 0 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-23 11:08 UTC (permalink / raw) To: Mark Brown Cc: Srinivas Kandagatla, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-sound, linux-arm-msm, linux-kernel, devicetree On 9/22/2026 5:51 PM, Mark Brown wrote: > On Tue, Sep 22, 2026 at 12:38:11AM +0530, Ajay Kumar Nandam wrote: >> On modem-DSP (mDSP) platforms the DSP runs in a stage-2 protected >> context and cannot use the SMMU. PCM buffers must live in reserved-memory >> carveouts that are SCM-assigned to the appropriate VMIDs. > > 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. Thanks Mark, I will fix the binding patch subject in v6 to match the ASoC subsystem style Thanks Ajay kumar Nandam ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-21 19:08 ` [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus Ajay Kumar Nandam 2026-09-21 19:17 ` sashiko-bot 2026-09-22 12:21 ` Mark Brown @ 2026-09-22 12:46 ` Rob Herring (Arm) 2026-09-23 11:12 ` Ajay Kumar Nandam 2026-09-23 11:57 ` Krzysztof Kozlowski 3 siblings, 1 reply; 15+ messages in thread From: Rob Herring (Arm) @ 2026-09-22 12:46 UTC (permalink / raw) To: Ajay Kumar Nandam Cc: linux-sound, Mark Brown, Pierre-Louis Bossart, Jaroslav Kysela, linux-arm-msm, linux-kernel, Liam Girdwood, devicetree, Conor Dooley, Krzysztof Kozlowski, Srinivas Kandagatla, Takashi Iwai On Tue, 22 Sep 2026 00:38:11 +0530, Ajay Kumar Nandam wrote: > On modem-DSP (mDSP) platforms the DSP runs in a stage-2 protected > context and cannot use the SMMU. PCM buffers must live in reserved-memory > carveouts that are SCM-assigned to the appropriate VMIDs. > > Add an optional memory-region property (1-2 entries) for the > control-path and data-path carveouts, and remove iommus from the > required list since mDSP platforms have neither IOMMU nor VMID DT > properties. > > Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com> > --- > Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml: properties:memory-region: {'minItems': 1, 'maxItems': 2, 'items': [{'description': 'Control-path buffer (audio_heap_mem) for platforms where the DSP runs in a stage-2 protected context (e.g. modem-DSP).'}, {'description': 'Data-path buffer pool from which per-stream DMA buffers are allocated.'}]} should not be valid under {'required': ['maxItems']} hint: "maxItems" is not needed with an "items" list from schema $id: http://devicetree.org/meta-schemas/items.yaml doc reference errors (make refcheckdocs): See https://patchwork.kernel.org/project/devicetree/patch/20260922-vmid-v4-v5-3-e79cfd7af5b3@oss.qualcomm.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-22 12:46 ` Rob Herring (Arm) @ 2026-09-23 11:12 ` Ajay Kumar Nandam 0 siblings, 0 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-23 11:12 UTC (permalink / raw) To: Rob Herring (Arm) Cc: linux-sound, Mark Brown, Pierre-Louis Bossart, Jaroslav Kysela, linux-arm-msm, linux-kernel, Liam Girdwood, devicetree, Conor Dooley, Krzysztof Kozlowski, Srinivas Kandagatla, Takashi Iwai On 9/22/2026 6:16 PM, Rob Herring (Arm) wrote: > > On Tue, 22 Sep 2026 00:38:11 +0530, Ajay Kumar Nandam wrote: >> On modem-DSP (mDSP) platforms the DSP runs in a stage-2 protected >> context and cannot use the SMMU. PCM buffers must live in reserved-memory >> carveouts that are SCM-assigned to the appropriate VMIDs. >> >> Add an optional memory-region property (1-2 entries) for the >> control-path and data-path carveouts, and remove iommus from the >> required list since mDSP platforms have neither IOMMU nor VMID DT >> properties. >> >> Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com> >> --- >> Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml: properties:memory-region: {'minItems': 1, 'maxItems': 2, 'items': [{'description': 'Control-path buffer (audio_heap_mem) for platforms where the DSP runs in a stage-2 protected context (e.g. modem-DSP).'}, {'description': 'Data-path buffer pool from which per-stream DMA buffers are allocated.'}]} should not be valid under {'required': ['maxItems']} > hint: "maxItems" is not needed with an "items" list > from schema $id: http://devicetree.org/meta-schemas/items.yaml > > doc reference errors (make refcheckdocs): > > See https://patchwork.kernel.org/project/devicetree/patch/20260922-vmid-v4-v5-3-e79cfd7af5b3@oss.qualcomm.com > > The base for the series is generally the latest rc1. A different dependency > should be noted in *this* patch. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your schema. > Thanks Rob, fixed in v6 by dropping the redundant maxItems from memory-region. The focused binding validation passes locally, and I will include the base-commit trailer in v6. Thanks Ajay Kumar Nandam ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-21 19:08 ` [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus Ajay Kumar Nandam ` (2 preceding siblings ...) 2026-09-22 12:46 ` Rob Herring (Arm) @ 2026-09-23 11:57 ` Krzysztof Kozlowski 2026-09-23 16:22 ` Ajay Kumar Nandam 3 siblings, 1 reply; 15+ messages in thread From: Krzysztof Kozlowski @ 2026-09-23 11:57 UTC (permalink / raw) To: Ajay Kumar Nandam, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-sound, linux-arm-msm, linux-kernel, devicetree On 21/09/2026 21:08, Ajay Kumar Nandam wrote: > On modem-DSP (mDSP) platforms the DSP runs in a stage-2 protected > context and cannot use the SMMU. PCM buffers must live in reserved-memory > carveouts that are SCM-assigned to the appropriate VMIDs. > > Add an optional memory-region property (1-2 entries) for the > control-path and data-path carveouts, and remove iommus from the > required list since mDSP platforms have neither IOMMU nor VMID DT > properties. > > Signed-off-by: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com> > --- > Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml > index 9e5b30d9c6e6..7d0415f2463a 100644 > --- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml > +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml > @@ -20,9 +20,19 @@ properties: > minItems: 1 > maxItems: 2 > > + memory-region: > + minItems: 1 > + maxItems: 2 > + items: > + - description: > + Control-path buffer (audio_heap_mem) for platforms where the DSP > + runs in a stage-2 protected context (e.g. modem-DSP). > + - description: > + Data-path buffer pool from which per-stream DMA buffers are > + allocated. > + > required: > - compatible > - - iommus I imagine you either have iommus or memory-region, no? IOW, oneOf: - required: - iommus - required: - memory-region ? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus 2026-09-23 11:57 ` Krzysztof Kozlowski @ 2026-09-23 16:22 ` Ajay Kumar Nandam 0 siblings, 0 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-23 16:22 UTC (permalink / raw) To: Krzysztof Kozlowski, Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-sound, linux-arm-msm, linux-kernel, devicetree On 9/23/2026 5:27 PM, Krzysztof Kozlowski wrote: >> diff --git a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml >> index 9e5b30d9c6e6..7d0415f2463a 100644 >> --- a/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml >> +++ b/Documentation/devicetree/bindings/sound/qcom,q6apm-dai.yaml >> @@ -20,9 +20,19 @@ properties: >> minItems: 1 >> maxItems: 2 >> >> + memory-region: >> + minItems: 1 >> + maxItems: 2 >> + items: >> + - description: >> + Control-path buffer (audio_heap_mem) for platforms where the DSP >> + runs in a stage-2 protected context (e.g. modem-DSP). >> + - description: >> + Data-path buffer pool from which per-stream DMA buffers are >> + allocated. >> + >> required: >> - compatible >> - - iommus > I imagine you either have iommus or memory-region, no? IOW, > oneOf: > - required: > - iommus > - required: > - memory-region > ? Yes, that matches the intended split. For existing ADSP/SMMU platforms, q6apm-dais uses iommus. For the mDSP/VMID path, the DSP cannot access normal SMMU-backed system memory,so the buffers have to come from reserved-memory carveouts which the driver SCM-assigns to the fixed VMIDs. I will add the oneOf constraint in v6 to require either iommus or memory-region. VMIDs remain internal to the driver and are not described in DT. Thanks Ajay Kumar Nandam > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms 2026-09-21 19:08 [PATCH v5 0/4] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP) Ajay Kumar Nandam ` (2 preceding siblings ...) 2026-09-21 19:08 ` [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus Ajay Kumar Nandam @ 2026-09-21 19:08 ` Ajay Kumar Nandam 2026-09-21 19:25 ` sashiko-bot 2026-09-22 12:30 ` Mark Brown 3 siblings, 2 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-21 19:08 UTC (permalink / raw) To: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: ajay.nandam, linux-sound, linux-arm-msm, linux-kernel, devicetree, Mohit Sharma 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. At probe time, the driver reads the GPR domain_id from the parent APM device to determine which DSP serves audio. When domain_id matches GPR_DOMAIN_ID_ADSP the existing SMMU/iommus path is taken and no new code is exercised. When domain_id matches GPR_DOMAIN_ID_MODEM, the driver verifies that qcom_scm is available (deferring otherwise) and that iommus is absent (the two mechanisms are mutually exclusive), then enables the SCM assignment path. In the SCM path the driver parses the optional memory-region entries in DT. The first entry (memory-region[0] / audio_heap_mem) is the control-path carveout used by the DSP firmware for command traffic; since the mDSP operates on stage-2 protected memory, this carveout must be SCM-assigned at probe time itself for the firmware to function. It is SCM-assigned to HLOS (retained as source owner, RW) plus MSS_MSA and LPASS (consumer VMIDs, both RW) and restored to HLOS-only ownership via devm_add_action_or_reset() at device removal. The second entry (memory-region[1]) is the data-path buffer pool from which per-stream DMA buffers are carved out. This pool is attached via of_reserved_mem_device_init_by_idx() so that PCM buffers allocate directly from the carveout instead of system RAM. The data-path pool (memory-region[1]) is SCM-assigned to the consumer VMIDs as a single whole-pool operation at probe time, rather than per-stream in pcm_new()/compr_open(). qcom_scm_assign_mem() consumes an entry in a small fixed-size TZ memory-protection table that is shared platform-wide; assigning individual per-stream slices (up to Q6APM_POOL_MAX_STREAMS times) exhausts that table and hangs the SMC call, which was observed as a crash while bringing up the sound card on Shikra. Since the whole pool is already accessible to the mDSP once assigned, pcm_new()/pcm_free()/compr_open()/compr_free() skip the per-buffer SCM assign/unassign entirely when the data-path pool is in use (has_reserved_mem), and only fall back to per-buffer assignment when use_scm_assign is set without a data-path pool present. Compressed audio streams follow the same pattern: the DMA buffer allocated in compr_open() is SCM-assigned immediately after allocation and unassigned in compr_free() before the buffer is freed, unless it was carved from the pre-assigned data-path pool. The VMIDs are static per SoC and hardcoded in the driver (HLOS, MSS_MSA, LPASS) rather than read from DT, following the upstream pattern used by rmtfs_mem and qcom_q6v5_pas. Buffer constraints are capped at reserved_buf_size when the data-path pool is present, and snd_pcm_set_fixed_buffer_all() is used for both paths so the carveout is not subject to the preallocate_dma module parameter. All new code paths are gated on use_scm_assign (false when domain_id is not GPR_DOMAIN_ID_MODEM), 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 | 300 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 280 insertions(+), 21 deletions(-) diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index e6e24f3b9922..991feb317940 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -102,6 +102,7 @@ config SND_SOC_QDSP6_ASM_DAI config SND_SOC_QDSP6_APM_DAI tristate select SND_SOC_COMPRESS + select QCOM_SCM config SND_SOC_QDSP6_APM_LPASS_DAI tristate diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c index bf1f872a09f4..06438fedad0f 100644 --- a/sound/soc/qcom/qdsp6/q6apm-dai.c +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c @@ -1,20 +1,24 @@ // SPDX-License-Identifier: GPL-2.0 // Copyright (c) 2021, Linaro Limited -#include <linux/init.h> +#include <dt-bindings/firmware/qcom,scm.h> +#include <dt-bindings/soc/qcom,gpr.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 <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/soc-dapm.h> #include <asm/div64.h> #include <asm/dma.h> -#include <linux/dma-mapping.h> -#include <sound/pcm_params.h> #include "q6apm.h" #define DRV_NAME "q6apm-dai" @@ -36,6 +40,16 @@ #define COMPR_PLAYBACK_MIN_NUM_FRAGMENTS (4) #define SID_MASK_DEFAULT 0xF +#define Q6APM_MAX_SCM_REGIONS 16 +#define Q6APM_POOL_MAX_STREAMS 8 + +struct q6apm_scm_region { + phys_addr_t addr; + size_t size; + u64 src_perms; + bool assigned; +}; + static const struct snd_compr_codec_caps q6apm_compr_caps = { .num_descriptors = 1, .descriptor[0].max_ch = 2, @@ -84,9 +98,88 @@ struct q6apm_dai_rtd { }; struct q6apm_dai_data { + struct device *dev; long long sid; + bool use_scm_assign; + bool has_reserved_mem; + size_t reserved_buf_size; + struct q6apm_scm_region scm_regions[Q6APM_MAX_SCM_REGIONS]; + int num_scm_regions; }; +static int q6apm_dai_scm_assign(struct q6apm_dai_data *pdata, + phys_addr_t addr, size_t size) +{ + struct qcom_scm_vmperm dst[] = { + { .vmid = QCOM_SCM_VMID_HLOS, .perm = QCOM_SCM_PERM_RW }, + { .vmid = QCOM_SCM_VMID_MSS_MSA, .perm = QCOM_SCM_PERM_RW }, + { .vmid = QCOM_SCM_VMID_LPASS, .perm = QCOM_SCM_PERM_RW }, + }; + struct q6apm_scm_region *r; + u64 src = BIT(QCOM_SCM_VMID_HLOS); + int ret; + + if (pdata->num_scm_regions >= Q6APM_MAX_SCM_REGIONS) + return -ENOSPC; + + ret = qcom_scm_assign_mem(addr, size, &src, dst, ARRAY_SIZE(dst)); + if (ret) + return ret; + + r = &pdata->scm_regions[pdata->num_scm_regions++]; + r->addr = addr; + r->size = size; + r->src_perms = src; + r->assigned = true; + + return 0; +} + +static void q6apm_dai_scm_unassign(struct q6apm_dai_data *pdata, + phys_addr_t addr) +{ + struct qcom_scm_vmperm hlos = { + .vmid = QCOM_SCM_VMID_HLOS, + .perm = QCOM_SCM_PERM_RW, + }; + int i; + + for (i = 0; i < pdata->num_scm_regions; i++) { + if (pdata->scm_regions[i].addr != addr || + !pdata->scm_regions[i].assigned) + continue; + + if (qcom_scm_assign_mem(addr, pdata->scm_regions[i].size, + &pdata->scm_regions[i].src_perms, + &hlos, 1)) { + dev_err(pdata->dev, "SCM unassign %pa failed\n", &addr); + return; + } + + pdata->scm_regions[i].assigned = false; + pdata->num_scm_regions--; + pdata->scm_regions[i] = pdata->scm_regions[pdata->num_scm_regions]; + return; + } +} + +static void q6apm_dai_scm_cleanup(void *data) +{ + struct q6apm_dai_data *pdata = data; + int i; + + for (i = pdata->num_scm_regions - 1; i >= 0; i--) { + if (pdata->scm_regions[i].assigned) + q6apm_dai_scm_unassign(pdata, + pdata->scm_regions[i].addr); + } +} + +static void q6apm_dai_reserved_mem_release(void *data) +{ + of_reserved_mem_device_release(data); +} + 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 +502,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 +527,20 @@ static int q6apm_dai_open(struct snd_soc_component *component, } runtime->private_data = prtd; - runtime->dma_bytes = BUFFER_BYTES_MAX; + runtime->dma_bytes = pdata->has_reserved_mem ? pdata->reserved_buf_size : + 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_max = 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_max; + pos_buffer = (void *)(substream->dma_buffer.area + buf_max); prtd->pos_buffer = (struct sh_mem_pull_push_mode_position_buffer *)(pos_buffer); } @@ -535,6 +634,7 @@ static int q6apm_dai_memory_map(struct snd_soc_component *component, { struct q6apm_dai_data *pdata; struct device *dev = component->dev; + size_t buf_max; phys_addr_t phys; int ret; @@ -544,20 +644,23 @@ static int q6apm_dai_memory_map(struct snd_soc_component *component, return -EINVAL; } + buf_max = pdata->has_reserved_mem ? pdata->reserved_buf_size : + BUFFER_BYTES_MAX; + if (pdata->sid < 0) phys = substream->dma_buffer.addr; 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, buf_max); if (ret < 0) dev_err(dev, "Audio Start: Buffer Allocation failed rc = %d\n", ret); if (is_push_pull) { if (pdata->sid < 0) - phys = substream->dma_buffer.addr + BUFFER_BYTES_MAX; + phys = substream->dma_buffer.addr + buf_max; else - phys = (substream->dma_buffer.addr + BUFFER_BYTES_MAX) | (pdata->sid << 32); + phys = (substream->dma_buffer.addr + buf_max) | (pdata->sid << 32); ret = q6apm_map_pos_buffer(dev, graph_id, phys, POS_BUFFER_BYTES); if (ret < 0) @@ -572,20 +675,22 @@ 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; struct snd_pcm *pcm = rtd->pcm; - /* - * Allocate one extra page as a workaround for a DSP bug where 32-bit - * address arithmetic can overflow when the buffer is placed near the - * end of the addressable range. - */ int size = BUFFER_BYTES_MAX + PAGE_SIZE; int graph_id, ret; bool is_push_pull; struct snd_pcm_substream *substream = NULL; + pdata = snd_soc_component_get_drvdata(component); + if (!pdata) + return -EINVAL; + + if (pdata->has_reserved_mem) + size = pdata->reserved_buf_size + PAGE_SIZE; + 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) @@ -603,9 +708,24 @@ static int q6apm_dai_pcm_new(struct snd_soc_component *component, struct snd_soc if (ret) return ret; + if (pdata->use_scm_assign && !pdata->has_reserved_mem) { + ret = q6apm_dai_scm_assign(pdata, + substream->dma_buffer.addr, + ALIGN(size, PAGE_SIZE)); + if (ret) { + dev_err(component->dev, + "SCM assign buffer failed: %d\n", ret); + return ret; + } + } + ret = q6apm_dai_memory_map(component, substream, graph_id, is_push_pull); - if (ret) + if (ret) { + if (pdata->use_scm_assign && !pdata->has_reserved_mem) + q6apm_dai_scm_unassign(pdata, + substream->dma_buffer.addr); return ret; + } } return 0; @@ -635,15 +755,26 @@ 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; struct snd_pcm_substream *substream; + pdata = snd_soc_component_get_drvdata(component); + substream = pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream; - if (substream) + if (substream) { q6apm_dai_memory_unmap(component, substream); + if (pdata && pdata->use_scm_assign && !pdata->has_reserved_mem) + q6apm_dai_scm_unassign(pdata, + substream->dma_buffer.addr); + } substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream; - if (substream) + if (substream) { q6apm_dai_memory_unmap(component, substream); + if (pdata && pdata->use_scm_assign && !pdata->has_reserved_mem) + q6apm_dai_scm_unassign(pdata, + substream->dma_buffer.addr); + } } static int q6apm_dai_compr_open(struct snd_soc_component *component, @@ -683,6 +814,17 @@ static int q6apm_dai_compr_open(struct snd_soc_component *component, if (ret) return ret; + if (pdata->use_scm_assign && !pdata->has_reserved_mem) { + ret = q6apm_dai_scm_assign(pdata, prtd->dma_buffer.addr, + ALIGN(size, PAGE_SIZE)); + if (ret) { + dev_err(dev, "SCM assign compr buffer failed: %d\n", + ret); + snd_dma_free_pages(&prtd->dma_buffer); + return ret; + } + } + if (pdata->sid < 0) prtd->phys = prtd->dma_buffer.addr; else @@ -700,11 +842,16 @@ static int q6apm_dai_compr_free(struct snd_soc_component *component, { struct snd_compr_runtime *runtime = stream->runtime; struct q6apm_dai_rtd *prtd = runtime->private_data; + struct q6apm_dai_data *pdata; + + pdata = snd_soc_component_get_drvdata(component); q6apm_graph_stop(prtd->graph); q6apm_free_fragments(prtd->graph, SNDRV_PCM_STREAM_PLAYBACK); q6apm_unmap_memory_fixed_region(component->dev, prtd->graph->id); q6apm_graph_close(prtd->graph); + if (pdata && pdata->use_scm_assign && !pdata->has_reserved_mem) + q6apm_dai_scm_unassign(pdata, prtd->dma_buffer.addr); snd_dma_free_pages(&prtd->dma_buffer); prtd->graph = NULL; kfree(prtd); @@ -1021,6 +1168,7 @@ static int q6apm_dai_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *node = dev->of_node; + struct q6apm *apm = dev_get_drvdata(dev->parent); struct q6apm_dai_data *pdata; struct of_phandle_args args; int rc; @@ -1029,12 +1177,122 @@ static int q6apm_dai_probe(struct platform_device *pdev) if (!pdata) return -ENOMEM; + pdata->dev = dev; + rc = of_parse_phandle_with_fixed_args(node, "iommus", 1, 0, &args); if (rc < 0) pdata->sid = -1; else pdata->sid = args.args[0] & SID_MASK_DEFAULT; + if (apm && apm->gdev && + apm->gdev->domain_id == GPR_DOMAIN_ID_MODEM) { + if (!qcom_scm_is_available()) + return -EPROBE_DEFER; + + if (pdata->sid >= 0) { + dev_err(dev, + "iommus and mDSP SCM path are mutually exclusive\n"); + return -EINVAL; + } + + pdata->use_scm_assign = true; + + rc = devm_add_action_or_reset(dev, q6apm_dai_scm_cleanup, + pdata); + if (rc) + return rc; + } + + if (pdata->use_scm_assign) { + int mem_count; + + mem_count = of_count_phandle_with_args(node, "memory-region", + NULL); + if (mem_count >= 1) { + struct device_node *mem_node; + struct reserved_mem *rmem; + + mem_node = of_parse_phandle(node, "memory-region", 0); + rmem = of_reserved_mem_lookup(mem_node); + of_node_put(mem_node); + if (!rmem) { + dev_err(dev, + "memory-region[0]: lookup failed\n"); + return -ENODEV; + } + + rc = q6apm_dai_scm_assign(pdata, rmem->base, + ALIGN(rmem->size, PAGE_SIZE)); + if (rc) { + dev_err(dev, + "SCM assign memory-region[0] failed: %d\n", + rc); + return rc; + } + } + + if (mem_count >= 2) { + struct device_node *mem_node; + struct reserved_mem *rmem; + size_t per_stream; + + mem_node = of_parse_phandle(node, "memory-region", 1); + rmem = of_reserved_mem_lookup(mem_node); + of_node_put(mem_node); + if (!rmem) { + dev_err(dev, + "memory-region[1]: lookup failed\n"); + return -ENODEV; + } + + per_stream = rmem->size / Q6APM_POOL_MAX_STREAMS; + if (per_stream <= POS_BUFFER_BYTES + PAGE_SIZE) { + dev_err(dev, + "reserved-memory pool too small: %llu bytes\n", + (u64)rmem->size); + return -EINVAL; + } + + /* + * Assign the whole pool to the consumer VMIDs once here, + * instead of per-stream in pcm_new()/compr_open(). Each + * qcom_scm_assign_mem() call consumes an entry in a small + * fixed-size TZ memory-protection table shared platform-wide; + * assigning per-stream-slice (up to Q6APM_POOL_MAX_STREAMS + * times) exhausts that table and hangs the SMC call. + */ + rc = q6apm_dai_scm_assign(pdata, rmem->base, + ALIGN(rmem->size, PAGE_SIZE)); + if (rc) { + dev_err(dev, + "SCM assign memory-region[1] failed: %d\n", + rc); + return rc; + } + + 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; + + pdata->reserved_buf_size = + min_t(size_t, + per_stream - POS_BUFFER_BYTES - PAGE_SIZE, + BUFFER_BYTES_MAX); + pdata->has_reserved_mem = true; + } + } + dev_set_drvdata(dev, pdata); return devm_snd_soc_register_component(dev, &q6apm_fe_dai_component, NULL, 0); -- 2.34.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms 2026-09-21 19:08 ` [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms Ajay Kumar Nandam @ 2026-09-21 19:25 ` sashiko-bot 2026-09-22 12:30 ` Mark Brown 1 sibling, 0 replies; 15+ messages in thread From: sashiko-bot @ 2026-09-21 19:25 UTC (permalink / raw) To: Ajay Kumar Nandam; +Cc: robh, devicetree, conor+dt Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] asoc: qcom: compress stream allocation bypasses reserved pool size limits - [Medium] asoc: qcom: SCM assignment expands beyond reserved memory bounds - [Medium] asoc: qcom: memory and graph leak in q6apm_dai_compr_open() error path - [Medium] asoc: qcom: capture stream lacks dynamic buffer size constraint -- commit 8016abb309ec8211ded913589c6a8b5656e3fed2 Author: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com> Subject: ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms This commit adds support for mDSP platforms by explicitly assigning PCM buffers to the appropriate Virtual Machine IDs via SCM calls at probe time and during stream opening. It manages whole-pool assignment for data-path buffers when a memory region is provided, falling back to per-buffer assignments when no data-path pool is present. > diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c > index 033197bdd8705..8f12b860e1250 100644 > --- a/sound/soc/qcom/qdsp6/q6apm-dai.c > +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c [ ... ] > @@ -409,8 +502,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] Since this constraint is only applied when the stream is SNDRV_PCM_STREAM_PLAYBACK, capture streams still advertise a static BUFFER_BYTES_MAX of 512KB. If the reserved memory pool limits reserved_buf_size to something smaller than 512KB, couldn't userspace request a capture size up to 512KB, causing hw_params to fail because the preallocated buffer is too small? > if (ret < 0) { [ ... ] > @@ -683,6 +814,17 @@ static int q6apm_dai_compr_open(struct snd_soc_component *component, > if (ret) > return ret; > > + if (pdata->use_scm_assign && !pdata->has_reserved_mem) { [Severity: High] When has_reserved_mem is true, the device's DMA allocations are fulfilled from the memory-region[1] pool. However, this function unconditionally allocates 8MB (COMPR_PLAYBACK_MAX_FRAGMENT_SIZE * COMPR_PLAYBACK_MAX_NUM_FRAGMENTS) without capping it to pdata->reserved_buf_size like the PCM streams do. Will this massive allocation immediately exhaust the shared data-path pool, starving all other streams, or fail outright if the pool slice is smaller than 8MB? > + ret = q6apm_dai_scm_assign(pdata, prtd->dma_buffer.addr, > + ALIGN(size, PAGE_SIZE)); > + if (ret) { > + dev_err(dev, "SCM assign compr buffer failed: %d\n", > + ret); > + snd_dma_free_pages(&prtd->dma_buffer); > + return ret; [Severity: Medium] If q6apm_dai_scm_assign() fails here, the driver frees the DMA buffer and returns directly. Doesn't this bypass freeing the prtd structure (allocated via kzalloc_obj) and closing the DSP graph (opened via q6apm_graph_open)? The ALSA core does not invoke the .free callback when .open returns an error, so this appears to cause a permanent memory and graph resource leak. > + } > + } > + [ ... ] > @@ -1029,12 +1177,122 @@ static int q6apm_dai_probe(struct platform_device *pdev) [ ... ] > + if (mem_count >= 1) { > + struct device_node *mem_node; > + struct reserved_mem *rmem; > + > + mem_node = of_parse_phandle(node, "memory-region", 0); > + rmem = of_reserved_mem_lookup(mem_node); > + of_node_put(mem_node); > + if (!rmem) { > + dev_err(dev, > + "memory-region[0]: lookup failed\n"); > + return -ENODEV; > + } > + > + rc = q6apm_dai_scm_assign(pdata, rmem->base, > + ALIGN(rmem->size, PAGE_SIZE)); [Severity: Medium] SCM assignments modify memory protections at the TrustZone level. Since the ALIGN macro indiscriminately rounds up the size, what happens if the device tree provides a carveout size that is not a multiple of PAGE_SIZE? It seems the driver will request TrustZone to reassign the trailing bytes that belong to adjacent, unreserved kernel memory, which revokes HLOS access to that adjacent memory and could cause a kernel crash when accessed by other subsystems. > + if (rc) { -- Sashiko AI review · https://sashiko.dev/#/patchset/20260922-vmid-v4-v5-0-e79cfd7af5b3@oss.qualcomm.com?part=4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms 2026-09-21 19:08 ` [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms Ajay Kumar Nandam 2026-09-21 19:25 ` sashiko-bot @ 2026-09-22 12:30 ` Mark Brown 2026-09-23 11:20 ` Ajay Kumar Nandam 1 sibling, 1 reply; 15+ messages in thread From: Mark Brown @ 2026-09-22 12:30 UTC (permalink / raw) To: Ajay Kumar Nandam Cc: Srinivas Kandagatla, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-sound, linux-arm-msm, linux-kernel, devicetree, Mohit Sharma [-- Attachment #1: Type: text/plain, Size: 904 bytes --] On Tue, Sep 22, 2026 at 12:38:12AM +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. > +static int q6apm_dai_scm_assign(struct q6apm_dai_data *pdata, > + phys_addr_t addr, size_t size) > +{ > + r = &pdata->scm_regions[pdata->num_scm_regions++]; This needs a lock. > @@ -1021,6 +1168,7 @@ static int q6apm_dai_probe(struct platform_device *pdev) > { > + if (per_stream <= POS_BUFFER_BYTES + PAGE_SIZE) { > + dev_err(dev, > + "reserved-memory pool too small: %llu bytes\n", > + (u64)rmem->size); %pa [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms 2026-09-22 12:30 ` Mark Brown @ 2026-09-23 11:20 ` Ajay Kumar Nandam 0 siblings, 0 replies; 15+ messages in thread From: Ajay Kumar Nandam @ 2026-09-23 11:20 UTC (permalink / raw) To: Mark Brown Cc: Srinivas Kandagatla, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-sound, linux-arm-msm, linux-kernel, devicetree, Mohit Sharma On 9/22/2026 6:00 PM, Mark Brown 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. >> +static int q6apm_dai_scm_assign(struct q6apm_dai_data *pdata, >> + phys_addr_t addr, size_t size) >> +{ >> + r = &pdata->scm_regions[pdata->num_scm_regions++]; > This needs a lock. > >> @@ -1021,6 +1168,7 @@ static int q6apm_dai_probe(struct platform_device *pdev) >> { >> + if (per_stream <= POS_BUFFER_BYTES + PAGE_SIZE) { >> + dev_err(dev, >> + "reserved-memory pool too small: %llu bytes\n", >> + (u64)rmem->size); > %pa Thanks Mark, fixed both in v6. I added a mutex to serialize scm_regions[] / num_scm_regions updates in q6apm_dai_scm_assign() and q6apm_dai_scm_unassign(), and changed the reserved-memory size diagnostic to use %pa Thanks Ajay Kumar Nandam ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-09-23 16:22 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-21 19:08 [PATCH v5 0/4] ASoC: qcom: enable audio on stage-2 protected DSPs (mDSP) Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 1/4] ASoC: qcom: q6apm: clear g_apm on driver removal Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 2/4] ASoC: qcom: qdsp6: generalize GPR service domain Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 3/4] dt-bindings: sound: qcom,q6apm-dai: add memory-region and relax iommus Ajay Kumar Nandam 2026-09-21 19:17 ` sashiko-bot 2026-09-22 12:21 ` Mark Brown 2026-09-23 11:08 ` Ajay Kumar Nandam 2026-09-22 12:46 ` Rob Herring (Arm) 2026-09-23 11:12 ` Ajay Kumar Nandam 2026-09-23 11:57 ` Krzysztof Kozlowski 2026-09-23 16:22 ` Ajay Kumar Nandam 2026-09-21 19:08 ` [PATCH v5 4/4] ASoC: qcom: q6apm-dai: add SCM buffer assignment for mDSP platforms Ajay Kumar Nandam 2026-09-21 19:25 ` sashiko-bot 2026-09-22 12:30 ` Mark Brown 2026-09-23 11:20 ` Ajay Kumar Nandam
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).