[PATCH v2 00/10] Renesas ipmmu-vmsa: Miscellaneous cleanups and fixes

[PATCH v2 00/10] Renesas ipmmu-vmsa: Miscellaneous cleanups and fixes

[Laurent Pinchart]

Hello,

This patch set cleans up and fixes small issues in the ipmmu-vmsa driver. The
patches are based on top of "[PATCH v3] iommu: Add driver for Renesas
VMSA-compatible IPMMU" that adds the ipmmu-vmsa driver.

The most interesting part of this series is the rewrite of the page table
management code. The IOMMU core guarantees that the map and unmap operations
will always be called only with page sizes advertised by the driver. We can
use that assumption to remove loops of PGD and PMD entries, simplifying the
code.

Joerg, is there still time to get this merged in v3.16 ? The patches have all
been posted previously and the only comment I've received was about a missing
#define in patch 04/10.

Cc: Joerg Roedel <joro@8bytes.org>

Changes since v1:

- Add missing IPMMU_PTRS_PER_PUD definition in patch 04/10

Laurent Pinchart (10):
  iommu/ipmmu-vmsa: Refactor micro-TLB lookup
  iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or
    attachment
  iommu/ipmmu-vmsa: Fix the supported page sizes
  iommu/ipmmu-vmsa: Define driver-specific page directory sizes
  iommu/ipmmu-vmsa: Set the PTE contiguous hint bit when possible
  iommu/ipmmu-vmsa: PMD is never folded, PUD always is
  iommu/ipmmu-vmsa: Rewrite page table management
  iommu/ipmmu-vmsa: Support 2MB mappings
  iommu/ipmmu-vmsa: Remove stage 2 PTE bits definitions
  iommu/ipmmu-vmsa: Support clearing mappings

 drivers/iommu/ipmmu-vmsa.c | 535 ++++++++++++++++++++++++++++++---------------
 1 file changed, 361 insertions(+), 174 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 01/10] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Laurent Pinchart]

Cache the micro-TLB number in archdata allocated in the .add_device
handler instead of looking it up when the deviced is attached and
detached. This simplifies the .attach_dev and .detach_dev operations and
prepares for DT support.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 92 ++++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 40 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index a8a940a..49e00f7 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -44,6 +44,11 @@ struct ipmmu_vmsa_domain {
 	pgd_t *pgd;
 };
 
+struct ipmmu_vmsa_archdata {
+	struct ipmmu_vmsa_device *mmu;
+	unsigned int utlb;
+};
+
 static DEFINE_SPINLOCK(ipmmu_devices_lock);
 static LIST_HEAD(ipmmu_devices);
 
@@ -265,14 +270,19 @@ static void ipmmu_tlb_invalidate(struct ipmmu_vmsa_domain *domain)
  * Enable MMU translation for the microTLB.
  */
 static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
-			      const struct ipmmu_vmsa_master *master)
+			      unsigned int utlb)
 {
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 
+	/*
+	 * TODO: Reference-count the microTLB as several bus masters can be
+	 * connected to the same microTLB.
+	 */
+
 	/* TODO: What should we set the ASID to ? */
-	ipmmu_write(mmu, IMUASID(master->utlb), 0);
+	ipmmu_write(mmu, IMUASID(utlb), 0);
 	/* TODO: Do we need to flush the microTLB ? */
-	ipmmu_write(mmu, IMUCTR(master->utlb),
+	ipmmu_write(mmu, IMUCTR(utlb),
 		    IMUCTR_TTSEL_MMU(domain->context_id) | IMUCTR_FLUSH |
 		    IMUCTR_MMUEN);
 }
@@ -281,11 +291,11 @@ static void ipmmu_utlb_enable(struct ipmmu_vmsa_domain *domain,
  * Disable MMU translation for the microTLB.
  */
 static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
-			       const struct ipmmu_vmsa_master *master)
+			       unsigned int utlb)
 {
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 
-	ipmmu_write(mmu, IMUCTR(master->utlb), 0);
+	ipmmu_write(mmu, IMUCTR(utlb), 0);
 }
 
 static void ipmmu_flush_pgtable(struct ipmmu_vmsa_device *mmu, void *addr,
@@ -674,21 +684,6 @@ static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
  * IOMMU Operations
  */
 
-static const struct ipmmu_vmsa_master *
-ipmmu_find_master(struct ipmmu_vmsa_device *ipmmu, struct device *dev)
-{
-	const struct ipmmu_vmsa_master *master = ipmmu->pdata->masters;
-	const char *devname = dev_name(dev);
-	unsigned int i;
-
-	for (i = 0; i < ipmmu->pdata->num_masters; ++i, ++master) {
-		if (strcmp(master->name, devname) = 0)
-			return master;
-	}
-
-	return NULL;
-}
-
 static int ipmmu_domain_init(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain;
@@ -727,9 +722,9 @@ static void ipmmu_domain_destroy(struct iommu_domain *io_domain)
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			       struct device *dev)
 {
-	struct ipmmu_vmsa_device *mmu = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_device *mmu = archdata->mmu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
-	const struct ipmmu_vmsa_master *master;
 	unsigned long flags;
 	int ret = 0;
 
@@ -759,11 +754,7 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 	if (ret < 0)
 		return ret;
 
-	master = ipmmu_find_master(mmu, dev);
-	if (!master)
-		return -EINVAL;
-
-	ipmmu_utlb_enable(domain, master);
+	ipmmu_utlb_enable(domain, archdata->utlb);
 
 	return 0;
 }
@@ -771,14 +762,10 @@ static int ipmmu_attach_device(struct iommu_domain *io_domain,
 static void ipmmu_detach_device(struct iommu_domain *io_domain,
 				struct device *dev)
 {
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
-	const struct ipmmu_vmsa_master *master;
 
-	master = ipmmu_find_master(domain->mmu, dev);
-	if (!master)
-		return;
-
-	ipmmu_utlb_disable(domain, master);
+	ipmmu_utlb_disable(domain, archdata->utlb);
 
 	/*
 	 * TODO: Optimize by disabling the context when no device is attached.
@@ -839,11 +826,26 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
 	return __pfn_to_phys(pte_pfn(pte)) | (iova & ~PAGE_MASK);
 }
 
+static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
+{
+	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
+	const char *devname = dev_name(dev);
+	unsigned int i;
+
+	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
+		if (strcmp(master->name, devname) = 0)
+			return master->utlb;
+	}
+
+	return -1;
+}
+
 static int ipmmu_add_device(struct device *dev)
 {
-	const struct ipmmu_vmsa_master *master = NULL;
+	struct ipmmu_vmsa_archdata *archdata;
 	struct ipmmu_vmsa_device *mmu;
 	struct iommu_group *group;
+	int utlb = -1;
 	int ret;
 
 	if (dev->archdata.iommu) {
@@ -856,10 +858,10 @@ static int ipmmu_add_device(struct device *dev)
 	spin_lock(&ipmmu_devices_lock);
 
 	list_for_each_entry(mmu, &ipmmu_devices, list) {
-		master = ipmmu_find_master(mmu, dev);
-		if (master) {
+		utlb = ipmmu_find_utlb(mmu, dev);
+		if (utlb >= 0) {
 			/*
-			 * TODO Take a reference to the master to protect
+			 * TODO Take a reference to the MMU to protect
 			 * against device removal.
 			 */
 			break;
@@ -868,10 +870,10 @@ static int ipmmu_add_device(struct device *dev)
 
 	spin_unlock(&ipmmu_devices_lock);
 
-	if (!master)
+	if (utlb < 0)
 		return -ENODEV;
 
-	if (!master->utlb >= mmu->num_utlbs)
+	if (utlb >= mmu->num_utlbs)
 		return -EINVAL;
 
 	/* Create a device group and add the device to it. */
@@ -889,7 +891,15 @@ static int ipmmu_add_device(struct device *dev)
 		return ret;
 	}
 
-	dev->archdata.iommu = mmu;
+	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
+	if (!archdata) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	archdata->mmu = mmu;
+	archdata->utlb = utlb;
+	dev->archdata.iommu = archdata;
 
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -923,6 +933,7 @@ static int ipmmu_add_device(struct device *dev)
 	return 0;
 
 error:
+	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 	iommu_group_remove_device(dev);
 	return ret;
@@ -932,6 +943,7 @@ static void ipmmu_remove_device(struct device *dev)
 {
 	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
+	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 }
 
-- 
1.8.5.5

[PATCH v2 02/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 49e00f7..49dbedd 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -917,7 +917,8 @@ static int ipmmu_add_device(struct device *dev)
 							SZ_1G, SZ_2G);
 		if (IS_ERR(mapping)) {
 			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
-			return PTR_ERR(mapping);
+			ret = PTR_ERR(mapping);
+			goto error;
 		}
 
 		mmu->mapping = mapping;
@@ -933,6 +934,7 @@ static int ipmmu_add_device(struct device *dev)
 	return 0;
 
 error:
+	arm_iommu_release_mapping(mmu->mapping);
 	kfree(dev->archdata.iommu);
 	dev->archdata.iommu = NULL;
 	iommu_group_remove_device(dev);
-- 
1.8.5.5

[PATCH v2 03/10] iommu/ipmmu-vmsa: Fix the supported page sizes

[Laurent Pinchart]

The hardware supports 2MB page sizes, not 1MB.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 49dbedd..313b5b9 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -959,7 +959,7 @@ static struct iommu_ops ipmmu_ops = {
 	.iova_to_phys = ipmmu_iova_to_phys,
 	.add_device = ipmmu_add_device,
 	.remove_device = ipmmu_remove_device,
-	.pgsize_bitmap = SZ_1M | SZ_64K | SZ_4K,
+	.pgsize_bitmap = SZ_2M | SZ_64K | SZ_4K,
 };
 
 /* -----------------------------------------------------------------------------
-- 
1.8.5.5

[PATCH v2 04/10] iommu/ipmmu-vmsa: Define driver-specific page directory sizes

[Laurent Pinchart]

The PTRS_PER_(PUD|PGD|PMD|PTE) macros evaluate to different values
depending on whether LPAE is enabled. The IPMMU driver uses a long
descriptor format regardless of LPAE, making those macros mismatch the
IPMMU configuration on non-LPAE systems.

Replace the macros by driver-specific versions that always evaluate to
the right value.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 313b5b9..b34a4b7b 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -209,6 +209,11 @@ static LIST_HEAD(ipmmu_devices);
 #define ARM_VMSA_PTE_MEMATTR_NC		(((pteval_t)0x5) << 2)
 #define ARM_VMSA_PTE_MEMATTR_DEV	(((pteval_t)0x1) << 2)
 
+#define IPMMU_PTRS_PER_PTE		512
+#define IPMMU_PTRS_PER_PMD		512
+#define IPMMU_PTRS_PER_PGD		4
+#define IPMMU_PTRS_PER_PUD		1
+
 /* -----------------------------------------------------------------------------
  * Read/Write Access
  */
@@ -327,7 +332,7 @@ static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 
 	/* TTBR0 */
 	ipmmu_flush_pgtable(domain->mmu, domain->pgd,
-			    PTRS_PER_PGD * sizeof(*domain->pgd));
+			    IPMMU_PTRS_PER_PGD * sizeof(*domain->pgd));
 	ttbr = __pa(domain->pgd);
 	ipmmu_ctx_write(domain, IMTTLBR0, ttbr);
 	ipmmu_ctx_write(domain, IMTTUBR0, ttbr >> 32);
@@ -469,7 +474,7 @@ static void ipmmu_free_pmds(pud_t *pud)
 	unsigned int i;
 
 	pmd = pmd_base;
-	for (i = 0; i < PTRS_PER_PMD; ++i) {
+	for (i = 0; i < IPMMU_PTRS_PER_PMD; ++i) {
 		if (pmd_none(*pmd))
 			continue;
 
@@ -486,7 +491,7 @@ static void ipmmu_free_puds(pgd_t *pgd)
 	unsigned int i;
 
 	pud = pud_base;
-	for (i = 0; i < PTRS_PER_PUD; ++i) {
+	for (i = 0; i < IPMMU_PTRS_PER_PUD; ++i) {
 		if (pud_none(*pud))
 			continue;
 
@@ -509,7 +514,7 @@ static void ipmmu_free_pgtables(struct ipmmu_vmsa_domain *domain)
 	 * tables.
 	 */
 	pgd = pgd_base;
-	for (i = 0; i < PTRS_PER_PGD; ++i) {
+	for (i = 0; i < IPMMU_PTRS_PER_PGD; ++i) {
 		if (pgd_none(*pgd))
 			continue;
 		ipmmu_free_puds(pgd);
@@ -694,7 +699,7 @@ static int ipmmu_domain_init(struct iommu_domain *io_domain)
 
 	spin_lock_init(&domain->lock);
 
-	domain->pgd = kzalloc(PTRS_PER_PGD * sizeof(pgd_t), GFP_KERNEL);
+	domain->pgd = kzalloc(IPMMU_PTRS_PER_PGD * sizeof(pgd_t), GFP_KERNEL);
 	if (!domain->pgd) {
 		kfree(domain);
 		return -ENOMEM;
-- 
1.8.5.5

[PATCH v2 05/10] iommu/ipmmu-vmsa: Set the PTE contiguous hint bit when possible

[Laurent Pinchart]

The contiguous hint bit signals to the IOMMU that a range of 16 PTEs
refer to physically contiguous memory. It improves performances by
dividing the number of TLB lookups by 16, effectively implementing 64kB
page sizes.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index b34a4b7b..150e18e 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -209,6 +209,9 @@ static LIST_HEAD(ipmmu_devices);
 #define ARM_VMSA_PTE_MEMATTR_NC		(((pteval_t)0x5) << 2)
 #define ARM_VMSA_PTE_MEMATTR_DEV	(((pteval_t)0x1) << 2)
 
+#define ARM_VMSA_PTE_CONT_ENTRIES	16
+#define ARM_VMSA_PTE_CONT_SIZE		(PAGE_SIZE * ARM_VMSA_PTE_CONT_ENTRIES)
+
 #define IPMMU_PTRS_PER_PTE		512
 #define IPMMU_PTRS_PER_PMD		512
 #define IPMMU_PTRS_PER_PGD		4
@@ -569,10 +572,44 @@ static int ipmmu_alloc_init_pte(struct ipmmu_vmsa_device *mmu, pmd_t *pmd,
 	pteval |= ARM_VMSA_PTE_SH_IS;
 	start = pte;
 
-	/* Install the page table entries. */
+	/*
+	 * Install the page table entries.
+	 *
+	 * Set the contiguous hint in the PTEs where possible. The hint
+	 * indicates a series of ARM_VMSA_PTE_CONT_ENTRIES PTEs mapping a
+	 * physically contiguous region with the following constraints:
+	 *
+	 * - The region start is aligned to ARM_VMSA_PTE_CONT_SIZE
+	 * - Each PTE in the region has the contiguous hint bit set
+	 *
+	 * We don't support partial unmapping so there's no need to care about
+	 * clearing the contiguous hint from neighbour PTEs.
+	 */
 	do {
-		*pte++ = pfn_pte(pfn++, __pgprot(pteval));
-		addr += PAGE_SIZE;
+		unsigned long chunk_end;
+
+		/*
+		 * If the address is aligned to a contiguous region size and the
+		 * mapping size is large enough, process the largest possible
+		 * number of PTEs multiple of ARM_VMSA_PTE_CONT_ENTRIES.
+		 * Otherwise process the smallest number of PTEs to align the
+		 * address to a contiguous region size or to complete the
+		 * mapping.
+		 */
+		if (IS_ALIGNED(addr, ARM_VMSA_PTE_CONT_SIZE) &&
+		    end - addr >= ARM_VMSA_PTE_CONT_SIZE) {
+			chunk_end = round_down(end, ARM_VMSA_PTE_CONT_SIZE);
+			pteval |= ARM_VMSA_PTE_CONT;
+		} else {
+			chunk_end = min(ALIGN(addr, ARM_VMSA_PTE_CONT_SIZE),
+					end);
+			pteval &= ~ARM_VMSA_PTE_CONT;
+		}
+
+		do {
+			*pte++ = pfn_pte(pfn++, __pgprot(pteval));
+			addr += PAGE_SIZE;
+		} while (addr != chunk_end);
 	} while (addr != end);
 
 	ipmmu_flush_pgtable(mmu, start, sizeof(*pte) * (pte - start));
-- 
1.8.5.5

[PATCH v2 06/10] iommu/ipmmu-vmsa: PMD is never folded, PUD always is

[Laurent Pinchart]

The driver only supports the 3-level long descriptor format that has no
PUD and always has a PMD.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 66 +++++++---------------------------------------
 1 file changed, 9 insertions(+), 57 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 150e18e..d388749 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -215,7 +215,6 @@ static LIST_HEAD(ipmmu_devices);
 #define IPMMU_PTRS_PER_PTE		512
 #define IPMMU_PTRS_PER_PMD		512
 #define IPMMU_PTRS_PER_PGD		4
-#define IPMMU_PTRS_PER_PUD		1
 
 /* -----------------------------------------------------------------------------
  * Read/Write Access
@@ -465,6 +464,8 @@ static irqreturn_t ipmmu_irq(int irq, void *dev)
  * Page Table Management
  */
 
+#define pud_pgtable(pud) pfn_to_page(__phys_to_pfn(pud_val(pud) & PHYS_MASK))
+
 static void ipmmu_free_ptes(pmd_t *pmd)
 {
 	pgtable_t table = pmd_pgtable(*pmd);
@@ -473,10 +474,10 @@ static void ipmmu_free_ptes(pmd_t *pmd)
 
 static void ipmmu_free_pmds(pud_t *pud)
 {
-	pmd_t *pmd, *pmd_base = pmd_offset(pud, 0);
+	pmd_t *pmd = pmd_offset(pud, 0);
+	pgtable_t table;
 	unsigned int i;
 
-	pmd = pmd_base;
 	for (i = 0; i < IPMMU_PTRS_PER_PMD; ++i) {
 		if (pmd_none(*pmd))
 			continue;
@@ -485,24 +486,8 @@ static void ipmmu_free_pmds(pud_t *pud)
 		pmd++;
 	}
 
-	pmd_free(NULL, pmd_base);
-}
-
-static void ipmmu_free_puds(pgd_t *pgd)
-{
-	pud_t *pud, *pud_base = pud_offset(pgd, 0);
-	unsigned int i;
-
-	pud = pud_base;
-	for (i = 0; i < IPMMU_PTRS_PER_PUD; ++i) {
-		if (pud_none(*pud))
-			continue;
-
-		ipmmu_free_pmds(pud);
-		pud++;
-	}
-
-	pud_free(NULL, pud_base);
+	table = pud_pgtable(*pud);
+	__free_page(table);
 }
 
 static void ipmmu_free_pgtables(struct ipmmu_vmsa_domain *domain)
@@ -520,7 +505,7 @@ static void ipmmu_free_pgtables(struct ipmmu_vmsa_domain *domain)
 	for (i = 0; i < IPMMU_PTRS_PER_PGD; ++i) {
 		if (pgd_none(*pgd))
 			continue;
-		ipmmu_free_puds(pgd);
+		ipmmu_free_pmds((pud_t *)pgd);
 		pgd++;
 	}
 
@@ -624,7 +609,6 @@ static int ipmmu_alloc_init_pmd(struct ipmmu_vmsa_device *mmu, pud_t *pud,
 	pmd_t *pmd;
 	int ret;
 
-#ifndef __PAGETABLE_PMD_FOLDED
 	if (pud_none(*pud)) {
 		pmd = (pmd_t *)get_zeroed_page(GFP_ATOMIC);
 		if (!pmd)
@@ -636,7 +620,6 @@ static int ipmmu_alloc_init_pmd(struct ipmmu_vmsa_device *mmu, pud_t *pud,
 
 		pmd += pmd_index(addr);
 	} else
-#endif
 		pmd = pmd_offset(pud, addr);
 
 	do {
@@ -648,38 +631,6 @@ static int ipmmu_alloc_init_pmd(struct ipmmu_vmsa_device *mmu, pud_t *pud,
 	return ret;
 }
 
-static int ipmmu_alloc_init_pud(struct ipmmu_vmsa_device *mmu, pgd_t *pgd,
-				unsigned long addr, unsigned long end,
-				phys_addr_t phys, int prot)
-{
-	unsigned long next;
-	pud_t *pud;
-	int ret;
-
-#ifndef __PAGETABLE_PUD_FOLDED
-	if (pgd_none(*pgd)) {
-		pud = (pud_t *)get_zeroed_page(GFP_ATOMIC);
-		if (!pud)
-			return -ENOMEM;
-
-		ipmmu_flush_pgtable(mmu, pud, PAGE_SIZE);
-		*pgd = __pgd(__pa(pud) | PMD_NSTABLE | PMD_TYPE_TABLE);
-		ipmmu_flush_pgtable(mmu, pgd, sizeof(*pgd));
-
-		pud += pud_index(addr);
-	} else
-#endif
-		pud = pud_offset(pgd, addr);
-
-	do {
-		next = pud_addr_end(addr, end);
-		ret = ipmmu_alloc_init_pmd(mmu, pud, addr, next, phys, prot);
-		phys += next - addr;
-	} while (pud++, addr = next, addr < end);
-
-	return ret;
-}
-
 static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
 				unsigned long iova, phys_addr_t paddr,
 				size_t size, int prot)
@@ -707,7 +658,8 @@ static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
 	do {
 		unsigned long next = pgd_addr_end(iova, end);
 
-		ret = ipmmu_alloc_init_pud(mmu, pgd, iova, next, paddr, prot);
+		ret = ipmmu_alloc_init_pmd(mmu, (pud_t *)pgd, iova, next, paddr,
+					   prot);
 		if (ret)
 			break;
 
-- 
1.8.5.5

[PATCH v2 07/10] iommu/ipmmu-vmsa: Rewrite page table management

[Laurent Pinchart]

The IOMMU core will only call us with page sizes advertized as supported
by the driver. We can thus simplify the code by removing loops over PGD
and PMD entries.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 193 ++++++++++++++++++++-------------------------
 1 file changed, 86 insertions(+), 107 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index d388749..159e09a 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -517,118 +517,97 @@ static void ipmmu_free_pgtables(struct ipmmu_vmsa_domain *domain)
  * functions as they would flush the CPU TLB.
  */
 
-static int ipmmu_alloc_init_pte(struct ipmmu_vmsa_device *mmu, pmd_t *pmd,
-				unsigned long addr, unsigned long end,
-				phys_addr_t phys, int prot)
+static pte_t *ipmmu_alloc_pte(struct ipmmu_vmsa_device *mmu, pmd_t *pmd,
+			      unsigned long iova)
 {
-	unsigned long pfn = __phys_to_pfn(phys);
-	pteval_t pteval = ARM_VMSA_PTE_PAGE | ARM_VMSA_PTE_NS | ARM_VMSA_PTE_AF
-			| ARM_VMSA_PTE_XN;
-	pte_t *pte, *start;
+	pte_t *pte;
 
-	if (pmd_none(*pmd)) {
-		/* Allocate a new set of tables */
-		pte = (pte_t *)get_zeroed_page(GFP_ATOMIC);
-		if (!pte)
-			return -ENOMEM;
+	if (!pmd_none(*pmd))
+		return pte_offset_kernel(pmd, iova);
 
-		ipmmu_flush_pgtable(mmu, pte, PAGE_SIZE);
-		*pmd = __pmd(__pa(pte) | PMD_NSTABLE | PMD_TYPE_TABLE);
-		ipmmu_flush_pgtable(mmu, pmd, sizeof(*pmd));
+	pte = (pte_t *)get_zeroed_page(GFP_ATOMIC);
+	if (!pte)
+		return NULL;
 
-		pte += pte_index(addr);
-	} else
-		pte = pte_offset_kernel(pmd, addr);
+	ipmmu_flush_pgtable(mmu, pte, PAGE_SIZE);
+	*pmd = __pmd(__pa(pte) | PMD_NSTABLE | PMD_TYPE_TABLE);
+	ipmmu_flush_pgtable(mmu, pmd, sizeof(*pmd));
 
-	pteval |= ARM_VMSA_PTE_AP_UNPRIV | ARM_VMSA_PTE_nG;
-	if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
-		pteval |= ARM_VMSA_PTE_AP_RDONLY;
+	return pte + pte_index(iova);
+}
 
-	if (prot & IOMMU_CACHE)
-		pteval |= (IMMAIR_ATTR_IDX_WBRWA <<
-			   ARM_VMSA_PTE_ATTRINDX_SHIFT);
+static pmd_t *ipmmu_alloc_pmd(struct ipmmu_vmsa_device *mmu, pgd_t *pgd,
+			      unsigned long iova)
+{
+	pud_t *pud = (pud_t *)pgd;
+	pmd_t *pmd;
 
-	/* If no access, create a faulting entry to avoid TLB fills */
-	if (prot & IOMMU_EXEC)
-		pteval &= ~ARM_VMSA_PTE_XN;
-	else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
-		pteval &= ~ARM_VMSA_PTE_PAGE;
+	if (!pud_none(*pud))
+		return pmd_offset(pud, iova);
 
-	pteval |= ARM_VMSA_PTE_SH_IS;
-	start = pte;
+	pmd = (pmd_t *)get_zeroed_page(GFP_ATOMIC);
+	if (!pmd)
+		return NULL;
 
-	/*
-	 * Install the page table entries.
-	 *
-	 * Set the contiguous hint in the PTEs where possible. The hint
-	 * indicates a series of ARM_VMSA_PTE_CONT_ENTRIES PTEs mapping a
-	 * physically contiguous region with the following constraints:
-	 *
-	 * - The region start is aligned to ARM_VMSA_PTE_CONT_SIZE
-	 * - Each PTE in the region has the contiguous hint bit set
-	 *
-	 * We don't support partial unmapping so there's no need to care about
-	 * clearing the contiguous hint from neighbour PTEs.
-	 */
-	do {
-		unsigned long chunk_end;
+	ipmmu_flush_pgtable(mmu, pmd, PAGE_SIZE);
+	*pud = __pud(__pa(pmd) | PMD_NSTABLE | PMD_TYPE_TABLE);
+	ipmmu_flush_pgtable(mmu, pud, sizeof(*pud));
 
-		/*
-		 * If the address is aligned to a contiguous region size and the
-		 * mapping size is large enough, process the largest possible
-		 * number of PTEs multiple of ARM_VMSA_PTE_CONT_ENTRIES.
-		 * Otherwise process the smallest number of PTEs to align the
-		 * address to a contiguous region size or to complete the
-		 * mapping.
-		 */
-		if (IS_ALIGNED(addr, ARM_VMSA_PTE_CONT_SIZE) &&
-		    end - addr >= ARM_VMSA_PTE_CONT_SIZE) {
-			chunk_end = round_down(end, ARM_VMSA_PTE_CONT_SIZE);
-			pteval |= ARM_VMSA_PTE_CONT;
-		} else {
-			chunk_end = min(ALIGN(addr, ARM_VMSA_PTE_CONT_SIZE),
-					end);
-			pteval &= ~ARM_VMSA_PTE_CONT;
-		}
+	return pmd + pmd_index(iova);
+}
 
-		do {
-			*pte++ = pfn_pte(pfn++, __pgprot(pteval));
-			addr += PAGE_SIZE;
-		} while (addr != chunk_end);
-	} while (addr != end);
+static u64 ipmmu_page_prot(unsigned int prot, u64 type)
+{
+	u64 pgprot = ARM_VMSA_PTE_XN | ARM_VMSA_PTE_nG | ARM_VMSA_PTE_AF
+		   | ARM_VMSA_PTE_SH_IS | ARM_VMSA_PTE_AP_UNPRIV
+		   | ARM_VMSA_PTE_NS | type;
 
-	ipmmu_flush_pgtable(mmu, start, sizeof(*pte) * (pte - start));
-	return 0;
+	if (!(prot & IOMMU_WRITE) && (prot & IOMMU_READ))
+		pgprot |= ARM_VMSA_PTE_AP_RDONLY;
+
+	if (prot & IOMMU_CACHE)
+		pgprot |= IMMAIR_ATTR_IDX_WBRWA << ARM_VMSA_PTE_ATTRINDX_SHIFT;
+
+	if (prot & IOMMU_EXEC)
+		pgprot &= ~ARM_VMSA_PTE_XN;
+	else if (!(prot & (IOMMU_READ | IOMMU_WRITE)))
+		/* If no access create a faulting entry to avoid TLB fills. */
+		pgprot &= ~ARM_VMSA_PTE_PAGE;
+
+	return pgprot;
 }
 
-static int ipmmu_alloc_init_pmd(struct ipmmu_vmsa_device *mmu, pud_t *pud,
-				unsigned long addr, unsigned long end,
-				phys_addr_t phys, int prot)
+static int ipmmu_alloc_init_pte(struct ipmmu_vmsa_device *mmu, pmd_t *pmd,
+				unsigned long iova, unsigned long pfn,
+				size_t size, int prot)
 {
-	unsigned long next;
-	pmd_t *pmd;
-	int ret;
+	pteval_t pteval = ipmmu_page_prot(prot, ARM_VMSA_PTE_PAGE);
+	unsigned int num_ptes = 1;
+	pte_t *pte, *start;
+	unsigned int i;
 
-	if (pud_none(*pud)) {
-		pmd = (pmd_t *)get_zeroed_page(GFP_ATOMIC);
-		if (!pmd)
-			return -ENOMEM;
+	pte = ipmmu_alloc_pte(mmu, pmd, iova);
+	if (!pte)
+		return -ENOMEM;
+
+	start = pte;
 
-		ipmmu_flush_pgtable(mmu, pmd, PAGE_SIZE);
-		*pud = __pud(__pa(pmd) | PMD_NSTABLE | PMD_TYPE_TABLE);
-		ipmmu_flush_pgtable(mmu, pud, sizeof(*pud));
+	/*
+	 * Install the page table entries. We can be called both for a single
+	 * page or for a block of 16 physically contiguous pages. In the latter
+	 * case set the PTE contiguous hint.
+	 */
+	if (size = SZ_64K) {
+		pteval |= ARM_VMSA_PTE_CONT;
+		num_ptes = ARM_VMSA_PTE_CONT_ENTRIES;
+	}
 
-		pmd += pmd_index(addr);
-	} else
-		pmd = pmd_offset(pud, addr);
+	for (i = num_ptes; i; --i)
+		*pte++ = pfn_pte(pfn++, __pgprot(pteval));
 
-	do {
-		next = pmd_addr_end(addr, end);
-		ret = ipmmu_alloc_init_pte(mmu, pmd, addr, end, phys, prot);
-		phys += next - addr;
-	} while (pmd++, addr = next, addr < end);
+	ipmmu_flush_pgtable(mmu, start, sizeof(*pte) * num_ptes);
 
-	return ret;
+	return 0;
 }
 
 static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
@@ -638,7 +617,8 @@ static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
 	struct ipmmu_vmsa_device *mmu = domain->mmu;
 	pgd_t *pgd = domain->pgd;
 	unsigned long flags;
-	unsigned long end;
+	unsigned long pfn;
+	pmd_t *pmd;
 	int ret;
 
 	if (!pgd)
@@ -650,26 +630,25 @@ static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
 	if (paddr & ~((1ULL << 40) - 1))
 		return -ERANGE;
 
-	spin_lock_irqsave(&domain->lock, flags);
-
+	pfn = __phys_to_pfn(paddr);
 	pgd += pgd_index(iova);
-	end = iova + size;
 
-	do {
-		unsigned long next = pgd_addr_end(iova, end);
+	/* Update the page tables. */
+	spin_lock_irqsave(&domain->lock, flags);
 
-		ret = ipmmu_alloc_init_pmd(mmu, (pud_t *)pgd, iova, next, paddr,
-					   prot);
-		if (ret)
-			break;
+	pmd = ipmmu_alloc_pmd(mmu, pgd, iova);
+	if (!pmd) {
+		ret = -ENOMEM;
+		goto done;
+	}
 
-		paddr += next - iova;
-		iova = next;
-	} while (pgd++, iova != end);
+	ret = ipmmu_alloc_init_pte(mmu, pmd, iova, pfn, size, prot);
 
+done:
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-	ipmmu_tlb_invalidate(domain);
+	if (!ret)
+		ipmmu_tlb_invalidate(domain);
 
 	return ret;
 }
@@ -953,7 +932,7 @@ static struct iommu_ops ipmmu_ops = {
 	.iova_to_phys = ipmmu_iova_to_phys,
 	.add_device = ipmmu_add_device,
 	.remove_device = ipmmu_remove_device,
-	.pgsize_bitmap = SZ_2M | SZ_64K | SZ_4K,
+	.pgsize_bitmap = SZ_64K | SZ_4K,
 };
 
 /* -----------------------------------------------------------------------------
-- 
1.8.5.5

[PATCH v2 08/10] iommu/ipmmu-vmsa: Support 2MB mappings

[Laurent Pinchart]

Add support for 2MB block mappings at the PMD level.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 159e09a..27fd4d0 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -479,7 +479,7 @@ static void ipmmu_free_pmds(pud_t *pud)
 	unsigned int i;
 
 	for (i = 0; i < IPMMU_PTRS_PER_PMD; ++i) {
-		if (pmd_none(*pmd))
+		if (!pmd_table(*pmd))
 			continue;
 
 		ipmmu_free_ptes(pmd);
@@ -610,6 +610,18 @@ static int ipmmu_alloc_init_pte(struct ipmmu_vmsa_device *mmu, pmd_t *pmd,
 	return 0;
 }
 
+static int ipmmu_alloc_init_pmd(struct ipmmu_vmsa_device *mmu, pmd_t *pmd,
+				unsigned long iova, unsigned long pfn,
+				int prot)
+{
+	pmdval_t pmdval = ipmmu_page_prot(prot, PMD_TYPE_SECT);
+
+	*pmd = pfn_pmd(pfn, __pgprot(pmdval));
+	ipmmu_flush_pgtable(mmu, pmd, sizeof(*pmd));
+
+	return 0;
+}
+
 static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
 				unsigned long iova, phys_addr_t paddr,
 				size_t size, int prot)
@@ -642,7 +654,18 @@ static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
 		goto done;
 	}
 
-	ret = ipmmu_alloc_init_pte(mmu, pmd, iova, pfn, size, prot);
+	switch (size) {
+	case SZ_2M:
+		ret = ipmmu_alloc_init_pmd(mmu, pmd, iova, pfn, prot);
+		break;
+	case SZ_64K:
+	case SZ_4K:
+		ret = ipmmu_alloc_init_pte(mmu, pmd, iova, pfn, size, prot);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
 
 done:
 	spin_unlock_irqrestore(&domain->lock, flags);
@@ -792,6 +815,9 @@ static phys_addr_t ipmmu_iova_to_phys(struct iommu_domain *io_domain,
 	if (pmd_none(pmd))
 		return 0;
 
+	if (pmd_sect(pmd))
+		return __pfn_to_phys(pmd_pfn(pmd)) | (iova & ~PMD_MASK);
+
 	pte = *(pmd_page_vaddr(pmd) + pte_index(iova));
 	if (pte_none(pte))
 		return 0;
@@ -932,7 +958,7 @@ static struct iommu_ops ipmmu_ops = {
 	.iova_to_phys = ipmmu_iova_to_phys,
 	.add_device = ipmmu_add_device,
 	.remove_device = ipmmu_remove_device,
-	.pgsize_bitmap = SZ_64K | SZ_4K,
+	.pgsize_bitmap = SZ_2M | SZ_64K | SZ_4K,
 };
 
 /* -----------------------------------------------------------------------------
-- 
1.8.5.5

[PATCH v2 09/10] iommu/ipmmu-vmsa: Remove stage 2 PTE bits definitions

[Laurent Pinchart]

We don't support stage 2 translation yet.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 27fd4d0..a4175d3 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -201,14 +201,6 @@ static LIST_HEAD(ipmmu_devices);
 #define ARM_VMSA_PTE_ATTRINDX_SHIFT	2
 #define ARM_VMSA_PTE_nG			(((pteval_t)1) << 11)
 
-/* Stage-2 PTE */
-#define ARM_VMSA_PTE_HAP_FAULT		(((pteval_t)0) << 6)
-#define ARM_VMSA_PTE_HAP_READ		(((pteval_t)1) << 6)
-#define ARM_VMSA_PTE_HAP_WRITE		(((pteval_t)2) << 6)
-#define ARM_VMSA_PTE_MEMATTR_OIWB	(((pteval_t)0xf) << 2)
-#define ARM_VMSA_PTE_MEMATTR_NC		(((pteval_t)0x5) << 2)
-#define ARM_VMSA_PTE_MEMATTR_DEV	(((pteval_t)0x1) << 2)
-
 #define ARM_VMSA_PTE_CONT_ENTRIES	16
 #define ARM_VMSA_PTE_CONT_SIZE		(PAGE_SIZE * ARM_VMSA_PTE_CONT_ENTRIES)
 
-- 
1.8.5.5

[PATCH v2 10/10] iommu/ipmmu-vmsa: Support clearing mappings

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/iommu/ipmmu-vmsa.c | 190 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 186 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index a4175d3..c003fae 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -192,14 +192,22 @@ static LIST_HEAD(ipmmu_devices);
 #define ARM_VMSA_PTE_SH_NS		(((pteval_t)0) << 8)
 #define ARM_VMSA_PTE_SH_OS		(((pteval_t)2) << 8)
 #define ARM_VMSA_PTE_SH_IS		(((pteval_t)3) << 8)
+#define ARM_VMSA_PTE_SH_MASK		(((pteval_t)3) << 8)
 #define ARM_VMSA_PTE_NS			(((pteval_t)1) << 5)
 #define ARM_VMSA_PTE_PAGE		(((pteval_t)3) << 0)
 
 /* Stage-1 PTE */
+#define ARM_VMSA_PTE_nG			(((pteval_t)1) << 11)
 #define ARM_VMSA_PTE_AP_UNPRIV		(((pteval_t)1) << 6)
 #define ARM_VMSA_PTE_AP_RDONLY		(((pteval_t)2) << 6)
+#define ARM_VMSA_PTE_AP_MASK		(((pteval_t)3) << 6)
+#define ARM_VMSA_PTE_ATTRINDX_MASK	(((pteval_t)3) << 2)
 #define ARM_VMSA_PTE_ATTRINDX_SHIFT	2
-#define ARM_VMSA_PTE_nG			(((pteval_t)1) << 11)
+
+#define ARM_VMSA_PTE_ATTRS_MASK \
+	(ARM_VMSA_PTE_XN | ARM_VMSA_PTE_CONT | ARM_VMSA_PTE_nG | \
+	 ARM_VMSA_PTE_AF | ARM_VMSA_PTE_SH_MASK | ARM_VMSA_PTE_AP_MASK | \
+	 ARM_VMSA_PTE_NS | ARM_VMSA_PTE_ATTRINDX_MASK)
 
 #define ARM_VMSA_PTE_CONT_ENTRIES	16
 #define ARM_VMSA_PTE_CONT_SIZE		(PAGE_SIZE * ARM_VMSA_PTE_CONT_ENTRIES)
@@ -614,7 +622,7 @@ static int ipmmu_alloc_init_pmd(struct ipmmu_vmsa_device *mmu, pmd_t *pmd,
 	return 0;
 }
 
-static int ipmmu_handle_mapping(struct ipmmu_vmsa_domain *domain,
+static int ipmmu_create_mapping(struct ipmmu_vmsa_domain *domain,
 				unsigned long iova, phys_addr_t paddr,
 				size_t size, int prot)
 {
@@ -668,6 +676,180 @@ done:
 	return ret;
 }
 
+static void ipmmu_clear_pud(struct ipmmu_vmsa_device *mmu, pud_t *pud)
+{
+	/* Free the page table. */
+	pgtable_t table = pud_pgtable(*pud);
+	__free_page(table);
+
+	/* Clear the PUD. */
+	*pud = __pud(0);
+	ipmmu_flush_pgtable(mmu, pud, sizeof(*pud));
+}
+
+static void ipmmu_clear_pmd(struct ipmmu_vmsa_device *mmu, pud_t *pud,
+			    pmd_t *pmd)
+{
+	unsigned int i;
+
+	/* Free the page table. */
+	if (pmd_table(*pmd)) {
+		pgtable_t table = pmd_pgtable(*pmd);
+		__free_page(table);
+	}
+
+	/* Clear the PMD. */
+	*pmd = __pmd(0);
+	ipmmu_flush_pgtable(mmu, pmd, sizeof(*pmd));
+
+	/* Check whether the PUD is still needed. */
+	pmd = pmd_offset(pud, 0);
+	for (i = 0; i < IPMMU_PTRS_PER_PMD; ++i) {
+		if (!pmd_none(pmd[i]))
+			return;
+	}
+
+	/* Clear the parent PUD. */
+	ipmmu_clear_pud(mmu, pud);
+}
+
+static void ipmmu_clear_pte(struct ipmmu_vmsa_device *mmu, pud_t *pud,
+			    pmd_t *pmd, pte_t *pte, unsigned int num_ptes)
+{
+	unsigned int i;
+
+	/* Clear the PTE. */
+	for (i = num_ptes; i; --i)
+		pte[i-1] = __pte(0);
+
+	ipmmu_flush_pgtable(mmu, pte, sizeof(*pte) * num_ptes);
+
+	/* Check whether the PMD is still needed. */
+	pte = pte_offset_kernel(pmd, 0);
+	for (i = 0; i < IPMMU_PTRS_PER_PTE; ++i) {
+		if (!pte_none(pte[i]))
+			return;
+	}
+
+	/* Clear the parent PMD. */
+	ipmmu_clear_pmd(mmu, pud, pmd);
+}
+
+static int ipmmu_split_pmd(struct ipmmu_vmsa_device *mmu, pmd_t *pmd)
+{
+	pte_t *pte, *start;
+	pteval_t pteval;
+	unsigned long pfn;
+	unsigned int i;
+
+	pte = (pte_t *)get_zeroed_page(GFP_ATOMIC);
+	if (!pte)
+		return -ENOMEM;
+
+	/* Copy the PMD attributes. */
+	pteval = (pmd_val(*pmd) & ARM_VMSA_PTE_ATTRS_MASK)
+	       | ARM_VMSA_PTE_CONT | ARM_VMSA_PTE_PAGE;
+
+	pfn = pmd_pfn(*pmd);
+	start = pte;
+
+	for (i = IPMMU_PTRS_PER_PTE; i; --i)
+		*pte++ = pfn_pte(pfn++, __pgprot(pteval));
+
+	ipmmu_flush_pgtable(mmu, start, PAGE_SIZE);
+	*pmd = __pmd(__pa(start) | PMD_NSTABLE | PMD_TYPE_TABLE);
+	ipmmu_flush_pgtable(mmu, pmd, sizeof(*pmd));
+
+	return 0;
+}
+
+static void ipmmu_split_pte(struct ipmmu_vmsa_device *mmu, pte_t *pte)
+{
+	unsigned int i;
+
+	for (i = ARM_VMSA_PTE_CONT_ENTRIES; i; --i)
+		pte[i-1] = __pte(pte_val(*pte) & ~ARM_VMSA_PTE_CONT);
+
+	ipmmu_flush_pgtable(mmu, pte, sizeof(*pte) * ARM_VMSA_PTE_CONT_ENTRIES);
+}
+
+static int ipmmu_clear_mapping(struct ipmmu_vmsa_domain *domain,
+			       unsigned long iova, size_t size)
+{
+	struct ipmmu_vmsa_device *mmu = domain->mmu;
+	unsigned long flags;
+	pgd_t *pgd = domain->pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	pte_t *pte;
+	int ret = 0;
+
+	if (!pgd)
+		return -EINVAL;
+
+	if (size & ~PAGE_MASK)
+		return -EINVAL;
+
+	pgd += pgd_index(iova);
+	pud = (pud_t *)pgd;
+
+	spin_lock_irqsave(&domain->lock, flags);
+
+	/* If there's no PUD or PMD we're done. */
+	if (pud_none(*pud))
+		goto done;
+
+	pmd = pmd_offset(pud, iova);
+	if (pmd_none(*pmd))
+		goto done;
+
+	/*
+	 * When freeing a 2MB block just clear the PMD. In the unlikely case the
+	 * block is mapped as individual pages this will free the corresponding
+	 * PTE page table.
+	 */
+	if (size = SZ_2M) {
+		ipmmu_clear_pmd(mmu, pud, pmd);
+		goto done;
+	}
+
+	/*
+	 * If the PMD has been mapped as a section remap it as pages to allow
+	 * freeing individual pages.
+	 */
+	if (pmd_sect(*pmd))
+		ipmmu_split_pmd(mmu, pmd);
+
+	pte = pte_offset_kernel(pmd, iova);
+
+	/*
+	 * When freeing a 64kB block just clear the PTE entries. We don't have
+	 * to care about the contiguous hint of the surrounding entries.
+	 */
+	if (size = SZ_64K) {
+		ipmmu_clear_pte(mmu, pud, pmd, pte, ARM_VMSA_PTE_CONT_ENTRIES);
+		goto done;
+	}
+
+	/*
+	 * If the PTE has been mapped with the contiguous hint set remap it and
+	 * its surrounding PTEs to allow unmapping a single page.
+	 */
+	if (pte_val(*pte) & ARM_VMSA_PTE_CONT)
+		ipmmu_split_pte(mmu, pte);
+
+	/* Clear the PTE. */
+	ipmmu_clear_pte(mmu, pud, pmd, pte, 1);
+
+done:
+	spin_unlock_irqrestore(&domain->lock, flags);
+
+	if (ret)
+		ipmmu_tlb_invalidate(domain);
+
+	return 0;
+}
+
 /* -----------------------------------------------------------------------------
  * IOMMU Operations
  */
@@ -768,7 +950,7 @@ static int ipmmu_map(struct iommu_domain *io_domain, unsigned long iova,
 	if (!domain)
 		return -ENODEV;
 
-	return ipmmu_handle_mapping(domain, iova, paddr, size, prot);
+	return ipmmu_create_mapping(domain, iova, paddr, size, prot);
 }
 
 static size_t ipmmu_unmap(struct iommu_domain *io_domain, unsigned long iova,
@@ -777,7 +959,7 @@ static size_t ipmmu_unmap(struct iommu_domain *io_domain, unsigned long iova,
 	struct ipmmu_vmsa_domain *domain = io_domain->priv;
 	int ret;
 
-	ret = ipmmu_handle_mapping(domain, iova, 0, size, 0);
+	ret = ipmmu_clear_mapping(domain, iova, size);
 	return ret ? 0 : size;
 }
 
-- 
1.8.5.5

Re: [PATCH v2 00/10] Renesas ipmmu-vmsa: Miscellaneous cleanups and fixes

[Laurent Pinchart]

Hi Joerg,

Ping ?

On Thursday 15 May 2014 12:40:41 Laurent Pinchart wrote:
> Hello,
> 
> This patch set cleans up and fixes small issues in the ipmmu-vmsa driver.
> The patches are based on top of "[PATCH v3] iommu: Add driver for Renesas
> VMSA-compatible IPMMU" that adds the ipmmu-vmsa driver.
> 
> The most interesting part of this series is the rewrite of the page table
> management code. The IOMMU core guarantees that the map and unmap operations
> will always be called only with page sizes advertised by the driver. We can
> use that assumption to remove loops of PGD and PMD entries, simplifying the
> code.
> 
> Joerg, is there still time to get this merged in v3.16 ? The patches have
> all been posted previously and the only comment I've received was about a
> missing #define in patch 04/10.
> 
> Cc: Joerg Roedel <joro@8bytes.org>
> 
> Changes since v1:
> 
> - Add missing IPMMU_PTRS_PER_PUD definition in patch 04/10
> 
> Laurent Pinchart (10):
>   iommu/ipmmu-vmsa: Refactor micro-TLB lookup
>   iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or
>     attachment
>   iommu/ipmmu-vmsa: Fix the supported page sizes
>   iommu/ipmmu-vmsa: Define driver-specific page directory sizes
>   iommu/ipmmu-vmsa: Set the PTE contiguous hint bit when possible
>   iommu/ipmmu-vmsa: PMD is never folded, PUD always is
>   iommu/ipmmu-vmsa: Rewrite page table management
>   iommu/ipmmu-vmsa: Support 2MB mappings
>   iommu/ipmmu-vmsa: Remove stage 2 PTE bits definitions
>   iommu/ipmmu-vmsa: Support clearing mappings
> 
> drivers/iommu/ipmmu-vmsa.c | 535 +++++++++++++++++++++++++++---------------
> 1 file changed, 361
> insertions(+), 174 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 02/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment

[Joerg Roedel]

On Thu, May 15, 2014 at 12:40:43PM +0200, Laurent Pinchart wrote:
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  drivers/iommu/ipmmu-vmsa.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 49e00f7..49dbedd 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -917,7 +917,8 @@ static int ipmmu_add_device(struct device *dev)
>  							SZ_1G, SZ_2G);
>  		if (IS_ERR(mapping)) {
>  			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
> -			return PTR_ERR(mapping);
> +			ret = PTR_ERR(mapping);
> +			goto error;
>  		}
>  
>  		mmu->mapping = mapping;
> @@ -933,6 +934,7 @@ static int ipmmu_add_device(struct device *dev)
>  	return 0;
>  
>  error:
> +	arm_iommu_release_mapping(mmu->mapping);
>  	kfree(dev->archdata.iommu);
>  	dev->archdata.iommu = NULL;
>  	iommu_group_remove_device(dev);

Skipped this one because it didn't apply. The others are applied.

Re: [PATCH v2 02/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment

[Laurent Pinchart]

Hi Joerg,

On Monday 26 May 2014 11:23:11 Joerg Roedel wrote:
> On Thu, May 15, 2014 at 12:40:43PM +0200, Laurent Pinchart wrote:
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/iommu/ipmmu-vmsa.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > index 49e00f7..49dbedd 100644
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -917,7 +917,8 @@ static int ipmmu_add_device(struct device *dev)
> >  							SZ_1G, SZ_2G);
> >  		if (IS_ERR(mapping)) {
> >  			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
> > -			return PTR_ERR(mapping);
> > +			ret = PTR_ERR(mapping);
> > +			goto error;
> >  		}
> >  		
> >  		mmu->mapping = mapping;
> > @@ -933,6 +934,7 @@ static int ipmmu_add_device(struct device *dev)
> >  	return 0;
> >  
> >  error:
> > +	arm_iommu_release_mapping(mmu->mapping);
> >  	kfree(dev->archdata.iommu);
> >  	dev->archdata.iommu = NULL;
> >  	iommu_group_remove_device(dev);
> 
> Skipped this one because it didn't apply. The others are applied.

Thank you. I'll rebase the patch on top of your tree as soon as you publish 
the related branch and resubmit.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 02/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment

[Joerg Roedel]

On Mon, May 26, 2014 at 12:08:37PM +0200, Laurent Pinchart wrote:
> 
> Thank you. I'll rebase the patch on top of your tree as soon as you publish 
> the related branch and resubmit.

I also put this patch on-top to fix a compile error.

From 720b0cef715ab97b21b33e7f3c328e2863411cab Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel@suse.de>
Date: Mon, 26 May 2014 13:07:01 +0200
Subject: [PATCH] arm/ipmmu-vmsa: Fix compile error

The function arm_iommu_create_mapping lost the order
parameter. Remove it from this IOMMU driver too to make it
build.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/ipmmu-vmsa.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 95b819a..53cde08 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1087,7 +1087,7 @@ static int ipmmu_add_device(struct device *dev)
 		struct dma_iommu_mapping *mapping;
 
 		mapping = arm_iommu_create_mapping(&platform_bus_type,
-							SZ_1G, SZ_2G, 0);
+						   SZ_1G, SZ_2G);
 		if (IS_ERR(mapping)) {
 			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
 			return PTR_ERR(mapping);
-- 
1.7.9.5

Re: [PATCH v2 02/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment

[Laurent Pinchart]

Hi Joerg,

On Monday 26 May 2014 13:31:17 Joerg Roedel wrote:
> On Mon, May 26, 2014 at 12:08:37PM +0200, Laurent Pinchart wrote:
> > Thank you. I'll rebase the patch on top of your tree as soon as you
> > publish the related branch and resubmit.
> 
> I also put this patch on-top to fix a compile error.

That looks good to me, thank you.

> From 720b0cef715ab97b21b33e7f3c328e2863411cab Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <jroedel@suse.de>
> Date: Mon, 26 May 2014 13:07:01 +0200
> Subject: [PATCH] arm/ipmmu-vmsa: Fix compile error
> 
> The function arm_iommu_create_mapping lost the order
> parameter. Remove it from this IOMMU driver too to make it
> build.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  drivers/iommu/ipmmu-vmsa.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 95b819a..53cde08 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1087,7 +1087,7 @@ static int ipmmu_add_device(struct device *dev)
>  		struct dma_iommu_mapping *mapping;
> 
>  		mapping = arm_iommu_create_mapping(&platform_bus_type,
> -							SZ_1G, SZ_2G, 0);
> +						   SZ_1G, SZ_2G);
>  		if (IS_ERR(mapping)) {
>  			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
>  			return PTR_ERR(mapping);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 02/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment

[Joerg Roedel]

Hi Laurent,

On Mon, May 26, 2014 at 12:08:37PM +0200, Laurent Pinchart wrote:
> > Skipped this one because it didn't apply. The others are applied.
> 
> Thank you. I'll rebase the patch on top of your tree as soon as you publish 
> the related branch and resubmit.

What happened to this patch? And how about the other iommu_domain
changes requested. Will you submit them for 3.17?


	Joerg

Re: [PATCH v2 02/10] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment

[Laurent Pinchart]

Hi Joerg,

On Monday 16 June 2014 17:11:42 Joerg Roedel wrote:
> On Mon, May 26, 2014 at 12:08:37PM +0200, Laurent Pinchart wrote:
> > > Skipped this one because it didn't apply. The others are applied.
> > 
> > Thank you. I'll rebase the patch on top of your tree as soon as you
> > publish the related branch and resubmit.
> 
> What happened to this patch?

It looks like it slipped through the cracks :-/ Should I submit it as a fix 
for v3.16 or wait until v3.17 ?

> And how about the other iommu_domain changes requested. Will you submit them
> for 3.17?

I plan to work on that, but I'm not sure whether I'll be able to finish the 
work for v3.17, as I need to concentrate on a DMA engine driver right now, and 
will then need to finish the DT bindings for the IPMMU driver (which may 
result in fixes for some of the iommu_domain-related problems).

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/10] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Khiem Nguyen]

On 5/15/2014 7:40 PM, Laurent Pinchart wrote:
> Cache the micro-TLB number in archdata allocated in the .add_device
> handler instead of looking it up when the deviced is attached and
> detached. This simplifies the .attach_dev and .detach_dev operations and
> prepares for DT support.
[snip]
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
[snip]
> +static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device *dev)
> +{
> +	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
> +	const char *devname = dev_name(dev);
> +	unsigned int i;
> +
> +	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
> +		if (strcmp(master->name, devname) = 0)
> +			return master->utlb;
> +	}
> +
> +	return -1;
> +}
[snip]
>  static int ipmmu_add_device(struct device *dev)
[snip]
>  	list_for_each_entry(mmu, &ipmmu_devices, list) {
> -		master = ipmmu_find_master(mmu, dev);
> -		if (master) {
> +		utlb = ipmmu_find_utlb(mmu, dev);
> +		if (utlb >= 0) {
>  			/*
> -			 * TODO Take a reference to the master to protect
> +			 * TODO Take a reference to the MMU to protect
>  			 * against device removal.
>  			 */
>  			break;
[snip]
> +	archdata->mmu = mmu;
> +	archdata->utlb = utlb;
[snip]

I have one question for ipmmu_add_device().

In my understanding, your code will find utlb for device
base on device name.
For any device, it will /only/ return utlb number of first match.

How about the case that a device name connected with more than 1 utlb ?
e.g DU device (rcar-du-r8a7790) in Lager

Was that case already covered in your code ?

Thanks.

Best regards,
KHIEM Nguyen

Re: [PATCH v2 01/10] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Laurent Pinchart]

On Thursday 10 July 2014 09:03:26 Khiem Nguyen wrote:
> On 5/15/2014 7:40 PM, Laurent Pinchart wrote:
> > Cache the micro-TLB number in archdata allocated in the .add_device
> > handler instead of looking it up when the deviced is attached and
> > detached. This simplifies the .attach_dev and .detach_dev operations and
> > prepares for DT support.
> 
> [snip]
> 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> 
> [snip]
> 
> > +static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device
> > *dev)
> > +{
> > +	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
> > +	const char *devname = dev_name(dev);
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
> > +		if (strcmp(master->name, devname) = 0)
> > +			return master->utlb;
> > +	}
> > +
> > +	return -1;
> > +}
> 
> [snip]
> 
> >  static int ipmmu_add_device(struct device *dev)
> 
> [snip]
> 
> >  	list_for_each_entry(mmu, &ipmmu_devices, list) {
> > -		master = ipmmu_find_master(mmu, dev);
> > -		if (master) {
> > +		utlb = ipmmu_find_utlb(mmu, dev);
> > +		if (utlb >= 0) {
> >  			/*
> > -			 * TODO Take a reference to the master to protect
> > +			 * TODO Take a reference to the MMU to protect
> >  			 * against device removal.
> >  			 */
> >  			break;
> 
> [snip]
> 
> > +	archdata->mmu = mmu;
> > +	archdata->utlb = utlb;
> 
> [snip]
> 
> I have one question for ipmmu_add_device().
> 
> In my understanding, your code will find utlb for device
> base on device name.
> For any device, it will /only/ return utlb number of first match.
> 
> How about the case that a device name connected with more than 1 utlb ?
> e.g DU device (rcar-du-r8a7790) in Lager
> 
> Was that case already covered in your code ?

For the DU case, the R8A7790 contains two DU devices, each connected to a 
single utlb. The IPMMU driver will thus work fine in that case.

I agree that this is a problem in general though, other devices (such as the 
DMAC) are connected to more than one utlb. This is currently not supported by 
the driver.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/10] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Khiem Nguyen]

Hi Laurent,

On 7/10/2014 7:37 PM, Laurent Pinchart wrote:
> On Thursday 10 July 2014 09:03:26 Khiem Nguyen wrote:
>> On 5/15/2014 7:40 PM, Laurent Pinchart wrote:
>>> Cache the micro-TLB number in archdata allocated in the .add_device
>>> handler instead of looking it up when the deviced is attached and
>>> detached. This simplifies the .attach_dev and .detach_dev operations and
>>> prepares for DT support.
>>
>> [snip]
>>
>>> Signed-off-by: Laurent Pinchart
>>> <laurent.pinchart+renesas@ideasonboard.com>
>>
>> [snip]
>>
>>> +static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device
>>> *dev)
>>> +{
>>> +	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
>>> +	const char *devname = dev_name(dev);
>>> +	unsigned int i;
>>> +
>>> +	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
>>> +		if (strcmp(master->name, devname) = 0)
>>> +			return master->utlb;
>>> +	}
>>> +
>>> +	return -1;
>>> +}
>>
>> [snip]
>>
>>>  static int ipmmu_add_device(struct device *dev)
>>
>> [snip]
>>
>>>  	list_for_each_entry(mmu, &ipmmu_devices, list) {
>>> -		master = ipmmu_find_master(mmu, dev);
>>> -		if (master) {
>>> +		utlb = ipmmu_find_utlb(mmu, dev);
>>> +		if (utlb >= 0) {
>>>  			/*
>>> -			 * TODO Take a reference to the master to protect
>>> +			 * TODO Take a reference to the MMU to protect
>>>  			 * against device removal.
>>>  			 */
>>>  			break;
>>
>> [snip]
>>
>>> +	archdata->mmu = mmu;
>>> +	archdata->utlb = utlb;
>>
>> [snip]
>>
>> I have one question for ipmmu_add_device().
>>
>> In my understanding, your code will find utlb for device
>> base on device name.
>> For any device, it will /only/ return utlb number of first match.
>>
>> How about the case that a device name connected with more than 1 utlb ?
>> e.g DU device (rcar-du-r8a7790) in Lager
>>
>> Was that case already covered in your code ?
> 
> For the DU case, the R8A7790 contains /two DU devices/, each connected to a 
> single utlb. The IPMMU driver will thus work fine in that case.

As my understanding, in board-lager-reference.c,
DU devices are registered with 1 device name "rcar-du-r8a7790".

I also added some logs in ipmmu_add_device() to observe
the mapping between device name and utlb number.
And I saw that only one utlb is mapped with DU.

Do I miss anything here ?

> 
> I agree that this is a problem in general though, other devices (such as the 
> DMAC) are connected to more than one utlb. This is currently not supported by 
> the driver.
> 
Thanks for your confirmation.
I guess the driver will be improved in near future.

-- 
Best regards,
KHIEM Nguyen

Re: [PATCH v2 01/10] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Laurent Pinchart]

Hi Khiem,

On Monday 14 July 2014 09:19:23 Khiem Nguyen wrote:
> On 7/10/2014 7:37 PM, Laurent Pinchart wrote:
> > On Thursday 10 July 2014 09:03:26 Khiem Nguyen wrote:
> >> On 5/15/2014 7:40 PM, Laurent Pinchart wrote:
> >>> Cache the micro-TLB number in archdata allocated in the .add_device
> >>> handler instead of looking it up when the deviced is attached and
> >>> detached. This simplifies the .attach_dev and .detach_dev operations and
> >>> prepares for DT support.
> >> 
> >> [snip]
> >> 
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@ideasonboard.com>
> >> 
> >> [snip]
> >> 
> >>> +static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device
> >>> *dev)
> >>> +{
> >>> +	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
> >>> +	const char *devname = dev_name(dev);
> >>> +	unsigned int i;
> >>> +
> >>> +	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
> >>> +		if (strcmp(master->name, devname) = 0)
> >>> +			return master->utlb;
> >>> +	}
> >>> +
> >>> +	return -1;
> >>> +}
> >> 
> >> [snip]
> >> 
> >>>  static int ipmmu_add_device(struct device *dev)
> >> 
> >> [snip]
> >> 
> >>>  	list_for_each_entry(mmu, &ipmmu_devices, list) {
> >>> -		master = ipmmu_find_master(mmu, dev);
> >>> -		if (master) {
> >>> +		utlb = ipmmu_find_utlb(mmu, dev);
> >>> +		if (utlb >= 0) {
> >>>  			/*
> >>> -			 * TODO Take a reference to the master to protect
> >>> +			 * TODO Take a reference to the MMU to protect
> >>>  			 * against device removal.
> >>>  			 */
> >>>  			break;
> >> 
> >> [snip]
> >> 
> >>> +	archdata->mmu = mmu;
> >>> +	archdata->utlb = utlb;
> >> 
> >> [snip]
> >> 
> >> I have one question for ipmmu_add_device().
> >> 
> >> In my understanding, your code will find utlb for device
> >> base on device name.
> >> For any device, it will /only/ return utlb number of first match.
> >> 
> >> How about the case that a device name connected with more than 1 utlb ?
> >> e.g DU device (rcar-du-r8a7790) in Lager
> >> 
> >> Was that case already covered in your code ?
> > 
> > For the DU case, the R8A7790 contains /two DU devices/, each connected to
> > a single utlb. The IPMMU driver will thus work fine in that case.
> 
> As my understanding, in board-lager-reference.c,
> DU devices are registered with 1 device name "rcar-du-r8a7790".
> 
> I also added some logs in ipmmu_add_device() to observe
> the mapping between device name and utlb number.
> And I saw that only one utlb is mapped with DU.
> 
> Do I miss anything here ?

No, you're right, my bad. This is definitely a limitation of the driver at the 
moment.

> > I agree that this is a problem in general though, other devices (such as
> > the DMAC) are connected to more than one utlb. This is currently not
> > supported by the driver.
> 
> Thanks for your confirmation.
> I guess the driver will be improved in near future.

Yes, it will.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 01/10] iommu/ipmmu-vmsa: Refactor micro-TLB lookup

[Khiem Nguyen]

Hi Laurent,

On 7/14/2014 6:01 PM, Laurent Pinchart wrote:
> On Monday 14 July 2014 09:19:23 Khiem Nguyen wrote:
>> On 7/10/2014 7:37 PM, Laurent Pinchart wrote:
>>> On Thursday 10 July 2014 09:03:26 Khiem Nguyen wrote:
>>>> On 5/15/2014 7:40 PM, Laurent Pinchart wrote:
>>>>> Cache the micro-TLB number in archdata allocated in the .add_device
>>>>> handler instead of looking it up when the deviced is attached and
>>>>> detached. This simplifies the .attach_dev and .detach_dev operations and
>>>>> prepares for DT support.
>>>>
>>>> [snip]
>>>>
>>>>> Signed-off-by: Laurent Pinchart
>>>>> <laurent.pinchart+renesas@ideasonboard.com>
>>>>
>>>> [snip]
>>>>
>>>>> +static int ipmmu_find_utlb(struct ipmmu_vmsa_device *mmu, struct device
>>>>> *dev)
>>>>> +{
>>>>> +	const struct ipmmu_vmsa_master *master = mmu->pdata->masters;
>>>>> +	const char *devname = dev_name(dev);
>>>>> +	unsigned int i;
>>>>> +
>>>>> +	for (i = 0; i < mmu->pdata->num_masters; ++i, ++master) {
>>>>> +		if (strcmp(master->name, devname) = 0)
>>>>> +			return master->utlb;
>>>>> +	}
>>>>> +
>>>>> +	return -1;
>>>>> +}
>>>>
>>>> [snip]
>>>>
>>>>>  static int ipmmu_add_device(struct device *dev)
>>>>
>>>> [snip]
>>>>
>>>>>  	list_for_each_entry(mmu, &ipmmu_devices, list) {
>>>>> -		master = ipmmu_find_master(mmu, dev);
>>>>> -		if (master) {
>>>>> +		utlb = ipmmu_find_utlb(mmu, dev);
>>>>> +		if (utlb >= 0) {
>>>>>  			/*
>>>>> -			 * TODO Take a reference to the master to protect
>>>>> +			 * TODO Take a reference to the MMU to protect
>>>>>  			 * against device removal.
>>>>>  			 */
>>>>>  			break;
>>>>
>>>> [snip]
>>>>
>>>>> +	archdata->mmu = mmu;
>>>>> +	archdata->utlb = utlb;
>>>>
>>>> [snip]
>>>>
>>>> I have one question for ipmmu_add_device().
>>>>
>>>> In my understanding, your code will find utlb for device
>>>> base on device name.
>>>> For any device, it will /only/ return utlb number of first match.
>>>>
>>>> How about the case that a device name connected with more than 1 utlb ?
>>>> e.g DU device (rcar-du-r8a7790) in Lager
>>>>
>>>> Was that case already covered in your code ?
>>>
>>> For the DU case, the R8A7790 contains /two DU devices/, each connected to
>>> a single utlb. The IPMMU driver will thus work fine in that case.
>>
>> As my understanding, in board-lager-reference.c,
>> DU devices are registered with 1 device name "rcar-du-r8a7790".
>>
>> I also added some logs in ipmmu_add_device() to observe
>> the mapping between device name and utlb number.
>> And I saw that only one utlb is mapped with DU.
>>
>> Do I miss anything here ?
> 
> No, you're right, my bad. This is definitely a limitation of the driver at the 
> moment.

OK. Thanks for your confirmation.

> 
>>> I agree that this is a problem in general though, other devices (such as
>>> the DMAC) are connected to more than one utlb. This is currently not
>>> supported by the driver.
>>
>> Thanks for your confirmation.
>> I guess the driver will be improved in near future.
> 
> Yes, it will.
> 
I'm happy to wait for your update. :)

-- 
Best regards,
KHIEM Nguyen