SMMU 52-bit address support

Here's a (hopefully final) update of 52-bit SMMU support. The only material changes from v1[1] are a small cosmetic tweak to patch #1, and correction of the silly error in patch #2 as reported by Nate in testing.

Before trying to add the SMMUv3.1 support for 52-bit addresses, make things bearable by cleaning up the various address mask definitions to use GENMASK_ULL() consistently. The fact that doing so reveals (and fixes) a latent off-by-one in Q_BASE_ADDR_MASK only goes to show what a jolly good idea it is...

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: Clean up one more now-unnecessary linewrap

 drivers/iommu/arm-smmu-v3.c | 53 ++++++++++++++++++---------------------------
 1 file changed, 21 insertions(+), 32 deletions(-)

Bring io-pgtable-arm in line with the ARMv8.2-LPA feature allowing 52-bit physical addresses when using the 64KB translation granule. This will be supported by SMMUv3.1.

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: Fix TCR_PS/TCR_IPS copy-paste error

 drivers/iommu/io-pgtable-arm.c | 65 ++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 18 deletions(-)

drivers/iommu/io-pgtable-arm.c +10 -7

 static bool selftest_running = false;
 
 static dma_addr_t __arm_lpae_dma_addr(void *pages)

@@ -287,7 +302,7 @@ static void __arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
 		pte |= ARM_LPAE_PTE_TYPE_BLOCK;
 
 	pte |= ARM_LPAE_PTE_AF | ARM_LPAE_PTE_SH_IS;
-	pte |= pfn_to_iopte(paddr >> data->pg_shift, data);
+	pte |= paddr_to_iopte(paddr, data);
 
 	__arm_lpae_set_pte(ptep, pte, &data->iop.cfg);
 }

@@ -528,7 +543,7 @@ static int arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data,
 	if (size == split_sz)
 		unmap_idx = ARM_LPAE_LVL_IDX(iova, lvl, data);
 
-	blk_paddr = iopte_to_pfn(blk_pte, data) << data->pg_shift;
+	blk_paddr = iopte_to_paddr(blk_pte, data);
 	pte = iopte_prot(blk_pte);
 
 	for (i = 0; i < tablesz / sizeof(pte); i++, blk_paddr += split_sz) {

@@ -652,12 +667,13 @@ static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
 
 found_translation:
 	iova &= (ARM_LPAE_BLOCK_SIZE(lvl, data) - 1);
-	return ((phys_addr_t)iopte_to_pfn(pte,data) << data->pg_shift) | iova;
+	return iopte_to_paddr(pte, data) | iova;
 }
 
 static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
 {
-	unsigned long granule;
+	unsigned long granule, page_sizes;
+	unsigned int max_addr_bits = 48;
 
 	/*
 	 * We need to restrict the supported page sizes to match the

@@ -677,17 +693,24 @@ static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
 
 	switch (granule) {
 	case SZ_4K:
-		cfg->pgsize_bitmap &= (SZ_4K | SZ_2M | SZ_1G);
+		page_sizes = (SZ_4K | SZ_2M | SZ_1G);
 		break;
 	case SZ_16K:
-		cfg->pgsize_bitmap &= (SZ_16K | SZ_32M);
+		page_sizes = (SZ_16K | SZ_32M);
 		break;
 	case SZ_64K:
-		cfg->pgsize_bitmap &= (SZ_64K | SZ_512M);
+		max_addr_bits = 52;
+		page_sizes = (SZ_64K | SZ_512M);
+		if (cfg->oas > 48)

Implement SMMUv3.1 support for 52-bit physical addresses. Since a 52-bit OAS implies 64KB translation granule support, permitting level 1 block entries there is simple, and the rest is just extending address fields.

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: No change

 drivers/iommu/arm-smmu-v3.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

drivers/iommu/arm-smmu-v3.c +10 -9

index 52cad776b31b..c9c4e6132e27 100644
@@ -101,6 +101,7 @@
 #define IDR5_OAS_42_BIT			(3 << IDR5_OAS_SHIFT)
 #define IDR5_OAS_44_BIT			(4 << IDR5_OAS_SHIFT)
 #define IDR5_OAS_48_BIT			(5 << IDR5_OAS_SHIFT)
+#define IDR5_OAS_52_BIT			(6 << IDR5_OAS_SHIFT)
 
 #define ARM_SMMU_CR0			0x20
 #define CR0_CMDQEN			(1 << 3)

@@ -159,7 +160,7 @@
 
 #define ARM_SMMU_STRTAB_BASE		0x80
 #define STRTAB_BASE_RA			(1UL << 62)
-#define STRTAB_BASE_ADDR_MASK		GENMASK_ULL(47, 6)
+#define STRTAB_BASE_ADDR_MASK		GENMASK_ULL(51, 6)
 
 #define ARM_SMMU_STRTAB_BASE_CFG	0x88
 #define STRTAB_BASE_CFG_LOG2SIZE_SHIFT	0

@@ -190,7 +191,7 @@
 #define ARM_SMMU_PRIQ_IRQ_CFG2		0xdc
 
 /* Common MSI config fields */
-#define MSI_CFG0_ADDR_MASK		GENMASK_ULL(47, 2)
+#define MSI_CFG0_ADDR_MASK		GENMASK_ULL(51, 2)
 #define MSI_CFG2_SH_SHIFT		4
 #define MSI_CFG2_SH_NSH			(0UL << MSI_CFG2_SH_SHIFT)
 #define MSI_CFG2_SH_OSH			(2UL << MSI_CFG2_SH_SHIFT)

@@ -206,7 +207,7 @@
 					 Q_IDX(q, p) * (q)->ent_dwords)
 
 #define Q_BASE_RWA			(1UL << 62)
-#define Q_BASE_ADDR_MASK		GENMASK_ULL(47, 5)
+#define Q_BASE_ADDR_MASK		GENMASK_ULL(51, 5)
 #define Q_BASE_LOG2SIZE_SHIFT		0
 #define Q_BASE_LOG2SIZE_MASK		0x1fUL
 

@@ -223,7 +224,7 @@
 #define STRTAB_L1_DESC_DWORDS		1
 #define STRTAB_L1_DESC_SPAN_SHIFT	0
 #define STRTAB_L1_DESC_SPAN_MASK	0x1fUL
-#define STRTAB_L1_DESC_L2PTR_MASK	GENMASK_ULL(47, 6)
+#define STRTAB_L1_DESC_L2PTR_MASK	GENMASK_ULL(51, 6)
 
 #define STRTAB_STE_DWORDS		8
 #define STRTAB_STE_0_V			(1UL << 0)

@@ -236,7 +237,7 @@
 
 #define STRTAB_STE_0_S1FMT_SHIFT	4
 #define STRTAB_STE_0_S1FMT_LINEAR	(0UL << STRTAB_STE_0_S1FMT_SHIFT)
-#define STRTAB_STE_0_S1CTXPTR_MASK	GENMASK_ULL(47, 6)
+#define STRTAB_STE_0_S1CTXPTR_MASK	GENMASK_ULL(51, 6)
 #define STRTAB_STE_0_S1CDMAX_SHIFT	59
 #define STRTAB_STE_0_S1CDMAX_MASK	0x1fUL
 

@@ -274,7 +275,7 @@
 #define STRTAB_STE_2_S2PTW		(1UL << 54)
 #define STRTAB_STE_2_S2R		(1UL << 58)
 
-#define STRTAB_STE_3_S2TTB_MASK		GENMASK_ULL(47, 4)
+#define STRTAB_STE_3_S2TTB_MASK		GENMASK_ULL(51, 4)
 
 /* Context descriptor (stage-1 only) */
 #define CTXDESC_CD_DWORDS		8

@@ -320,7 +321,7 @@
 #define CTXDESC_CD_0_ASID_SHIFT		48
 #define CTXDESC_CD_0_ASID_MASK		0xffffUL
 
-#define CTXDESC_CD_1_TTB0_MASK		GENMASK_ULL(47, 4)
+#define CTXDESC_CD_1_TTB0_MASK		GENMASK_ULL(51, 4)
 
 #define CTXDESC_CD_3_MAIR_SHIFT		0
 

@@ -357,7 +358,7 @@
 #define CMDQ_TLBI_0_ASID_SHIFT		48
 #define CMDQ_TLBI_1_LEAF		(1UL << 0)
 #define CMDQ_TLBI_1_VA_MASK		GENMASK_ULL(63, 12)
