[PATCH 00/12] iommu: qcom_iommu: implement support for instances on MSM8974

[PATCH 00/12] iommu: qcom_iommu: implement support for instances on MSM8974

[Dmitry Baryshkov]

Qualcomm MSM8974 platform has five SMMU instances, used by display, GPU,
Venus, VFE (camera) and JPEG encoder. Each of them follows ARM SMMU v1
spec, however they differ from other Qualcomm platforms in the
implementation-specific registers and also in interaction with TZ.
Venus, MDP and VFE SMMUs are secured and require programming only of
CBs, while GPU and JPEG require full programming.

This series skips IOMMUs which can't be tested right now (VFE and JPEG),
and adds only MDP, GPU and Venus (although untested, it is required for
display to work) SMMU instances.

Note, to get display to work properly one fix is necessary, [1]

[1] https://patch.msgid.link/20260809-msm8974-mmcc-fix-v1-1-50f2dcf18d2e@oss.qualcomm.com

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (12):
      dt-bindings: iommu: qcom,iommu: document the MSM8974 IOMMUs
      iommu: arm-smmu: add global register definitions used by the QSMMU
      iommu: qcom_iommu: extract context bank programming into a helper
      iommu: qcom_iommu: support the short-descriptor pagetable format
      iommu: qcom_iommu: handle the SMMU global register space
      iommu: qcom_iommu: support non-TZ-managed instances
      iommu: qcom_iommu: restore context bank state after power collapse
      iommu: qcom_iommu: halt the micro-MMU while programming context banks
      iommu: qcom_iommu: allow faulting transactions to terminate
      iommu: qcom_iommu: program BFP (prefetch) settings
      iommu: qcom_iommu: add the MSM8974 instances
      ARM: dts: qcom: msm8974: add the IOMMUs

 .../devicetree/bindings/iommu/qcom,iommu.yaml      |  16 +-
 arch/arm/boot/dts/qcom/qcom-msm8974.dtsi           |  85 +++-
 drivers/iommu/arm/Kconfig                          |   1 +
 drivers/iommu/arm/arm-smmu/arm-smmu.h              |  10 +
 drivers/iommu/arm/arm-smmu/qcom_iommu.c            | 445 +++++++++++++++++++--
 5 files changed, 517 insertions(+), 40 deletions(-)
---
base-commit: 6c5c106bff878247a9cc7255077aab9040df8bf8
change-id: 20260809-msm8974-iommu-upstream-144b1e2a15f0

Best regards,
--  
With best wishes
Dmitry

[PATCH 01/12] dt-bindings: iommu: qcom,iommu: document the MSM8974 IOMMUs

[Dmitry Baryshkov]

MSM8974 has several IOMMU instances, based on ARM SMMU v1 spec.
Nevertheless, the combination of implementation-specific registers and
the way TZ and hypervisor protect those instances make them not
compatible with the normal ARM SMMU programming sequences (nor with the
MSM8916 and later programming interfaces).

Add per-instance compatibles for the display, Venus and GPU MSM8974
IOMMU instances. The per-instance (rather than per-SoC) compatibles let
the driver carry the per-instance prefetch tuning tables and stream ID
maps, which are not derivable from the hardware. Document the extended
meaning of a missing qcom,iommu-secure-id and of reg on these instances.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/iommu/qcom,iommu.yaml | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml b/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml
index 93a489025317..43dffe41e3b8 100644
--- a/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml
@@ -30,6 +30,10 @@ properties:
               - qcom,msm8953-iommu
               - qcom,msm8976-iommu
           - const: qcom,msm-iommu-v2
+      - enum:
+          - qcom,msm8974-gpu-iommu
+          - qcom,msm8974-mdp-iommu
+          - qcom,msm8974-venus-iommu
 
   clocks:
     minItems: 2
@@ -50,13 +54,23 @@ properties:
 
   reg:
     maxItems: 1
+    description:
+      For instances using the qcom,msm-iommu-v1/-v2 fallbacks, the
+      optional "SMMU local" register region used for interrupt routing
+      configuration. For instances with per-instance compatibles, the
+      SMMU global register space, used for stream mapping configuration
+      on non-secured instances and for the implementation-defined
+      prefetch registers on all instances.
 
   ranges: true
 
   qcom,iommu-secure-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
-      The SCM secure ID of the IOMMU instance.
+      The SCM secure ID of the IOMMU instance. When absent on instances
+      with per-instance compatibles, the instance is not managed by the
+      secure world and the OS programs the whole global register space
+      (stream mapping included) directly.
 
   '#address-cells':
     const: 1

-- 
2.47.3

Re: [PATCH 01/12] dt-bindings: iommu: qcom,iommu: document the MSM8974 IOMMUs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing top-level 'interrupts' property and incorrect unconditional requirements for new direct-mapped instances.
- [Low] The schema fails to conditionally require the `reg` property for the newly added MSM8974 per-instance compatibles.
--

--- Patch [1]: [PATCH 01/12] dt-bindings: iommu: qcom,iommu: document the MSM8974 IOMMUs ---
commit b29444ad6a6fa61aa71980a7c581393333c88d6a
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

dt-bindings: iommu: qcom,iommu: document the MSM8974 IOMMUs

This patch adds per-instance compatibles for the display, Venus and GPU
MSM8974 IOMMU instances based on the ARM SMMU v1 spec. It allows the driver
to carry per-instance prefetch tuning tables and stream ID maps, and updates
property documentation for direct-mapped instances.

> diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml b/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml
> index 93a4890253170..43dffe41e3b8f 100644
> --- a/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.yaml
> @@ -30,6 +30,10 @@ properties:
>                - qcom,msm8953-iommu
>                - qcom,msm8976-iommu
>            - const: qcom,msm-iommu-v2
> +      - enum:
> +          - qcom,msm8974-gpu-iommu
> +          - qcom,msm8974-mdp-iommu
> +          - qcom,msm8974-venus-iommu

[Severity: Medium]
Will valid configurations for these direct-mapped instances fail schema
validation when qcom,iommu-secure-id is missing?

If the OS programs the global register space directly, statically assigned
iommu-ctx@ child nodes aren't used, requiring interrupts to be defined at the
top level. Because interrupts is not defined at the top level and child node
properties are unconditionally required, adding a node for these compatibles
without qcom,iommu-secure-id appears to fail dt_binding_check.

Could the global schema constraints be adjusted to allow top-level interrupts
and conditionally relax child-node requirements for these new non-secured
instances?

[ ... ]

> @@ -50,13 +54,23 @@ properties:
>  
>    reg:
>      maxItems: 1
> +    description:
> +      For instances using the qcom,msm-iommu-v1/-v2 fallbacks, the
> +      optional "SMMU local" register region used for interrupt routing
> +      configuration. For instances with per-instance compatibles, the
> +      SMMU global register space, used for stream mapping configuration
> +      on non-secured instances and for the implementation-defined
> +      prefetch registers on all instances.

[Severity: Low]
Is a conditional allOf block missing to enforce the presence of the reg
property for the new MSM8974 per-instance compatibles?

The description states that reg is used for the implementation-defined
prefetch registers on all instances with per-instance compatibles. Without
requiring it conditionally, incomplete device tree nodes missing the reg
property will silently pass validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=1

[PATCH 02/12] iommu: arm-smmu: add global register definitions used by the QSMMU

[Dmitry Baryshkov]

The MSM8974 QSMMU support about to be added to qcom_iommu has to
program the SMMU global register space itself, and several of the
architectural registers and fields it needs are missing from
arm-smmu.h: the secure configuration and fault registers sCR2, sGFAR
and sGFSRRESTORE, the sCR0 SMCFCFG and STALLD fields and the S2CR
NSCFG and MEMATTR fields.

Add them, sorted by their register offset and bit positions.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/arm-smmu.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 26d2e33cd328..49424d8aca6b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -26,11 +26,13 @@
 /* Configuration registers */
 #define ARM_SMMU_GR0_sCR0		0x0
 #define ARM_SMMU_sCR0_VMID16EN		BIT(31)
+#define ARM_SMMU_sCR0_SMCFCFG		BIT(21)
 #define ARM_SMMU_sCR0_BSU		GENMASK(15, 14)
 #define ARM_SMMU_sCR0_FB		BIT(13)
 #define ARM_SMMU_sCR0_PTM		BIT(12)
 #define ARM_SMMU_sCR0_VMIDPNE		BIT(11)
 #define ARM_SMMU_sCR0_USFCFG		BIT(10)
+#define ARM_SMMU_sCR0_STALLD		BIT(8)
 #define ARM_SMMU_sCR0_GCFGFIE		BIT(5)
 #define ARM_SMMU_sCR0_GCFGFRE		BIT(4)
 #define ARM_SMMU_sCR0_EXIDENABLE	BIT(3)
@@ -38,6 +40,8 @@
 #define ARM_SMMU_sCR0_GFRE		BIT(1)
 #define ARM_SMMU_sCR0_CLIENTPD		BIT(0)
 
+#define ARM_SMMU_GR0_sCR2		0x8
+
 /* Auxiliary Configuration register */
 #define ARM_SMMU_GR0_sACR		0x10
 
@@ -80,9 +84,13 @@
 #define ARM_SMMU_ID7_MAJOR		GENMASK(7, 4)
 #define ARM_SMMU_ID7_MINOR		GENMASK(3, 0)
 
+#define ARM_SMMU_GR0_sGFAR		0x40
+
 #define ARM_SMMU_GR0_sGFSR		0x48
 #define ARM_SMMU_sGFSR_USF		BIT(1)
 
+#define ARM_SMMU_GR0_sGFSRRESTORE	0x4c
+
 #define ARM_SMMU_GR0_sGFSYNR0		0x50
 #define ARM_SMMU_GR0_sGFSYNR1		0x54
 #define ARM_SMMU_GR0_sGFSYNR2		0x58
@@ -110,12 +118,14 @@ enum arm_smmu_s2cr_privcfg {
 	S2CR_PRIVCFG_UNPRIV,
 	S2CR_PRIVCFG_PRIV,
 };
+#define ARM_SMMU_S2CR_NSCFG		GENMASK(19, 18)
 #define ARM_SMMU_S2CR_TYPE		GENMASK(17, 16)
 enum arm_smmu_s2cr_type {
 	S2CR_TYPE_TRANS,
 	S2CR_TYPE_BYPASS,
 	S2CR_TYPE_FAULT,
 };
+#define ARM_SMMU_S2CR_MEMATTR		GENMASK(15, 12)
 #define ARM_SMMU_S2CR_EXIDVALID		BIT(10)
 #define ARM_SMMU_S2CR_CBNDX		GENMASK(7, 0)
 

-- 
2.47.3

[PATCH 03/12] iommu: qcom_iommu: extract context bank programming into a helper

[Dmitry Baryshkov]

qcom_iommu_init_domain() programs the context bank registers inline,
with values computed on the spot, so no record of the programmed state
is kept. The MSM8974 QSMMU instances about to be supported sit in MMSS
GDSCs and lose the context bank state whenever the power domain
collapses, which requires the driver to reprogram the banks at runtime
resume.

