iommu: arm smmu: driver updates for 3.12

PATCH New Will Deacon 2 participants 9 messages Jul 31, 2013 – Aug 15, 2013 IOMMU

lore

Share & tools

lore

Will Deacon

author

2013-07-31 18:21 UTC #0 lore raw

cover iommu: arm smmu: driver updates for 3.12

Hi all,

Here's a set of small drivers updates to the ARM SMMU driver that went in during the merge window. The main changes are:

- Removal of a broken big-endian special case when setting the pgd
- Better error reporting when running with a single, combined IRQ line
- Removing the dynamic use of VMIDs for stage-1 mappings. This is basically to make life easier for hypervisors, who apparently have a hard life if guest OSs allocate their own VMIDs
- Simplification of the VMID/ASID allocation scheme based on the fact that we're just trying to assign one ID per context-bank.

I've tested these patches with an AArch32 MMU-500 model and not seen any problems running stage-1 DMA transactions through the SMMU.

All feedback welcome,

Will

Will Deacon (4):
iommu: arm smmu: remove broken big-endian check
iommu: arm smmu: tighten up global fault reporting
iommu: arm smmu: don't use VMIDs for stage-1 translations
iommu: arm smmu: simplify VMID and ASID allocation

 drivers/iommu/arm-smmu.c | 93 +++++++++++++++++++++++++++++-------------------
 1 file changed, 56 insertions(+), 37 deletions(-)

Signature · 1 line

1.8.2.2

Will Deacon

author

2013-07-31 18:21 UTC #1 lore raw

PATCH 1/4 iommu: arm smmu: remove broken big-endian check

The bottom word of the pgd should always be written to the low half of the TTBR, so we don't need to swap anything for big-endian.

Signed-off-by Will Deacon <[email protected]>

---
 drivers/iommu/arm-smmu.c | 6 ------
 1 file changed, 6 deletions(-)

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

index ebd0a4c..da8af45 100644
@@ -687,15 +687,9 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 
 	/* TTBR0 */
 	reg = __pa(root_cfg->pgd);
-#ifndef __BIG_ENDIAN
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
 	reg = (phys_addr_t)__pa(root_cfg->pgd) >> 32;
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI);
-#else
-	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI);
-	reg = (phys_addr_t)__pa(root_cfg->pgd) >> 32;
-	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
-#endif
 
 	/*
 	 * TTBCR

Signature · 1 line

1.8.2.2

Will Deacon

author

2013-07-31 18:21 UTC #2 lore raw

PATCH 2/4 iommu: arm smmu: tighten up global fault reporting

In reply to Will Deacon

On systems which use a single, combined irq line for the SMMU, context faults may result in us spuriously reporting global faults with zero status registers.

This patch fixes up the fsr checks in both the context and global fault interrupt handlers, so that we only report the fault if the fsr indicates something did indeed go awry.

Signed-off-by Will Deacon <[email protected]>

---
 drivers/iommu/arm-smmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/iommu/arm-smmu.c +5 -2

index da8af45..3a595bb 100644
@@ -305,7 +305,7 @@
 #define FSR_IGN				(FSR_AFF | FSR_ASF | FSR_TLBMCF |	\
 					 FSR_TLBLKF)
 #define FSR_FAULT			(FSR_MULTI | FSR_SS | FSR_UUT |		\
-					 FSR_EF | FSR_PF | FSR_TF)
+					 FSR_EF | FSR_PF | FSR_TF | FSR_IGN)
 
 #define FSYNR0_WNR			(1 << 4)

@@ -590,6 +590,9 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 
 	gfsr = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSR);
+	if (!gfsr)
+		return IRQ_NONE;
+
 	gfsynr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR0);
 	gfsynr1 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR1);
 	gfsynr2 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sGFSYNR2);

@@ -601,7 +604,7 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
 		gfsr, gfsynr0, gfsynr1, gfsynr2);
 
 	writel(gfsr, gr0_base + ARM_SMMU_GR0_sGFSR);
-	return IRQ_NONE;
+	return IRQ_HANDLED;
 }
 
 static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)

Signature · 1 line

1.8.2.2

Will Deacon

author

2013-07-31 18:21 UTC #3 lore raw

PATCH 3/4 iommu: arm smmu: don't use VMIDs for stage-1 translations

In reply to Will Deacon

Although permitted by the architecture, using VMIDs for stage-1 translations causes a complete nightmare for hypervisors, who end up having to virtualise the VMID space across VMs, which may be using multiple VMIDs each.

To make life easier for hypervisors (which might just decide not to support this VMID virtualisation), this patch reworks the stage-1 context-bank TLB invalidation so that:

- Stage-1 mappings are marked non-global in the ptes
- Each Stage-1 context-bank is assigned an ASID in TTBR0
- VMID 0 is reserved for Stage-1 context-banks

This allows the hypervisor to overwrite the Stage-1 VMID in the CBAR when trapping the write from the guest.

Signed-off-by Will Deacon <[email protected]>

---
 drivers/iommu/arm-smmu.c | 59 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 13 deletions(-)

drivers/iommu/arm-smmu.c +46 ~~-13~~

index 3a595bb..07a0308 100644
@@ -87,6 +87,7 @@
 #define ARM_SMMU_PTE_AP_UNPRIV		(((pteval_t)1) << 6)
 #define ARM_SMMU_PTE_AP_RDONLY		(((pteval_t)2) << 6)
 #define ARM_SMMU_PTE_ATTRINDX_SHIFT	2
+#define ARM_SMMU_PTE_nG			(((pteval_t)1) << 11)
 
 /* Stage-2 PTE */
 #define ARM_SMMU_PTE_HAP_FAULT		(((pteval_t)0) << 6)

@@ -223,6 +224,7 @@
 #define ARM_SMMU_CB_FAR_LO		0x60
 #define ARM_SMMU_CB_FAR_HI		0x64
 #define ARM_SMMU_CB_FSYNR0		0x68
+#define ARM_SMMU_CB_S1_TLBIASID		0x610
 
 #define SCTLR_S1_ASIDPNE		(1 << 12)
 #define SCTLR_CFCFG			(1 << 7)

@@ -282,6 +284,8 @@
 #define TTBCR2_ADDR_44			4
 #define TTBCR2_ADDR_48			5
 
+#define TTBRn_HI_ASID_SHIFT		16
+
 #define MAIR_ATTR_SHIFT(n)		((n) << 3)
 #define MAIR_ATTR_MASK			0xff
 #define MAIR_ATTR_DEVICE		0x04

@@ -533,6 +537,23 @@ static void arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
 	}
 }
 
+static void arm_smmu_tlb_inv_context(struct arm_smmu_cfg *cfg)
+{
+	struct arm_smmu_device *smmu = cfg->smmu;
+	void __iomem *base = ARM_SMMU_GR0(smmu);
+	bool stage1 = cfg->cbar != CBAR_TYPE_S2_TRANS;
+
+	if (stage1) {
+		base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
+		writel_relaxed(cfg->vmid, base + ARM_SMMU_CB_S1_TLBIASID);
+	} else {
+		base = ARM_SMMU_GR0(smmu);
+		writel_relaxed(cfg->vmid, base + ARM_SMMU_GR0_TLBIVMID);
+	}
+
+	arm_smmu_tlb_sync(smmu);
+}
+
 static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
 {
 	int flags, ret;

@@ -621,14 +642,15 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
 
 	/* CBAR */
-	reg = root_cfg->cbar |
-	      (root_cfg->vmid << CBAR_VMID_SHIFT);
+	reg = root_cfg->cbar;
 	if (smmu->version == 1)
 	      reg |= root_cfg->irptndx << CBAR_IRPTNDX_SHIFT;
 
 	/* Use the weakest memory type, so it is overridden by the pte */
 	if (stage1)
 		reg |= (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
+	else
+		reg |= root_cfg->vmid << CBAR_VMID_SHIFT;
 	writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(root_cfg->cbndx));
 
 	if (smmu->version > 1) {

@@ -692,6 +714,8 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 	reg = __pa(root_cfg->pgd);
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
 	reg = (phys_addr_t)__pa(root_cfg->pgd) >> 32;
+	if (stage1)
+		reg |= root_cfg->vmid << TTBRn_HI_ASID_SHIFT;
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI);
 
 	/*

@@ -747,10 +771,6 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 		writel_relaxed(reg, cb_base + ARM_SMMU_CB_S1_MAIR0);
 	}
 
-	/* Nuke the TLB */
-	writel_relaxed(root_cfg->vmid, gr0_base + ARM_SMMU_GR0_TLBIVMID);
-	arm_smmu_tlb_sync(smmu);
-
 	/* SCTLR */
 	reg = SCTLR_CFCFG | SCTLR_CFIE | SCTLR_CFRE | SCTLR_M | SCTLR_EAE_SBOP;
 	if (stage1)

@@ -787,7 +807,8 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		return -ENODEV;
 	}
 
-	ret = __arm_smmu_alloc_bitmap(smmu->vmid_map, 0, ARM_SMMU_NUM_VMIDS);
+	/* VMID zero is reserved for stage-1 mappings */
+	ret = __arm_smmu_alloc_bitmap(smmu->vmid_map, 1, ARM_SMMU_NUM_VMIDS);
 	if (IS_ERR_VALUE(ret))
 		return ret;

@@ -847,11 +868,17 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 	struct arm_smmu_domain *smmu_domain = domain->priv;
 	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
 	struct arm_smmu_device *smmu = root_cfg->smmu;
+	void __iomem *cb_base;
 	int irq;
 
 	if (!smmu)
 		return;
 
+	/* Disable the context bank and nuke the TLB before freeing it. */
+	cb_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, root_cfg->cbndx);
+	writel_relaxed(0, cb_base + ARM_SMMU_CB_SCTLR);
+	arm_smmu_tlb_inv_context(root_cfg);
+
 	if (root_cfg->irptndx != -1) {
 		irq = smmu->irqs[smmu->num_global_irqs + root_cfg->irptndx];
 		free_irq(irq, domain);

@@ -956,6 +983,11 @@ static void arm_smmu_free_pgtables(struct arm_smmu_domain *smmu_domain)
 static void arm_smmu_domain_destroy(struct iommu_domain *domain)
 {
 	struct arm_smmu_domain *smmu_domain = domain->priv;
+
+	/*
+	 * Free the domain resources. We assume that all devices have
+	 * already been detached.
+	 */
 	arm_smmu_destroy_domain_context(domain);
 	arm_smmu_free_pgtables(smmu_domain);
 	kfree(smmu_domain);

@@ -1196,7 +1228,7 @@ static int arm_smmu_alloc_init_pte(struct arm_smmu_device *smmu, pmd_t *pmd,
 	}
 
 	if (stage == 1) {
-		pteval |= ARM_SMMU_PTE_AP_UNPRIV;
+		pteval |= ARM_SMMU_PTE_AP_UNPRIV | ARM_SMMU_PTE_nG;
 		if (!(flags & IOMMU_WRITE) && (flags & IOMMU_READ))
 			pteval |= ARM_SMMU_PTE_AP_RDONLY;

@@ -1412,13 +1444,9 @@ static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
 {
 	int ret;
 	struct arm_smmu_domain *smmu_domain = domain->priv;
-	struct arm_smmu_cfg *root_cfg = &smmu_domain->root_cfg;
-	struct arm_smmu_device *smmu = root_cfg->smmu;
-	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
 
 	ret = arm_smmu_handle_mapping(smmu_domain, iova, 0, size, 0);
-	writel_relaxed(root_cfg->vmid, gr0_base + ARM_SMMU_GR0_TLBIVMID);
-	arm_smmu_tlb_sync(smmu);
+	arm_smmu_tlb_inv_context(&smmu_domain->root_cfg);
 	return ret ? ret : size;
 }

@@ -1541,6 +1569,7 @@ static struct iommu_ops arm_smmu_ops = {
 static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
 {
 	void __iomem *gr0_base = ARM_SMMU_GR0(smmu);
+	void __iomem *sctlr_base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB_SCTLR;
 	int i = 0;
 	u32 scr0 = readl_relaxed(gr0_base + ARM_SMMU_GR0_sCR0);

@@ -1550,6 +1579,10 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
 		writel_relaxed(S2CR_TYPE_BYPASS, gr0_base + ARM_SMMU_GR0_S2CR(i));
 	}
 
+	/* Make sure all context banks are disabled */
+	for (i = 0; i < smmu->num_context_banks; ++i)
+		writel_relaxed(0, sctlr_base + ARM_SMMU_CB(smmu, i));
+
 	/* Invalidate the TLB, just in case */
 	writel_relaxed(0, gr0_base + ARM_SMMU_GR0_STLBIALL);
 	writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH);

Signature · 1 line

1.8.2.2

Will Deacon

author

2013-07-31 18:21 UTC #4 lore raw

PATCH 4/4 iommu: arm smmu: simplify VMID and ASID allocation

In reply to Will Deacon

We only use ASIDs and VMIDs to identify individual stage-1 and stage-2 context-banks respectively, so rather than allocate these separately from the context-banks, just calculate them based on the context bank index.

Note that VMIDs are offset by 1, since VMID 0 is reserved for stage-1. This doesn't cause us any issues with the numberspaces, since the maximum number of context banks is half the minimum number of VMIDs.

Signed-off-by Will Deacon <[email protected]>

---
 drivers/iommu/arm-smmu.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

drivers/iommu/arm-smmu.c +11 ~~-22~~

index 07a0308..f417e89 100644
@@ -56,9 +56,6 @@
 /* Maximum number of mapping groups per SMMU */
 #define ARM_SMMU_MAX_SMRS		128
 
-/* Number of VMIDs per SMMU */
-#define ARM_SMMU_NUM_VMIDS		256
-
 /* SMMU global address space */
 #define ARM_SMMU_GR0(smmu)		((smmu)->base)
 #define ARM_SMMU_GR1(smmu)		((smmu)->base + (smmu)->pagesize)

@@ -369,21 +366,21 @@ struct arm_smmu_device {
 	u32				num_context_irqs;
 	unsigned int			*irqs;
 
-	DECLARE_BITMAP(vmid_map, ARM_SMMU_NUM_VMIDS);
-
 	struct list_head		list;
 	struct rb_root			masters;
 };
 
 struct arm_smmu_cfg {
 	struct arm_smmu_device		*smmu;
-	u8				vmid;
 	u8				cbndx;
 	u8				irptndx;
 	u32				cbar;
 	pgd_t				*pgd;
 };
 
+#define ARM_SMMU_CB_ASID(cfg)		((cfg)->cbndx)
+#define ARM_SMMU_CB_VMID(cfg)		((cfg)->cbndx + 1)
+
 struct arm_smmu_domain {
 	/*
 	 * A domain can span across multiple, chained SMMUs and requires

@@ -545,10 +542,12 @@ static void arm_smmu_tlb_inv_context(struct arm_smmu_cfg *cfg)
 
 	if (stage1) {
 		base = ARM_SMMU_CB_BASE(smmu) + ARM_SMMU_CB(smmu, cfg->cbndx);
-		writel_relaxed(cfg->vmid, base + ARM_SMMU_CB_S1_TLBIASID);
+		writel_relaxed(ARM_SMMU_CB_ASID(cfg),
+			       base + ARM_SMMU_CB_S1_TLBIASID);
 	} else {
 		base = ARM_SMMU_GR0(smmu);
-		writel_relaxed(cfg->vmid, base + ARM_SMMU_GR0_TLBIVMID);
+		writel_relaxed(ARM_SMMU_CB_VMID(cfg),
+			       base + ARM_SMMU_GR0_TLBIVMID);
 	}
 
 	arm_smmu_tlb_sync(smmu);

@@ -650,7 +649,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 	if (stage1)
 		reg |= (CBAR_S1_MEMATTR_WB << CBAR_S1_MEMATTR_SHIFT);
 	else
-		reg |= root_cfg->vmid << CBAR_VMID_SHIFT;
+		reg |= ARM_SMMU_CB_VMID(root_cfg) << CBAR_VMID_SHIFT;
 	writel_relaxed(reg, gr1_base + ARM_SMMU_GR1_CBAR(root_cfg->cbndx));
 
 	if (smmu->version > 1) {

@@ -715,7 +714,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain)
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_LO);
 	reg = (phys_addr_t)__pa(root_cfg->pgd) >> 32;
 	if (stage1)
-		reg |= root_cfg->vmid << TTBRn_HI_ASID_SHIFT;
+		reg |= ARM_SMMU_CB_ASID(root_cfg) << TTBRn_HI_ASID_SHIFT;
 	writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBR0_HI);
 
 	/*

@@ -807,12 +806,6 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		return -ENODEV;
 	}
 
-	/* VMID zero is reserved for stage-1 mappings */
-	ret = __arm_smmu_alloc_bitmap(smmu->vmid_map, 1, ARM_SMMU_NUM_VMIDS);
-	if (IS_ERR_VALUE(ret))
-		return ret;
-
-	root_cfg->vmid = ret;
 	if (smmu->features & ARM_SMMU_FEAT_TRANS_NESTED) {
 		/*
 		 * We will likely want to change this if/when KVM gets

@@ -831,10 +824,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 	ret = __arm_smmu_alloc_bitmap(smmu->context_map, start,
 				      smmu->num_context_banks);
 	if (IS_ERR_VALUE(ret))
-		goto out_free_vmid;
+		return ret;
 
 	root_cfg->cbndx = ret;
-
 	if (smmu->version == 1) {
 		root_cfg->irptndx = atomic_inc_return(&smmu->irptndx);
 		root_cfg->irptndx %= smmu->num_context_irqs;

@@ -858,8 +850,6 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 
 out_free_context:
 	__arm_smmu_free_bitmap(smmu->context_map, root_cfg->cbndx);
-out_free_vmid:
-	__arm_smmu_free_bitmap(smmu->vmid_map, root_cfg->vmid);
 	return ret;
 }

@@ -884,7 +874,6 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
 		free_irq(irq, domain);
 	}
 
-	__arm_smmu_free_bitmap(smmu->vmid_map, root_cfg->vmid);
 	__arm_smmu_free_bitmap(smmu->context_map, root_cfg->cbndx);
 }

@@ -1936,7 +1925,7 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
 		of_node_put(master->of_node);
 	}
 
-	if (!bitmap_empty(smmu->vmid_map, ARM_SMMU_NUM_VMIDS))
+	if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS))
 		dev_err(dev, "removing device with active domains!\n");
 
 	for (i = 0; i < smmu->num_global_irqs; ++i)

Signature · 1 line

1.8.2.2

Will Deacon

author

2013-08-12 11:45 UTC #5 lore raw

cover iommu: arm smmu: driver updates for 3.12

In reply to Will Deacon

Hi Joerg,

Quoted 11 lines · Will Deacon

Here's a set of small drivers updates to the ARM SMMU driver that went in during the merge window. The main changes are:
- Removal of a broken big-endian special case when setting the pgd
- Better error reporting when running with a single, combined IRQ line
- Removing the dynamic use of VMIDs for stage-1 mappings. This is basically to make life easier for hypervisors, who apparently have a hard life if guest OSs allocate their own VMIDs
- Simplification of the VMID/ASID allocation scheme based on the fact that we're just trying to assign one ID per context-bank.

Quoted 2 lines

I've tested these patches with an AArch32 MMU-500 model and not seen any problems running stage-1 DMA transactions through the SMMU.

Could you queue these for 3.12 please?

Thanks,

Will

joro at 8bytes.org

maintainer

2013-08-14 10:13 UTC #6 lore raw

cover iommu: arm smmu: driver updates for 3.12

Hi Will,

Quoted 1 line · Will Deacon

Could you queue these for 3.12 please?

Done. Queued the patches into the arm/smmu branch and will include it into next, thanks.

I also fixed the commit-headlines. Please prefix your iommu-patches with 'iommu/arm-smmu: ' next time and start the description after the colon with a capital letter.

Thanks,

Joerg

Will Deacon

author

2013-08-15 17:23 UTC #7 lore raw

cover iommu: arm smmu: driver updates for 3.12

Quoted 1 line · On Wed, Aug 14, 2013 at 11:13:18AM +0100, joro at 8bytes.org wrote:

Hi Will,

Hello Joerg,

Quoted 9 lines · Will Deacon

Quoted 1 line
Could you queue these for 3.12 please?
Done. Queued the patches into the arm/smmu branch and will include it into next, thanks.
I also fixed the commit-headlines. Please prefix your iommu-patches with 'iommu/arm-smmu: ' next time and start the description after the colon with a capital letter.

Sorry, I wasn't aware of the conventions here. Will follow this next time.

Cheers,

Will

joro at 8bytes.org

maintainer

2013-08-15 18:04 UTC #8 lore raw

cover iommu: arm smmu: driver updates for 3.12

Hi Will,

Quoted 1 line · Will Deacon

Sorry, I wasn't aware of the conventions here. Will follow this next time.

No problem, I am just starting to comuticating them more, so that I havn't to fix the commit-msg myself all the time anymore :)

Joerg