-#define CMDQ_TLBI_1_IPA_MASK		GENMASK_ULL(47, 12)
+#define CMDQ_TLBI_1_IPA_MASK		GENMASK_ULL(51, 12)
 
 #define CMDQ_PRI_0_SSID_SHIFT		12
 #define CMDQ_PRI_0_SSID_MASK		0xfffffUL

@@ -380,7 +381,7 @@
 #define CMDQ_SYNC_0_MSIATTR_OIWB	(0xfUL << CMDQ_SYNC_0_MSIATTR_SHIFT)
 #define CMDQ_SYNC_0_MSIDATA_SHIFT	32
 #define CMDQ_SYNC_0_MSIDATA_MASK	0xffffffffUL
-#define CMDQ_SYNC_1_MSIADDR_MASK	GENMASK_ULL(47, 2)
+#define CMDQ_SYNC_1_MSIADDR_MASK	GENMASK_ULL(51, 2)
 
 /* Event queue */
 #define EVTQ_ENT_DWORDS			4

Stage 1 input addresses are effectively 64-bit in SMMUv3 anyway, so really all that's involved is letting io-pgtable know the appropriate upper bound for T0SZ.

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: No change

 drivers/iommu/arm-smmu-v3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/iommu/arm-smmu-v3.c +0 -0

index c9c4e6132e27..7059a0805bd1 100644

drivers/iommu/arm-smmu-v3.c +1 -0

 #define ARM_SMMU_CR0			0x20
 #define CR0_CMDQEN			(1 << 3)

@@ -604,6 +605,7 @@ struct arm_smmu_device {
 #define ARM_SMMU_FEAT_STALLS		(1 << 11)
 #define ARM_SMMU_FEAT_HYP		(1 << 12)
 #define ARM_SMMU_FEAT_STALL_FORCE	(1 << 13)
+#define ARM_SMMU_FEAT_VAX		(1 << 14)
 	u32				features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)

Hi Robin,

Quoted 12 lines · Robin Murphy

Before trying to add the SMMUv3.1 support for 52-bit addresses, make things bearable by cleaning up the various address mask definitions to use GENMASK_ULL() consistently. The fact that doing so reveals (and fixes) a latent off-by-one in Q_BASE_ADDR_MASK only goes to show what a jolly good idea it is...

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: Clean up one more now-unnecessary linewrap

Quoted 2 lines

  drivers/iommu/arm-smmu-v3.c | 53 ++++++++++++++++++---------------------------
  1 file changed, 21 insertions(+), 32 deletions(-)

Whilst I agree that using GENMASK is better, this patch does mean that the driver is (more) inconsistent with its _MASK terminology in that you can't generally tell whether a definition that ends in _MASK is shifted or not, and this isn't even consistent for fields within the same register.

Should we be using GENMASK/BIT for all fields instead and removing all of the _SHIFT definitions?

Quoted 12 lines · Robin Murphy

Bring io-pgtable-arm in line with the ARMv8.2-LPA feature allowing 52-bit physical addresses when using the 64KB translation granule. This will be supported by SMMUv3.1.

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: Fix TCR_PS/TCR_IPS copy-paste error

  drivers/iommu/io-pgtable-arm.c | 65 ++++++++++++++++++++++++++++++------------
  1 file changed, 47 insertions(+), 18 deletions(-)

[...]

Quoted 4 lines

drivers/iommu/io-pgtable-arm.c +0 -0

@@ -203,6 +199,25 @@ struct arm_lpae_io_pgtable {
 
  typedef u64 arm_lpae_iopte;
 

drivers/iommu/io-pgtable-arm.c struct arm_lpae_io_pgtable { · line 209

Quoted 8 lines

drivers/iommu/io-pgtable-arm.c +8 -0

+static arm_lpae_iopte paddr_to_iopte(phys_addr_t paddr,
+				     struct arm_lpae_io_pgtable *data)
+{
+	arm_lpae_iopte pte = paddr;
+
+	/* Of the bits which overlap, either 51:48 or 15:12 are always RES0 */
+	return (pte | (pte >> 36)) & ARM_LPAE_PTE_ADDR_MASK;
+}

I don't particularly like relying on properties of the paddr for correct construction of the pte here. The existing macro doesn't have this limitation. I suspect it's all fine at the moment because we only use TTBR0, but I'd rather not bake that in if we can avoid it.

drivers/iommu/io-pgtable-arm.c struct arm_lpae_io_pgtable { · line 218

Quoted 8 lines

drivers/iommu/io-pgtable-arm.c +8 -0

+static phys_addr_t iopte_to_paddr(arm_lpae_iopte pte,
+				  struct arm_lpae_io_pgtable *data)
+{
+	phys_addr_t paddr = pte & ARM_LPAE_PTE_ADDR_MASK;
+	phys_addr_t paddr_hi = paddr & (ARM_LPAE_GRANULE(data) - 1);
+
+	/* paddr_hi spans nothing for 4K granule, and only RES0 bits for 16K */
+	return (paddr ^ paddr_hi) | (paddr_hi << 36);

Why do we need xor here?

Quoted 61 lines

drivers/iommu/io-pgtable-arm.c +14 -8

  static bool selftest_running = false;
 
  static dma_addr_t __arm_lpae_dma_addr(void *pages)

@@ -287,7 +302,7 @@ static void __arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
  		pte |= ARM_LPAE_PTE_TYPE_BLOCK;
 
  	pte |= ARM_LPAE_PTE_AF | ARM_LPAE_PTE_SH_IS;
-	pte |= pfn_to_iopte(paddr >> data->pg_shift, data);
+	pte |= paddr_to_iopte(paddr, data);
 
  	__arm_lpae_set_pte(ptep, pte, &data->iop.cfg);
  }

@@ -528,7 +543,7 @@ static int arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data,
  	if (size == split_sz)
  		unmap_idx = ARM_LPAE_LVL_IDX(iova, lvl, data);
 
-	blk_paddr = iopte_to_pfn(blk_pte, data) << data->pg_shift;
+	blk_paddr = iopte_to_paddr(blk_pte, data);
  	pte = iopte_prot(blk_pte);
 
  	for (i = 0; i < tablesz / sizeof(pte); i++, blk_paddr += split_sz) {

@@ -652,12 +667,13 @@ static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops *ops,
 
  found_translation:
  	iova &= (ARM_LPAE_BLOCK_SIZE(lvl, data) - 1);
-	return ((phys_addr_t)iopte_to_pfn(pte,data) << data->pg_shift) | iova;
+	return iopte_to_paddr(pte, data) | iova;
  }
 
  static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
  {
-	unsigned long granule;
+	unsigned long granule, page_sizes;
+	unsigned int max_addr_bits = 48;
 
  	/*
  	 * We need to restrict the supported page sizes to match the

@@ -677,17 +693,24 @@ static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg)
 
  	switch (granule) {
  	case SZ_4K:
-		cfg->pgsize_bitmap &= (SZ_4K | SZ_2M | SZ_1G);
+		page_sizes = (SZ_4K | SZ_2M | SZ_1G);
  		break;
  	case SZ_16K:
-		cfg->pgsize_bitmap &= (SZ_16K | SZ_32M);
+		page_sizes = (SZ_16K | SZ_32M);
  		break;
  	case SZ_64K:
-		cfg->pgsize_bitmap &= (SZ_64K | SZ_512M);
+		max_addr_bits = 52;
+		page_sizes = (SZ_64K | SZ_512M);
+		if (cfg->oas > 48)
+			page_sizes |= 1ULL << 42; /* 4TB */
  		break;
  	default:
-		cfg->pgsize_bitmap = 0;
+		page_sizes = 0;
  	}
+
+	cfg->pgsize_bitmap &= page_sizes;

drivers/iommu/io-pgtable-arm.c static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg) · line 713

Quoted 2 lines

drivers/iommu/io-pgtable-arm.c +2 -0

+	cfg->ias = min(cfg->ias, max_addr_bits);
+	cfg->oas = min(cfg->oas, max_addr_bits);

I don't think we should be writing to the ias/oas fields here, at least now without auditing the drivers and updating the comments about the io-pgtable API. For example, the SMMUv3 driver uses its own ias local variable to initialise the domain geometry, and won't pick up any changes made here.

Quoted 13 lines · Robin Murphy

Stage 1 input addresses are effectively 64-bit in SMMUv3 anyway, so really all that's involved is letting io-pgtable know the appropriate upper bound for T0SZ.

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: No change

  drivers/iommu/arm-smmu-v3.c | 8 ++++++++
  1 file changed, 8 insertions(+)

drivers/iommu/arm-smmu-v3.c line 105

Quoted 9 lines

drivers/iommu/arm-smmu-v3.c +1 -0

index c9c4e6132e27..7059a0805bd1 100644
@@ -102,6 +102,7 @@
  #define IDR5_OAS_44_BIT			(4 << IDR5_OAS_SHIFT)
  #define IDR5_OAS_48_BIT			(5 << IDR5_OAS_SHIFT)
  #define IDR5_OAS_52_BIT			(6 << IDR5_OAS_SHIFT)
+#define IDR5_VAX			(1 << 10)

I think this is actually a two-bit field, so we should check the whole thing.

Quoted 12 lines

drivers/iommu/arm-smmu-v3.c +1 -0

  #define ARM_SMMU_CR0			0x20
  #define CR0_CMDQEN			(1 << 3)

@@ -604,6 +605,7 @@ struct arm_smmu_device {
  #define ARM_SMMU_FEAT_STALLS		(1 << 11)
  #define ARM_SMMU_FEAT_HYP		(1 << 12)
  #define ARM_SMMU_FEAT_STALL_FORCE	(1 << 13)
+#define ARM_SMMU_FEAT_VAX		(1 << 14)
  	u32				features;
 
  #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)

drivers/iommu/arm-smmu-v3.c static int arm_smmu_domain_finalise(struct iommu_domain *domain) · line 1661

Quoted 4 lines

drivers/iommu/arm-smmu-v3.c +1 -0

@@ -1656,6 +1658,8 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain)
  	switch (smmu_domain->stage) {
  	case ARM_SMMU_DOMAIN_S1:
  		ias = VA_BITS;
+		if (VA_BITS > 48 && !(smmu->features & ARM_SMMU_FEAT_VAX))

nit, but I'd rather the if condition was checking ias instead of VA_BITS.

drivers/iommu/arm-smmu-v3.c static int arm_smmu_domain_finalise(struct iommu_domain *domain) · line 1663

Quoted 2 lines

drivers/iommu/arm-smmu-v3.c +1 -0

+			ias = 48;
  		oas = smmu->ias;

Should we also be sanitising the oas here if the SMMU doesn't support 64k pages? It looks like the ias/oas sanitisation isn't cleanly split between the pgtable code and the SMMU driver.

Quoted 13 lines · Will Deacon

Quoted 12 lines · Robin Murphy

Before trying to add the SMMUv3.1 support for 52-bit addresses, make things bearable by cleaning up the various address mask definitions to use GENMASK_ULL() consistently. The fact that doing so reveals (and fixes) a latent off-by-one in Q_BASE_ADDR_MASK only goes to show what a jolly good idea it is...

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: Clean up one more now-unnecessary linewrap

Quoted 7 lines

Quoted 2 lines

 drivers/iommu/arm-smmu-v3.c | 53 ++++++++++++++++++---------------------------
 1 file changed, 21 insertions(+), 32 deletions(-)

Whilst I agree that using GENMASK is better, this patch does mean that the driver is (more) inconsistent with its _MASK terminology in that you can't generally tell whether a definition that ends in _MASK is shifted or not, and this isn't even consistent for fields within the same register.

The apparently slightly-less-than-obvious internal consistency is that every mask used for an *address field* is now in-place, while other types of field are still handled as inconsistently as they were before. It should also be the case that every x_MASK without a corresponding x_SHIFT defined next to it is unshifted.

Either way it's certainly no *worse* than the current situation where address masks sometimes have a nonzero shift, sometimes have zero bits at the bottom and a shift of 0, and sometimes have no shift defined at all.

Thinking about it some more, the address masks should only ever be needed when *extracting* an address from a register/structure word, or validating them in the context of an address *before* inserting into a field - if we can't trust input to be correct then just silently masking off bits probably isn't the best idea either way - so IMHO there is plenty of contextual disambiguation too.

Quoted 2 lines

Should we be using GENMASK/BIT for all fields instead and removing all of the _SHIFT definitions?

I'm all aboard using BIT() consistently for single-bit boolean fields, but for multi-bit fields in general we do have to keep an explicit shift defined *somewhere* in order to make sensible use of the value, i.e. either:

	val = (reg >> 22) & 0x1f;
	reg = (val & 0x1f) << 22;

or:

	val = (reg & 0x07c00000) >> 22;
	reg = (val << 22) & 0x07c00000;

[ but ideally not this mess we currently have in some places:

	val = (reg & 0x1f << 22) >> 22;

]

Again, I'd gladly clean everything up to at least be self-consistent (and line up more with how we did things in SMMUv2) if you think it's worthwhile. Although I guess that means I'd get the job of fixing up future stable backport conflicts too ;)

Quoted 16 lines · Will Deacon

Quoted 12 lines · Robin Murphy

Bring io-pgtable-arm in line with the ARMv8.2-LPA feature allowing 52-bit physical addresses when using the 64KB translation granule. This will be supported by SMMUv3.1.

Tested-by Nate Watterson <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---

v2: Fix TCR_PS/TCR_IPS copy-paste error

 drivers/iommu/io-pgtable-arm.c | 65 ++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 18 deletions(-)

[...]

Quoted 15 lines

drivers/iommu/io-pgtable-arm.c struct arm_lpae_io_pgtable { · line 207

Quoted 10 lines

drivers/iommu/io-pgtable-arm.c +8 -0

@@ -203,6 +199,25 @@ struct arm_lpae_io_pgtable {
 typedef u64 arm_lpae_iopte;
+static arm_lpae_iopte paddr_to_iopte(phys_addr_t paddr,
+				     struct arm_lpae_io_pgtable *data)
+{
+	arm_lpae_iopte pte = paddr;
+
+	/* Of the bits which overlap, either 51:48 or 15:12 are always RES0 */
+	return (pte | (pte >> 36)) & ARM_LPAE_PTE_ADDR_MASK;
+}

I don't particularly like relying on properties of the paddr for correct construction of the pte here. The existing macro doesn't have this limitation. I suspect it's all fine at the moment because we only use TTBR0, but I'd rather not bake that in if we can avoid it.

What's the relevance of TTBR0 to physical addresses? :/

Note that by this point paddr has been validated against cfg->oas by arm_lpae_map(), and validated to be granule-aligned by iommu_map(), so it really can't be wrong under reasonable conditions.

Quoted 10 lines

drivers/iommu/io-pgtable-arm.c struct arm_lpae_io_pgtable { · line 218

Quoted 8 lines

drivers/iommu/io-pgtable-arm.c +8 -0

+static phys_addr_t iopte_to_paddr(arm_lpae_iopte pte,
+				  struct arm_lpae_io_pgtable *data)
+{
+	phys_addr_t paddr = pte & ARM_LPAE_PTE_ADDR_MASK;
+	phys_addr_t paddr_hi = paddr & (ARM_LPAE_GRANULE(data) - 1);
+
+	/* paddr_hi spans nothing for 4K granule, and only RES0 bits for 16K */
+	return (paddr ^ paddr_hi) | (paddr_hi << 36);

Why do we need xor here?

Because "(paddr ^ paddr_hi)" is more concise than "(paddr & ~(phys_addr_t)(ARM_LPAE_GRANULE(data) - 1)" or variants thereof. It's potentially a teeny bit more efficient too, I think, but it's mostly about the readability.

Quoted 9 lines

drivers/iommu/io-pgtable-arm.c static void arm_lpae_restrict_pgsizes(struct io_pgtable_cfg *cfg) · line 713

Quoted 3 lines

drivers/iommu/io-pgtable-arm.c +3 -0

+	cfg->pgsize_bitmap &= page_sizes;
+	cfg->ias = min(cfg->ias, max_addr_bits);
+	cfg->oas = min(cfg->oas, max_addr_bits);

I don't think we should be writing to the ias/oas fields here, at least now without auditing the drivers and updating the comments about the io-pgtable API. For example, the SMMUv3 driver uses its own ias local variable to initialise the domain geometry, and won't pick up any changes made here.

As you've discovered, the driver thing is indeed true. More generally, though, we've always adjusted cfg->pgsize_bitmap here, so I don't see why other fields should be treated differently - I've always assumed the cfg which the driver passes in here just represents its total maximum capabilities, from which it's io-pgtable's job to pick an appropriate configuration.