Stash the computed register values in struct qcom_iommu_ctx and move
the programming sequence into a new qcom_iommu_program_ctx() helper
that can later be replayed from the resume path. No functional change.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 66 ++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 21d18ce67b98..5bbb51d9bd65 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -61,6 +61,11 @@ struct qcom_iommu_ctx {
 	bool			 secured_ctx;
 	u8			 asid;      /* asid and ctx bank # are 1:1 */
 	struct iommu_domain	*domain;
+	/* programmed context bank state */
+	u64			 ttbr0;
+	u32			 tcr[2];
+	u32			 mair[2];
+	u32			 sctlr;
 };
 
 struct qcom_iommu_domain {
@@ -213,6 +218,32 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
+				   struct qcom_iommu_ctx *ctx)
+{
+	/* Disable context bank before programming */
+	iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
+
+	/* Clear context bank fault address fault status registers */
+	iommu_writel(ctx, ARM_SMMU_CB_FAR, 0);
+	iommu_writel(ctx, ARM_SMMU_CB_FSR, ARM_SMMU_CB_FSR_FAULT);
+
+	/* TTBRs */
+	iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, ctx->ttbr0);
+	iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0);
+
+	/* TCR */
+	iommu_writel(ctx, ARM_SMMU_CB_TCR2, ctx->tcr[1]);
+	iommu_writel(ctx, ARM_SMMU_CB_TCR, ctx->tcr[0]);
+
+	/* MAIRs (stage-1 only) */
+	iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0, ctx->mair[0]);
+	iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR1, ctx->mair[1]);
+
+	/* SCTLR */
+	iommu_writel(ctx, ARM_SMMU_CB_SCTLR, ctx->sctlr);
+}
+
 static int qcom_iommu_init_domain(struct iommu_domain *domain,
 				  struct qcom_iommu_dev *qcom_iommu,
 				  struct device *dev)
@@ -267,32 +298,13 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 			continue;
 		}
 
-		/* Disable context bank before programming */
-		iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
+		ctx->ttbr0 = pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
+			     FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid);
+		ctx->tcr[0] = arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE;
+		ctx->tcr[1] = arm_smmu_lpae_tcr2(&pgtbl_cfg);
+		ctx->mair[0] = pgtbl_cfg.arm_lpae_s1_cfg.mair;
+		ctx->mair[1] = pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32;
 
-		/* Clear context bank fault address fault status registers */
-		iommu_writel(ctx, ARM_SMMU_CB_FAR, 0);
-		iommu_writel(ctx, ARM_SMMU_CB_FSR, ARM_SMMU_CB_FSR_FAULT);
-
-		/* TTBRs */
-		iommu_writeq(ctx, ARM_SMMU_CB_TTBR0,
-				pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
-				FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid));
-		iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0);
-
-		/* TCR */
-		iommu_writel(ctx, ARM_SMMU_CB_TCR2,
-				arm_smmu_lpae_tcr2(&pgtbl_cfg));
-		iommu_writel(ctx, ARM_SMMU_CB_TCR,
-			     arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE);
-
-		/* MAIRs (stage-1 only) */
-		iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0,
-				pgtbl_cfg.arm_lpae_s1_cfg.mair);
-		iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR1,
-				pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32);
-
-		/* SCTLR */
 		reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE |
 		      ARM_SMMU_SCTLR_AFE | ARM_SMMU_SCTLR_TRE |
 		      ARM_SMMU_SCTLR_M | ARM_SMMU_SCTLR_S1_ASIDPNE |
@@ -301,7 +313,9 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
 			reg |= ARM_SMMU_SCTLR_E;
 
-		iommu_writel(ctx, ARM_SMMU_CB_SCTLR, reg);
+		ctx->sctlr = reg;
+
+		qcom_iommu_program_ctx(qcom_iommu, ctx);
 
 		ctx->domain = domain;
 	}

-- 
2.47.3

Re: [PATCH 03/12] iommu: qcom_iommu: extract context bank programming into a helper

[sashiko-bot]

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

New issues:
- [Low] The `qcom_iommu` parameter in the new helper `qcom_iommu_program_ctx` is never used.

Pre-existing issues:
- [High] A use-after-free (UAF) condition exists on the error path of `qcom_iommu_init_domain` where a dangling pointer is left in `ctx->domain` and accessed by the IRQ handler.
--

--- Patch [3]: [PATCH 03/12] iommu: qcom_iommu: extract context bank programming into a helper ---
commit 7d60ff7e8f25f085f5b849ae6ddac1aa47c3809d
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Subject: iommu: qcom_iommu: extract context bank programming into a helper

This patch extracts the inline context bank programming sequence into a new
helper function qcom_iommu_program_ctx(). It also stores the computed
register values into the qcom_iommu_ctx struct. This allows the registers to
be reprogrammed upon runtime resume for MSM8974 QSMMU instances.

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 21d18ce67b982..5bbb51d9bd653 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c

[ ... ]

> @@ -213,6 +218,32 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  
> +static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
> +				   struct qcom_iommu_ctx *ctx)
> +{

[Severity: Low]
Is the qcom_iommu parameter needed here? It does not appear to be used
anywhere inside this helper function.

> +	/* Disable context bank before programming */
> +	iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);

[ ... ]

> @@ -301,7 +313,9 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
>  		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
>  			reg |= ARM_SMMU_SCTLR_E;
>  
> -		iommu_writel(ctx, ARM_SMMU_CB_SCTLR, reg);
> +		ctx->sctlr = reg;
> +
> +		qcom_iommu_program_ctx(qcom_iommu, ctx);
>  
>  		ctx->domain = domain;
>  	}

[Severity: High]
This is a pre-existing issue, but does the error path in
qcom_iommu_init_domain() handle partially initialized context banks correctly?

If an early iteration of the context bank setup succeeds, ctx->domain is set:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_init_domain() {
    ...
    qcom_iommu_program_ctx(qcom_iommu, ctx);
    ctx->domain = domain;
    ...
}

If a subsequent iteration fails, the function jumps to out_clear_iommu
but leaves ctx->domain pointing to the domain.

When the IOMMU core frees the domain, could a subsequent hardware fault
trigger a use-after-free when the IRQ handler dereferences it?

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_fault() {
    ...
    if (report_iommu_fault(ctx->domain, ctx->dev, iova, 0)) {
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=3

[PATCH 04/12] iommu: qcom_iommu: support the short-descriptor pagetable format

[Dmitry Baryshkov]

The MSM8974-generation QSMMU does not walk the LPAE tables this
driver hardcodes: every access faults with an access flag fault
despite AF being set in the descriptors, and with SCTLR.AFE cleared
the walks still return garbage translations (observed on the APQ8074
dragonboard GPU, which locks up on its first ringbuffer fetch). The
downstream kernel only ever used the short-descriptor format on
MSM8974; its LPAE support was reserved for the newer APQ8084 IP / TZ
revision.

Introduce a per-instance configuration structure, selected through
the of_device_id match data, carrying a pagetable format field and an
AFE quirk flag. For the ARM_V7S format, program the ASID through
CONTEXTIDR instead of the TTBR0 ASID field, reuse the MAIR register
offsets for PRRR/NMRR, restrict the output address size to 32 bits
and keep SCTLR.AFE disabled like the downstream driver does when the
no_afe flag is set. No instance sets a configuration yet.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/Kconfig               |  1 +
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 54 +++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/arm/Kconfig b/drivers/iommu/arm/Kconfig
index 5fac08b89dee..596e643961c3 100644
--- a/drivers/iommu/arm/Kconfig
+++ b/drivers/iommu/arm/Kconfig
@@ -138,6 +138,7 @@ config QCOM_IOMMU
 	select QCOM_SCM
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
+	select IOMMU_IO_PGTABLE_ARMV7S
 	select ARM_DMA_USE_IOMMU
 	help
 	  Support for IOMMU on certain Qualcomm SoCs.
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 5bbb51d9bd65..77c01f217d7e 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -43,10 +43,18 @@ enum qcom_iommu_clk {
 
 struct qcom_iommu_ctx;
 
+/* Per-instance configuration, absent on msm8916-style instances */
+struct qcom_iommu_cfg {
+	enum io_pgtable_fmt		 fmt;
+	/* the walker faults on the AF bit despite it being set */
+	bool				 no_afe;
+};
+
 struct qcom_iommu_dev {
 	/* IOMMU core code handle */
 	struct iommu_device	 iommu;
 	struct device		*dev;
+	const struct qcom_iommu_cfg *cfg;
 	struct clk_bulk_data clks[CLK_NUM];
 	void __iomem		*local_base;
 	u32			 sec_id;
@@ -54,6 +62,11 @@ struct qcom_iommu_dev {
 	struct qcom_iommu_ctx	*ctxs[];   /* indexed by asid */
 };
 
+static enum io_pgtable_fmt qcom_iommu_pgtbl_fmt(struct qcom_iommu_dev *qcom_iommu)
+{
+	return qcom_iommu->cfg ? qcom_iommu->cfg->fmt : ARM_32_LPAE_S1;
+}
+
 struct qcom_iommu_ctx {
 	struct device		*dev;
 	void __iomem		*base;
@@ -65,6 +78,7 @@ struct qcom_iommu_ctx {
 	u64			 ttbr0;
 	u32			 tcr[2];
 	u32			 mair[2];
+	u32			 contextidr;
 	u32			 sctlr;
 };
 
@@ -232,8 +246,11 @@ static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
 	iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, ctx->ttbr0);
 	iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0);
 
-	/* TCR */
-	iommu_writel(ctx, ARM_SMMU_CB_TCR2, ctx->tcr[1]);
+	/* TCR; the v7s ASID lives in CONTEXTIDR instead of TTBR0 */
+	if (qcom_iommu_pgtbl_fmt(qcom_iommu) == ARM_V7S)
+		iommu_writel(ctx, ARM_SMMU_CB_CONTEXTIDR, ctx->contextidr);
+	else
+		iommu_writel(ctx, ARM_SMMU_CB_TCR2, ctx->tcr[1]);
 	iommu_writel(ctx, ARM_SMMU_CB_TCR, ctx->tcr[0]);
 
 	/* MAIRs (stage-1 only) */
@@ -252,6 +269,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct io_pgtable_ops *pgtbl_ops;
 	struct io_pgtable_cfg pgtbl_cfg;
+	enum io_pgtable_fmt fmt;
 	int i, ret = 0;
 	u32 reg;
 
@@ -259,10 +277,12 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 	if (qcom_domain->iommu)
 		goto out_unlock;
 
+	fmt = qcom_iommu_pgtbl_fmt(qcom_iommu);
+
 	pgtbl_cfg = (struct io_pgtable_cfg) {
 		.pgsize_bitmap	= domain->pgsize_bitmap,
 		.ias		= 32,
-		.oas		= 40,
+		.oas		= fmt == ARM_V7S ? 32 : 40,
 		.tlb		= &qcom_flush_ops,
 		.iommu_dev	= qcom_iommu->dev,
 	};
@@ -270,7 +290,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 	qcom_domain->iommu = qcom_iommu;
 	qcom_domain->fwspec = fwspec;
 
-	pgtbl_ops = alloc_io_pgtable_ops(ARM_32_LPAE_S1, &pgtbl_cfg, qcom_domain);
+	pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, qcom_domain);
 	if (!pgtbl_ops) {
 		dev_err(qcom_iommu->dev, "failed to allocate pagetable ops\n");
 		ret = -ENOMEM;
@@ -298,12 +318,22 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 			continue;
 		}
 
-		ctx->ttbr0 = pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
-			     FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid);
-		ctx->tcr[0] = arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE;
-		ctx->tcr[1] = arm_smmu_lpae_tcr2(&pgtbl_cfg);
-		ctx->mair[0] = pgtbl_cfg.arm_lpae_s1_cfg.mair;
-		ctx->mair[1] = pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32;
+		if (fmt == ARM_V7S) {
+			ctx->ttbr0 = pgtbl_cfg.arm_v7s_cfg.ttbr;
+			ctx->tcr[0] = pgtbl_cfg.arm_v7s_cfg.tcr;
+			ctx->tcr[1] = 0;
+			/* PRRR/NMRR share the MAIR0/MAIR1 offsets */
+			ctx->mair[0] = pgtbl_cfg.arm_v7s_cfg.prrr;
+			ctx->mair[1] = pgtbl_cfg.arm_v7s_cfg.nmrr;
+			ctx->contextidr = ctx->asid;
+		} else {
+			ctx->ttbr0 = pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
+				     FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid);
+			ctx->tcr[0] = arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE;
+			ctx->tcr[1] = arm_smmu_lpae_tcr2(&pgtbl_cfg);
+			ctx->mair[0] = pgtbl_cfg.arm_lpae_s1_cfg.mair;
+			ctx->mair[1] = pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32;
+		}
 
 		reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE |
 		      ARM_SMMU_SCTLR_AFE | ARM_SMMU_SCTLR_TRE |
@@ -313,6 +343,9 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 		if (IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
 			reg |= ARM_SMMU_SCTLR_E;
 
+		if (qcom_iommu->cfg && qcom_iommu->cfg->no_afe)
+			reg &= ~ARM_SMMU_SCTLR_AFE;
+
 		ctx->sctlr = reg;
 
 		qcom_iommu_program_ctx(qcom_iommu, ctx);
@@ -808,6 +841,7 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	qcom_iommu->max_asid = max_asid;
 	qcom_iommu->dev = dev;
+	qcom_iommu->cfg = of_device_get_match_data(dev);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res) {

-- 
2.47.3

Re: [PATCH 04/12] iommu: qcom_iommu: support the short-descriptor pagetable format

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing rollback in qcom_iommu_init_domain leaves dangling ctx->domain pointers and active contexts, causing UAF in the fault handler.
- [High] Missing reset of `iova` base address in TLB invalidation loop causes missed TLB flushes for subsequent context banks.
- [High] Unprogrammed contexts cause NULL pointer dereference in fault handler.
--

--- Patch [4]: [PATCH 04/12] iommu: qcom_iommu: support the short-descriptor pagetable format ---
commit 058d015d111c3926c8cd1da34f2ceba59b5078db
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

iommu: qcom_iommu: support the short-descriptor pagetable format
    
The MSM8974-generation QSMMU does not walk the LPAE tables this
driver hardcodes: every access faults with an access flag fault
despite AF being set in the descriptors.

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 5bbb51d9bd653..77c01f217d7ea 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c

[ ... ]

> @@ -298,12 +318,22 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
>  			continue;
>  		}
>  
> -		ctx->ttbr0 = pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
> -			     FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid);
> -		ctx->tcr[0] = arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE;
> -		ctx->tcr[1] = arm_smmu_lpae_tcr2(&pgtbl_cfg);
> -		ctx->mair[0] = pgtbl_cfg.arm_lpae_s1_cfg.mair;
> -		ctx->mair[1] = pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32;
> +		if (fmt == ARM_V7S) {
> +			ctx->ttbr0 = pgtbl_cfg.arm_v7s_cfg.ttbr;
> +			ctx->tcr[0] = pgtbl_cfg.arm_v7s_cfg.tcr;
> +			ctx->tcr[1] = 0;
> +			/* PRRR/NMRR share the MAIR0/MAIR1 offsets */
> +			ctx->mair[0] = pgtbl_cfg.arm_v7s_cfg.prrr;
> +			ctx->mair[1] = pgtbl_cfg.arm_v7s_cfg.nmrr;
> +			ctx->contextidr = ctx->asid;
> +		} else {
> +			ctx->ttbr0 = pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
> +				     FIELD_PREP(ARM_SMMU_TTBRn_ASID, ctx->asid);
> +			ctx->tcr[0] = arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE;
> +			ctx->tcr[1] = arm_smmu_lpae_tcr2(&pgtbl_cfg);
> +			ctx->mair[0] = pgtbl_cfg.arm_lpae_s1_cfg.mair;
> +			ctx->mair[1] = pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32;
> +		}
>  
>  		reg = ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE |
>  		      ARM_SMMU_SCTLR_AFE | ARM_SMMU_SCTLR_TRE |

[Severity: High]
This isn't a bug introduced by this patch, but does qcom_iommu_init_domain()
properly roll back if a later context bank fails to initialize?

If qcom_scm_restore_sec_cfg() fails for a later context bank, it jumps to the
out_clear_iommu error path without un-programming the earlier context banks
or resetting their ctx->domain pointers to NULL:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_init_domain() {
...
			ret = qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, ctx->asid);
			if (ret) {
				dev_err(qcom_iommu->dev, "secure init failed: %d\n", ret);
				goto out_clear_iommu;
			}
...
}

If a hardware fault subsequently occurs on one of the still-active context
banks, would qcom_iommu_fault() pass the dangling ctx->domain pointer to
report_iommu_fault(), potentially causing a use-after-free?

[Severity: High]
This is a pre-existing issue, but does qcom_iommu_tlb_inv_range_nosync() miss
TLB flushes for subsequent context banks?

In qcom_iommu_tlb_inv_range_nosync(), when there are multiple context banks,
the iova variable is incremented in the inner loop but isn't reset for the
next iteration of the outer loop:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_tlb_inv_range_nosync() {
...
		iova = (iova >> 12) << 12;
		iova |= ctx->asid;
		do {
			iommu_writel(ctx, reg, iova);
			iova += granule;
		} while (s -= granule);
...
}

Could this heavily incremented iova value cause missed TLB flushes for
subsequent context banks, leading to stale TLB translations and incorrect
memory accesses?

[Severity: High]
This is also a pre-existing issue, but can unprogrammed contexts cause a NULL
pointer dereference in the fault handler?

During qcom_iommu_ctx_probe(), if a bootloader left the context active, a DMA
fault can fire before qcom_iommu_attach_dev() initializes the context and sets
ctx->domain. If this occurs, qcom_iommu_fault() passes a NULL ctx->domain to
report_iommu_fault():

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_fault() {
...
	if (report_iommu_fault(ctx->domain, ctx->dev, iova, 0)) {
...
}

Which then dereferences the domain pointer without checking if it is NULL:

drivers/iommu/iommu.c:report_iommu_fault() {
...
	if (domain->cookie_type == IOMMU_COOKIE_FAULT_HANDLER &&
	    domain->handler)
...
}

Would this lead to a kernel panic from a NULL pointer dereference during
device initialization?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=4

[PATCH 05/12] iommu: qcom_iommu: handle the SMMU global register space

[Dmitry Baryshkov]

On msm8974 the OS has to access the SMMU global register space: the
non-secured instances need it fully programmed and even the TZ-managed
ones expose OS-writable implementation-defined registers in it. The
driver only knows about the msm8916-style "SMMU local" region in
resource 0, and has no accessors for the global space.

For instances with a per-instance configuration, treat resource 0 as
the global register space and require it. Never perform the
SMMU_INTR_SEL_NS write on such instances - offset 0x2000 from the global
base is the implementation-defined MICRO_MMU_CTRL halt request register
on this IP, not the interrupt select of the separate msm8916-style local
region. Provide gr0/gr1 accessors for the following changes.

Do not clear the FSR of the context banks at probe time on such
instances either: they sit in MMSS power domains which may well be off
at that point, and an access to an unpowered register block stalls the
bus. Skipping the clear is safe and preferable to powering the domain
up for it. Safe, because a collapsed context bank has no active logic:
it can neither latch a fault nor assert its interrupt, so the window
the probe-time clear protects against on msm8916 does not exist while
the domain is off, and the first runtime resume fully reprograms the
context bank - including the FSR - before translation is enabled, so
any fault state the bootloader left behind is wiped before it could
matter. Preferable, because taking a runtime PM reference just for the
clear would be a power cycle with no lasting effect (the cleared state
is lost again when the domain collapses after the put), and the
get/put pair is actively harmful for the MDP instance: at IOMMU probe
time the display is still scanning out from bootloader-configured
state with no kernel driver holding the MDSS GDSC, so the final put
would power the scanout domain off mid-boot. The same treatment
already exists for the TZ-secured contexts, which skip the clear for
banks the OS cannot reach.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 44 +++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 77c01f217d7e..428b18697244 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -34,6 +34,9 @@
 
 #define SMMU_INTR_SEL_NS     0x2000
 
+/* GR1 sits one 4K page above GR0 on the msm8974 QSMMU */
+#define QCOM_IOMMU_GR1			0x1000
+
 enum qcom_iommu_clk {
 	CLK_IFACE,
 	CLK_BUS,
@@ -57,6 +60,7 @@ struct qcom_iommu_dev {
 	const struct qcom_iommu_cfg *cfg;
 	struct clk_bulk_data clks[CLK_NUM];
 	void __iomem		*local_base;
+	void __iomem		*global_base;
 	u32			 sec_id;
 	u8			 max_asid;
 	struct qcom_iommu_ctx	*ctxs[];   /* indexed by asid */
@@ -130,6 +134,26 @@ iommu_readq(struct qcom_iommu_ctx *ctx, unsigned reg)
 	return readq_relaxed(ctx->base + reg);
 }
 
+static inline void
+qcom_iommu_gr0_write(struct qcom_iommu_dev *qcom_iommu, unsigned int reg,
+		      u32 val)
+{
+	writel_relaxed(val, qcom_iommu->global_base + reg);
+}
+
+static inline u32
+qcom_iommu_gr0_read(struct qcom_iommu_dev *qcom_iommu, unsigned int reg)
+{
+	return readl_relaxed(qcom_iommu->global_base + reg);
+}
+
+static inline void
+qcom_iommu_gr1_write(struct qcom_iommu_dev *qcom_iommu, unsigned int reg,
+		      u32 val)
+{
+	writel_relaxed(val, qcom_iommu->global_base + QCOM_IOMMU_GR1 + reg);
+}
+
 static void qcom_iommu_tlb_sync(void *cookie)
 {
 	struct qcom_iommu_domain *qcom_domain = cookie;
@@ -749,9 +773,10 @@ static int qcom_iommu_ctx_probe(struct platform_device *pdev)
 		ctx->secured_ctx = true;
 
 	/* clear IRQs before registering fault handler, just in case the
-	 * boot-loader left us a surprise:
+	 * boot-loader left us a surprise.  Instances with a power domain
+	 * may not be accessible yet; they are reset at first resume.
 	 */
-	if (!ctx->secured_ctx) {
+	if (!ctx->secured_ctx && !qcom_iommu->cfg) {
 		ret = pm_runtime_resume_and_get(dev->parent);
 		if (ret)
 			return ret;
@@ -845,9 +870,18 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res) {
-		qcom_iommu->local_base = devm_ioremap_resource(dev, res);
-		if (IS_ERR(qcom_iommu->local_base))
-			return PTR_ERR(qcom_iommu->local_base);
+		if (qcom_iommu->cfg) {
+			qcom_iommu->global_base = devm_ioremap_resource(dev, res);
+			if (IS_ERR(qcom_iommu->global_base))
+				return PTR_ERR(qcom_iommu->global_base);
+		} else {
+			qcom_iommu->local_base = devm_ioremap_resource(dev, res);
+			if (IS_ERR(qcom_iommu->local_base))
+				return PTR_ERR(qcom_iommu->local_base);
+		}
+	} else if (qcom_iommu->cfg) {
+		return dev_err_probe(dev, -EINVAL,
+				     "missing global register space\n");
 	}
 
 	clk = devm_clk_get(dev, "iface");

-- 
2.47.3

Re: [PATCH 05/12] iommu: qcom_iommu: handle the SMMU global register space

[sashiko-bot]

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

New issues:
- [High] Context bank state is not reprogrammed upon runtime resume, leading to translation failures for instances in power domains.
- [High] Skipping the FSR clear at probe time allows a pending bootloader fault to trigger an interrupt before `ctx->domain` is set, causing a NULL pointer dereference.

Pre-existing issues:
- [High] The shared IRQ handler accesses hardware registers without checking if the device is active, and there is no IRQ synchronization in the suspend path.
- [High] Unbinding the parent IOMMU device causes a NULL pointer dereference in the child context device's remove callback.
--

--- Patch [5]: [PATCH 05/12] iommu: qcom_iommu: handle the SMMU global register space ---
commit bf26052ef6d0329129f71ba618d88c9f30a2725a
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

iommu: qcom_iommu: handle the SMMU global register space

On msm8974 the OS has to access the SMMU global register space: the
non-secured instances need it fully programmed and even the TZ-managed
ones expose OS-writable implementation-defined registers in it. The
driver only knows about the msm8916-style "SMMU local" region in
resource 0, and has no accessors for the global space.

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 77c01f217d7ea..428b18697244c 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c

[ ... ]

> @@ -213,6 +256,35 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
>  	return IRQ_HANDLED;
>  }
>  

[Severity: High]
This is a pre-existing issue, but shouldn't the shared IRQ handler check if
the device is active before accessing hardware registers?

The interrupt line is registered with IRQF_SHARED. If another device sharing
the line asserts an interrupt while the IOMMU's power domain is suspended,
qcom_iommu_fault() executes and accesses ARM_SMMU_CB_FSR without waking the
device via pm_runtime_get_if_active(). Accessing unpowered MMIO registers
can trigger an asynchronous external abort or bus stall.

Also, qcom_iommu_suspend() disables clocks without calling
synchronize_irq(), allowing race conditions if the shared handler is running
mid-flight.

> +static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
> +				   struct qcom_iommu_ctx *ctx)
> +{
> +	/* Disable context bank before programming */
> +	iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);

[Severity: High]
Is it necessary to reprogram this cached state during runtime resume?

When the PM domain collapses and subsequently resumes, the non-secure context
banks remain unconfigured, breaking any further DMA translations.

Looking at qcom_iommu_resume(), it restores secure configurations but
doesn't restore the OS-managed (non-secure) context banks by looping over
them and calling qcom_iommu_program_ctx().

[ ... ]

> @@ -749,9 +773,10 @@ static int qcom_iommu_ctx_probe(struct platform_device *pdev)
>  		ctx->secured_ctx = true;
>  
>  	/* clear IRQs before registering fault handler, just in case the
> -	 * boot-loader left us a surprise:
> +	 * boot-loader left us a surprise.  Instances with a power domain
> +	 * may not be accessible yet; they are reset at first resume.
>  	 */
> -	if (!ctx->secured_ctx) {
> +	if (!ctx->secured_ctx && !qcom_iommu->cfg) {

[Severity: High]
If a bootloader fault is pending and we bypass clearing the Fault Status
Register (FSR) here, could it trigger an interrupt before ctx->domain is set?

When devm_request_irq() enables the shared IRQ line a few lines later, the
pending interrupt could fire immediately. The handler qcom_iommu_fault()
executes and calls report_iommu_fault(ctx->domain, ...).

Because ctx->domain is only assigned later during device attachment
(qcom_iommu_init_domain), it is currently NULL, which report_iommu_fault()
will dereference.

>  		ret = pm_runtime_resume_and_get(dev->parent);
>  		if (ret)
>  			return ret;

[ ... ]

[Severity: High]
This is a pre-existing issue, but can unbinding the parent IOMMU device cause
a NULL pointer dereference in the child context device's remove callback?

When the parent driver is unbound, qcom_iommu_device_remove() sets its private
data to NULL via platform_set_drvdata(pdev, NULL). Subsequently, devres tears
down the child devices, invoking qcom_iommu_ctx_remove().

This child callback calls dev_get_drvdata(pdev->dev.parent) to retrieve the
parent's data, which is now NULL, and immediately dereferences it:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_ctx_remove() {
    ...
    struct qcom_iommu_dev *qcom_iommu = dev_get_drvdata(pdev->dev.parent);
    ...
    qcom_iommu->ctxs[ctx->asid] = NULL;
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=5

[PATCH 06/12] iommu: qcom_iommu: support non-TZ-managed instances

[Dmitry Baryshkov]

The GPU on MSM8974 has never been usable upstream because its IOMMU
cannot be driven: unlike the MSM8916-style instances this driver
supports, where the secure world owns the SMMU global register space
and the OS programs just the context banks after a restore_sec_cfg SCM
call, the MSM8974 GPU IOMMU is not managed by TZ at all. Nobody sets
up its stream mapping, so attaching a domain leaves every transaction
unmatched and the GPU faults on its first memory access. Verified on
the APQ8074 dragonboard: restore_sec_cfg is only accepted for the
secured instances, while the GPU instance exposes its ID registers to
the OS (IDR0 0x78034404 / IDR1 0x10000003).

Add a non-secure programming mode, derived from the downstream
msm_iommu-v1.c driver. When qcom,iommu-secure-id is absent, reset and
program the global space at runtime resume instead of calling into TZ:
clear sACR/sCR2 and the global fault state, invalidate the TLB and all
SMRs, install the per-instance stream mapping (SMR/S2CR/CBAR, with the
downstream MEMATTR/NSCFG/VMID/IRPTNDX values) and enable the SMMU with
fault reporting enabled and stalling disabled (sCR0.STALLD, matching
the downstream terminate model). The stream ID map is per-instance
configuration data since it is not discoverable from the hardware; the
SMR index allocation keeps ASID == context bank index, which this
hardware requires (downstream msm-3.10 commit 4d92ea10aac3 ("iommu:
msm: Use CB index as ASID")). Also set CB_ACTLR to redirect cacheable
requests to the L2 slave port, as the downstream driver does for every
context on this IP generation - on MSM8916 TZ owns CB_ACTLR, so the
driver never needed to touch it before.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 107 ++++++++++++++++++++++++++++++--
 1 file changed, 101 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 428b18697244..3b07c4cf23fe 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -37,6 +37,9 @@
 /* GR1 sits one 4K page above GR0 on the msm8974 QSMMU */
 #define QCOM_IOMMU_GR1			0x1000
 
+/* Redirect all cacheable requests to the L2 slave port */
+#define QCOM_IOMMU_ACTLR_BPRC		(BIT(28) | BIT(29) | BIT(30))
+
 enum qcom_iommu_clk {
 	CLK_IFACE,
 	CLK_BUS,
@@ -46,11 +49,23 @@ enum qcom_iommu_clk {
 
 struct qcom_iommu_ctx;
 
-/* Per-instance configuration, absent on msm8916-style instances */
+struct qcom_iommu_sid {
+	u8			 cbndx;
+	u8			 sid;
+};
+
+/*
+ * Per-instance configuration for instances whose global register space
+ * is at least partially OS-managed (reg points at the global space and
+ * SMMU_INTR_SEL_NS must not be written). The stream ID map is only used
+ * on non-secured instances.
+ */
 struct qcom_iommu_cfg {
 	enum io_pgtable_fmt		 fmt;
 	/* the walker faults on the AF bit despite it being set */
 	bool				 no_afe;
+	const struct qcom_iommu_sid	*sids;	/* one SMR slot per entry */
+	unsigned int			 num_sids;
 };
 
 struct qcom_iommu_dev {
@@ -62,6 +77,7 @@ struct qcom_iommu_dev {
 	void __iomem		*local_base;
 	void __iomem		*global_base;
 	u32			 sec_id;
+	bool			 non_secure;
 	u8			 max_asid;
 	struct qcom_iommu_ctx	*ctxs[];   /* indexed by asid */
 };
@@ -256,6 +272,71 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+/*
+ * Reset and configure the global register space of an instance the
+ * secure world does not manage: global fault state, TLB, stream
+ * mapping (SMR/S2CR/CBAR) and the global configuration register.
+ */
+static int qcom_iommu_reset_ns(struct qcom_iommu_dev *qcom_iommu)
+{
+	const struct qcom_iommu_cfg *cfg = qcom_iommu->cfg;
+	unsigned int i, num_smr;
+	u32 reg;
+	int ret;
+
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_sACR, 0);
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_sCR2, 0);
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_sGFAR, 0);
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_sGFAR + 4, 0);
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_sGFSRRESTORE, 0);
+
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_TLBIALLNSNH, 0);
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_sTLBGSYNC, 0);
+	ret = read_poll_timeout(qcom_iommu_gr0_read, reg,
+				!(reg & ARM_SMMU_sTLBGSTATUS_GSACTIVE),
+				0, 5000000, false,
+				qcom_iommu, ARM_SMMU_GR0_sTLBGSTATUS);
+	if (ret) {
+		dev_err(qcom_iommu->dev,
+			"timeout waiting for global TLB SYNC\n");
+		return ret;
+	}
+
+	num_smr = FIELD_GET(ARM_SMMU_ID0_NUMSMRG,
+			    qcom_iommu_gr0_read(qcom_iommu, ARM_SMMU_GR0_ID0));
+	for (i = 0; i < num_smr; i++)
+		qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_SMR(i), 0);
+
+	for (i = 0; i < cfg->num_sids; i++) {
+		const struct qcom_iommu_sid *sid = &cfg->sids[i];
+
+		qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_SMR(i),
+				      ARM_SMMU_SMR_VALID |
+				      FIELD_PREP(ARM_SMMU_SMR_ID, sid->sid));
+		qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_S2CR(i),
+				      FIELD_PREP(ARM_SMMU_S2CR_TYPE, S2CR_TYPE_TRANS) |
+				      FIELD_PREP(ARM_SMMU_S2CR_CBNDX, sid->cbndx) |
+				      FIELD_PREP(ARM_SMMU_S2CR_MEMATTR, 0xa) |
+				      FIELD_PREP(ARM_SMMU_S2CR_NSCFG, 3));
+		qcom_iommu_gr1_write(qcom_iommu,
+				      ARM_SMMU_GR1_CBAR(sid->cbndx),
+				      FIELD_PREP(ARM_SMMU_CBAR_TYPE,
+						 CBAR_TYPE_S1_TRANS_S2_BYPASS) |
+				      FIELD_PREP(ARM_SMMU_CBAR_IRPTNDX, 1) |
+				      FIELD_PREP(ARM_SMMU_CBAR_VMID, 3) |
+				      FIELD_PREP(ARM_SMMU_CBAR_S1_BPSHCFG, 2) |
+				      FIELD_PREP(ARM_SMMU_CBAR_S1_MEMATTR, 0xa));
+	}
+
+	qcom_iommu_gr0_write(qcom_iommu, ARM_SMMU_GR0_sCR0,
+			      ARM_SMMU_sCR0_SMCFCFG | ARM_SMMU_sCR0_USFCFG |
+			      ARM_SMMU_sCR0_STALLD | ARM_SMMU_sCR0_GCFGFIE |
+			      ARM_SMMU_sCR0_GCFGFRE | ARM_SMMU_sCR0_GFIE |
+			      ARM_SMMU_sCR0_GFRE);
+
+	return 0;
+}
+
 static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
 				   struct qcom_iommu_ctx *ctx)
 {
@@ -266,6 +347,9 @@ static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
 	iommu_writel(ctx, ARM_SMMU_CB_FAR, 0);
 	iommu_writel(ctx, ARM_SMMU_CB_FSR, ARM_SMMU_CB_FSR_FAULT);
 
+	if (qcom_iommu->cfg)
+		iommu_writel(ctx, ARM_SMMU_CB_ACTLR, QCOM_IOMMU_ACTLR_BPRC);
+
 	/* TTBRs */
 	iommu_writeq(ctx, ARM_SMMU_CB_TTBR0, ctx->ttbr0);
 	iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0);
@@ -327,7 +411,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 	for (i = 0; i < fwspec->num_ids; i++) {
 		struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]);
 
-		if (!ctx->secure_init) {
+		if (!qcom_iommu->non_secure && !ctx->secure_init) {
 			ret = qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, ctx->asid);
 			if (ret) {
 				dev_err(qcom_iommu->dev, "secure init failed: %d\n", ret);
@@ -907,8 +991,12 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
 
 	if (of_property_read_u32(dev->of_node, "qcom,iommu-secure-id",
 				 &qcom_iommu->sec_id)) {
-		dev_err(dev, "missing qcom,iommu-secure-id property\n");
-		return -ENODEV;
+		if (!qcom_iommu->cfg) {
+			dev_err(dev, "missing qcom,iommu-secure-id property\n");
+			return -ENODEV;
+		}
+		/* The secure world does not manage this instance at all */
+		qcom_iommu->non_secure = true;
 	}
 
 	if (qcom_iommu_has_secure_context(qcom_iommu)) {
@@ -981,8 +1069,15 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
 	if (ret < 0)
 		return ret;
 
-	if (dev->pm_domain)
-		return qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, 0);
+	if (qcom_iommu->non_secure) {
+		ret = qcom_iommu_reset_ns(qcom_iommu);
+		if (ret)
+			return ret;
+	} else if (dev->pm_domain) {
+		ret = qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, 0);
+		if (ret)
+			return ret;
+	}
 
 	return ret;
 }

-- 
2.47.3

Re: [PATCH 06/12] iommu: qcom_iommu: support non-TZ-managed instances

[sashiko-bot]

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

New issues:
- [High] Missing configuration data in `qcom_iommu_of_match` makes the non-secure code path unreachable.
- [High] Context bank registers are not restored during runtime resume for non-secure IOMMU instances.

Pre-existing issues:
- [High] Dangling pointer and potential UAF when `qcom_iommu_init_domain()` fails halfway through context bank initialization.
- [High] The shared interrupt handler `qcom_iommu_fault` can be invoked while the device is suspended or before a domain is attached, leading to unclocked register accesses and a NULL pointer dereference.
- [High] `qcom_iommu_unmap` calls `pm_runtime_get_sync`, which sleeps, but IOMMU unmap operations can be invoked from atomic context.
--

--- Patch [6]: [PATCH 06/12] iommu: qcom_iommu: support non-TZ-managed instances ---
commit e923920b01d0c5363e8c876cd9db3360e29f7e5a
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
iommu: qcom_iommu: support non-TZ-managed instances

This patch adds non-secure programming mode support for MSM8974 GPU IOMMU
instances that are not managed by TZ. It configures the global space at
runtime resume, clears global faults, and sets up stream mapping instead of
relying on secure calls.

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 428b18697244c..3b07c4cf23fe1 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
[ ... ]
> @@ -327,7 +411,7 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
>  	for (i = 0; i < fwspec->num_ids; i++) {
>  		struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain, fwspec->ids[i]);
>  
> -		if (!ctx->secure_init) {
> +		if (!qcom_iommu->non_secure && !ctx->secure_init) {

[Severity: High]
This is a pre-existing issue, but is there a dangling pointer and potential
use-after-free when qcom_iommu_init_domain() fails halfway through context
bank initialization?

If context bank initialization fails (for example, qcom_scm_restore_sec_cfg()
fails on a subsequent iteration), the function returns an error without
clearing ctx->domain for the already processed context banks. The IOMMU core
then frees the domain memory.

If a hardware fault interrupt fires, will qcom_iommu_fault() dereference the
freed ctx->domain?

>  			ret = qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, ctx->asid);
>  			if (ret) {
>  				dev_err(qcom_iommu->dev, "secure init failed: %d\n", ret);
[ ... ]
> @@ -907,8 +991,12 @@ static int qcom_iommu_device_probe(struct platform_device *pdev)
>  
>  	if (of_property_read_u32(dev->of_node, "qcom,iommu-secure-id",
>  				 &qcom_iommu->sec_id)) {
> -		dev_err(dev, "missing qcom,iommu-secure-id property\n");
> -		return -ENODEV;
> +		if (!qcom_iommu->cfg) {
> +			dev_err(dev, "missing qcom,iommu-secure-id property\n");
> +			return -ENODEV;
> +		}

[Severity: High]
Does the missing configuration data in qcom_iommu_of_match make this
non-secure code path unreachable?

During device probe, of_device_get_match_data() returns NULL because the newly
introduced compatibles (like qcom,msm8974-gpu-iommu) and their struct
qcom_iommu_cfg data were not added to the qcom_iommu_of_match array in this
patch.

This causes qcom_iommu->cfg to evaluate to NULL, unconditionally failing
with -ENODEV here when probing a non-secure instance.

> +		/* The secure world does not manage this instance at all */
> +		qcom_iommu->non_secure = true;
>  	}
>  
>  	if (qcom_iommu_has_secure_context(qcom_iommu)) {
[ ... ]
> @@ -981,8 +1069,15 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
>  	if (ret < 0)
>  		return ret;
>  
> -	if (dev->pm_domain)
> -		return qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, 0);
> +	if (qcom_iommu->non_secure) {
> +		ret = qcom_iommu_reset_ns(qcom_iommu);
> +		if (ret)
> +			return ret;

[Severity: High]
Are context bank registers restored during runtime resume for non-secure
IOMMU instances?

When the non-secure IOMMU is runtime resumed (e.g., after GDSC power
collapse), qcom_iommu_resume() restores the global registers via
qcom_iommu_reset_ns(), but it seems to completely omit iterating over and
reprogramming the attached context banks.

Since the context bank child devices lack PM operations, do they remain
disabled (SCTLR = 0) after resume, causing translation faults for all
non-secure instances?

> +	} else if (dev->pm_domain) {
> +		ret = qcom_scm_restore_sec_cfg(qcom_iommu->sec_id, 0);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but can the shared interrupt handler
qcom_iommu_fault() be invoked while the device is suspended or before a
domain is attached, leading to unclocked register accesses and a NULL
pointer dereference?

In qcom_iommu_ctx_probe(), a shared IRQ is requested before a domain is
attached:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_ctx_probe() {
    ...
	ret = devm_request_irq(dev, irq,
			       qcom_iommu_fault,
			       IRQF_SHARED,
    ...
}

If another device sharing the IRQ fires, qcom_iommu_fault() executes and
accesses ARM_SMMU_CB_FSR. If the device is unclocked, this crashes the bus.

If clocked and a bootloader fault is pending, it dereferences a NULL
ctx->domain. The target patch seems to explicitly bypass clearing pending
bootloader faults for non-secure instances when qcom_iommu->cfg is present:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_ctx_probe() {
    ...
	if (!ctx->secured_ctx && !qcom_iommu->cfg) {
		ret = pm_runtime_resume_and_get(dev->parent);
    ...
}

Will this ensure the crash triggers immediately upon shared IRQ firing?

[Severity: High]
This is a pre-existing issue, but does qcom_iommu_unmap() call
pm_runtime_get_sync() which sleeps, even though IOMMU unmap operations can
be invoked from atomic context?

The DMA API invokes the IOMMU unmap operation from an atomic context
(e.g., interrupt handlers):

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_unmap() {
    ...
	pm_runtime_get_sync(qcom_domain->iommu->dev);
	spin_lock_irqsave(&qcom_domain->pgtbl_lock, flags);
    ...
}

Because the device is not marked with pm_runtime_irq_safe(),
pm_runtime_get_sync() acquires a mutex and sleeps. Can this lead to a "BUG:
sleeping function called from invalid context" panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=6

[PATCH 07/12] iommu: qcom_iommu: restore context bank state after power collapse

[Dmitry Baryshkov]

The MSM8974 QSMMU instances sit in MMSS GDSCs: whenever the power
domain collapses, the SMMU loses the global and context bank state the
driver programmed, and the next master transaction faults or bypasses
translation.

Cache the computed context bank register values at attach time and
replay the global reset, stream mapping and context bank programming
on runtime resume, for instances that opt in through the new
ctx_restore configuration flag.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 3b07c4cf23fe..8e9488c47d5c 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -64,6 +64,8 @@ struct qcom_iommu_cfg {
 	enum io_pgtable_fmt		 fmt;
 	/* the walker faults on the AF bit despite it being set */
 	bool				 no_afe;
+	/* context banks lose their state over GDSC power collapse */
+	bool				 ctx_restore;
 	const struct qcom_iommu_sid	*sids;	/* one SMR slot per entry */
 	unsigned int			 num_sids;
 };
@@ -94,7 +96,7 @@ struct qcom_iommu_ctx {
 	bool			 secured_ctx;
 	u8			 asid;      /* asid and ctx bank # are 1:1 */
 	struct iommu_domain	*domain;
-	/* programmed context bank state */
+	/* CB state replayed after power collapse on OS-managed instances */
 	u64			 ttbr0;
 	u32			 tcr[2];
 	u32			 mair[2];
@@ -1063,6 +1065,7 @@ static void qcom_iommu_device_remove(struct platform_device *pdev)
 static int __maybe_unused qcom_iommu_resume(struct device *dev)
 {
 	struct qcom_iommu_dev *qcom_iommu = dev_get_drvdata(dev);
+	unsigned int i;
 	int ret;
 
 	ret = clk_bulk_prepare_enable(CLK_NUM, qcom_iommu->clks);
@@ -1079,6 +1082,16 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
 			return ret;
 	}
 
+	if (qcom_iommu->cfg && qcom_iommu->cfg->ctx_restore) {
+		/* Restore context banks lost over power collapse */
+		for (i = 0; i <= qcom_iommu->max_asid; i++) {
+			struct qcom_iommu_ctx *ctx = qcom_iommu->ctxs[i];
+
+			if (ctx && ctx->domain && !ctx->secured_ctx)
+				qcom_iommu_program_ctx(qcom_iommu, ctx);
+		}
+	}
+
 	return ret;
 }
 

-- 
2.47.3

Re: [PATCH 07/12] iommu: qcom_iommu: restore context bank state after power collapse

[sashiko-bot]

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

Pre-existing issues:
- [High] Use-After-Free and hardware page table corruption due to dangling `ctx->domain` on initialization error path.
--

--- Patch [7]: [PATCH 07/12] iommu: qcom_iommu: restore context bank state after power collapse ---
commit 4c924588306637fe44ecea534620e5e790b49aa8
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

iommu: qcom_iommu: restore context bank state after power collapse

This commit caches the computed context bank register values at attach
time and replays the global reset, stream mapping and context bank
programming on runtime resume. Instances opt in through the new
ctx_restore configuration flag.

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 3b07c4cf23fe1..8e9488c47d5c1 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
[ ... ]
> @@ -1079,6 +1082,16 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
>  			return ret;
>  	}
>  
> +	if (qcom_iommu->cfg && qcom_iommu->cfg->ctx_restore) {
> +		/* Restore context banks lost over power collapse */
> +		for (i = 0; i <= qcom_iommu->max_asid; i++) {
> +			struct qcom_iommu_ctx *ctx = qcom_iommu->ctxs[i];
> +
> +			if (ctx && ctx->domain && !ctx->secured_ctx)
> +				qcom_iommu_program_ctx(qcom_iommu, ctx);

[Severity: High]
This is a pre-existing issue, but could this new resume logic trigger a
use-after-free and corrupt hardware page tables if initialization previously
failed?

In drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_init_domain(), the error
path frees the allocated page tables but fails to clear the ctx->domain pointer
for context banks that were successfully processed before the error occurred:

out_clear_iommu:
	free_io_pgtable_ops(pgtbl_ops);
	qcom_domain->iommu = NULL;

If a device with multiple context banks fails initialization midway (for example,
during a firmware SCM call for a subsequent bank), the error path leaves
ctx->domain dangling and the page tables freed.

Upon the next PM runtime resume, this new loop in qcom_iommu_resume() sees the
truthy ctx->domain and programs the hardware with the stale ctx->ttbr0, causing
the IOMMU to use freed memory for DMA translation page tables.

> +		}
> +	}
> +
>  	return ret;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=7

[PATCH 08/12] iommu: qcom_iommu: halt the micro-MMU while programming context banks

[Dmitry Baryshkov]

The context bank registers are reprogrammed underneath a live translation
front-end. This happens at attach time, and again on every runtime resume
for instances that have to replay their context bank state after power
collapse. Rewriting SCTLR, TTBR0, TCR and the MAIRs while the micro-MMU
keeps accepting client transactions races the in-flight traffic against
the new configuration.

The hardware provides a handshake for exactly this: setting HALT_REQ in
MICRO_MMU_CTRL, at the start of the implementation-defined register
space, stops new transactions from being accepted and reports IDLE once
the outstanding ones have retired. Downstream drives it through a
qcom,iommu-enable-halt property.

Add the handshake, under a per-instance flag as the handshake is not
wanted on every instance, and bound the wait rather than spinning
forever, so that an unresponsive front-end degrades to a diagnostic
instead of a hang.

The halt is confined to context bank programming. Applying it around the
global reset as well resets the SoC before the console comes up,
reproducibly, on an APQ8074 DragonBoard.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 44 +++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 8e9488c47d5c..f27f8722ee64 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -37,6 +37,14 @@
 /* GR1 sits one 4K page above GR0 on the msm8974 QSMMU */
 #define QCOM_IOMMU_GR1			0x1000
 
+/* The implementation-defined space sits two 4K pages above GR0 */
+#define QCOM_IOMMU_IMPL_DEF		0x2000
+
+/* Micro-MMU control, at the start of the implementation-defined space */
+#define QCOM_IOMMU_MICRO_MMU_CTRL	(QCOM_IOMMU_IMPL_DEF + 0x000)
+#define MICRO_MMU_CTRL_HALT_REQ		BIT(2)
+#define MICRO_MMU_CTRL_IDLE		BIT(3)
+
 /* Redirect all cacheable requests to the L2 slave port */
 #define QCOM_IOMMU_ACTLR_BPRC		(BIT(28) | BIT(29) | BIT(30))
 
@@ -66,6 +74,8 @@ struct qcom_iommu_cfg {
 	bool				 no_afe;
 	/* context banks lose their state over GDSC power collapse */
 	bool				 ctx_restore;
+	/* the micro-MMU must be halted while its registers are programmed */
+	bool				 halt;
 	const struct qcom_iommu_sid	*sids;	/* one SMR slot per entry */
 	unsigned int			 num_sids;
 };
@@ -339,9 +349,41 @@ static int qcom_iommu_reset_ns(struct qcom_iommu_dev *qcom_iommu)
 	return 0;
 }
 
+/*
+ * Halting the micro-MMU quiesces the translation front-end: it stops new
+ * client transactions being accepted and waits for the outstanding ones to
+ * retire, so that the context bank registers can be reprogrammed without
+ * in-flight traffic racing the change.
+ */
+static void qcom_iommu_halt(struct qcom_iommu_dev *qcom_iommu)
+{
+	void __iomem *reg = qcom_iommu->global_base + QCOM_IOMMU_MICRO_MMU_CTRL;
+	u32 val;
+
+	if (!qcom_iommu->cfg || !qcom_iommu->cfg->halt)
+		return;
+
+	writel_relaxed(readl_relaxed(reg) | MICRO_MMU_CTRL_HALT_REQ, reg);
+
+	if (readl_poll_timeout(reg, val, val & MICRO_MMU_CTRL_IDLE, 0, 100000))
+		dev_err(qcom_iommu->dev, "timeout waiting for micro-MMU halt\n");
+}
+
+static void qcom_iommu_unhalt(struct qcom_iommu_dev *qcom_iommu)
+{
+	void __iomem *reg = qcom_iommu->global_base + QCOM_IOMMU_MICRO_MMU_CTRL;
+
+	if (!qcom_iommu->cfg || !qcom_iommu->cfg->halt)
+		return;
+
+	writel_relaxed(readl_relaxed(reg) & ~MICRO_MMU_CTRL_HALT_REQ, reg);
+}
+
 static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
 				   struct qcom_iommu_ctx *ctx)
 {
+	qcom_iommu_halt(qcom_iommu);
+
 	/* Disable context bank before programming */
 	iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
 
@@ -369,6 +411,8 @@ static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
 
 	/* SCTLR */
 	iommu_writel(ctx, ARM_SMMU_CB_SCTLR, ctx->sctlr);
+
+	qcom_iommu_unhalt(qcom_iommu);
 }
 
 static int qcom_iommu_init_domain(struct iommu_domain *domain,

-- 
2.47.3

Re: [PATCH 08/12] iommu: qcom_iommu: halt the micro-MMU while programming context banks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Concurrent domain attachments and context programming cause race conditions in the global micro-MMU halt state, leading to premature unhalting and data races on the control register.
- [Medium] Spinning for up to 100ms in a tight loop using `cpu_relax()` risks degrading system responsiveness on hardware timeout.
--

--- Patch [8]: [PATCH 08/12] iommu: qcom_iommu: halt the micro-MMU while programming context banks ---
commit b970b685e94c99403661414d09a198b270daac8a
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

iommu: qcom_iommu: halt the micro-MMU while programming context banks

This commit adds a hardware handshake to halt the micro-MMU before
reprogramming context bank registers. It introduces qcom_iommu_halt()
and qcom_iommu_unhalt() to wrap the context programming sequence.

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 8e9488c47d5c1..f27f8722ee640 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c

[ ... ]

> @@ -339,9 +349,41 @@ static int qcom_iommu_reset_ns(struct qcom_iommu_dev *qcom_iommu)
>  	return 0;
>  }
>  
> +/*
> + * Halting the micro-MMU quiesces the translation front-end: it stops new
> + * client transactions being accepted and waits for the outstanding ones to
> + * retire, so that the context bank registers can be reprogrammed without
> + * in-flight traffic racing the change.
> + */
> +static void qcom_iommu_halt(struct qcom_iommu_dev *qcom_iommu)
> +{
> +	void __iomem *reg = qcom_iommu->global_base + QCOM_IOMMU_MICRO_MMU_CTRL;
> +	u32 val;
> +
> +	if (!qcom_iommu->cfg || !qcom_iommu->cfg->halt)
> +		return;
> +
> +	writel_relaxed(readl_relaxed(reg) | MICRO_MMU_CTRL_HALT_REQ, reg);

[Severity: High]
Could this read-modify-write on the global MICRO_MMU_CTRL register race if
devices mapped to different IOMMU domains are probing or attaching
concurrently?

> +
> +	if (readl_poll_timeout(reg, val, val & MICRO_MMU_CTRL_IDLE, 0, 100000))

[Severity: Medium]
Does setting the delay parameter to 0 cause this to spin continuously in a
tight cpu_relax() loop for up to 100ms?

If the hardware becomes unresponsive, especially in callers like
qcom_iommu_resume() which loop over multiple context banks, could this block
the CPU for over a second in a non-preemptible kernel?

> +		dev_err(qcom_iommu->dev, "timeout waiting for micro-MMU halt\n");
> +}
> +
> +static void qcom_iommu_unhalt(struct qcom_iommu_dev *qcom_iommu)
> +{
> +	void __iomem *reg = qcom_iommu->global_base + QCOM_IOMMU_MICRO_MMU_CTRL;
> +
> +	if (!qcom_iommu->cfg || !qcom_iommu->cfg->halt)
> +		return;
> +
> +	writel_relaxed(readl_relaxed(reg) & ~MICRO_MMU_CTRL_HALT_REQ, reg);

[Severity: High]
If qcom_iommu_program_ctx() is called concurrently for different domains,
could this unconditional clear prematurely unhalt the micro-MMU while another
thread is still actively programming a different context bank?

Since this modifies the global micro-MMU state without a device-level lock or
reference count, does this risk allowing in-flight traffic to race against the
reconfiguration?

> +}
> +
>  static void qcom_iommu_program_ctx(struct qcom_iommu_dev *qcom_iommu,
>  				   struct qcom_iommu_ctx *ctx)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=8

[PATCH 09/12] iommu: qcom_iommu: allow faulting transactions to terminate

[Dmitry Baryshkov]

Context banks are programmed with SCTLR.CFCFG, which selects the stall
model: rather than terminating, a faulting transaction is held pending a
write to CB_RESUME. Held transactions stop the micro-MMU draining, and
anything that waits for the front-end to quiesce - the halt handshake, and
TLBSYNC - waits on exactly that.

Not every instance wants the stall model, and some cannot tolerate it.
Add a per-instance no_stall flag that clears CFCFG so faults terminate
instead. CFRE and CFIE are untouched, so faults are still reported
through the existing interrupt handler; only the stall goes away, and
with it the handler's RESUME write becomes a formality.

No instance sets the flag yet.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index f27f8722ee64..2b2cf665c39d 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -76,6 +76,8 @@ struct qcom_iommu_cfg {
 	bool				 ctx_restore;
 	/* the micro-MMU must be halted while its registers are programmed */
 	bool				 halt;
+	/* faulting transactions must terminate rather than stall */
+	bool				 no_stall;
 	const struct qcom_iommu_sid	*sids;	/* one SMR slot per entry */
 	unsigned int			 num_sids;
 };
@@ -500,6 +502,9 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 		if (qcom_iommu->cfg && qcom_iommu->cfg->no_afe)
 			reg &= ~ARM_SMMU_SCTLR_AFE;
 
+		if (qcom_iommu->cfg && qcom_iommu->cfg->no_stall)
+			reg &= ~ARM_SMMU_SCTLR_CFCFG;
+
 		ctx->sctlr = reg;
 
 		qcom_iommu_program_ctx(qcom_iommu, ctx);

-- 
2.47.3

Re: [PATCH 09/12] iommu: qcom_iommu: allow faulting transactions to terminate

[sashiko-bot]

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

New issues:
- [High] The `qcom_iommu_fault` interrupt handler unconditionally writes to `ARM_SMMU_CB_RESUME`, causing CONSTRAINED UNPREDICTABLE behavior when `no_stall` is enabled.

Pre-existing issues:
- [High] The driver ignores the return value of `pm_runtime_get_sync()`, risking a synchronous external abort on QCOM platforms if power enablement fails.
--

--- Patch [9]: [PATCH 09/12] iommu: qcom_iommu: allow faulting transactions to terminate ---
commit 7786a25b4251a5907f3352e2d3045ccfe7aefaab
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

iommu: qcom_iommu: allow faulting transactions to terminate

This commit introduces a no_stall flag to qcom_iommu_cfg. When set, this
flag clears the CFCFG bit in SCTLR so that faulting transactions terminate
instead of stalling. The interrupt handler remains unchanged.

> through the existing interrupt handler; only the stall goes away, and
> with it the handler's RESUME write becomes a formality.

[Severity: High]
Does this write actually become a formality, or could it cause a regression
with unpredictable behavior?

If stalling is disabled, FSR.SS will remain 0 when a context fault occurs.
According to the ARM SMMUv2 spec, writing to CB_RESUME when the stall status
(FSR.SS) is 0 is constrained unpredictable and can trigger a synchronous
abort.

The qcom_iommu_fault() interrupt handler unconditionally writes to
ARM_SMMU_CB_RESUME:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_fault() {
    ...
	iommu_writel(ctx, ARM_SMMU_CB_FSR, fsr);
	iommu_writel(ctx, ARM_SMMU_CB_RESUME, ARM_SMMU_RESUME_TERMINATE);
    ...
}

Should the unconditional write to CB_RESUME be guarded by a check for
FSR.SS?

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index f27f8722ee640..2b2cf665c39db 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
[ ... ]
> @@ -500,6 +502,9 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
>  		if (qcom_iommu->cfg && qcom_iommu->cfg->no_afe)
>  			reg &= ~ARM_SMMU_SCTLR_AFE;
>  
> +		if (qcom_iommu->cfg && qcom_iommu->cfg->no_stall)
> +			reg &= ~ARM_SMMU_SCTLR_CFCFG;
> +
>  		ctx->sctlr = reg;
>  
>  		qcom_iommu_program_ctx(qcom_iommu, ctx);

[Severity: High]
This is a pre-existing issue, but is it safe to proceed with initializing
the domain without checking if power enablement succeeded?

This function is called by qcom_iommu_attach_dev(), which ignores the return
value of pm_runtime_get_sync():

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_attach_dev() {
    ...
	pm_runtime_get_sync(qcom_iommu->dev);
	ret = qcom_iommu_init_domain(domain, qcom_iommu, dev);
	pm_runtime_put_sync(qcom_iommu->dev);
    ...
}

If pm_runtime_get_sync() fails, qcom_iommu_init_domain() will perform MMIO
accesses on unpowered hardware. On Qualcomm platforms, accessing unclocked or
unpowered IOMMU hardware registers causes a synchronous external abort.

Should qcom_iommu_attach_dev() check the return value and safely abort before
attempting to access unclocked registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=9

[PATCH 10/12] iommu: qcom_iommu: program BFP (prefetch) settings

[Dmitry Baryshkov]

MSM8974 QSMMU instances require additional programming of the prefetch
settings in the instance-specific space, with the tables being specific
to a particular instance (rather than the SoC). Implement support for
programming BFB tables.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 2b2cf665c39d..9523ae95fa12 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -57,6 +57,11 @@ enum qcom_iommu_clk {
 
 struct qcom_iommu_ctx;
 
+struct qcom_iommu_bfb_reg {
+	u32			 offset;
+	u32			 value;
+};
+
 struct qcom_iommu_sid {
 	u8			 cbndx;
 	u8			 sid;
@@ -78,6 +83,8 @@ struct qcom_iommu_cfg {
 	bool				 halt;
 	/* faulting transactions must terminate rather than stall */
 	bool				 no_stall;
+	const struct qcom_iommu_bfb_reg	*bfb;
+	unsigned int			 num_bfb;
 	const struct qcom_iommu_sid	*sids;	/* one SMR slot per entry */
 	unsigned int			 num_sids;
 };
@@ -286,6 +293,25 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+static void qcom_iommu_bfb_setup(struct qcom_iommu_dev *qcom_iommu)
+{
+	const struct qcom_iommu_cfg *cfg = qcom_iommu->cfg;
+	unsigned int i;
+
+	if (!cfg)
+		return;
+
+	/* The BFB registers sit in the implementation-defined space */
+	for (i = 0; i < cfg->num_bfb; i++) {
+		if (WARN_ON_ONCE(cfg->bfb[i].offset >= SZ_4K))
+			continue;
+
+		writel_relaxed(cfg->bfb[i].value,
+			       qcom_iommu->global_base + QCOM_IOMMU_IMPL_DEF +
+			       cfg->bfb[i].offset);
+	}
+}
+
 /*
  * Reset and configure the global register space of an instance the
  * secure world does not manage: global fault state, TLB, stream
@@ -1131,6 +1157,8 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
 			return ret;
 	}
 
+	qcom_iommu_bfb_setup(qcom_iommu);
+
 	if (qcom_iommu->cfg && qcom_iommu->cfg->ctx_restore) {
 		/* Restore context banks lost over power collapse */
 		for (i = 0; i <= qcom_iommu->max_asid; i++) {

-- 
2.47.3

Re: [PATCH 10/12] iommu: qcom_iommu: program BFP (prefetch) settings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing clock disable/unprepare on error paths in qcom_iommu_resume() leading to reference leaks.
--

--- Patch [10]: [PATCH 10/12] iommu: qcom_iommu: program BFP (prefetch) settings ---
commit 7f97a52dc093659d163957e201f4e9c73067ebaf
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

iommu: qcom_iommu: program BFP (prefetch) settings

MSM8974 QSMMU instances require additional programming of the prefetch
settings in the instance-specific space, with the tables being specific
to a particular instance. Implement support for programming BFB tables.

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 2b2cf665c39db..9523ae95fa125 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c

[ ... ]

> @@ -1131,6 +1157,8 @@ static int __maybe_unused qcom_iommu_resume(struct device *dev)
>  			return ret;

[Severity: Medium]
Does this code leak the clock reference?

If qcom_iommu_reset_ns() or qcom_scm_restore_sec_cfg() fails during device
resume, this error path returns directly without calling
clk_bulk_disable_unprepare() to clean up the clocks that were enabled
earlier in qcom_iommu_resume() via clk_bulk_prepare_enable().

>  	}
>  
> +	qcom_iommu_bfb_setup(qcom_iommu);
> +
>  	if (qcom_iommu->cfg && qcom_iommu->cfg->ctx_restore) {
>  		/* Restore context banks lost over power collapse */
>  		for (i = 0; i <= qcom_iommu->max_asid; i++) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=10

[PATCH 11/12] iommu: qcom_iommu: add the MSM8974 instances

[Dmitry Baryshkov]

All the infrastructure for driving the MSM8974 IOMMUs is in place, but
no instance uses it yet, so the GPU, display, video codec and camera
blocks still cannot be put behind their IOMMUs.

Add the per-instance configurations for the five MSM8974 IOMMUs. MDP and
Venus are TZ-secured (SCM secure IDs 1 and 0), so only their context
banks are programmed, like on MSM8916. GPU is fully OS-managed. The
stream ID maps come from the downstream MSM8974-v2 DT. All instances use
the short-descriptor pagetable format with the AFE quirk.

All instances terminate faulting transactions rather than stalling them,
as downstream does. The display IOMMU also halts the micro-MMU while its
context banks are programmed: a hung GPU cannot drain, so halting it
during GPU recovery resets the SoC.

Verified on the APQ8074 dragonboard: kmscube on the Adreno 330
renders through the GPU IOMMU (119 frames offscreen, freedreno FD330)
and MDP5 scanout works through the MDP IOMMU.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c | 102 ++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 9523ae95fa12..bc7d692a2043 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -1187,9 +1187,111 @@ static const struct dev_pm_ops qcom_iommu_pm_ops = {
 				pm_runtime_force_resume)
 };
 
+static const struct qcom_iommu_bfb_reg msm8974_gpu_bfb[] = {
+	{ 0x04c, 0x00000003 },
+	{ 0x050, 0x00000000 },
+	{ 0x514, 0x00000004 },
+	{ 0x540, 0x00000010 },
+	{ 0x56c, 0x00000000 },
+	{ 0x0ac, 0x00000000 },
+	{ 0x15c, 0x00000000 },
+	{ 0x20c, 0x00000020 },
+	{ 0x314, 0x00000000 },
+	{ 0x394, 0x00000001 },
+	{ 0x414, 0x00000081 },
+	{ 0x008, 0x00000000 },
+};
+
+static const struct qcom_iommu_sid msm8974_gpu_sids[] = {
+	{ .cbndx = 0, .sid = 0 },	/* GFX3D_USER */
+	{ .cbndx = 1, .sid = 1 },	/* GFX3D_PRIV */
+};
+
+static const struct qcom_iommu_cfg msm8974_gpu_cfg = {
+	.no_stall = true,
+	.fmt = ARM_V7S,
+	.no_afe = true,
+	.ctx_restore = true,
+	.bfb = msm8974_gpu_bfb,
+	.num_bfb = ARRAY_SIZE(msm8974_gpu_bfb),
+	.sids = msm8974_gpu_sids,
+	.num_sids = ARRAY_SIZE(msm8974_gpu_sids),
+};
+
+static const struct qcom_iommu_bfb_reg msm8974_mdp_bfb[] = {
+	{ 0x04c, 0xffffffff },
+	{ 0x050, 0x00000000 },
+	{ 0x514, 0x00000004 },
+	{ 0x540, 0x00000010 },
+	{ 0x56c, 0x00000000 },
+	{ 0x0ac, 0x00006800 },
+	{ 0x15c, 0x00006221 },
+	{ 0x20c, 0x00016231 },
+	{ 0x314, 0x00000000 },
+	{ 0x394, 0x00000034 },
+	{ 0x414, 0x00000074 },
+	{ 0x008, 0x00000000 },
+	{ 0x00c, 0x00000000 },
+	{ 0x010, 0x00000000 },
+	{ 0x014, 0x00000000 },
+	{ 0x018, 0x00000000 },
+	{ 0x01c, 0x00000000 },
+	{ 0x020, 0x00000000 },
+};
+
+static const struct qcom_iommu_cfg msm8974_mdp_cfg = {
+	.halt = true,
+	.no_stall = true,
+	.fmt = ARM_V7S,
+	.no_afe = true,
+	.ctx_restore = true,
+	.bfb = msm8974_mdp_bfb,
+	.num_bfb = ARRAY_SIZE(msm8974_mdp_bfb),
+};
+
+static const struct qcom_iommu_bfb_reg msm8974_venus_bfb[] = {
+	{ 0x04c, 0xffffffff },
+	{ 0x050, 0xffffffff },
+	{ 0x514, 0x00000004 },
+	{ 0x540, 0x00000008 },
+	{ 0x56c, 0x00000000 },
+	{ 0x0ac, 0x00013205 },
+	{ 0x15c, 0x00004000 },
+	{ 0x20c, 0x00014020 },
+	{ 0x314, 0x00000000 },
+	{ 0x394, 0x00000094 },
+	{ 0x414, 0x00000114 },
+	{ 0x008, 0x00000000 },
+	{ 0x00c, 0x00000000 },
+	{ 0x010, 0x00000000 },
+	{ 0x014, 0x00000000 },
+	{ 0x018, 0x00000000 },
+	{ 0x01c, 0x00000000 },
+	{ 0x020, 0x00000000 },
+	{ 0x024, 0x00000000 },
+	{ 0x028, 0x00000000 },
+	{ 0x02c, 0x00000000 },
+	{ 0x030, 0x00000000 },
+	{ 0x034, 0x00000000 },
+	{ 0x038, 0x00000000 },
+};
+
+static const struct qcom_iommu_cfg msm8974_venus_cfg = {
+	.halt = true,
+	.no_stall = true,
+	.fmt = ARM_V7S,
+	.no_afe = true,
+	.ctx_restore = true,
+	.bfb = msm8974_venus_bfb,
+	.num_bfb = ARRAY_SIZE(msm8974_venus_bfb),
+};
+
 static const struct of_device_id qcom_iommu_of_match[] = {
 	{ .compatible = "qcom,msm-iommu-v1" },
 	{ .compatible = "qcom,msm-iommu-v2" },
+	{ .compatible = "qcom,msm8974-gpu-iommu", .data = &msm8974_gpu_cfg },
+	{ .compatible = "qcom,msm8974-mdp-iommu", .data = &msm8974_mdp_cfg },
+	{ .compatible = "qcom,msm8974-venus-iommu", .data = &msm8974_venus_cfg },
 	{ /* sentinel */ }
 };
 

-- 
2.47.3

Re: [PATCH 11/12] iommu: qcom_iommu: add the MSM8974 instances

[sashiko-bot]

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

New issues:
- [Medium] The commit message claims to add per-instance configurations for five MSM8974 IOMMUs (explicitly mentioning camera blocks), but only three are provided.
- [Low] The Venus IOMMU is configured to halt the micro-MMU, but the commit message only justifies this behavior for the display (MDP) IOMMU.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the qcom_iommu_fault interrupt handler accesses device hardware registers directly without checking the device's PM state.
--

--- Patch [11]: [PATCH 11/12] iommu: qcom_iommu: add the MSM8974 instances ---
commit bfa29c5d93fde62303c2c0ea8deeb5f5b59f6431
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

iommu: qcom_iommu: add the MSM8974 instances

This commit adds per-instance configurations for MSM8974 IOMMUs. It configures
them to terminate faulting transactions rather than stalling.

> Add the per-instance configurations for the five MSM8974 IOMMUs. MDP and
> Venus are TZ-secured (SCM secure IDs 1 and 0), so only their context
> banks are programmed, like on MSM8916. GPU is fully OS-managed. The
> stream ID maps come from the downstream MSM8974-v2 DT. All instances use
> the short-descriptor pagetable format with the AFE quirk.

[Severity: Medium]
The commit message claims to add per-instance configurations for five
MSM8974 IOMMUs, but only three are provided in this patch. Are the other
instances missing from the patch, or does the commit message need updating?

> All instances terminate faulting transactions rather than stalling them,
> as downstream does. The display IOMMU also halts the micro-MMU while its
> context banks are programmed: a hung GPU cannot drain, so halting it
> during GPU recovery resets the SoC.

[Severity: Low]
Does the Venus IOMMU also need to halt the micro-MMU? The commit message
only justifies this behavior for the display (MDP) IOMMU, but the Venus
configuration in msm8974_venus_cfg sets halt = true. Could the commit
message be expanded to document this?

> diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> index 9523ae95fa125..bc7d692a20436 100644
> --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
> +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c

[ ... ]

> +static const struct qcom_iommu_cfg msm8974_venus_cfg = {
> +	.halt = true,
> +	.no_stall = true,
> +	.fmt = ARM_V7S,
> +	.no_afe = true,
> +	.ctx_restore = true,
> +	.bfb = msm8974_venus_bfb,
> +	.num_bfb = ARRAY_SIZE(msm8974_venus_bfb),
> +};

[ ... ]

>  static const struct of_device_id qcom_iommu_of_match[] = {
>  	{ .compatible = "qcom,msm-iommu-v1" },
>  	{ .compatible = "qcom,msm-iommu-v2" },
> +	{ .compatible = "qcom,msm8974-gpu-iommu", .data = &msm8974_gpu_cfg },
> +	{ .compatible = "qcom,msm8974-mdp-iommu", .data = &msm8974_mdp_cfg },
> +	{ .compatible = "qcom,msm8974-venus-iommu", .data = &msm8974_venus_cfg },
>  	{ /* sentinel */ }
>  };

[Severity: High]
This is a pre-existing issue, but the qcom_iommu_fault interrupt handler
accesses hardware registers directly without checking the device PM state:

drivers/iommu/arm/arm-smmu/qcom_iommu.c:qcom_iommu_fault() {
    ...
	fsr = iommu_readl(ctx, ARM_SMMU_CB_FSR);

	if (!(fsr & ARM_SMMU_CB_FSR_FAULT))
		return IRQ_NONE;
    ...
}

Because the interrupt is shared (IRQF_SHARED), an interrupt from another
device on the same line could trigger this handler while the IOMMU is in a
suspended PM state. Can reading from the unpowered hardware register
ARM_SMMU_CB_FSR cause a bus stall or return 0xffffffff? Should the handler
use pm_runtime_get_if_active before accessing the registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-msm8974-iommu-upstream-v1-0-87f5cd492560@oss.qualcomm.com?part=11

[PATCH 12/12] ARM: dts: qcom: msm8974: add the IOMMUs

[Dmitry Baryshkov]

Since the switch to DRM_GPUVM the drm/msm driver can't work without the
IOMMU, breaking display and GPU support on MSM8974 (and MSM8226).

Describe the IOMMU instances and point the GPU and the MDP at theirs.
The nodes follow the MSM8916 convention: ranges points at the context
bank array (base + 0x8000) so that context node offsets equal the
context bank indices.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 arch/arm/boot/dts/qcom/qcom-msm8974.dtsi | 85 +++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
index 06c0af0aa317..ac3058995d49 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
@@ -1958,6 +1958,8 @@ mdp: display-controller@fd900000 {
 				interconnects = <&mmssnoc MNOC_MAS_MDP_PORT0 &bimc BIMC_SLV_EBI_CH0>;
 				interconnect-names = "mdp0-mem";
 
+				iommus = <&mdp_iommu 0>;
+
 				ports {
 					#address-cells = <1>;
 					#size-cells = <0>;
@@ -2125,6 +2127,28 @@ mdss_dsi1_phy: phy@fd923000 {
 			};
 		};
 
+		mdp_iommu: iommu@fd928000 {
+			compatible = "qcom,msm8974-mdp-iommu";
+			reg = <0xfd928000 0x2600>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#iommu-cells = <1>;
+			ranges = <0 0xfd930000 0x1000>;
+
+			clocks = <&mmcc MDSS_AHB_CLK>,
+				 <&mmcc MDSS_AXI_CLK>;
+			clock-names = "iface", "bus";
+			power-domains = <&mmcc MDSS_GDSC>;
+			qcom,iommu-secure-id = <1>;
+
+			/* MDP_0 */
+			iommu-ctx@0 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x0 0x1000>;
+				interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+			};
+		};
+
 		cci: cci@fda0c000 {
 			compatible = "qcom,msm8974-cci";
 			#address-cells = <1>;
@@ -2180,7 +2204,7 @@ gpu: gpu@fdb00000 {
 					<&ocmemnoc OCMEM_VNOC_MAS_GFX3D &ocmemnoc OCMEM_SLV_OCMEM>;
 			interconnect-names = "gfx-mem", "ocmem";
 
-			// iommus = <&gpu_iommu 0>;
+			iommus = <&gpu_iommu 0>;
 
 			status = "disabled";
 
@@ -2201,6 +2225,65 @@ opp-27000000 {
 			};
 		};
 
+		gpu_iommu: iommu@fdb10000 {
+			compatible = "qcom,msm8974-gpu-iommu";
+			reg = <0xfdb10000 0x2600>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#iommu-cells = <1>;
+			ranges = <0 0xfdb18000 0x2000>;
+
+			clocks = <&mmcc OXILICX_AHB_CLK>,
+				 <&mmcc OXILICX_AXI_CLK>,
+				 <&mmcc OXILI_GFX3D_CLK>;
+			clock-names = "iface", "bus", "tbu";
+			power-domains = <&mmcc OXILICX_GDSC>;
+
+			/* GFX3D_USER */
+			iommu-ctx@0 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x0 0x1000>;
+				interrupts = <GIC_SPI 241 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			/* GFX3D_PRIV */
+			iommu-ctx@1000 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x1000 0x1000>;
+				interrupts = <GIC_SPI 241 IRQ_TYPE_LEVEL_HIGH>;
+			};
+		};
+
+		venus_iommu: iommu@fdc84000 {
+			compatible = "qcom,msm8974-venus-iommu";
+			reg = <0xfdc84000 0x2600>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#iommu-cells = <1>;
+			ranges = <0 0xfdc8c000 0x3000>;
+
+			clocks = <&mmcc VENUS0_AHB_CLK>,
+				 <&mmcc VENUS0_AXI_CLK>,
+				 <&mmcc VENUS0_VCODEC0_CLK>;
+			clock-names = "iface", "bus", "tbu";
+			power-domains = <&mmcc VENUS0_GDSC>;
+			qcom,iommu-secure-id = <0>;
+
+			/* VENUS_NS */
+			iommu-ctx@0 {
+				compatible = "qcom,msm-iommu-v1-ns";
+				reg = <0x0 0x1000>;
+				interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			/* VENUS_FW */
+			iommu-ctx@2000 {
+				compatible = "qcom,msm-iommu-v1-sec";
+				reg = <0x2000 0x1000>;
+				interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+			};
+		};
+
 		sram@fdd00000 {
 			compatible = "qcom,msm8974-ocmem";
 			reg = <0xfdd00000 0x2000>,

-- 
2.47.3

Re: [PATCH 00/12] iommu: qcom_iommu: implement support for instances on MSM8974

[Luca Weiss]

Hi Dmitry,

Many thanks for working on this and sending this patch series!

On Sun Aug 9, 2026 at 10:15 PM CEST, Dmitry Baryshkov wrote:
> Qualcomm MSM8974 platform has five SMMU instances, used by display, GPU,
> Venus, VFE (camera) and JPEG encoder. Each of them follows ARM SMMU v1
> spec, however they differ from other Qualcomm platforms in the
> implementation-specific registers and also in interaction with TZ.
> Venus, MDP and VFE SMMUs are secured and require programming only of
> CBs, while GPU and JPEG require full programming.
>
> This series skips IOMMUs which can't be tested right now (VFE and JPEG),
> and adds only MDP, GPU and Venus (although untested, it is required for
> display to work) SMMU instances.

I do have a patch series (sent years ago to the mailing lists as well)
for CAMSS so I can definitely test this in the future.

>
> Note, to get display to work properly one fix is necessary, [1]
>
> [1] https://patch.msgid.link/20260809-msm8974-mmcc-fix-v1-1-50f2dcf18d2e@oss.qualcomm.com

I've applied this series on v7.2-rc7, with the extra commits between
that and linux-next for qcom_iommu.c backported so that your series
applies without conflicts.

So far I'm stuck with the GPU not being able to probe, adding some
printk's shows that in msm_iommu_new() the call for
iommu_attach_device() is failing.

[    5.971154] msm_mdp fd900100.display-controller: failed to load adreno gpu
[    5.972991] msm_mdp fd900100.display-controller: failed to bind fdb00000.gpu (ops a3xx_ops [msm]): -16
[    5.974073] msm_mdp fd900100.display-controller: adev bind failed: -16
[    5.974152] panel-s6d6fa1 fd922800.dsi.0: error -EBUSY: Failed to attach to DSI host
[    5.974230] panel-s6d6fa1 fd922800.dsi.0: probe with driver panel-s6d6fa1 failed with error -16

~ # dmesg | grep iommu
[    0.127014] iommu: Default domain type: Translated
[    0.127052] iommu: DMA domain TLB invalidation policy: strict mode
[    0.739899] platform fdb00000.gpu: Adding to iommu group 0
[    0.741358] qcom-iommu fdc84000.iommu: iommu sec: pgtable size: 90112
[    6.006877] platform fd900100.display-controller: Adding to iommu group 1

Have you seen any issues like this?

Regards
Luca

>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
> Dmitry Baryshkov (12):
>       dt-bindings: iommu: qcom,iommu: document the MSM8974 IOMMUs
>       iommu: arm-smmu: add global register definitions used by the QSMMU
>       iommu: qcom_iommu: extract context bank programming into a helper
>       iommu: qcom_iommu: support the short-descriptor pagetable format
>       iommu: qcom_iommu: handle the SMMU global register space
>       iommu: qcom_iommu: support non-TZ-managed instances
>       iommu: qcom_iommu: restore context bank state after power collapse
>       iommu: qcom_iommu: halt the micro-MMU while programming context banks
>       iommu: qcom_iommu: allow faulting transactions to terminate
>       iommu: qcom_iommu: program BFP (prefetch) settings
>       iommu: qcom_iommu: add the MSM8974 instances
>       ARM: dts: qcom: msm8974: add the IOMMUs
>
>  .../devicetree/bindings/iommu/qcom,iommu.yaml      |  16 +-
>  arch/arm/boot/dts/qcom/qcom-msm8974.dtsi           |  85 +++-
>  drivers/iommu/arm/Kconfig                          |   1 +
>  drivers/iommu/arm/arm-smmu/arm-smmu.h              |  10 +
>  drivers/iommu/arm/arm-smmu/qcom_iommu.c            | 445 +++++++++++++++++++--
>  5 files changed, 517 insertions(+), 40 deletions(-)
> ---
> base-commit: 6c5c106bff878247a9cc7255077aab9040df8bf8
> change-id: 20260809-msm8974-iommu-upstream-144b1e2a15f0
>
> Best regards,
> --  
> With best wishes
> Dmitry