[PATCH] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses

[PATCH] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses

[Daniel Mentz]

The SMMU specification defines several types of SMMU-originated memory
accesses, including Stage 1 and Stage 2 translation table walks, stream
table accesses (L1STD and STE fetches), CD table accesses (L1CD and CD
fetches), and queue accesses (CMDQ fetch, EVENTQ write, PRIQ write).

While the memory attributes used for Stage 1 and Stage 2 translation
table walks are configured in io-pgtable-arm based on whether the SMMU
is coherent (ARM_SMMU_FEAT_COHERENCY), the attributes for stream tables,
CD tables, and queues are currently hardcoded to Inner Shareable,
Write-Back.

On non-coherent systems, however, memory for stream tables, CD tables,
and queues is allocated via dma_alloc_coherent() /
dmam_alloc_coherent(), which provides CPU mappings with Normal
Non-Cacheable attributes. Having a non-coherent SMMU access these
buffers with Inner Shareable, Write-Back attributes results in
mismatched memory attributes between the CPU and the SMMU.

Configure the memory attributes for tables and queues in
arm_smmu_device_reset() and arm_smmu_make_cdtable_ste() based on
ARM_SMMU_FEAT_COHERENCY, matching the attributes used for translation
table walks:

- In SMMU_CR1, use Outer Shareable, Non-Cacheable for non-coherent
  SMMUs, while retaining Inner Shareable, Write-Back for coherent SMMUs.
  This applies to stream table accesses as well as queue accesses (CMDQ
  fetch, EVENTQ write, PRIQ write).
- In STE.{S1CIR, S1COR, S1CSH}, use Outer Shareable, Non-Cacheable for
  non-coherent SMMUs, while retaining Inner Shareable, Write-Back
  Read-Allocate for coherent SMMUs. This applies to CD table accesses.

Assisted-by: LLM
Signed-off-by: Daniel Mentz <danielmentz@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 37 +++++++++++++++------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 5732f3ba0122..84cfaeeb4a0d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1881,6 +1881,15 @@ void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
 {
 	struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table;
 	struct arm_smmu_device *smmu = master->smmu;
+	u64 s1c, s1csh;
+
+	if (smmu->features & ARM_SMMU_FEAT_COHERENCY) {
+		s1c = STRTAB_STE_1_S1C_CACHE_WBRA;
+		s1csh = ARM_SMMU_SH_ISH;
+	} else {
+		s1c = STRTAB_STE_1_S1C_CACHE_NC;
+		s1csh = ARM_SMMU_SH_OSH;
+	}
 
 	memset(target, 0, sizeof(*target));
 	target->data[0] = cpu_to_le64(
@@ -1892,9 +1901,9 @@ void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
 
 	target->data[1] = cpu_to_le64(
 		FIELD_PREP(STRTAB_STE_1_S1DSS, s1dss) |
-		FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
-		FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
-		FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
+		FIELD_PREP(STRTAB_STE_1_S1CIR, s1c) |
+		FIELD_PREP(STRTAB_STE_1_S1COR, s1c) |
+		FIELD_PREP(STRTAB_STE_1_S1CSH, s1csh) |
 		((smmu->features & ARM_SMMU_FEAT_STALLS &&
 		  !master->stall_enabled) ?
 			 STRTAB_STE_1_S1STALLD :
@@ -4844,7 +4853,7 @@ static void arm_smmu_write_strtab(struct arm_smmu_device *smmu)
 static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
 {
 	int ret;
-	u32 reg, enables;
+	u32 reg, enables, cache, sh;
 
 	/* Clear CR0 and sync (disables SMMU and queue processing) */
 	reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
@@ -4858,12 +4867,20 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
 		return ret;
 
 	/* CR1 (table and queue memory attributes) */
-	reg = FIELD_PREP(CR1_TABLE_SH, ARM_SMMU_SH_ISH) |
-	      FIELD_PREP(CR1_TABLE_OC, CR1_CACHE_WB) |
-	      FIELD_PREP(CR1_TABLE_IC, CR1_CACHE_WB) |
-	      FIELD_PREP(CR1_QUEUE_SH, ARM_SMMU_SH_ISH) |
-	      FIELD_PREP(CR1_QUEUE_OC, CR1_CACHE_WB) |
-	      FIELD_PREP(CR1_QUEUE_IC, CR1_CACHE_WB);
+	if (smmu->features & ARM_SMMU_FEAT_COHERENCY) {
+		cache = CR1_CACHE_WB;
+		sh = ARM_SMMU_SH_ISH;
+	} else {
+		cache = CR1_CACHE_NC;
+		sh = ARM_SMMU_SH_OSH;
+	}
+
+	reg = FIELD_PREP(CR1_TABLE_SH, sh) |
+	      FIELD_PREP(CR1_TABLE_OC, cache) |
+	      FIELD_PREP(CR1_TABLE_IC, cache) |
+	      FIELD_PREP(CR1_QUEUE_SH, sh) |
+	      FIELD_PREP(CR1_QUEUE_OC, cache) |
+	      FIELD_PREP(CR1_QUEUE_IC, cache);
 	writel_relaxed(reg, smmu->base + ARM_SMMU_CR1);
 
 	/* CR2 (random crap) */
-- 
2.56.0.rc1.315.gc6ed9934b7-goog

Re: [PATCH] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses

[Nicolin Chen]

On Tue, Sep 29, 2026 at 03:22:29AM +0000, Daniel Mentz wrote:
> @@ -4858,12 +4867,20 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
>  		return ret;
>  
>  	/* CR1 (table and queue memory attributes) */
> -	reg = FIELD_PREP(CR1_TABLE_SH, ARM_SMMU_SH_ISH) |
> -	      FIELD_PREP(CR1_TABLE_OC, CR1_CACHE_WB) |
> -	      FIELD_PREP(CR1_TABLE_IC, CR1_CACHE_WB) |
> -	      FIELD_PREP(CR1_QUEUE_SH, ARM_SMMU_SH_ISH) |
> -	      FIELD_PREP(CR1_QUEUE_OC, CR1_CACHE_WB) |
> -	      FIELD_PREP(CR1_QUEUE_IC, CR1_CACHE_WB);
> +	if (smmu->features & ARM_SMMU_FEAT_COHERENCY) {
> +		cache = CR1_CACHE_WB;
> +		sh = ARM_SMMU_SH_ISH;
> +	} else {
> +		cache = CR1_CACHE_NC;
> +		sh = ARM_SMMU_SH_OSH;
> +	}
> +
> +	reg = FIELD_PREP(CR1_TABLE_SH, sh) |
> +	      FIELD_PREP(CR1_TABLE_OC, cache) |
> +	      FIELD_PREP(CR1_TABLE_IC, cache) |
> +	      FIELD_PREP(CR1_QUEUE_SH, sh) |
> +	      FIELD_PREP(CR1_QUEUE_OC, cache) |
> +	      FIELD_PREP(CR1_QUEUE_IC, cache);
>  	writel_relaxed(reg, smmu->base + ARM_SMMU_CR1);

Nit: this is a big function. Perhaps, cr1_sh and cr1_cache?

Otherwise,

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Re: [PATCH] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses

[Mostafa Saleh]

Hi Daniel,

On Tue, Sep 29, 2026 at 4:22 AM Daniel Mentz <danielmentz@google.com> wrote:
>
> The SMMU specification defines several types of SMMU-originated memory
> accesses, including Stage 1 and Stage 2 translation table walks, stream
> table accesses (L1STD and STE fetches), CD table accesses (L1CD and CD
> fetches), and queue accesses (CMDQ fetch, EVENTQ write, PRIQ write).
>
> While the memory attributes used for Stage 1 and Stage 2 translation
> table walks are configured in io-pgtable-arm based on whether the SMMU
> is coherent (ARM_SMMU_FEAT_COHERENCY), the attributes for stream tables,
> CD tables, and queues are currently hardcoded to Inner Shareable,
> Write-Back.
>
> On non-coherent systems, however, memory for stream tables, CD tables,
> and queues is allocated via dma_alloc_coherent() /
> dmam_alloc_coherent(), which provides CPU mappings with Normal
> Non-Cacheable attributes. Having a non-coherent SMMU access these
> buffers with Inner Shareable, Write-Back attributes results in
> mismatched memory attributes between the CPU and the SMMU.
>
> Configure the memory attributes for tables and queues in
> arm_smmu_device_reset() and arm_smmu_make_cdtable_ste() based on
> ARM_SMMU_FEAT_COHERENCY, matching the attributes used for translation
> table walks:
>
> - In SMMU_CR1, use Outer Shareable, Non-Cacheable for non-coherent
>   SMMUs, while retaining Inner Shareable, Write-Back for coherent SMMUs.
>   This applies to stream table accesses as well as queue accesses (CMDQ
>   fetch, EVENTQ write, PRIQ write).
> - In STE.{S1CIR, S1COR, S1CSH}, use Outer Shareable, Non-Cacheable for
>   non-coherent SMMUs, while retaining Inner Shareable, Write-Back
>   Read-Allocate for coherent SMMUs. This applies to CD table accesses.
>

A similar patch was posted previously, and Robin highlighted the
reasoning for keeping it as is:
https://lore.kernel.org/linux-iommu/c25309d1-0424-495e-82af-d025b3e6d8c8@arm.com/

It'd be great to highlight any changes since then.

Thanks,
Mostafa

Re: [PATCH] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses

[Daniel Mentz]

Hi Mostafa,

On Thu, Oct 1, 2026 at 2:22 AM Mostafa Saleh <smostafa@google.com> wrote:
> A similar patch was posted previously, and Robin highlighted the
> reasoning for keeping it as is:
> https://lore.kernel.org/linux-iommu/c25309d1-0424-495e-82af-d025b3e6d8c8@arm.com/
>
> It'd be great to highlight any changes since then.

Thanks for the pointer. I wasn't previously aware of that email thread.
Having read through the full discussion now, I don't think Dawei Li's
patch was rejected on principle, nor did Robin argue that the driver
should keep programming Write-Back attributes for non-coherent SMMUs:

1. In that message, Robin was explaining why the existing code happened
   to work in practice on common AXI implementations ("the Normal
   Cacheable attribute will inherently degrade to a non-snooping (and
   thus effectively Normal Non-Cacheable) one, as that's essentially
   what AXI will do in practice"), and pushing back against the Fixes: /
   stable tags and against abusing firmware COHACC overrides to operate
   a coherent SMMU non-coherently.

   In his follow-up reply in the same thread [1], Robin clarified:

   "Oh, I'm not saying that we *shouldn't* set our attributes more
   exactly - this would still be needed for doing things the "right"
   way too - I just want to be very clear on the reasons why."

   Will also noted in the same thread [2]:

   "I'm not against being more careful about the memory attributes used
   by the non-coherent walker..."

   Dawei agreed to address the review feedback (such as dropping the
   unnecessary Q_BASE_RWA / STRTAB_BASE_RA changes, since allocation
   hints are ignored when Non-Cacheable is configured per IHI 0070
   section 6.3.11), and simply never sent a v2.

2. More importantly, Robin's observation about AXI degrading Normal
   Cacheable to non-snooping is based on the behavior of common
   implementations, not on the Arm SMMUv3 Architecture Specification
   (ARM IHI 0070).

   Under ARM IHI 0070 section 3.15 ("Coherency considerations and memory
   access types"):

   "ARM expects that this be IO-coherent access to normal shared memory,
   but in implementations that cannot support cache coherency, this
   might be non-cached access. ... The degree to which the different
   memory access types and attributes are supported is IMPLEMENTATION
   DEFINED."

   Furthermore, section 6.3.11 ("SMMU_CR1") and section 13.1.2
   ("Attribute support") explicitly permit an SMMU and memory system to
   reject unsupported memory attributes with an External Abort:

   "Where an SMMU and the memory system support only a subset of memory
   types, and where access to an unsupported type can be reported to the
   SMMU, the SMMU will record an external abort for the cause of the
   access in question in the architected manner."

   Section 13.1.2 specifically lists the resulting faults for
   SMMU-originated accesses using unsupported memory types:
   - STE fetch: F_STE_FETCH
   - CD fetch: F_CD_FETCH
   - Command queue read: GERROR.CMDQ_ERR and CERROR_ABT
   - Event queue access: GERROR.EVENTQ_ABT_ERR
   - PRI queue access: GERROR.PRIQ_ABT_ERR
   - Translation table walk: F_WALK_EABT

   Therefore, a non-coherent SMMUv3 implementation (SMMU_IDR0.COHACC == 0)
   where every SMMU-originated access configured with Normal Write-Back,
   Inner Shareable attributes fails and records an External Abort is
   completely architecturally compliant, yet unsupported by the current
   arm-smmu-v3 driver.

3. Finally, the Arm Architecture Reference Manual (ARM DDI 0487, section
   B2.8 "Mismatched memory attributes") explicitly warns against
   accessing the same memory location with mismatched attributes across
   agents:

   "There might be a loss of coherency when multiple agents attempt to
   access a memory Location [with mismatched memory attributes]."

   On a non-coherent SMMU, stream tables, CD tables, and queues are
   allocated via dma_alloc_coherent() / dmam_alloc_coherent(), which
   maps them as Normal Non-Cacheable on the CPU. Programming the SMMU to
   access those same buffers as Normal Write-Back, Inner Shareable
   creates mismatched memory attributes between the CPU and the SMMU.

   We already avoid this for Stage 1 and Stage 2 translation table walks
   in io-pgtable-arm by selecting Non-Cacheable, Outer Shareable when
   !coherent_walk. This patch simply brings SMMU_CR1 and
   STE.{S1CIR,S1COR,S1CSH} into compliance with the same rule.

[1] https://lore.kernel.org/linux-iommu/f253d6aa-1dc2-4b1a-85df-f43b06719c04@arm.com/
[2] https://lore.kernel.org/linux-iommu/aW9xs1ko3nWq5VbS@willie-the-truck/

Thanks,
Daniel

Re: [PATCH] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses

[Will Deacon]

On Wed, Sep 30, 2026 at 04:03:53PM -0700, Nicolin Chen wrote:
> On Tue, Sep 29, 2026 at 03:22:29AM +0000, Daniel Mentz wrote:
> > @@ -4858,12 +4867,20 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
> >  		return ret;
> >  
> >  	/* CR1 (table and queue memory attributes) */
> > -	reg = FIELD_PREP(CR1_TABLE_SH, ARM_SMMU_SH_ISH) |
> > -	      FIELD_PREP(CR1_TABLE_OC, CR1_CACHE_WB) |
> > -	      FIELD_PREP(CR1_TABLE_IC, CR1_CACHE_WB) |
> > -	      FIELD_PREP(CR1_QUEUE_SH, ARM_SMMU_SH_ISH) |
> > -	      FIELD_PREP(CR1_QUEUE_OC, CR1_CACHE_WB) |
> > -	      FIELD_PREP(CR1_QUEUE_IC, CR1_CACHE_WB);
> > +	if (smmu->features & ARM_SMMU_FEAT_COHERENCY) {
> > +		cache = CR1_CACHE_WB;
> > +		sh = ARM_SMMU_SH_ISH;
> > +	} else {
> > +		cache = CR1_CACHE_NC;
> > +		sh = ARM_SMMU_SH_OSH;
> > +	}
> > +
> > +	reg = FIELD_PREP(CR1_TABLE_SH, sh) |
> > +	      FIELD_PREP(CR1_TABLE_OC, cache) |
> > +	      FIELD_PREP(CR1_TABLE_IC, cache) |
> > +	      FIELD_PREP(CR1_QUEUE_SH, sh) |
> > +	      FIELD_PREP(CR1_QUEUE_OC, cache) |
> > +	      FIELD_PREP(CR1_QUEUE_IC, cache);
> >  	writel_relaxed(reg, smmu->base + ARM_SMMU_CR1);
> 
> Nit: this is a big function. Perhaps, cr1_sh and cr1_cache?

Yes, I'd much prefer that naming.

Daniel -- please can you send a v2 with that change and also cc lkml
so that Sashiko can have a look as well?

Cheers,

Will