* [PATCH v2] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses
@ 2026-10-04 19:50 Daniel Mentz
2026-10-05 16:42 ` Robin Murphy
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Mentz @ 2026-10-04 19:50 UTC (permalink / raw)
To: iommu
Cc: will, robin.murphy, joro, nicolinc, smostafa, linux-arm-kernel,
linux-kernel, dawei.li, jgg, praan, 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
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Daniel Mentz <danielmentz@google.com>
---
Changes in v2:
- Rename local variables 'cache' and 'sh' to 'cr1_cache' and 'cr1_sh' in
arm_smmu_device_reset() (Nicolin, Will)
- Collect Reviewed-by from Nicolin
- Link to v1: https://lore.kernel.org/linux-iommu/20260929032229.3532247-1-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 34e916ea339f..02cc6fb83461 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1905,6 +1905,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(
@@ -1916,9 +1925,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 :
@@ -5102,7 +5111,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, cr1_cache, cr1_sh;
/* Clear CR0 and sync (disables SMMU and queue processing) */
reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
@@ -5116,12 +5125,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) {
+ cr1_cache = CR1_CACHE_WB;
+ cr1_sh = ARM_SMMU_SH_ISH;
+ } else {
+ cr1_cache = CR1_CACHE_NC;
+ cr1_sh = ARM_SMMU_SH_OSH;
+ }
+
+ reg = FIELD_PREP(CR1_TABLE_SH, cr1_sh) |
+ FIELD_PREP(CR1_TABLE_OC, cr1_cache) |
+ FIELD_PREP(CR1_TABLE_IC, cr1_cache) |
+ FIELD_PREP(CR1_QUEUE_SH, cr1_sh) |
+ FIELD_PREP(CR1_QUEUE_OC, cr1_cache) |
+ FIELD_PREP(CR1_QUEUE_IC, cr1_cache);
writel_relaxed(reg, smmu->base + ARM_SMMU_CR1);
/* CR2 (random crap) */
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses 2026-10-04 19:50 [PATCH v2] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses Daniel Mentz @ 2026-10-05 16:42 ` Robin Murphy 2026-10-05 21:26 ` Daniel Mentz 0 siblings, 1 reply; 4+ messages in thread From: Robin Murphy @ 2026-10-05 16:42 UTC (permalink / raw) To: Daniel Mentz, iommu Cc: will, joro, nicolinc, smostafa, linux-arm-kernel, linux-kernel, dawei.li, jgg, praan On 04/10/2026 8:50 pm, Daniel Mentz 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: This still isn't answering the question of "why?" though. Yes the architecture says some things, but if we were strict about avoiding mismatched attributes then Linux couldn't ever support non-coherent DMA at all! Similarly while the architecture does permit SMMU implementations to be picky about their output attributes, does any such implementation actually exist at all, let alone in a system capable of running mainline Linux? And yes, io-pgtable-arm does happen to be a little more fastidious with attributes, but that is more to do with Qualcomm SMMUv2 platforms that gave a magic meaning to the outer-cacheable attribute on a still-otherwise-non-coherent downstream interconnect - which we support via IO_PGTABLE_QUIRK_ARM_OUTER_WBWA - but is not directly relevant to arm-smmu-v3 itself. So, short of a justification why it is unavoidably _necessary_ to change an assumption that in practice has held since day one, I'm still going to hold the opinion that at worst this smells like a sneaky hack around DT/ACPI describing coherency incorrectly, which really should be fixed in the firmware; or at best is just churn for no practical benefit that will only help hide that bug in future. And if someone does have a justifiable use-case for wanting to use a coherent SMMU non-coherently, then it's all the more reason to add proper DMA API support for that rather than bodging firmware to try to trick Linux, as there's already a wishlist of similar use-cases (PCIe No Snoop, Panfrost's tiler heap, etc.) Thanks, Robin. > - 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 > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com> > Signed-off-by: Daniel Mentz <danielmentz@google.com> > --- > Changes in v2: > - Rename local variables 'cache' and 'sh' to 'cr1_cache' and 'cr1_sh' in > arm_smmu_device_reset() (Nicolin, Will) > - Collect Reviewed-by from Nicolin > - Link to v1: https://lore.kernel.org/linux-iommu/20260929032229.3532247-1-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 34e916ea339f..02cc6fb83461 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -1905,6 +1905,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( > @@ -1916,9 +1925,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 : > @@ -5102,7 +5111,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, cr1_cache, cr1_sh; > > /* Clear CR0 and sync (disables SMMU and queue processing) */ > reg = readl_relaxed(smmu->base + ARM_SMMU_CR0); > @@ -5116,12 +5125,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) { > + cr1_cache = CR1_CACHE_WB; > + cr1_sh = ARM_SMMU_SH_ISH; > + } else { > + cr1_cache = CR1_CACHE_NC; > + cr1_sh = ARM_SMMU_SH_OSH; > + } > + > + reg = FIELD_PREP(CR1_TABLE_SH, cr1_sh) | > + FIELD_PREP(CR1_TABLE_OC, cr1_cache) | > + FIELD_PREP(CR1_TABLE_IC, cr1_cache) | > + FIELD_PREP(CR1_QUEUE_SH, cr1_sh) | > + FIELD_PREP(CR1_QUEUE_OC, cr1_cache) | > + FIELD_PREP(CR1_QUEUE_IC, cr1_cache); > writel_relaxed(reg, smmu->base + ARM_SMMU_CR1); > > /* CR2 (random crap) */ ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses 2026-10-05 16:42 ` Robin Murphy @ 2026-10-05 21:26 ` Daniel Mentz 2026-10-07 16:39 ` Robin Murphy 0 siblings, 1 reply; 4+ messages in thread From: Daniel Mentz @ 2026-10-05 21:26 UTC (permalink / raw) To: Robin Murphy Cc: iommu, will, joro, nicolinc, smostafa, linux-arm-kernel, linux-kernel, dawei.li, jgg, praan On Mon, Oct 5, 2026 at 9:42 AM Robin Murphy <robin.murphy@arm.com> wrote: > This still isn't answering the question of "why?" though. Yes the > architecture says some things, but if we were strict about avoiding > mismatched attributes then Linux couldn't ever support non-coherent DMA > at all! Similarly while the architecture does permit SMMU > implementations to be picky about their output attributes, does any such > implementation actually exist at all, let alone in a system capable of > running mainline Linux? Fair point that existing implementations have been forgiving in practice. My thinking here is simply that a small, self-contained cleanup that brings the driver into line with the architecture spec is worthwhile on its own merits, even without a known implementation where this currently causes problems. This is really in the same spirit as your commit 7618e4790982 ("iommu/io-pgtable-arm: Improve attribute handling"), which aligned the attribute handling with the architecture specification on the basis that: "Although the SMMU architectures seem to give some slightly stronger guarantees of Non-Cacheable output types becoming implicitly Outer Shareable in most cases, we may as well be explicit and not take any chances." As I noted in my reply on the v1 thread [1], under ARM IHI 0070 (sections 3.15, 6.3.11, and 13.1.2), a non-coherent SMMUv3 implementation (SMMU_IDR0.COHACC == 0) in which every SMMU-originated access configured with Normal Write-Back, Inner Shareable attributes fails and records an External Abort (F_STE_FETCH, F_CD_FETCH, CERROR_ABT, etc.) is completely architecturally compliant, yet wouldn't work with the current arm-smmu-v3 driver. It seems reasonable to be explicit here too and program attributes that are valid per the spec, rather than relying on the interconnect to silently degrade Write-Back attributes to Non-Cacheable. [1] https://lore.kernel.org/linux-iommu/CAE2F3rACz6Z7X3NNfLWEfjTD9K9YJyYHHyGzcBafEemTFGhgqQ@mail.gmail.com/ Thanks, Daniel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses 2026-10-05 21:26 ` Daniel Mentz @ 2026-10-07 16:39 ` Robin Murphy 0 siblings, 0 replies; 4+ messages in thread From: Robin Murphy @ 2026-10-07 16:39 UTC (permalink / raw) To: Daniel Mentz Cc: iommu, will, joro, nicolinc, smostafa, linux-arm-kernel, linux-kernel, dawei.li, jgg, praan On 05/10/2026 10:26 pm, Daniel Mentz wrote: > On Mon, Oct 5, 2026 at 9:42 AM Robin Murphy <robin.murphy@arm.com> wrote: >> This still isn't answering the question of "why?" though. Yes the >> architecture says some things, but if we were strict about avoiding >> mismatched attributes then Linux couldn't ever support non-coherent DMA >> at all! Similarly while the architecture does permit SMMU >> implementations to be picky about their output attributes, does any such >> implementation actually exist at all, let alone in a system capable of >> running mainline Linux? > > Fair point that existing implementations have been forgiving in > practice. My thinking here is simply that a small, self-contained > cleanup that brings the driver into line with the architecture spec is > worthwhile on its own merits, even without a known implementation where > this currently causes problems. > > This is really in the same spirit as your commit 7618e4790982 > ("iommu/io-pgtable-arm: Improve attribute handling"), which aligned the > attribute handling with the architecture specification on the basis > that: > > "Although the SMMU architectures seem to give some slightly stronger > guarantees of Non-Cacheable output types becoming implicitly Outer > Shareable in most cases, we may as well be explicit and not take any > chances." That was more about being self-consistent and technically compliant with VMSAv7, which again is not relevant to SMMUv3. In fact if we _only_ had to support SMMUv3 and not arbitrary other io-pgtable users then we could point to 13.1.7 "Ensuring consistent output attributes" to prove that that change would not have been necessary. > As I noted in my reply on the v1 thread [1], under ARM IHI 0070 > (sections 3.15, 6.3.11, and 13.1.2), a non-coherent SMMUv3 > implementation (SMMU_IDR0.COHACC == 0) in which every SMMU-originated > access configured with Normal Write-Back, Inner Shareable attributes > fails and records an External Abort (F_STE_FETCH, F_CD_FETCH, > CERROR_ABT, etc.) is completely architecturally compliant, yet wouldn't > work with the current arm-smmu-v3 driver. Sure, and another system could only support iNC-oWB, wherein this change still wouldn't work. If you want to argue against making assumptions about the implementation/interconnect, you can't simply make a slightly different assumption about the implementation/interconnect ;) In fact for maximum fun, you could even have an interconnect that only supports the iWB-oWB-ISH type, but the SMMU is still non-coherent since it's in a _different_ inner shareability domain from the CPUs... Yes, 13.1.2 "Attribute support" says that the system may not support all memory types, and unsupported ones may abort, but then equally it says "[...] the SMMU is not required to generate attributes that it does not use. With the exception of R/W, INST, and PRIV all configuration fields that affect unused attributes are IGNORED." And this is why the mismatched attributes argument doesn't stand up on its own - without knowing the system-specific details of exactly what is being ignored from what we think we've programmed, how can we say what the actual attributes used to access memory really are, and thus what is or isn't mismatched? Thanks, Robin. > It seems reasonable to be explicit here too and program attributes that > are valid per the spec, rather than relying on the interconnect to > silently degrade Write-Back attributes to Non-Cacheable. > > [1] https://lore.kernel.org/linux-iommu/CAE2F3rACz6Z7X3NNfLWEfjTD9K9YJyYHHyGzcBafEemTFGhgqQ@mail.gmail.com/ > > Thanks, > Daniel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-07 16:40 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-04 19:50 [PATCH v2] iommu/arm-smmu-v3: Align memory attributes for SMMU-originated accesses Daniel Mentz 2026-10-05 16:42 ` Robin Murphy 2026-10-05 21:26 ` Daniel Mentz 2026-10-07 16:39 ` Robin Murphy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox