Skip to contentSkip to conversation

Finish off the split page table prep work

PATCH In review Will Deacon 3 participants 11 messages Jan 10, 2020 – Feb 19, 2020

Subsystems (6)
Arm Qualcomm mailing list Arm MediaTek IOMMU Arm SMMU drivers +2 more subsystems
lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

cover Finish off the split page table prep work

Hi all,

Last merge window, I merged most of the split page table prep work from Robin [1], but there were a few patches left pending some rework. I think Robin was hoping to get that done for 5.5, but what with the holidays falling like they did and other committments, I've ended up picked up the bits that were left over.

I'm pretty limited with regards to SMMU hardware on which I can test this lot, so I might have broken something.

Applies against for-joerg/arm-smmu/updates.

Will

[1] https://lore.kernel.org/lkml/20191104202012.GN24909@willie-the-truck

Cc Robin Murphy <[email protected]>
Cc Jordan Crouse <[email protected]>

--->8

Robin Murphy (3):
iommu/io-pgtable-arm: Rationalise TTBRn handling iommu/io-pgtable-arm: Rationalise TCR handling iommu/io-pgtable-arm: Prepare for TTBR1 usage

Will Deacon (5):
iommu/io-pgtable-arm: Support non-coherent stage-2 page tables iommu/io-pgtable-arm: Ensure non-cacheable mappings are Outer
Shareable
iommu/io-pgtable-arm: Ensure ARM_64_LPAE_S2_TCR_RES1 is unsigned iommu/arm-smmu: Rename public #defines under ARM_SMMU_ namespace iommu/io-pgtable-arm: Rationalise VTCR handling

 drivers/iommu/arm-smmu-impl.c      |   2 +-
 drivers/iommu/arm-smmu-v3.c        |  60 ++++----
 drivers/iommu/arm-smmu.c           | 171 ++++++++++++----------
 drivers/iommu/arm-smmu.h           | 228 ++++++++++++++++++-----------
 drivers/iommu/io-pgtable-arm-v7s.c |  23 ++-
 drivers/iommu/io-pgtable-arm.c     | 155 +++++++++-----------
 drivers/iommu/io-pgtable.c         |   2 +-
 drivers/iommu/ipmmu-vmsa.c         |   2 +-
 drivers/iommu/msm_iommu.c          |   4 +-
 drivers/iommu/mtk_iommu.c          |   4 +-
 drivers/iommu/qcom_iommu.c         |  25 ++--
 include/linux/io-pgtable.h         |  27 +++-
 12 files changed, 381 insertions(+), 322 deletions(-)
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#1 lore raw

PATCH 1/8 iommu/io-pgtable-arm: Rationalise TTBRn handling

From: Robin Murphy <[email protected]>

TTBR1 values have so far been redundant since no users implement any support for split address spaces. Crucially, though, one of the main reasons for wanting to do so is to be able to manage each half entirely independently, e.g. context-switching one set of mappings without disturbing the other. Thus it seems unlikely that tying two tables together in a single io_pgtable_cfg would ever be particularly desirable or useful.

Streamline the configs to just a single conceptual TTBR value representing the allocated table. This paves the way for future users to support split address spaces by simply allocating a table and dealing with the detailed TTBRn logistics themselves.

Tested-by Jordan Crouse <[email protected]>
Signed-off-by Robin Murphy <[email protected]>
[will: Keep v7s non-coherent mappings explicitly outer-shareable]
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/arm-smmu-v3.c        |  2 +-
 drivers/iommu/arm-smmu.c           |  9 ++++-----
 drivers/iommu/io-pgtable-arm-v7s.c | 17 ++++++++---------
 drivers/iommu/io-pgtable-arm.c     |  5 ++---
 drivers/iommu/ipmmu-vmsa.c         |  2 +-
 drivers/iommu/msm_iommu.c          |  4 ++--
 drivers/iommu/mtk_iommu.c          |  4 ++--
 drivers/iommu/qcom_iommu.c         |  3 +--
 include/linux/io-pgtable.h         |  4 ++--
 9 files changed, 23 insertions(+), 27 deletions(-)
drivers/iommu/arm-smmu-v3.c +1 -1
index aa7e53023585..cf2ae065a6c2 100644
@@ -2166,7 +2166,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
 	}
 
 	cfg->cd.asid	= (u16)asid;
-	cfg->cd.ttbr	= pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
+	cfg->cd.ttbr	= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
 	cfg->cd.tcr	= pgtbl_cfg->arm_lpae_s1_cfg.tcr;
 	cfg->cd.mair	= pgtbl_cfg->arm_lpae_s1_cfg.mair;
 	return 0;
drivers/iommu/arm-smmu.c +4 -5
index 46b87740d708..72640e045268 100644
@@ -553,13 +553,12 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
 	/* TTBRs */
 	if (stage1) {
 		if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH32_S) {
-			cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr[0];
-			cb->ttbr[1] = pgtbl_cfg->arm_v7s_cfg.ttbr[1];
+			cb->ttbr[0] = pgtbl_cfg->arm_v7s_cfg.ttbr;
+			cb->ttbr[1] = 0;
 		} else {
-			cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[0];
+			cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
 			cb->ttbr[0] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
-			cb->ttbr[1] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr[1];
-			cb->ttbr[1] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
+			cb->ttbr[1] = FIELD_PREP(TTBRn_ASID, cfg->asid);
 		}
 	} else {
 		cb->ttbr[0] = pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
drivers/iommu/io-pgtable-arm-v7s.c +8 -9
index 7c3bd2c3cdca..714cb406b0a8 100644
@@ -822,15 +822,14 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
 	/* Ensure the empty pgd is visible before any actual TTBR write */
 	wmb();
 
-	/* TTBRs */
-	cfg->arm_v7s_cfg.ttbr[0] = virt_to_phys(data->pgd) |
-				   ARM_V7S_TTBR_S | ARM_V7S_TTBR_NOS |
-				   (cfg->coherent_walk ?
-				   (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) |
-				    ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBWA)) :
-				   (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) |
-				    ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC)));
-	cfg->arm_v7s_cfg.ttbr[1] = 0;
+	/* TTBR */
+	cfg->arm_v7s_cfg.ttbr = virt_to_phys(data->pgd) |
+				ARM_V7S_TTBR_S | ARM_V7S_TTBR_NOS |
+				(cfg->coherent_walk ?
+				 (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_WBWA) |
+				  ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_WBWA)) :
+				 (ARM_V7S_TTBR_IRGN_ATTR(ARM_V7S_RGN_NC) |
+				  ARM_V7S_TTBR_ORGN_ATTR(ARM_V7S_RGN_NC)));
 	return &data->iop;
 
 out_free_data:
drivers/iommu/io-pgtable-arm.c +2 -3
index bdf47f745268..7b422b9fe05b 100644
@@ -872,9 +872,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, void *cookie)
 	/* Ensure the empty pgd is visible before any actual TTBR write */
 	wmb();
 
-	/* TTBRs */
-	cfg->arm_lpae_s1_cfg.ttbr[0] = virt_to_phys(data->pgd);
-	cfg->arm_lpae_s1_cfg.ttbr[1] = 0;
+	/* TTBR */
+	cfg->arm_lpae_s1_cfg.ttbr = virt_to_phys(data->pgd);
 	return &data->iop;
 
 out_free_data:
drivers/iommu/ipmmu-vmsa.c +1 -1
index d02edd2751f3..ecb3f9464dd5 100644
@@ -374,7 +374,7 @@ static void ipmmu_domain_setup_context(struct ipmmu_vmsa_domain *domain)
 	u32 tmp;
 
 	/* TTBR0 */
-	ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
+	ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr;
 	ipmmu_ctx_write_root(domain, IMTTLBR0, ttbr);
 	ipmmu_ctx_write_root(domain, IMTTUBR0, ttbr >> 32);
drivers/iommu/msm_iommu.c +2 -2
index 93f14bca26ee..94a6df1bddd6 100644
@@ -279,8 +279,8 @@ static void __program_context(void __iomem *base, int ctx,
 	SET_V2PCFG(base, ctx, 0x3);
 
 	SET_TTBCR(base, ctx, priv->cfg.arm_v7s_cfg.tcr);
-	SET_TTBR0(base, ctx, priv->cfg.arm_v7s_cfg.ttbr[0]);
-	SET_TTBR1(base, ctx, priv->cfg.arm_v7s_cfg.ttbr[1]);
+	SET_TTBR0(base, ctx, priv->cfg.arm_v7s_cfg.ttbr);
+	SET_TTBR1(base, ctx, 0);
 
 	/* Set prrr and nmrr */
 	SET_PRRR(base, ctx, priv->cfg.arm_v7s_cfg.prrr);
drivers/iommu/mtk_iommu.c +2 -2
index 6fc1f5ecf91e..95945f467c03 100644
@@ -367,7 +367,7 @@ static int mtk_iommu_attach_device(struct iommu_domain *domain,
 	/* Update the pgtable base address register of the M4U HW */
 	if (!data->m4u_dom) {
 		data->m4u_dom = dom;
-		writel(dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
+		writel(dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
 		       data->base + REG_MMU_PT_BASE_ADDR);
 	}
 

@@ -765,7 +765,7 @@ static int __maybe_unused mtk_iommu_resume(struct device *dev)
 	writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);
 	writel_relaxed(reg->vld_pa_rng, base + REG_MMU_VLD_PA_RNG);
 	if (m4u_dom)
-		writel(m4u_dom->cfg.arm_v7s_cfg.ttbr[0] & MMU_PT_ADDR_MASK,
+		writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
 		       base + REG_MMU_PT_BASE_ADDR);
 	return 0;
 }
drivers/iommu/qcom_iommu.c +1 -2
index 52f38292df5b..c200bc066257 100644
@@ -269,10 +269,9 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 
 		/* TTBRs */
 		iommu_writeq(ctx, ARM_SMMU_CB_TTBR0,
-				pgtbl_cfg.arm_lpae_s1_cfg.ttbr[0] |
+				pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
 				FIELD_PREP(TTBRn_ASID, ctx->asid));
 		iommu_writeq(ctx, ARM_SMMU_CB_TTBR1,
-				pgtbl_cfg.arm_lpae_s1_cfg.ttbr[1] |
 				FIELD_PREP(TTBRn_ASID, ctx->asid));
 
 		/* TCR */
include/linux/io-pgtable.h +2 -2
index ee21eedafe98..53bca5343f52 100644
@@ -100,7 +100,7 @@ struct io_pgtable_cfg {
 	/* Low-level data specific to the table format */
 	union {
 		struct {
-			u64	ttbr[2];
+			u64	ttbr;
 			u64	tcr;
 			u64	mair;
 		} arm_lpae_s1_cfg;

@@ -111,7 +111,7 @@ struct io_pgtable_cfg {
 		} arm_lpae_s2_cfg;
 
 		struct {
-			u32	ttbr[2];
+			u32	ttbr;
 			u32	tcr;
 			u32	nmrr;
 			u32	prrr;
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#2 lore raw

PATCH 2/8 iommu/io-pgtable-arm: Support non-coherent stage-2 page tables

In reply to Will Deacon

Commit 9e6ea59f3ff3 ("iommu/io-pgtable: Support non-coherent page tables") added support for non-coherent page-table walks to the Arm IOMMU page-table backends. Unfortunately, it left the stage-2 allocator unchanged, so let's hook that up in the same way.

Cc Bjorn Andersson <[email protected]>
Cc Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/io-pgtable-arm.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
drivers/iommu/io-pgtable-arm.c +10 -4
index 7b422b9fe05b..ab440b52a5f4 100644
@@ -910,10 +910,16 @@ arm_64_lpae_alloc_pgtable_s2(struct io_pgtable_cfg *cfg, void *cookie)
 	}
 
 	/* VTCR */
-	reg = ARM_64_LPAE_S2_TCR_RES1 |
-	     (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
-	     (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) |
-	     (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT);
+	reg = ARM_64_LPAE_S2_TCR_RES1;
+	if (cfg->coherent_walk) {
+		reg |= (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
+		       (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) |
+		       (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT);
+	} else {
+		reg |= (ARM_LPAE_TCR_SH_OS << ARM_LPAE_TCR_SH0_SHIFT) |
+		       (ARM_LPAE_TCR_RGN_NC << ARM_LPAE_TCR_IRGN0_SHIFT) |
+		       (ARM_LPAE_TCR_RGN_NC << ARM_LPAE_TCR_ORGN0_SHIFT);
+	}
 
 	sl = data->start_level;
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#3 lore raw

PATCH 3/8 iommu/io-pgtable-arm: Ensure non-cacheable mappings are Outer Shareable

In reply to Will Deacon

The Armv7 ARM states that Normal, Non-cacheable mappings must explicitly be marked as Outer Shareable in order to avoid UNPREDICTABLE behaviour:

  | Overlaying the shareability attribute (B3-1377, ARM DDI 0406C.c)
  |
  | A memory region with a resultant memory type attribute of Normal, and
  | a resultant cacheability attribute of Inner Non-cacheable, Outer
  | Non-cacheable, must have a resultant shareability attribute of Outer
  | Shareable, otherwise shareability is UNPREDICTABLE

Although this requirement doesn't appear to exist in the Armv8 docs, where the 'SH' field is simply ignored in this case, it's straightforward enough to set it here.

Cc Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/io-pgtable-arm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/iommu/io-pgtable-arm.c +6 -2
index ab440b52a5f4..4b437ead2300 100644
@@ -303,9 +303,8 @@ static void __arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
 
 	if (data->iop.fmt != ARM_MALI_LPAE)
 		pte |= ARM_LPAE_PTE_AF;
-	pte |= ARM_LPAE_PTE_SH_IS;
-	pte |= paddr_to_iopte(paddr, data);
 
+	pte |= paddr_to_iopte(paddr, data);
 	__arm_lpae_set_pte(ptep, pte, &data->iop.cfg);
 }
 

@@ -463,6 +462,11 @@ static arm_lpae_iopte arm_lpae_prot_to_pte(struct arm_lpae_io_pgtable *data,
 	if (prot & IOMMU_NOEXEC)
 		pte |= ARM_LPAE_PTE_XN;
 
+	if (prot & IOMMU_CACHE)
+		pte |= ARM_LPAE_PTE_SH_IS;
+	else
+		pte |= ARM_LPAE_PTE_SH_OS;
+
 	return pte;
 }
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#4 lore raw

PATCH 4/8 iommu/io-pgtable-arm: Ensure ARM_64_LPAE_S2_TCR_RES1 is unsigned

In reply to Will Deacon

ARM_64_LPAE_S2_TCR_RES1 is intended to map to bit 31 of the VTCR register, which is required to be set to 1 by the architecture. Unfortunately, we accidentally treat this as a signed quantity which means we also set the upper 32 bits of the VTCR to one, and they are required to be zero.

Treat ARM_64_LPAE_S2_TCR_RES1 as unsigned to avoid the unwanted sign-extension up to 64 bits.

Cc Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/io-pgtable-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/io-pgtable-arm.c +1 -1
index 4b437ead2300..89216bf37282 100644
@@ -101,7 +101,7 @@
 
 /* Register bits */
 #define ARM_32_LPAE_TCR_EAE		(1 << 31)
-#define ARM_64_LPAE_S2_TCR_RES1		(1 << 31)
+#define ARM_64_LPAE_S2_TCR_RES1		(1U << 31)
 
 #define ARM_LPAE_TCR_EPD1		(1 << 23)
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#5 lore raw

PATCH 5/8 iommu/io-pgtable-arm: Rationalise TCR handling

In reply to Will Deacon

From: Robin Murphy <[email protected]>

Although it's conceptually nice for the io_pgtable_cfg to provide a standard VMSA TCR value, the reality is that no VMSA-compliant IOMMU looks exactly like an Arm CPU, and they all have various other TCR controls which io-pgtable can't be expected to understand. Thus since there is an expectation that drivers will have to add to the given TCR value anyway, let's strip it down to just the essentials that are directly relevant to io-pgtable's inner workings - namely the various sizes and the walk attributes.

Tested-by Jordan Crouse <[email protected]>
Signed-off-by Robin Murphy <[email protected]>
[will: Add missing include of bitfield.h]
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/arm-smmu-v3.c        | 41 +++----------
 drivers/iommu/arm-smmu.c           |  7 ++-
 drivers/iommu/arm-smmu.h           | 28 +++++++++
 drivers/iommu/io-pgtable-arm-v7s.c |  6 +-
 drivers/iommu/io-pgtable-arm.c     | 98 ++++++++++++------------------
 drivers/iommu/io-pgtable.c         |  2 +-
 drivers/iommu/qcom_iommu.c         |  8 +--
 include/linux/io-pgtable.h         |  9 ++-
 8 files changed, 95 insertions(+), 104 deletions(-)
drivers/iommu/arm-smmu-v3.c +9 -32
index cf2ae065a6c2..d127974afdb7 100644
@@ -260,27 +260,18 @@
 /* Context descriptor (stage-1 only) */
 #define CTXDESC_CD_DWORDS		8
 #define CTXDESC_CD_0_TCR_T0SZ		GENMASK_ULL(5, 0)
-#define ARM64_TCR_T0SZ			GENMASK_ULL(5, 0)
 #define CTXDESC_CD_0_TCR_TG0		GENMASK_ULL(7, 6)
-#define ARM64_TCR_TG0			GENMASK_ULL(15, 14)
 #define CTXDESC_CD_0_TCR_IRGN0		GENMASK_ULL(9, 8)
-#define ARM64_TCR_IRGN0			GENMASK_ULL(9, 8)
 #define CTXDESC_CD_0_TCR_ORGN0		GENMASK_ULL(11, 10)
-#define ARM64_TCR_ORGN0			GENMASK_ULL(11, 10)
 #define CTXDESC_CD_0_TCR_SH0		GENMASK_ULL(13, 12)
-#define ARM64_TCR_SH0			GENMASK_ULL(13, 12)
 #define CTXDESC_CD_0_TCR_EPD0		(1ULL << 14)
-#define ARM64_TCR_EPD0			(1ULL << 7)
 #define CTXDESC_CD_0_TCR_EPD1		(1ULL << 30)
-#define ARM64_TCR_EPD1			(1ULL << 23)
 
 #define CTXDESC_CD_0_ENDI		(1UL << 15)
 #define CTXDESC_CD_0_V			(1UL << 31)
 
 #define CTXDESC_CD_0_TCR_IPS		GENMASK_ULL(34, 32)
-#define ARM64_TCR_IPS			GENMASK_ULL(34, 32)
 #define CTXDESC_CD_0_TCR_TBI0		(1ULL << 38)
-#define ARM64_TCR_TBI0			(1ULL << 37)
 
 #define CTXDESC_CD_0_AA64		(1UL << 41)
 #define CTXDESC_CD_0_S			(1UL << 44)

@@ -291,10 +282,6 @@
 
 #define CTXDESC_CD_1_TTB0_MASK		GENMASK_ULL(51, 4)
 
-/* Convert between AArch64 (CPU) TCR format and SMMU CD format */
-#define ARM_SMMU_TCR2CD(tcr, fld)	FIELD_PREP(CTXDESC_CD_0_TCR_##fld, \
-					FIELD_GET(ARM64_TCR_##fld, tcr))
-
 /* Command queue */
 #define CMDQ_ENT_SZ_SHIFT		4
 #define CMDQ_ENT_DWORDS			((1 << CMDQ_ENT_SZ_SHIFT) >> 3)

@@ -1439,23 +1426,6 @@ static int arm_smmu_cmdq_issue_sync(struct arm_smmu_device *smmu)
 }
 
 /* Context descriptor manipulation functions */
-static u64 arm_smmu_cpu_tcr_to_cd(u64 tcr)
-{
-	u64 val = 0;
-
-	/* Repack the TCR. Just care about TTBR0 for now */
-	val |= ARM_SMMU_TCR2CD(tcr, T0SZ);
-	val |= ARM_SMMU_TCR2CD(tcr, TG0);
-	val |= ARM_SMMU_TCR2CD(tcr, IRGN0);
-	val |= ARM_SMMU_TCR2CD(tcr, ORGN0);
-	val |= ARM_SMMU_TCR2CD(tcr, SH0);
-	val |= ARM_SMMU_TCR2CD(tcr, EPD0);
-	val |= ARM_SMMU_TCR2CD(tcr, EPD1);
-	val |= ARM_SMMU_TCR2CD(tcr, IPS);
-
-	return val;
-}
-
 static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
 				    struct arm_smmu_s1_cfg *cfg)
 {

@@ -1465,7 +1435,7 @@ static void arm_smmu_write_ctx_desc(struct arm_smmu_device *smmu,
 	 * We don't need to issue any invalidation here, as we'll invalidate
 	 * the STE when installing the new entry anyway.
 	 */
-	val = arm_smmu_cpu_tcr_to_cd(cfg->cd.tcr) |
+	val = cfg->cd.tcr |
 #ifdef __BIG_ENDIAN
 	      CTXDESC_CD_0_ENDI |
 #endif

@@ -2151,6 +2121,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
 	int asid;
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	struct arm_smmu_s1_cfg *cfg = &smmu_domain->s1_cfg;
+	typeof(&pgtbl_cfg->arm_lpae_s1_cfg.tcr) tcr = &pgtbl_cfg->arm_lpae_s1_cfg.tcr;
 
 	asid = arm_smmu_bitmap_alloc(smmu->asid_map, smmu->asid_bits);
 	if (asid < 0)

@@ -2167,7 +2138,13 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
 
 	cfg->cd.asid	= (u16)asid;
 	cfg->cd.ttbr	= pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
-	cfg->cd.tcr	= pgtbl_cfg->arm_lpae_s1_cfg.tcr;
+	cfg->cd.tcr	= FIELD_PREP(CTXDESC_CD_0_TCR_T0SZ, tcr->tsz) |
+			  FIELD_PREP(CTXDESC_CD_0_TCR_TG0, tcr->tg) |
+			  FIELD_PREP(CTXDESC_CD_0_TCR_IRGN0, tcr->irgn) |
+			  FIELD_PREP(CTXDESC_CD_0_TCR_ORGN0, tcr->orgn) |
+			  FIELD_PREP(CTXDESC_CD_0_TCR_SH0, tcr->sh) |
+			  FIELD_PREP(CTXDESC_CD_0_TCR_IPS, tcr->ips) |
+			  CTXDESC_CD_0_TCR_EPD1 | CTXDESC_CD_0_AA64;
 	cfg->cd.mair	= pgtbl_cfg->arm_lpae_s1_cfg.mair;
 	return 0;
drivers/iommu/arm-smmu.c +4 -3
index 72640e045268..e65eb60812ec 100644
@@ -540,11 +540,12 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
 		if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH32_S) {
 			cb->tcr[0] = pgtbl_cfg->arm_v7s_cfg.tcr;
 		} else {
-			cb->tcr[0] = pgtbl_cfg->arm_lpae_s1_cfg.tcr;
-			cb->tcr[1] = pgtbl_cfg->arm_lpae_s1_cfg.tcr >> 32;
-			cb->tcr[1] |= FIELD_PREP(TCR2_SEP, TCR2_SEP_UPSTREAM);
+			cb->tcr[0] = arm_smmu_lpae_tcr(pgtbl_cfg);
+			cb->tcr[1] = arm_smmu_lpae_tcr2(pgtbl_cfg);
 			if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH64)
 				cb->tcr[1] |= TCR2_AS;
+			else
+				cb->tcr[0] |= TCR_EAE;
 		}
 	} else {
 		cb->tcr[0] = pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
drivers/iommu/arm-smmu.h +28 -0
index 62b9f0cec49b..aade2b0ae175 100644
@@ -11,6 +11,7 @@
 #define _ARM_SMMU_H
 
 #include <linux/atomic.h>
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/clk.h>
 #include <linux/device.h>

@@ -158,12 +159,24 @@ enum arm_smmu_cbar_type {
 #define TCR2_SEP			GENMASK(17, 15)
 #define TCR2_SEP_UPSTREAM		0x7
 #define TCR2_AS				BIT(4)
+#define TCR2_PASIZE			GENMASK(3, 0)
 
 #define ARM_SMMU_CB_TTBR0		0x20
 #define ARM_SMMU_CB_TTBR1		0x28
 #define TTBRn_ASID			GENMASK_ULL(63, 48)
 
+/* arm64 headers leak this somehow :( */
+#undef TCR_T0SZ
+
 #define ARM_SMMU_CB_TCR			0x30
+#define TCR_EAE				BIT(31)
+#define TCR_EPD1			BIT(23)
+#define TCR_TG0				GENMASK(15, 14)
+#define TCR_SH0				GENMASK(13, 12)
+#define TCR_ORGN0			GENMASK(11, 10)
+#define TCR_IRGN0			GENMASK(9, 8)
+#define TCR_T0SZ			GENMASK(5, 0)
+
 #define ARM_SMMU_CB_CONTEXTIDR		0x34
 #define ARM_SMMU_CB_S1_MAIR0		0x38
 #define ARM_SMMU_CB_S1_MAIR1		0x3c

@@ -318,6 +331,21 @@ struct arm_smmu_domain {
 	struct iommu_domain		domain;
 };
 
+static inline u32 arm_smmu_lpae_tcr(struct io_pgtable_cfg *cfg)
+{
+	return TCR_EPD1 |
+	       FIELD_PREP(TCR_TG0, cfg->arm_lpae_s1_cfg.tcr.tg) |
+	       FIELD_PREP(TCR_SH0, cfg->arm_lpae_s1_cfg.tcr.sh) |
+	       FIELD_PREP(TCR_ORGN0, cfg->arm_lpae_s1_cfg.tcr.orgn) |
+	       FIELD_PREP(TCR_IRGN0, cfg->arm_lpae_s1_cfg.tcr.irgn) |
+	       FIELD_PREP(TCR_T0SZ, cfg->arm_lpae_s1_cfg.tcr.tsz);
+}
+
+static inline u32 arm_smmu_lpae_tcr2(struct io_pgtable_cfg *cfg)
+{
+	return FIELD_PREP(TCR2_PASIZE, cfg->arm_lpae_s1_cfg.tcr.ips) |
+	       FIELD_PREP(TCR2_SEP, TCR2_SEP_UPSTREAM);
+}
 
 /* Implementation details, yay! */
 struct arm_smmu_impl {
drivers/iommu/io-pgtable-arm-v7s.c +2 -4
index 714cb406b0a8..ccedf50e5998 100644
@@ -149,8 +149,6 @@
 #define ARM_V7S_TTBR_IRGN_ATTR(attr)					\
 	((((attr) & 0x1) << 6) | (((attr) & 0x2) >> 1))
 
-#define ARM_V7S_TCR_PD1			BIT(5)
-
 #ifdef CONFIG_ZONE_DMA32
 #define ARM_V7S_TABLE_GFP_DMA GFP_DMA32
 #define ARM_V7S_TABLE_SLAB_FLAGS SLAB_CACHE_DMA32

@@ -798,8 +796,8 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
 	 */
 	cfg->pgsize_bitmap &= SZ_4K | SZ_64K | SZ_1M | SZ_16M;
 
-	/* TCR: T0SZ=0, disable TTBR1 */
-	cfg->arm_v7s_cfg.tcr = ARM_V7S_TCR_PD1;
+	/* TCR: T0SZ=0, EAE=0 (if applicable) */
+	cfg->arm_v7s_cfg.tcr = 0;
 
 	/*
 	 * TEX remap: the indices used map to the closest equivalent types
drivers/iommu/io-pgtable-arm.c +40 -58

215 lines not on this page · raw message

drivers/iommu/io-pgtable.c +1 -1
index ced53e5b72b5..94394c81468f 100644
@@ -63,7 +63,7 @@ void free_io_pgtable_ops(struct io_pgtable_ops *ops)
 	if (!ops)
 		return;
 
-	iop = container_of(ops, struct io_pgtable, ops);
+	iop = io_pgtable_ops_to_pgtable(ops);
 	io_pgtable_tlb_flush_all(iop);
 	io_pgtable_init_table[iop->fmt]->free(iop);
 }
drivers/iommu/qcom_iommu.c +3 -5
index c200bc066257..2eeaf2eec946 100644
@@ -271,15 +271,13 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 		iommu_writeq(ctx, ARM_SMMU_CB_TTBR0,
 				pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
 				FIELD_PREP(TTBRn_ASID, ctx->asid));
-		iommu_writeq(ctx, ARM_SMMU_CB_TTBR1,
-				FIELD_PREP(TTBRn_ASID, ctx->asid));
+		iommu_writeq(ctx, ARM_SMMU_CB_TTBR1, 0);
 
 		/* TCR */
 		iommu_writel(ctx, ARM_SMMU_CB_TCR2,
-				(pgtbl_cfg.arm_lpae_s1_cfg.tcr >> 32) |
-				FIELD_PREP(TCR2_SEP, TCR2_SEP_UPSTREAM));
+				arm_smmu_lpae_tcr2(&pgtbl_cfg));
 		iommu_writel(ctx, ARM_SMMU_CB_TCR,
-				pgtbl_cfg.arm_lpae_s1_cfg.tcr);
+				arm_smmu_lpae_tcr(&pgtbl_cfg) | TCR_EAE);
 
 		/* MAIRs (stage-1 only) */
 		iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0,
include/linux/io-pgtable.h +8 -1
index 53bca5343f52..6ae104cedfd7 100644
@@ -101,7 +101,14 @@ struct io_pgtable_cfg {
 	union {
 		struct {
 			u64	ttbr;
-			u64	tcr;
+			struct {
+				u32	ips:3;
+				u32	tg:2;
+				u32	sh:2;
+				u32	orgn:2;
+				u32	irgn:2;
+				u32	tsz:6;
+			}	tcr;
 			u64	mair;
 		} arm_lpae_s1_cfg;
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#6 lore raw

PATCH 6/8 iommu/arm-smmu: Rename public #defines under ARM_SMMU_ namespace

In reply to Will Deacon

Now that we have arm-smmu.h defining various SMMU constants, ensure that they are namespaced with the ARM_SMMU_ prefix in order to avoid conflicts with the CPU, such as the one we're currently bodging around with the TCR.

Cc Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/arm-smmu-impl.c |   2 +-
 drivers/iommu/arm-smmu.c      | 157 ++++++++++++------------
 drivers/iommu/arm-smmu.h      | 217 +++++++++++++++++-----------------
 drivers/iommu/qcom_iommu.c    |  16 +--
 4 files changed, 204 insertions(+), 188 deletions(-)
drivers/iommu/arm-smmu-impl.c +1 -1
index b2fe72a8f019..74d97a886e93 100644
@@ -119,7 +119,7 @@ int arm_mmu500_reset(struct arm_smmu_device *smmu)
 	 * Secure has also cleared SACR.CACHE_LOCK for this to take effect...
 	 */
 	reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_ID7);
-	major = FIELD_GET(ID7_MAJOR, reg);
+	major = FIELD_GET(ARM_SMMU_ID7_MAJOR, reg);
 	reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sACR);
 	if (major >= 2)
 		reg &= ~ARM_MMU500_ACR_CACHE_LOCK;
drivers/iommu/arm-smmu.c +83 -74

405 lines not on this page · raw message

drivers/iommu/arm-smmu.h +111 -106

323 lines not on this page · raw message

drivers/iommu/qcom_iommu.c +9 -7
index 2eeaf2eec946..39759db4f003 100644
@@ -201,7 +201,7 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
 
 	fsr = iommu_readl(ctx, ARM_SMMU_CB_FSR);
 
-	if (!(fsr & FSR_FAULT))
+	if (!(fsr & ARM_SMMU_FSR_FAULT))
 		return IRQ_NONE;
 
 	fsynr = iommu_readl(ctx, ARM_SMMU_CB_FSYNR0);

@@ -215,7 +215,7 @@ static irqreturn_t qcom_iommu_fault(int irq, void *dev)
 	}
 
 	iommu_writel(ctx, ARM_SMMU_CB_FSR, fsr);
-	iommu_writel(ctx, ARM_SMMU_CB_RESUME, RESUME_TERMINATE);
+	iommu_writel(ctx, ARM_SMMU_CB_RESUME, ARM_SMMU_RESUME_TERMINATE);
 
 	return IRQ_HANDLED;
 }

@@ -270,14 +270,14 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 		/* TTBRs */
 		iommu_writeq(ctx, ARM_SMMU_CB_TTBR0,
 				pgtbl_cfg.arm_lpae_s1_cfg.ttbr |
-				FIELD_PREP(TTBRn_ASID, ctx->asid));
+				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) | TCR_EAE);
+			     arm_smmu_lpae_tcr(&pgtbl_cfg) | ARM_SMMU_TCR_EAE);
 
 		/* MAIRs (stage-1 only) */
 		iommu_writel(ctx, ARM_SMMU_CB_S1_MAIR0,

@@ -286,11 +286,13 @@ static int qcom_iommu_init_domain(struct iommu_domain *domain,
 				pgtbl_cfg.arm_lpae_s1_cfg.mair >> 32);
 
 		/* SCTLR */
-		reg = SCTLR_CFIE | SCTLR_CFRE | SCTLR_AFE | SCTLR_TRE |
-			SCTLR_M | SCTLR_S1_ASIDPNE | SCTLR_CFCFG;
+		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 |
+		      ARM_SMMU_SCTLR_CFCFG;
 
 		if (IS_ENABLED(CONFIG_BIG_ENDIAN))
-			reg |= SCTLR_E;
+			reg |= ARM_SMMU_SCTLR_E;
 
 		iommu_writel(ctx, ARM_SMMU_CB_SCTLR, reg);
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#7 lore raw

PATCH 7/8 iommu/io-pgtable-arm: Rationalise VTCR handling

In reply to Will Deacon

Commit 05a648cd2dd7 ("iommu/io-pgtable-arm: Rationalise TCR handling") reworked the way in which the TCR register value is returned from the io-pgtable code when targetting the Arm long-descriptor format, in preparation for allowing page-tables to target TTBR1.

As it turns out, the new interface is a lot nicer to use, so do the same conversion for the VTCR register even though there is only a single base register for stage-2 translation.

Cc Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/arm-smmu-v3.c    | 17 +++++++++-
 drivers/iommu/arm-smmu.c       |  2 +-
 drivers/iommu/arm-smmu.h       | 21 +++++++++++++
 drivers/iommu/io-pgtable-arm.c | 57 +++++++++++++---------------------
 include/linux/io-pgtable.h     | 10 +++++-
 5 files changed, 68 insertions(+), 39 deletions(-)
drivers/iommu/arm-smmu-v3.c +16 -1
index d127974afdb7..4443e1890077 100644
@@ -250,6 +250,13 @@
 
 #define STRTAB_STE_2_S2VMID		GENMASK_ULL(15, 0)
 #define STRTAB_STE_2_VTCR		GENMASK_ULL(50, 32)
+#define STRTAB_STE_2_VTCR_S2T0SZ	GENMASK_ULL(5, 0)
+#define STRTAB_STE_2_VTCR_S2SL0		GENMASK_ULL(7, 6)
+#define STRTAB_STE_2_VTCR_S2IR0		GENMASK_ULL(9, 8)
+#define STRTAB_STE_2_VTCR_S2OR0		GENMASK_ULL(11, 10)
+#define STRTAB_STE_2_VTCR_S2SH0		GENMASK_ULL(13, 12)
+#define STRTAB_STE_2_VTCR_S2TG		GENMASK_ULL(15, 14)
+#define STRTAB_STE_2_VTCR_S2PS		GENMASK_ULL(18, 16)
 #define STRTAB_STE_2_S2AA64		(1UL << 51)
 #define STRTAB_STE_2_S2ENDI		(1UL << 52)
 #define STRTAB_STE_2_S2PTW		(1UL << 54)

@@ -2159,14 +2166,22 @@ static int arm_smmu_domain_finalise_s2(struct arm_smmu_domain *smmu_domain,
 	int vmid;
 	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	struct arm_smmu_s2_cfg *cfg = &smmu_domain->s2_cfg;
+	typeof(&pgtbl_cfg->arm_lpae_s2_cfg.vtcr) vtcr;
 
 	vmid = arm_smmu_bitmap_alloc(smmu->vmid_map, smmu->vmid_bits);
 	if (vmid < 0)
 		return vmid;
 
+	vtcr = &pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
 	cfg->vmid	= (u16)vmid;
 	cfg->vttbr	= pgtbl_cfg->arm_lpae_s2_cfg.vttbr;
-	cfg->vtcr	= pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
+	cfg->vtcr	= FIELD_PREP(STRTAB_STE_2_VTCR_S2T0SZ, vtcr->tsz) |
+			  FIELD_PREP(STRTAB_STE_2_VTCR_S2SL0, vtcr->sl) |
+			  FIELD_PREP(STRTAB_STE_2_VTCR_S2IR0, vtcr->irgn) |
+			  FIELD_PREP(STRTAB_STE_2_VTCR_S2OR0, vtcr->orgn) |
+			  FIELD_PREP(STRTAB_STE_2_VTCR_S2SH0, vtcr->sh) |
+			  FIELD_PREP(STRTAB_STE_2_VTCR_S2TG, vtcr->tg) |
+			  FIELD_PREP(STRTAB_STE_2_VTCR_S2PS, vtcr->ps);
 	return 0;
 }
drivers/iommu/arm-smmu.c +1 -1
index 214be09f6ded..f067783ebd59 100644
@@ -548,7 +548,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
 				cb->tcr[0] |= ARM_SMMU_TCR_EAE;
 		}
 	} else {
-		cb->tcr[0] = pgtbl_cfg->arm_lpae_s2_cfg.vtcr;
+		cb->tcr[0] = arm_smmu_lpae_vtcr(pgtbl_cfg);
 	}
 
 	/* TTBRs */
drivers/iommu/arm-smmu.h +21 -0
index 6501f38a5966..8d1cd54d82a6 100644
@@ -174,6 +174,15 @@ enum arm_smmu_cbar_type {
 #define ARM_SMMU_TCR_IRGN0		GENMASK(9, 8)
 #define ARM_SMMU_TCR_T0SZ		GENMASK(5, 0)
 
+#define ARM_SMMU_VTCR_RES1		BIT(31)
+#define ARM_SMMU_VTCR_PS		GENMASK(18, 16)
+#define ARM_SMMU_VTCR_TG0		ARM_SMMU_TCR_TG0
+#define ARM_SMMU_VTCR_SH0		ARM_SMMU_TCR_SH0
+#define ARM_SMMU_VTCR_ORGN0		ARM_SMMU_TCR_ORGN0
+#define ARM_SMMU_VTCR_IRGN0		ARM_SMMU_TCR_IRGN0
+#define ARM_SMMU_VTCR_SL0		GENMASK(7, 6)
+#define ARM_SMMU_VTCR_T0SZ		ARM_SMMU_TCR_T0SZ
+
 #define ARM_SMMU_CB_CONTEXTIDR		0x34
 #define ARM_SMMU_CB_S1_MAIR0		0x38
 #define ARM_SMMU_CB_S1_MAIR1		0x3c

@@ -352,6 +361,18 @@ static inline u32 arm_smmu_lpae_tcr2(struct io_pgtable_cfg *cfg)
 	       FIELD_PREP(ARM_SMMU_TCR2_SEP, ARM_SMMU_TCR2_SEP_UPSTREAM);
 }
 
+static inline u32 arm_smmu_lpae_vtcr(struct io_pgtable_cfg *cfg)
+{
+	return ARM_SMMU_VTCR_RES1 |
+	       FIELD_PREP(ARM_SMMU_VTCR_PS, cfg->arm_lpae_s2_cfg.vtcr.ps) |
+	       FIELD_PREP(ARM_SMMU_VTCR_TG0, cfg->arm_lpae_s2_cfg.vtcr.tg) |
+	       FIELD_PREP(ARM_SMMU_VTCR_SH0, cfg->arm_lpae_s2_cfg.vtcr.sh) |
+	       FIELD_PREP(ARM_SMMU_VTCR_ORGN0, cfg->arm_lpae_s2_cfg.vtcr.orgn) |
+	       FIELD_PREP(ARM_SMMU_VTCR_IRGN0, cfg->arm_lpae_s2_cfg.vtcr.irgn) |
+	       FIELD_PREP(ARM_SMMU_VTCR_SL0, cfg->arm_lpae_s2_cfg.vtcr.sl) |
+	       FIELD_PREP(ARM_SMMU_VTCR_T0SZ, cfg->arm_lpae_s2_cfg.vtcr.tsz);
+}
+
 /* Implementation details, yay! */
 struct arm_smmu_impl {
 	u32 (*read_reg)(struct arm_smmu_device *smmu, int page, int offset);
drivers/iommu/io-pgtable-arm.c +21 -36

140 lines not on this page · raw message

include/linux/io-pgtable.h +9 -1
index 6ae104cedfd7..40c1b7745fb6 100644
@@ -114,7 +114,15 @@ struct io_pgtable_cfg {
 
 		struct {
 			u64	vttbr;
-			u64	vtcr;
+			struct {
+				u32	ps:3;
+				u32	tg:2;
+				u32	sh:2;
+				u32	orgn:2;
+				u32	irgn:2;
+				u32	sl:2;
+				u32	tsz:6;
+			}	vtcr;
 		} arm_lpae_s2_cfg;
 
 		struct {
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
author maintainer
#8 lore raw

PATCH 8/8 iommu/io-pgtable-arm: Prepare for TTBR1 usage

In reply to Will Deacon

From: Robin Murphy <[email protected]>

Now that we can correctly extract top-level indices without relying on the remaining upper bits being zero, the only remaining impediments to using a given table for TTBR1 are the address validation on map/unmap and the awkward TCR translation granule format. Add a quirk so that we can do the right thing at those points.

Tested-by Jordan Crouse <[email protected]>
Signed-off-by Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++------
 include/linux/io-pgtable.h     |  4 ++++
 2 files changed, 23 insertions(+), 6 deletions(-)
drivers/iommu/io-pgtable-arm.c +19 -6
index 846963c87e0f..5e966ef0bacf 100644
@@ -104,6 +104,10 @@
 #define ARM_LPAE_TCR_TG0_64K		1
 #define ARM_LPAE_TCR_TG0_16K		2
 
+#define ARM_LPAE_TCR_TG1_16K		1
+#define ARM_LPAE_TCR_TG1_4K		2
+#define ARM_LPAE_TCR_TG1_64K		3
+
 #define ARM_LPAE_TCR_SH_NS		0
 #define ARM_LPAE_TCR_SH_OS		2
 #define ARM_LPAE_TCR_SH_IS		3

@@ -463,6 +467,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	arm_lpae_iopte *ptep = data->pgd;
 	int ret, lvl = data->start_level;
 	arm_lpae_iopte prot;
+	long iaext = (long)iova >> cfg->ias;
 
 	/* If no access, then nothing to do */
 	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))

@@ -471,7 +476,9 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return -EINVAL;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias || paddr >> data->iop.cfg.oas))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext || paddr >> cfg->oas))
 		return -ERANGE;
 
 	prot = arm_lpae_prot_to_pte(data, iommu_prot);

@@ -637,11 +644,14 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
 	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
 	struct io_pgtable_cfg *cfg = &data->iop.cfg;
 	arm_lpae_iopte *ptep = data->pgd;
+	long iaext = (long)iova >> cfg->ias;
 
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return 0;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext))
 		return 0;
 
 	return __arm_lpae_unmap(data, gather, iova, size, data->start_level, ptep);

@@ -777,9 +787,11 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, void *cookie)
 	u64 reg;
 	struct arm_lpae_io_pgtable *data;
 	typeof(&cfg->arm_lpae_s1_cfg.tcr) tcr = &cfg->arm_lpae_s1_cfg.tcr;
+	bool tg1;
 
 	if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
-			    IO_PGTABLE_QUIRK_NON_STRICT))
+			    IO_PGTABLE_QUIRK_NON_STRICT |
+			    IO_PGTABLE_QUIRK_ARM_TTBR1))
 		return NULL;
 
 	data = arm_lpae_alloc_pgtable(cfg);

@@ -797,15 +809,16 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg, void *cookie)
 		tcr->orgn = ARM_LPAE_TCR_RGN_NC;
 	}
 
+	tg1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1;
 	switch (ARM_LPAE_GRANULE(data)) {
 	case SZ_4K:
-		tcr->tg = ARM_LPAE_TCR_TG0_4K;
+		tcr->tg = tg1 ? ARM_LPAE_TCR_TG1_4K : ARM_LPAE_TCR_TG0_4K;
 		break;
 	case SZ_16K:
-		tcr->tg = ARM_LPAE_TCR_TG0_16K;
+		tcr->tg = tg1 ? ARM_LPAE_TCR_TG1_16K : ARM_LPAE_TCR_TG0_16K;
 		break;
 	case SZ_64K:
-		tcr->tg = ARM_LPAE_TCR_TG0_64K;
+		tcr->tg = tg1 ? ARM_LPAE_TCR_TG1_64K : ARM_LPAE_TCR_TG0_64K;
 		break;
 	}
include/linux/io-pgtable.h +4 -0
index 40c1b7745fb6..53d53c6c2be9 100644
@@ -83,12 +83,16 @@ struct io_pgtable_cfg {
 	 * IO_PGTABLE_QUIRK_NON_STRICT: Skip issuing synchronous leaf TLBIs
 	 *	on unmap, for DMA domains using the flush queue mechanism for
 	 *	delayed invalidation.
+	 *
+	 * IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the table
+	 *	for use in the upper half of a split address space.
 	 */
 	#define IO_PGTABLE_QUIRK_ARM_NS		BIT(0)
 	#define IO_PGTABLE_QUIRK_NO_PERMS	BIT(1)
 	#define IO_PGTABLE_QUIRK_TLBI_ON_MAP	BIT(2)
 	#define IO_PGTABLE_QUIRK_ARM_MTK_EXT	BIT(3)
 	#define IO_PGTABLE_QUIRK_NON_STRICT	BIT(4)
+	#define IO_PGTABLE_QUIRK_ARM_TTBR1	BIT(5)
 	unsigned long			quirks;
 	unsigned long			pgsize_bitmap;
 	unsigned int			ias;
Signature · 6 lines
2.25.0.rc1.283.g88dfdc4193-goog

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
#9 lore raw

Re: [PATCH 8/8] iommu/io-pgtable-arm: Prepare for TTBR1 usage

Hi Will, Hi Robin,

Quoted 56 lines · Will Deacon

From: Robin Murphy <[email protected]>

Now that we can correctly extract top-level indices without relying on the remaining upper bits being zero, the only remaining impediments to using a given table for TTBR1 are the address validation on map/unmap and the awkward TCR translation granule format. Add a quirk so that we can do the right thing at those points.

Tested-by Jordan Crouse <[email protected]>
Signed-off-by Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
 drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++------
 include/linux/io-pgtable.h     |  4 ++++
 2 files changed, 23 insertions(+), 6 deletions(-)
drivers/iommu/io-pgtable-arm.c +9 -1
index 846963c87e0f..5e966ef0bacf 100644
@@ -104,6 +104,10 @@
 #define ARM_LPAE_TCR_TG0_64K		1
 #define ARM_LPAE_TCR_TG0_16K		2
 
+#define ARM_LPAE_TCR_TG1_16K		1
+#define ARM_LPAE_TCR_TG1_4K		2
+#define ARM_LPAE_TCR_TG1_64K		3
+
 #define ARM_LPAE_TCR_SH_NS		0
 #define ARM_LPAE_TCR_SH_OS		2
 #define ARM_LPAE_TCR_SH_IS		3

@@ -463,6 +467,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	arm_lpae_iopte *ptep = data->pgd;
 	int ret, lvl = data->start_level;
 	arm_lpae_iopte prot;
+	long iaext = (long)iova >> cfg->ias;
 
 	/* If no access, then nothing to do */
 	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))

@@ -471,7 +476,9 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return -EINVAL;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias || paddr >> data->iop.cfg.oas))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext || paddr >> cfg->oas))
 		return -ERANGE;
 
 	prot = arm_lpae_prot_to_pte(data, iommu_prot);

@@ -637,11 +644,14 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
 	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
 	struct io_pgtable_cfg *cfg = &data->iop.cfg;
 	arm_lpae_iopte *ptep = data->pgd;
+	long iaext = (long)iova >> cfg->ias;

drivers/iommu/io-pgtable-arm.c static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova, · line 657

Quoted 10 lines
drivers/iommu/io-pgtable-arm.c +3 -1
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return 0;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext))
 		return 0;
 
 	return __arm_lpae_unmap(data, gather, iova, size, data->start_level, ptep);

This part of the patch seems to break io-pgtable-arm.c on ARM32 to some extent. I have a device using qcom_iommu that could normally run ARM64, but I'm unable to because of outdated (signed) firmware. :( So it's running a lot of code that would normally run only on ARM64.

It used to work quite well but now qcom-venus fails to probe on:

    if (WARN_ON(iaext || paddr >> cfg->oas))

(I added some debug prints for clarity.)

    qcom-venus 1d00000.video-codec: Adding to iommu group 0
    arm-lpae io-pgtable: quirks: 0x0
    arm-lpae io-pgtable: arm_lpae_map: iova: 0xdd800000, paddr: 0xebe3f000, iaext: 0xffffffff, paddr >> oas: 0x0
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 954 at drivers/iommu/io-pgtable-arm.c:487 arm_lpae_map+0xe4/0x510
    Hardware name: Generic DT based system
    ...
    [<c04bafb0>] (arm_lpae_map) from [<c04bcd6c>] (qcom_iommu_map+0x50/0x78)
    [<c04bcd6c>] (qcom_iommu_map) from [<c04b7290>] (__iommu_map+0xe8/0x1cc)
    [<c04b7290>] (__iommu_map) from [<c04b7bbc>] (__iommu_map_sg+0xa8/0x118)
    [<c04b7bbc>] (__iommu_map_sg) from [<c04b7c64>] (iommu_map_sg_atomic+0x18/0x20)
    [<c04b7c64>] (iommu_map_sg_atomic) from [<c04b94f8>] (iommu_dma_alloc+0x4dc/0x5a0)
    [<c04b94f8>] (iommu_dma_alloc) from [<c0196a50>] (dma_alloc_attrs+0x104/0x114)
    [<c0196a50>] (dma_alloc_attrs) from [<bf302c60>] (venus_hfi_create+0xa4/0x260 [venus_core])
    [<bf302c60>] (venus_hfi_create [venus_core]) from [<bf2fe6cc>] (venus_probe+0x1e4/0x328 [venus_core])
    [<bf2fe6cc>] (venus_probe [venus_core]) from [<c04c8eb4>] (platform_drv_probe+0x48/0x98)
    ...
    Exception stack(0xc2587fa8 to 0xc2587ff0)
    7fa0:                   b6f3dc70 b5051010 b5051010 0017388c b6e645b0 b6e645b0
    7fc0: b6f3dc70 b5051010 00020000 00000080 b6e30790 be84ea54 0046ac91 00000000
    7fe0: b6e75f1c be84e940 b6e5fb51 b6eec8a4
    ---[ end trace 2a0ed284f6d82f16 ]---
    qcom-venus: probe of 1d00000.video-codec failed with error -12

Note that iaext = 0xffffffff != 0.
It seems to be some int/long size problem that only happens with larger iova addresses on 32-bit systems.

Without the (long) cast for iova everything is working correctly again:

drivers/iommu/io-pgtable-arm.c +2 -2
index 983b08477e64..f7ecc763c706 100644
@@ -468,7 +468,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
 	arm_lpae_iopte *ptep = data->pgd;
 	int ret, lvl = data->start_level;
 	arm_lpae_iopte prot;
-	long iaext = (long)iova >> cfg->ias;
+	long iaext = iova >> cfg->ias;
 
 	/* If no access, then nothing to do */
 	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))

@@ -645,7 +645,7 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
 	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
 	struct io_pgtable_cfg *cfg = &data->iop.cfg;
 	arm_lpae_iopte *ptep = data->pgd;
-	long iaext = (long)iova >> cfg->ias;
+	long iaext = iova >> cfg->ias;
 
 	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
 		return 0;

But I'm not sure if this is really the correct "fix"...

This problem can be also easily reproduced by enabling IOMMU_IO_PGTABLE_LPAE_SELFTEST on ARM32.
(Shouldn't there be some system that runs these automatically? ;))

Without this patch all of them are passing:

    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
    arm-lpae io-pgtable: selftest: completed with 18 PASS 0 FAIL

But with this patch 6 of them are failing (with a similar warning as I posted above):

    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
    ------------[ cut here ]------------
    WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:482 arm_lpae_map+0xa4/0x4e4
    Hardware name: Generic DT based system
    [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
    [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
    [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
    [<c012d76c>] (__warn) from [<c012d7ec>] (warn_slowpath_fmt+0x64/0xbc)
    [<c012d7ec>] (warn_slowpath_fmt) from [<c04baf70>] (arm_lpae_map+0xa4/0x4e4)
    [<c04baf70>] (arm_lpae_map) from [<c0a1de3c>] (arm_lpae_do_selftests+0x234/0x688)
    [<c0a1de3c>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
    [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
    [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
    [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
    Exception stack(0xee867fb0 to 0xee867ff8)
    7fa0:                                     00000000 00000000 00000000 00000000
    7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
    7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
    ---[ end trace 89e831c50a111e7c ]---
    ------------[ cut here ]------------
    WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:1182 arm_lpae_do_selftests+0x4b8/0x688
    selftest: test failed for fmt idx 0
    Hardware name: Generic DT based system
    [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
    [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
    [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
    [<c012d76c>] (__warn) from [<c012d820>] (warn_slowpath_fmt+0x98/0xbc)
    [<c012d820>] (warn_slowpath_fmt) from [<c0a1e0c0>] (arm_lpae_do_selftests+0x4b8/0x688)
    [<c0a1e0c0>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
    [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
    [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
    [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
    Exception stack(0xee867fb0 to 0xee867ff8)
    7fa0:                                     00000000 00000000 00000000 00000000
    7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
    7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
    ---[ end trace 89e831c50a111e7d ]---
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 32-bit
    arm-lpae io-pgtable: data: 3 levels, 0x20 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 36-bit
    arm-lpae io-pgtable: data: 3 levels, 0x200 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 40-bit
    arm-lpae io-pgtable: data: 4 levels, 0x10 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 42-bit
    arm-lpae io-pgtable: data: 4 levels, 0x40 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 44-bit
    arm-lpae io-pgtable: data: 4 levels, 0x100 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
    [warning as above]
    arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 48-bit
    arm-lpae io-pgtable: data: 4 levels, 0x1000 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
    arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
    arm-lpae io-pgtable: selftest: completed with 12 PASS 6 FAIL

Any suggestions how to fix this properly?

Thanks,
Stephan
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

reviewer
#10 lore raw

Re: [PATCH 8/8] iommu/io-pgtable-arm: Prepare for TTBR1 usage

Quoted 132 lines · Stephan Gerhold

Hi Will, Hi Robin,

Quoted 56 lines · Will Deacon

From: Robin Murphy <[email protected]>

Now that we can correctly extract top-level indices without relying on the remaining upper bits being zero, the only remaining impediments to using a given table for TTBR1 are the address validation on map/unmap and the awkward TCR translation granule format. Add a quirk so that we can do the right thing at those points.

Tested-by Jordan Crouse <[email protected]>
Signed-off-by Robin Murphy <[email protected]>
Signed-off-by Will Deacon <[email protected]>
---
  drivers/iommu/io-pgtable-arm.c | 25 +++++++++++++++++++------
  include/linux/io-pgtable.h     |  4 ++++
  2 files changed, 23 insertions(+), 6 deletions(-)
drivers/iommu/io-pgtable-arm.c +9 -1
index 846963c87e0f..5e966ef0bacf 100644
@@ -104,6 +104,10 @@
  #define ARM_LPAE_TCR_TG0_64K		1
  #define ARM_LPAE_TCR_TG0_16K		2
 
+#define ARM_LPAE_TCR_TG1_16K		1
+#define ARM_LPAE_TCR_TG1_4K		2
+#define ARM_LPAE_TCR_TG1_64K		3
+
  #define ARM_LPAE_TCR_SH_NS		0
  #define ARM_LPAE_TCR_SH_OS		2
  #define ARM_LPAE_TCR_SH_IS		3

@@ -463,6 +467,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
  	arm_lpae_iopte *ptep = data->pgd;
  	int ret, lvl = data->start_level;
  	arm_lpae_iopte prot;
+	long iaext = (long)iova >> cfg->ias;
 
  	/* If no access, then nothing to do */
  	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))

@@ -471,7 +476,9 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
  	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
  		return -EINVAL;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias || paddr >> data->iop.cfg.oas))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext || paddr >> cfg->oas))
  		return -ERANGE;
 
  	prot = arm_lpae_prot_to_pte(data, iommu_prot);

@@ -637,11 +644,14 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
  	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
  	struct io_pgtable_cfg *cfg = &data->iop.cfg;
  	arm_lpae_iopte *ptep = data->pgd;
+	long iaext = (long)iova >> cfg->ias;

drivers/iommu/io-pgtable-arm.c static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova, · line 657

Quoted 10 lines
drivers/iommu/io-pgtable-arm.c +3 -1
  	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
  		return 0;
 
-	if (WARN_ON(iova >> data->iop.cfg.ias))
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1)
+		iaext = ~iaext;
+	if (WARN_ON(iaext))
  		return 0;
 
  	return __arm_lpae_unmap(data, gather, iova, size, data->start_level, ptep);

This part of the patch seems to break io-pgtable-arm.c on ARM32 to some extent. I have a device using qcom_iommu that could normally run ARM64, but I'm unable to because of outdated (signed) firmware. :( So it's running a lot of code that would normally run only on ARM64.

It used to work quite well but now qcom-venus fails to probe on:

     if (WARN_ON(iaext || paddr >> cfg->oas))

(I added some debug prints for clarity.)

     qcom-venus 1d00000.video-codec: Adding to iommu group 0
     arm-lpae io-pgtable: quirks: 0x0
     arm-lpae io-pgtable: arm_lpae_map: iova: 0xdd800000, paddr: 0xebe3f000, iaext: 0xffffffff, paddr >> oas: 0x0
     ------------[ cut here ]------------
     WARNING: CPU: 0 PID: 954 at drivers/iommu/io-pgtable-arm.c:487 arm_lpae_map+0xe4/0x510
     Hardware name: Generic DT based system
     ...
     [<c04bafb0>] (arm_lpae_map) from [<c04bcd6c>] (qcom_iommu_map+0x50/0x78)
     [<c04bcd6c>] (qcom_iommu_map) from [<c04b7290>] (__iommu_map+0xe8/0x1cc)
     [<c04b7290>] (__iommu_map) from [<c04b7bbc>] (__iommu_map_sg+0xa8/0x118)
     [<c04b7bbc>] (__iommu_map_sg) from [<c04b7c64>] (iommu_map_sg_atomic+0x18/0x20)
     [<c04b7c64>] (iommu_map_sg_atomic) from [<c04b94f8>] (iommu_dma_alloc+0x4dc/0x5a0)
     [<c04b94f8>] (iommu_dma_alloc) from [<c0196a50>] (dma_alloc_attrs+0x104/0x114)
     [<c0196a50>] (dma_alloc_attrs) from [<bf302c60>] (venus_hfi_create+0xa4/0x260 [venus_core])
     [<bf302c60>] (venus_hfi_create [venus_core]) from [<bf2fe6cc>] (venus_probe+0x1e4/0x328 [venus_core])
     [<bf2fe6cc>] (venus_probe [venus_core]) from [<c04c8eb4>] (platform_drv_probe+0x48/0x98)
     ...
     Exception stack(0xc2587fa8 to 0xc2587ff0)
     7fa0:                   b6f3dc70 b5051010 b5051010 0017388c b6e645b0 b6e645b0
     7fc0: b6f3dc70 b5051010 00020000 00000080 b6e30790 be84ea54 0046ac91 00000000
     7fe0: b6e75f1c be84e940 b6e5fb51 b6eec8a4
     ---[ end trace 2a0ed284f6d82f16 ]---
     qcom-venus: probe of 1d00000.video-codec failed with error -12

Note that iaext = 0xffffffff != 0.
It seems to be some int/long size problem that only happens with larger iova addresses on 32-bit systems.

Without the (long) cast for iova everything is working correctly again:

drivers/iommu/io-pgtable-arm.c +2 -2
index 983b08477e64..f7ecc763c706 100644
@@ -468,7 +468,7 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, unsigned long iova,
  	arm_lpae_iopte *ptep = data->pgd;
  	int ret, lvl = data->start_level;
  	arm_lpae_iopte prot;
-	long iaext = (long)iova >> cfg->ias;
+	long iaext = iova >> cfg->ias;
 
  	/* If no access, then nothing to do */
  	if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE)))

@@ -645,7 +645,7 @@ static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
  	struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
  	struct io_pgtable_cfg *cfg = &data->iop.cfg;
  	arm_lpae_iopte *ptep = data->pgd;
-	long iaext = (long)iova >> cfg->ias;
+	long iaext = iova >> cfg->ias;
 
  	if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size))
  		return 0;
Quoted 1 line

But I'm not sure if this is really the correct "fix"...

No, that would end up breaking the TTBR1 case. The quirk isn't intended to be supported on 32-bit platforms, but somehow it's slipped through that this sign-extension still affects them (and goes wrong to boot), sorry about that.

I think quick fixes would be either to replace the "(long)" cast with "(s64)(u64)", or to just elide the iaext check altogether when 32-bit longs make it impossible to pass an out-of-range IOVA anyway. I'll have a think and see what works out nicest.

Quoted 17 lines

This problem can be also easily reproduced by enabling IOMMU_IO_PGTABLE_LPAE_SELFTEST on ARM32.
(Shouldn't there be some system that runs these automatically? ;))

Without this patch all of them are passing:

     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
Quoted 8 lines
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
     arm-lpae io-pgtable: selftest: completed with 18 PASS 0 FAIL

Hmm, we probably shouldn't be running IAS > 32 tests on 32-bit, given that such configs are effectively impossible to use.

Robin.

Quoted 77 lines

But with this patch 6 of them are failing (with a similar warning as I posted above):

     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 32
     ------------[ cut here ]------------
     WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:482 arm_lpae_map+0xa4/0x4e4
     Hardware name: Generic DT based system
     [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
     [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
     [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
     [<c012d76c>] (__warn) from [<c012d7ec>] (warn_slowpath_fmt+0x64/0xbc)
     [<c012d7ec>] (warn_slowpath_fmt) from [<c04baf70>] (arm_lpae_map+0xa4/0x4e4)
     [<c04baf70>] (arm_lpae_map) from [<c0a1de3c>] (arm_lpae_do_selftests+0x234/0x688)
     [<c0a1de3c>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
     [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
     [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
     [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
     Exception stack(0xee867fb0 to 0xee867ff8)
     7fa0:                                     00000000 00000000 00000000 00000000
     7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
     7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
     ---[ end trace 89e831c50a111e7c ]---
     ------------[ cut here ]------------
     WARNING: CPU: 3 PID: 1 at drivers/iommu/io-pgtable-arm.c:1182 arm_lpae_do_selftests+0x4b8/0x688
     selftest: test failed for fmt idx 0
     Hardware name: Generic DT based system
     [<c01104ac>] (unwind_backtrace) from [<c010c0b4>] (show_stack+0x10/0x14)
     [<c010c0b4>] (show_stack) from [<c06cfb34>] (dump_stack+0x90/0xa4)
     [<c06cfb34>] (dump_stack) from [<c012d76c>] (__warn+0xb8/0xd4)
     [<c012d76c>] (__warn) from [<c012d820>] (warn_slowpath_fmt+0x98/0xbc)
     [<c012d820>] (warn_slowpath_fmt) from [<c0a1e0c0>] (arm_lpae_do_selftests+0x4b8/0x688)
     [<c0a1e0c0>] (arm_lpae_do_selftests) from [<c0102788>] (do_one_initcall+0x50/0x1b8)
     [<c0102788>] (do_one_initcall) from [<c0a0148c>] (kernel_init_freeable+0x1ec/0x25c)
     [<c0a0148c>] (kernel_init_freeable) from [<c06e6dc4>] (kernel_init+0x8/0x110)
     [<c06e6dc4>] (kernel_init) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
     Exception stack(0xee867fb0 to 0xee867ff8)
     7fa0:                                     00000000 00000000 00000000 00000000
     7fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
     7fe0: 00000000 00000000 00000000 00000000 00000013 00000000
     ---[ end trace 89e831c50a111e7d ]---
     arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 32-bit
     arm-lpae io-pgtable: data: 3 levels, 0x20 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 36
     [warning as above]
     arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 36-bit
     arm-lpae io-pgtable: data: 3 levels, 0x200 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 40
     [warning as above]
     arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 40-bit
     arm-lpae io-pgtable: data: 4 levels, 0x10 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 42
     [warning as above]
     arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 42-bit
     arm-lpae io-pgtable: data: 4 levels, 0x40 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 44
     [warning as above]
     arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 44-bit
     arm-lpae io-pgtable: data: 4 levels, 0x100 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x40201000, IAS 48
     [warning as above]
     arm-lpae io-pgtable: cfg: pgsize_bitmap 0x40201000, ias 48-bit
     arm-lpae io-pgtable: data: 4 levels, 0x1000 pgd_size, 12 pg_shift, 9 bits_per_level, pgd @ (ptrval)
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 32
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 36
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 40
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 42
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 44
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x02004000, IAS 48
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 32
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 36
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 40
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 42
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 44
     arm-lpae io-pgtable: selftest: pgsize_bitmap 0x20010000, IAS 48
     arm-lpae io-pgtable: selftest: completed with 12 PASS 6 FAIL
Quoted 5 lines

Any suggestions how to fix this properly?

Thanks,
Stephan

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu