* [PATCH 0/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID
@ 2026-07-14 5:00 Vishnu Santhosh
2026-07-14 5:00 ` [PATCH 1/2] dt-bindings: dma: qcom,bam-dma: Add optional qcom,vmid property Vishnu Santhosh
2026-07-14 5:00 ` [PATCH 2/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID Vishnu Santhosh
0 siblings, 2 replies; 4+ messages in thread
From: Vishnu Santhosh @ 2026-07-14 5:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andy Gross, Bjorn Andersson
Cc: linux-arm-msm, dmaengine, devicetree, linux-kernel,
Vishnu Santhosh, chris.lew, Deepak Kumar Singh
On Qualcomm SoCs where the BAM is powered by a remote processor that
enforces XPU (eXternal Protection Unit) access control (e.g. the mDSP
on Shikra, VMID 43 / NAV), the BAM reads the descriptor FIFO as an AXI
master under the remote execution environment. Without an SCM grant
for the remote VMID, the first descriptor enqueue faults with an XPU
violation.
This series adds an optional qcom,vmid DT property to the BAM DMA
controller node. When present, the driver SCM-assigns each channel's
descriptor FIFO to HLOS plus the listed VMID(s) so the remote EE can
access it without faulting.
Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
---
Vishnu Santhosh (2):
dt-bindings: dma: qcom,bam-dma: Add optional qcom,vmid property
dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID
.../devicetree/bindings/dma/qcom,bam-dma.yaml | 26 +++
drivers/dma/qcom/Kconfig | 1 +
drivers/dma/qcom/bam_dma.c | 197 +++++++++++++++++++--
3 files changed, 208 insertions(+), 16 deletions(-)
---
base-commit: 3b029c035b34bbc693405ddf759f0e9b920c27f1
change-id: 20260714-qcom-bam-dma-vmid-ext-8a3bd3c5c00e
Best regards,
--
Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] dt-bindings: dma: qcom,bam-dma: Add optional qcom,vmid property
2026-07-14 5:00 [PATCH 0/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID Vishnu Santhosh
@ 2026-07-14 5:00 ` Vishnu Santhosh
2026-07-14 5:00 ` [PATCH 2/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID Vishnu Santhosh
1 sibling, 0 replies; 4+ messages in thread
From: Vishnu Santhosh @ 2026-07-14 5:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andy Gross, Bjorn Andersson
Cc: linux-arm-msm, dmaengine, devicetree, linux-kernel,
Vishnu Santhosh, chris.lew, Deepak Kumar Singh
A SoC can have multiple BAM DMA instances. Some of these BAMs are
powered by a remote processor that enforces XPU (eXternal Protection
Unit) access control and reads the per-channel descriptor FIFOs as an
AXI master under that remote processor's execution environment, so
their FIFOs must be accessible to the remote processor's VMID; other
BAM instances on the same SoC are not behind such a remote processor
and must not have this property set.
Add an optional qcom,vmid property listing the destination VMID(s)
that the affected BAM instance's descriptor FIFOs must be accessible
to. HLOS is always the source owner and must not be listed.
Co-developed-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
---
.../devicetree/bindings/dma/qcom,bam-dma.yaml | 26 ++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
index 0923fb189ada9ee435144e8490c64ecb81edc57d..d256340d3f32e81eaa8c1e275c2a99aac888aa90 100644
--- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
@@ -12,6 +12,15 @@ maintainers:
allOf:
- $ref: dma-controller.yaml#
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ const: qcom,shikra-bam-dma
+ then:
+ properties:
+ qcom,vmid: false
properties:
compatible:
@@ -29,6 +38,8 @@ properties:
- enum:
# SDM845, SM6115, SM8150, SM8250 and QCM2290
- qcom,bam-v1.7.4
+ # Shikra
+ - qcom,shikra-bam-dma
- const: qcom,bam-v1.7.0
clocks:
@@ -81,6 +92,21 @@ properties:
Indicates that the bam is powered up by a remote processor but must be
initialized by the local processor.
+ qcom,vmid:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ minItems: 1
+ maxItems: 8
+ items:
+ minimum: 1
+ maximum: 63
+ description:
+ Destination VMIDs of the remote processor(s) that read the per-channel
+ descriptor FIFOs as an AXI master. When present, the driver SCM-assigns
+ each FIFO to these VMIDs so the remote access does not trigger an XPU
+ violation. HLOS is always retained as the source owner and must not be
+ listed. Optional even when the qcom,shikra-bam-dma compatible is
+ present; not valid on any other compatible in this schema.
+
reg:
maxItems: 1
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID
2026-07-14 5:00 [PATCH 0/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID Vishnu Santhosh
2026-07-14 5:00 ` [PATCH 1/2] dt-bindings: dma: qcom,bam-dma: Add optional qcom,vmid property Vishnu Santhosh
@ 2026-07-14 5:00 ` Vishnu Santhosh
2026-07-14 5:16 ` sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Vishnu Santhosh @ 2026-07-14 5:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andy Gross, Bjorn Andersson
Cc: linux-arm-msm, dmaengine, devicetree, linux-kernel,
Vishnu Santhosh, chris.lew, Deepak Kumar Singh
On Qualcomm SoCs where the BAM is powered by a remote processor that
enforces XPU access control (e.g. the mDSP on Shikra, VMID 43 / NAV),
the BAM reads the descriptor FIFO as an AXI master under the remote
execution environment. Without an SCM grant for the remote VMID, the
first descriptor enqueue faults with an XPU violation.
Parse the optional qcom,vmid DT property as a list of destination
VMIDs. When present, SCM-assign each channel's descriptor FIFO to HLOS
plus the listed VMIDs; num_vmids being non-zero is derived purely from
qcom,vmid, a board-integration property, not from the per-SoC IP data,
and is distinct from qcom,powered-remotely.
A BAM with configured VMIDs has two properties that shape the channel
lifecycle:
1. The remote firmware owns the BAM's power and reset. It may remove
power during error recovery before the driver releases its
channels, so any pipe/block register access at teardown can raise
a synchronous external abort, and a local reset is redundant as
the remote re-initialises the hardware on the next power-on.
2. TZ does not revoke the SCM grant when the remote powers down. A
FIFO assigned once stays assigned across every power cycle, and
re-assigning or reclaiming it while the remote is mid-teardown is
rejected by TZ with -EINVAL.
Handle both by keeping the descriptor FIFO as a persistent resource on
such BAMs: allocate and SCM-assign it once on the first
bam_alloc_chan(), keep it (and its grant) across power cycles, and
reclaim it to HLOS and free it only once in bam_dma_remove(). If the
final reclaim fails the remote still has access, so the buffer is
leaked rather than returned to the page allocator; the source-VMID
bitmask stored by qcom_scm_assign_mem() drives that reclaim.
bam_free_chan() on such a BAM therefore only drops local channel state
(clear ->initialized, decrement active_channels) with no MMIO and no
SCM call, so the block and pipe are re-initialised on the next
power-on while power is present. The bam_chan_init_hw() pipe reset in
bam_dma_terminate_all() is likewise skipped. reclaiming the FIFO is an
SCM call, not a register access, so bam_dma_remove() stays safe.
Platforms that do not set qcom,vmid keep num_vmids 0, make no SCM
call, and leave the alloc/free and register-access paths unchanged.
Co-developed-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
---
drivers/dma/qcom/Kconfig | 1 +
drivers/dma/qcom/bam_dma.c | 197 +++++++++++++++++++++++++++++++++++++++++----
2 files changed, 182 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/qcom/Kconfig b/drivers/dma/qcom/Kconfig
index c71b0b5d536bdd5242df645454d8dc673a2ea659..8f20b33bce84de5fc48caa7411258c69b5b7e79f 100644
--- a/drivers/dma/qcom/Kconfig
+++ b/drivers/dma/qcom/Kconfig
@@ -15,6 +15,7 @@ config QCOM_BAM_DMA
depends on ARCH_QCOM || (COMPILE_TEST && OF && ARM)
select DMA_ENGINE
select DMA_VIRTUAL_CHANNELS
+ select QCOM_SCM
help
Enable support for the QCOM BAM DMA controller. This controller
provides DMA capabilities for a variety of on-chip devices.
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 1bb26af0405f3a16f97e0d4b86c945c252d97f57..389ca1a9d70e814b722bcc51f0433ce0214a2042 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -42,6 +42,7 @@
#include <linux/pm_runtime.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
+#include <linux/firmware/qcom/qcom_scm.h>
#include "../dmaengine.h"
#include "../virt-dma.h"
@@ -389,6 +390,9 @@ struct bam_chan {
struct bam_desc_hw *fifo_virt;
dma_addr_t fifo_phys;
+ /* SCM source-VMID bitmask of the FIFO, 0 if not SCM-assigned */
+ u64 fifo_src_perms;
+
/* fifo markers */
unsigned short head; /* start of active descriptor entries */
unsigned short tail; /* end of active descriptor entries */
@@ -423,6 +427,10 @@ struct bam_device {
const struct reg_offset_data *layout;
+ /* destination VMIDs for SCM assignment of descriptor FIFOs */
+ u32 *vmids;
+ int num_vmids;
+
struct clk *bamclk;
int irq;
@@ -559,6 +567,126 @@ static void bam_chan_init_hw(struct bam_chan *bchan,
bchan->tail = 0;
}
+/**
+ * bam_parse_vmids - Parse the optional qcom,vmid property
+ * @bdev: bam device
+ *
+ * Reads the list of destination VMIDs from qcom,vmid, if present. HLOS is
+ * always the source owner and must not be listed. Absent property leaves
+ * num_vmids 0.
+ */
+static int bam_parse_vmids(struct bam_device *bdev)
+{
+ struct device *dev = bdev->dev;
+ int n, i, ret;
+
+ n = of_property_count_u32_elems(dev->of_node, "qcom,vmid");
+ if (n == -EINVAL)
+ return 0;
+
+ if (n < 0)
+ return n;
+
+ if (!qcom_scm_is_available())
+ return -EPROBE_DEFER;
+
+ bdev->vmids = devm_kcalloc(dev, n, sizeof(*bdev->vmids), GFP_KERNEL);
+ if (!bdev->vmids)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(dev->of_node, "qcom,vmid",
+ bdev->vmids, n);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < n; i++) {
+ if (bdev->vmids[i] == QCOM_SCM_VMID_HLOS) {
+ dev_err(dev, "qcom,vmid must not include HLOS (%u)\n",
+ QCOM_SCM_VMID_HLOS);
+ return -EINVAL;
+ }
+ }
+
+ bdev->num_vmids = n;
+
+ return 0;
+}
+
+/**
+ * bam_assign_fifo - SCM-assign a channel's descriptor FIFO to the remote VMIDs
+ * @bdev: bam device
+ * @bchan: bam channel
+ *
+ * Grants HLOS plus the configured qcom,vmid VMIDs access to the FIFO, so
+ * the remote EE can read it. The updated source-VMID bitmask is stored in
+ * bchan->fifo_src_perms for bam_fifo_can_free() to reverse.
+ */
+static int bam_assign_fifo(struct bam_device *bdev, struct bam_chan *bchan)
+{
+ struct qcom_scm_vmperm *dst __free(kfree) = NULL;
+ u64 src = BIT_ULL(QCOM_SCM_VMID_HLOS);
+ int i, ret;
+
+ if (!bdev->num_vmids)
+ return 0;
+
+ dst = kcalloc(bdev->num_vmids + 1, sizeof(*dst), GFP_KERNEL);
+ if (!dst)
+ return -ENOMEM;
+
+ dst[0].vmid = QCOM_SCM_VMID_HLOS;
+ dst[0].perm = QCOM_SCM_PERM_RW;
+
+ for (i = 0; i < bdev->num_vmids; i++) {
+ dst[i + 1].vmid = bdev->vmids[i];
+ dst[i + 1].perm = QCOM_SCM_PERM_RW;
+ }
+
+ ret = qcom_scm_assign_mem(bchan->fifo_phys, BAM_DESC_FIFO_SIZE,
+ &src, dst, bdev->num_vmids + 1);
+ if (ret) {
+ dev_err(bdev->dev, "SCM assign fifo chan %u failed: %d\n",
+ bchan->id, ret);
+ return ret;
+ }
+
+ bchan->fifo_src_perms = src;
+
+ return 0;
+}
+
+/**
+ * bam_fifo_can_free - Reclaim a channel's descriptor FIFO to HLOS
+ * @bdev: bam device
+ * @bchan: bam channel
+ *
+ * Returns true if the FIFO may now be freed. On SCM failure the remote VMID
+ * still has access, so the caller must leak the buffer instead of freeing it.
+ */
+static bool bam_fifo_can_free(struct bam_device *bdev, struct bam_chan *bchan)
+{
+ struct qcom_scm_vmperm hlos = {
+ .vmid = QCOM_SCM_VMID_HLOS,
+ .perm = QCOM_SCM_PERM_RW,
+ };
+ int ret;
+
+ if (!bchan->fifo_src_perms)
+ return true;
+
+ ret = qcom_scm_assign_mem(bchan->fifo_phys, BAM_DESC_FIFO_SIZE,
+ &bchan->fifo_src_perms, &hlos, 1);
+ if (ret) {
+ dev_err(bdev->dev, "SCM reclaim fifo chan %u failed: %d; leaking\n",
+ bchan->id, ret);
+ return false;
+ }
+
+ bchan->fifo_src_perms = 0;
+
+ return true;
+}
+
/**
* bam_alloc_chan - Allocate channel resources for DMA channel.
* @chan: specified channel
@@ -570,16 +698,29 @@ static int bam_alloc_chan(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
- if (bchan->fifo_virt)
- return 0;
-
- /* allocate FIFO descriptor space, but only if necessary */
- bchan->fifo_virt = dma_alloc_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
- &bchan->fifo_phys, GFP_KERNEL);
-
+ /*
+ * Remote-owned BAMs keep the FIFO allocated and SCM-assigned to the
+ * remote VMID across power cycles (see bam_free_chan), so allocate and
+ * assign it only once; the block and pipe are still re-initialised on
+ * every power-on below.
+ */
if (!bchan->fifo_virt) {
- dev_err(bdev->dev, "Failed to allocate desc fifo\n");
- return -ENOMEM;
+ /* allocate FIFO descriptor space, but only if necessary */
+ bchan->fifo_virt = dma_alloc_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
+ &bchan->fifo_phys, GFP_KERNEL);
+ if (!bchan->fifo_virt) {
+ dev_err(bdev->dev, "Failed to allocate desc fifo\n");
+ return -ENOMEM;
+ }
+
+ if (bam_assign_fifo(bdev, bchan)) {
+ dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
+ bchan->fifo_virt, bchan->fifo_phys);
+ bchan->fifo_virt = NULL;
+ return -EIO;
+ }
+ } else if (!bdev->num_vmids) {
+ return 0;
}
if (bdev->active_channels++ == 0 && bdev->powered_remotely)
@@ -613,12 +754,29 @@ static void bam_free_chan(struct dma_chan *chan)
goto err;
}
+ /*
+ * Remote-owned BAMs (qcom,vmid) keep the descriptor FIFO allocated and
+ * SCM-assigned across power cycles: the remote may already have cut
+ * power, so pipe-register access would fault, and TZ still holds the
+ * grant for the next restart (the FIFO is reclaimed and freed once in
+ * bam_dma_remove). Only drop local channel state here so the block and
+ * pipe are re-initialised on the next power-on; skip all MMIO.
+ */
+ if (bdev->num_vmids) {
+ scoped_guard(spinlock_irqsave, &bchan->vc.lock)
+ bchan->initialized = 0;
+ bdev->active_channels--;
+ goto err;
+ }
+
scoped_guard(spinlock_irqsave, &bchan->vc.lock)
bam_reset_channel(bchan);
- dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE, bchan->fifo_virt,
- bchan->fifo_phys);
+ if (bam_fifo_can_free(bdev, bchan))
+ dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
+ bchan->fifo_virt, bchan->fifo_phys);
bchan->fifo_virt = NULL;
+ bdev->active_channels--;
/* mask irq for pipe/channel */
val = readl_relaxed(bam_addr(bdev, 0, BAM_IRQ_SRCS_MSK_EE));
@@ -628,7 +786,7 @@ static void bam_free_chan(struct dma_chan *chan)
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
- if (--bdev->active_channels == 0 && bdev->powered_remotely) {
+ if (bdev->active_channels == 0 && bdev->powered_remotely) {
/* s/w reset bam */
val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL));
val |= BAM_SW_RST;
@@ -767,7 +925,9 @@ static int bam_dma_terminate_all(struct dma_chan *chan)
if (!list_empty(&bchan->desc_list)) {
async_desc = list_first_entry(&bchan->desc_list,
struct bam_async_desc, desc_node);
- bam_chan_init_hw(bchan, async_desc->dir);
+ /* Remote-owned BAM: pipe reset may fault, skip it. */
+ if (!bchan->bdev->num_vmids)
+ bam_chan_init_hw(bchan, async_desc->dir);
}
list_for_each_entry_safe(async_desc, tmp,
@@ -1282,6 +1442,10 @@ static int bam_dma_probe(struct platform_device *pdev)
bdev->powered_remotely = of_property_read_bool(pdev->dev.of_node,
"qcom,powered-remotely");
+ ret = bam_parse_vmids(bdev);
+ if (ret)
+ return ret;
+
if (bdev->controlled_remotely || bdev->powered_remotely)
bdev->bamclk = devm_clk_get_optional(bdev->dev, "bam_clk");
else
@@ -1418,9 +1582,10 @@ static void bam_dma_remove(struct platform_device *pdev)
if (!bdev->channels[i].fifo_virt)
continue;
- dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
- bdev->channels[i].fifo_virt,
- bdev->channels[i].fifo_phys);
+ if (bam_fifo_can_free(bdev, &bdev->channels[i]))
+ dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
+ bdev->channels[i].fifo_virt,
+ bdev->channels[i].fifo_phys);
}
tasklet_kill(&bdev->task);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID
2026-07-14 5:00 ` [PATCH 2/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID Vishnu Santhosh
@ 2026-07-14 5:16 ` sashiko-bot
0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-14 5:16 UTC (permalink / raw)
To: Vishnu Santhosh; +Cc: devicetree, robh, vkoul, dmaengine, Frank.Li, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Critical] Passing an IOVA to the TrustZone SCM call instead of a strictly required physical address.
- [High] Skipping hardware reset during channel termination causes DMA Use-After-Free and persistent state corruption.
- [High] Skipping interrupt disable during channel free leaves hardware interrupts unmasked for a destroyed channel.
Pre-existing issues:
- [High] Unconditional MMIO register access in driver removal causes a synchronous external abort on remote-powered BAMs.
--
--- Patch [2]: [PATCH 2/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID ---
commit 91f2fbb1c7215cade81c38c8732865591d19254f
Author: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID
On Qualcomm SoCs where the BAM is powered by a remote processor that
enforces XPU access control, the BAM reads the descriptor FIFO as an
AXI master under the remote execution environment. This patch handles
such BAMs by keeping the descriptor FIFO as a persistent resource,
SCM-assigning it once and reclaiming it during remove.
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 1bb26af0405f3..389ca1a9d70e8 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
[ ... ]
> @@ -559,6 +567,126 @@ static void bam_chan_init_hw(struct bam_chan *bchan,
[ ... ]
> +static int bam_assign_fifo(struct bam_device *bdev, struct bam_chan *bchan)
> +{
> + struct qcom_scm_vmperm *dst __free(kfree) = NULL;
> + u64 src = BIT_ULL(QCOM_SCM_VMID_HLOS);
> + int i, ret;
[ ... ]
> + ret = qcom_scm_assign_mem(bchan->fifo_phys, BAM_DESC_FIFO_SIZE,
> + &src, dst, bdev->num_vmids + 1);
[Severity: Critical]
Is it safe to pass the IOVA returned by the DMA allocation API to TrustZone
here?
When the device is behind an IOMMU, bchan->fifo_phys contains a dma_addr_t
IOVA. However, TrustZone operates on raw physical memory and expects a physical
address to program hardware XPUs, completely bypassing the local IOMMU
translation.
Could this cause TrustZone to configure XPU memory permissions for the wrong
physical address?
> + if (ret) {
> + dev_err(bdev->dev, "SCM assign fifo chan %u failed: %d\n",
> + bchan->id, ret);
> + return ret;
> + }
> +
> + bchan->fifo_src_perms = src;
> +
> + return 0;
> +}
> +
> +/**
> + * bam_fifo_can_free - Reclaim a channel's descriptor FIFO to HLOS
[ ... ]
> +static bool bam_fifo_can_free(struct bam_device *bdev, struct bam_chan *bchan)
> +{
> + struct qcom_scm_vmperm hlos = {
> + .vmid = QCOM_SCM_VMID_HLOS,
> + .perm = QCOM_SCM_PERM_RW,
> + };
> + int ret;
> +
> + if (!bchan->fifo_src_perms)
> + return true;
> +
> + ret = qcom_scm_assign_mem(bchan->fifo_phys, BAM_DESC_FIFO_SIZE,
> + &bchan->fifo_src_perms, &hlos, 1);
[Severity: Critical]
Since bchan->fifo_phys is an IOVA when an IOMMU is present, will passing it
back during the reclaim operation compound the memory permission corruption
noted above?
> + if (ret) {
> + dev_err(bdev->dev, "SCM reclaim fifo chan %u failed: %d; leaking\n",
> + bchan->id, ret);
> + return false;
> + }
[ ... ]
> @@ -613,12 +754,29 @@ static void bam_free_chan(struct dma_chan *chan)
> goto err;
> }
>
> + /*
> + * Remote-owned BAMs (qcom,vmid) keep the descriptor FIFO allocated and
> + * SCM-assigned across power cycles: the remote may already have cut
> + * power, so pipe-register access would fault, and TZ still holds the
> + * grant for the next restart (the FIFO is reclaimed and freed once in
> + * bam_dma_remove). Only drop local channel state here so the block and
> + * pipe are re-initialised on the next power-on; skip all MMIO.
> + */
> + if (bdev->num_vmids) {
> + scoped_guard(spinlock_irqsave, &bchan->vc.lock)
> + bchan->initialized = 0;
> + bdev->active_channels--;
> + goto err;
> + }
[Severity: High]
Does this early exit skip masking and disabling the channel's interrupts in the
BAM controller?
If the hardware reset was also skipped during termination, the BAM might still
be actively running. Could this assert an unmasked interrupt when a pending
transaction completes after the channel is freed, resulting in an interrupt
storm or use-after-free in the interrupt handler?
> +
> scoped_guard(spinlock_irqsave, &bchan->vc.lock)
> bam_reset_channel(bchan);
[ ... ]
> @@ -767,7 +925,9 @@ static int bam_dma_terminate_all(struct dma_chan *chan)
> if (!list_empty(&bchan->desc_list)) {
> async_desc = list_first_entry(&bchan->desc_list,
> struct bam_async_desc, desc_node);
> - bam_chan_init_hw(bchan, async_desc->dir);
> + /* Remote-owned BAM: pipe reset may fault, skip it. */
> + if (!bchan->bdev->num_vmids)
> + bam_chan_init_hw(bchan, async_desc->dir);
[Severity: High]
By skipping the hardware reset for remote-owned BAMs without providing an
alternative mechanism to halt the active hardware pipe, won't the BAM continue
performing DMA?
When the client subsequently frees its data buffers, the still-running DMA
hardware could overwrite the freed memory. Furthermore, since the internal
software state flag for initialization isn't cleared, might future transfers
start with desynchronized FIFO pointers?
> }
[ ... ]
> @@ -1418,9 +1582,10 @@ static void bam_dma_remove(struct platform_device *pdev)
> if (!bdev->channels[i].fifo_virt)
> continue;
>
> - dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
> - bdev->channels[i].fifo_virt,
> - bdev->channels[i].fifo_phys);
> + if (bam_fifo_can_free(bdev, &bdev->channels[i]))
> + dma_free_wc(bdev->dev, BAM_DESC_FIFO_SIZE,
> + bdev->channels[i].fifo_virt,
> + bdev->channels[i].fifo_phys);
> }
[Severity: High]
This is a pre-existing issue, but I noticed an unconditional MMIO register
access in bam_dma_remove() happens right before this loop:
/* mask all interrupts for this execution environment */
writel_relaxed(0, bam_addr(bdev, 0, BAM_IRQ_SRCS_MSK_EE));
If the BAM driver is unbound while the remote firmware has cut power to the
hardware, wouldn't this unconditional MMIO write in the removal function fault?
The patch intentionally skipped MMIO operations in other teardown paths (like
channel free and terminate) to prevent this exact crash on remote-powered BAMs.
Could we also fix this in the removal path?
>
> tasklet_kill(&bdev->task);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-qcom-bam-dma-vmid-ext-v1-0-cef87c57b7dc@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-14 5:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-14 5:00 [PATCH 0/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID Vishnu Santhosh
2026-07-14 5:00 ` [PATCH 1/2] dt-bindings: dma: qcom,bam-dma: Add optional qcom,vmid property Vishnu Santhosh
2026-07-14 5:00 ` [PATCH 2/2] dmaengine: qcom: bam_dma: SCM-assign descriptor FIFOs to a remote VMID Vishnu Santhosh
2026-07-14 5:16 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox