[PATCH v7 00/07] iommu/ipmmu-vmsa: IPMMU multi-arch update V7

[PATCH v7 00/07] iommu/ipmmu-vmsa: IPMMU multi-arch update V7

[Magnus Damm]

iommu/ipmmu-vmsa: IPMMU multi-arch update V7

[PATCH v7 01/07] iommu/ipmmu-vmsa: Remove platform data handling
[PATCH v7 02/07] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context
[PATCH v7 03/07] iommu/ipmmu-vmsa: Break out utlb parsing code
[PATCH v7 04/07] iommu/ipmmu-vmsa: Break out domain allocation code
[PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
[PATCH v7 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access
[PATCH v7 07/07] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

These patches update the IPMMU driver with a couple of changes to support
build on multiple architectures. In the process of doing so the interrupt
code gets reworked, the foundation for supporting multiple contexts are
added and in case of CONFIG_IOMMU_DMA=y (on 64-bit or 32-bit ARM) devices
are grouped together and handled via ->xlate() - thanks Robin! Support for
existing 32-bit ARM SoCs from R-Car Gen2 is kept as-is.

Changes since V6:
 - Rebased on top of v4.11-rc1 and the following fast-tracked change:
   3b6bb5b iommu/ipmmu-vmsa: Restrict IOMMU Domain Geometry to 32-bit address spac
 - Updated patch 5/7 to roll in a few patches from other series
   See individual patch for more details
 - Build tested on 32-bit and 64-bit ARM
 - Run time tested on 64-bit ARM (with additional SoC-specific patches)

Changes since V5:
 - Rebased series on top of next-20161019
 - Updated patch 5/7 to simplify domain allocation/free code - thanks Joerg!
 - Added reviewed-by tag from Joerg for patch 1-4 and 6-7.
 
Changes since V4:
 - Updated patch 3/7 to work on top on the following commit in next-20160920:
   b1e2afc iommu/ipmmu-vmsa: Fix wrong error handle of ipmmu_add_device
 - Add Kconfig hunk to patch 5/7 to avoid undeclared ipmmu_ops if COMPILE_TEST
 - Rebased patch 7/7 to fit on top of new Kconfig bits in 5/7

Changes since V3:
 - Updated patch 3/7 to fix hang-on-boot issue on 32-bit ARM - thanks Geert!
 - Reworked group parameter handling in patch 3/7 and 5/7.
 - Added patch 6/7 to fix build of the driver on s390/tile/um architectures

Changes since V2:
 - Got rid of patch 3 from the V2 however patch 1, 2 and 4 are kept.
 - V3 patch 3, 4 and 5 come from
   [PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
 - Patch 5 has been reworked to include patch 3 of the V1 of this series 

Changes since V1:
 - Got rid of patch 2 and 3 from initial series
 - Updated bitmap code locking and also used lighter bitop functions
 - Updated the Kconfig bits to apply on top of ARCH_RENESAS

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
---

 Built on top v4.11-rc1:

 drivers/iommu/Kconfig      |    2 
 drivers/iommu/ipmmu-vmsa.c |  359 ++++++++++++++++++++++++++++++++++++--------
 2 files changed, 299 insertions(+), 62 deletions(-)

[PATCH v7 01/07] iommu/ipmmu-vmsa: Remove platform data handling

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

The IPMMU driver is using DT these days, and platform data is no longer
used by the driver. Remove unused code.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---

 Changes since V6:
 - None

 drivers/iommu/ipmmu-vmsa.c |    5 -----
 1 file changed, 5 deletions(-)

--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 17:25:49.000000000 +0900
@@ -768,11 +768,6 @@ static int ipmmu_probe(struct platform_d
 	int irq;
 	int ret;
 
-	if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
-		dev_err(&pdev->dev, "missing platform data\n");
-		return -EINVAL;
-	}
-
 	mmu = devm_kzalloc(&pdev->dev, sizeof(*mmu), GFP_KERNEL);
 	if (!mmu) {
 		dev_err(&pdev->dev, "cannot allocate device data\n");

Re: [PATCH v7 01/07] iommu/ipmmu-vmsa: Remove platform data handling

[Geert Uytterhoeven]

On Tue, Mar 7, 2017 at 4:16 AM, Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
>
> The IPMMU driver is using DT these days, and platform data is no longer
> used by the driver. Remove unused code.
>
> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v7 02/07] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Introduce a bitmap for context handing and convert the
interrupt routine to handle all registered contexts.

At this point the number of contexts are still limited.

Also remove the use of the ARM specific mapping variable
from ipmmu_irq() to allow compile on ARM64.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---

 Changes since V6:
 - None

 drivers/iommu/ipmmu-vmsa.c |   76 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 66 insertions(+), 10 deletions(-)

--- 0003/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 18:32:38.350607110 +0900
@@ -8,6 +8,7 @@
  * the Free Software Foundation; version 2 of the License.
  */
 
+#include <linux/bitmap.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
@@ -26,12 +27,17 @@
 
 #include "io-pgtable.h"
 
+#define IPMMU_CTX_MAX 1
+
 struct ipmmu_vmsa_device {
 	struct device *dev;
 	void __iomem *base;
 	struct list_head list;
 
 	unsigned int num_utlbs;
+	spinlock_t lock;			/* Protects ctx and domains[] */
+	DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
 	struct dma_iommu_mapping *mapping;
 };
@@ -293,9 +299,29 @@ static struct iommu_gather_ops ipmmu_gat
  * Domain/Context Management
  */
 
+static int ipmmu_domain_allocate_context(struct ipmmu_vmsa_device *mmu,
+					 struct ipmmu_vmsa_domain *domain)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&mmu->lock, flags);
+
+	ret = find_first_zero_bit(mmu->ctx, IPMMU_CTX_MAX);
+	if (ret != IPMMU_CTX_MAX) {
+		mmu->domains[ret] = domain;
+		set_bit(ret, mmu->ctx);
+	}
+
+	spin_unlock_irqrestore(&mmu->lock, flags);
+
+	return ret;
+}
+
 static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 {
 	u64 ttbr;
+	int ret;
 
 	/*
 	 * Allocate the page table operations.
@@ -327,10 +353,15 @@ static int ipmmu_domain_init_context(str
 		return -EINVAL;
 
 	/*
-	 * TODO: When adding support for multiple contexts, find an unused
-	 * context.
+	 * Find an unused context.
 	 */
-	domain->context_id = 0;
+	ret = ipmmu_domain_allocate_context(domain->mmu, domain);
+	if (ret == IPMMU_CTX_MAX) {
+		free_io_pgtable_ops(domain->iop);
+		return -EBUSY;
+	}
+
+	domain->context_id = ret;
 
 	/* TTBR0 */
 	ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -372,6 +403,19 @@ static int ipmmu_domain_init_context(str
 	return 0;
 }
 
+static void ipmmu_domain_free_context(struct ipmmu_vmsa_device *mmu,
+				      unsigned int context_id)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&mmu->lock, flags);
+
+	clear_bit(context_id, mmu->ctx);
+	mmu->domains[context_id] = NULL;
+
+	spin_unlock_irqrestore(&mmu->lock, flags);
+}
+
 static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
 {
 	/*
@@ -382,6 +426,7 @@ static void ipmmu_domain_destroy_context
 	 */
 	ipmmu_ctx_write(domain, IMCTR, IMCTR_FLUSH);
 	ipmmu_tlb_sync(domain);
+	ipmmu_domain_free_context(domain->mmu, domain->context_id);
 }
 
 /* -----------------------------------------------------------------------------
@@ -439,16 +484,25 @@ static irqreturn_t ipmmu_domain_irq(stru
 static irqreturn_t ipmmu_irq(int irq, void *dev)
 {
 	struct ipmmu_vmsa_device *mmu = dev;
-	struct iommu_domain *io_domain;
-	struct ipmmu_vmsa_domain *domain;
+	irqreturn_t status = IRQ_NONE;
+	unsigned int i;
+	unsigned long flags;
 
-	if (!mmu->mapping)
-		return IRQ_NONE;
+	spin_lock_irqsave(&mmu->lock, flags);
+
+	/*
+	 * Check interrupts for all active contexts.
+	 */
+	for (i = 0; i < IPMMU_CTX_MAX; i++) {
+		if (!mmu->domains[i])
+			continue;
+		if (ipmmu_domain_irq(mmu->domains[i]) == IRQ_HANDLED)
+			status = IRQ_HANDLED;
+	}
 
-	io_domain = mmu->mapping->domain;
-	domain = to_vmsa_domain(io_domain);
+	spin_unlock_irqrestore(&mmu->lock, flags);
 
-	return ipmmu_domain_irq(domain);
+	return status;
 }
 
 /* -----------------------------------------------------------------------------
@@ -776,6 +830,8 @@ static int ipmmu_probe(struct platform_d
 
 	mmu->dev = &pdev->dev;
 	mmu->num_utlbs = 32;
+	spin_lock_init(&mmu->lock);
+	bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
 
 	/* Map I/O memory and request IRQ. */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

[PATCH v7 03/07] iommu/ipmmu-vmsa: Break out utlb parsing code

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Break out the utlb parsing code and dev_data allocation into a
separate function. This is preparation for future code sharing.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---

Changes since V6:
 - None

 drivers/iommu/ipmmu-vmsa.c |   58 ++++++++++++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 21 deletions(-)

--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 18:33:38.600607110 +0900
@@ -649,22 +649,15 @@ static int ipmmu_find_utlbs(struct ipmmu
 	return 0;
 }
 
-static int ipmmu_add_device(struct device *dev)
+static int ipmmu_init_platform_device(struct device *dev)
 {
 	struct ipmmu_vmsa_archdata *archdata;
 	struct ipmmu_vmsa_device *mmu;
-	struct iommu_group *group = NULL;
 	unsigned int *utlbs;
 	unsigned int i;
 	int num_utlbs;
 	int ret = -ENODEV;
 
-	if (dev->archdata.iommu) {
-		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
-			 dev_name(dev));
-		return -EINVAL;
-	}
-
 	/* Find the master corresponding to the device. */
 
 	num_utlbs = of_count_phandle_with_args(dev->of_node, "iommus",
@@ -701,6 +694,36 @@ static int ipmmu_add_device(struct devic
 		}
 	}
 
+	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
+	if (!archdata) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
+	archdata->mmu = mmu;
+	archdata->utlbs = utlbs;
+	archdata->num_utlbs = num_utlbs;
+	dev->archdata.iommu = archdata;
+	return 0;
+
+error:
+	kfree(utlbs);
+	return ret;
+}
+
+static int ipmmu_add_device(struct device *dev)
+{
+	struct ipmmu_vmsa_archdata *archdata;
+	struct ipmmu_vmsa_device *mmu = NULL;
+	struct iommu_group *group;
+	int ret;
+
+	if (dev->archdata.iommu) {
+		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
+			 dev_name(dev));
+		return -EINVAL;
+	}
+
 	/* Create a device group and add the device to it. */
 	group = iommu_group_alloc();
 	if (IS_ERR(group)) {
@@ -718,16 +741,9 @@ static int ipmmu_add_device(struct devic
 		goto error;
 	}
 
-	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
-	if (!archdata) {
-		ret = -ENOMEM;
+	ret = ipmmu_init_platform_device(dev);
+	if (ret < 0)
 		goto error;
-	}
-
-	archdata->mmu = mmu;
-	archdata->utlbs = utlbs;
-	archdata->num_utlbs = num_utlbs;
-	dev->archdata.iommu = archdata;
 
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -738,6 +754,8 @@ static int ipmmu_add_device(struct devic
 	 * - Make the mapping size configurable ? We currently use a 2GB mapping
 	 *   at a 1GB offset to ensure that NULL VAs will fault.
 	 */
+	archdata = dev->archdata.iommu;
+	mmu = archdata->mmu;
 	if (!mmu->mapping) {
 		struct dma_iommu_mapping *mapping;
 
@@ -762,10 +780,8 @@ static int ipmmu_add_device(struct devic
 	return 0;
 
 error:
-	arm_iommu_release_mapping(mmu->mapping);
-
-	kfree(dev->archdata.iommu);
-	kfree(utlbs);
+	if (mmu)
+		arm_iommu_release_mapping(mmu->mapping);
 
 	dev->archdata.iommu = NULL;
 

[PATCH v7 04/07] iommu/ipmmu-vmsa: Break out domain allocation code

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Break out the domain allocation code into a separate function.
This is preparation for future code sharing.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---

 Changes since V6:
 - None

 drivers/iommu/ipmmu-vmsa.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

--- 0006/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 18:34:59.520607110 +0900
@@ -509,13 +509,10 @@ static irqreturn_t ipmmu_irq(int irq, vo
  * IOMMU Operations
  */
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+static struct iommu_domain *__ipmmu_domain_alloc(unsigned type)
 {
 	struct ipmmu_vmsa_domain *domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
-
 	domain = kzalloc(sizeof(*domain), GFP_KERNEL);
 	if (!domain)
 		return NULL;
@@ -525,6 +522,14 @@ static struct iommu_domain *ipmmu_domain
 	return &domain->io_domain;
 }
 
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+	if (type != IOMMU_DOMAIN_UNMANAGED)
+		return NULL;
+
+	return __ipmmu_domain_alloc(type);
+}
+
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);

Re: [PATCH v7 04/07] iommu/ipmmu-vmsa: Break out domain allocation code

[Geert Uytterhoeven]

On Tue, Mar 7, 2017 at 4:17 AM, Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
>
> Break out the domain allocation code into a separate function.
> This is preparation for future code sharing.
>
> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Introduce an alternative set of iommu_ops suitable for 64-bit ARM
as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
Kconfig to depend on ARM or IOMMU_DMA. Initialize the device
from ->xlate() when CONFIG_IOMMU_DMA=y.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
---

 Changes since V6:
 - Rolled in the following patches from "r8a7795 support V2":
   [PATCH v2 04/11] iommu/ipmmu-vmsa: Reuse iommu groups
   [PATCH v2 06/11] iommu/ipmmu-vmsa: Teach xlate() to skip disabled iommus
 - Moved find_group() implementation to prevent warning on 32-bit ARM
 - Rolled in the following patch from "IPMMU slave device whitelist V2":
   [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()

 drivers/iommu/Kconfig      |    1 
 drivers/iommu/ipmmu-vmsa.c |  164 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 157 insertions(+), 8 deletions(-)

--- 0001/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig	2017-03-06 18:42:42.000000000 +0900
@@ -274,6 +274,7 @@ config EXYNOS_IOMMU_DEBUG
 
 config IPMMU_VMSA
 	bool "Renesas VMSA-compatible IPMMU"
+	depends on ARM || IOMMU_DMA
 	depends on ARM_LPAE
 	depends on ARCH_RENESAS || COMPILE_TEST
 	select IOMMU_API
--- 0009/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 19:22:27.700607110 +0900
@@ -10,6 +10,7 @@
 
 #include <linux/bitmap.h>
 #include <linux/delay.h>
+#include <linux/dma-iommu.h>
 #include <linux/dma-mapping.h>
 #include <linux/err.h>
 #include <linux/export.h>
@@ -22,8 +23,10 @@
 #include <linux/sizes.h>
 #include <linux/slab.h>
 
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 #include <asm/dma-iommu.h>
 #include <asm/pgalloc.h>
+#endif
 
 #include "io-pgtable.h"
 
@@ -57,6 +60,8 @@ struct ipmmu_vmsa_archdata {
 	struct ipmmu_vmsa_device *mmu;
 	unsigned int *utlbs;
 	unsigned int num_utlbs;
+	struct device *dev;
+	struct list_head list;
 };
 
 static DEFINE_SPINLOCK(ipmmu_devices_lock);
@@ -522,14 +527,6 @@ static struct iommu_domain *__ipmmu_doma
 	return &domain->io_domain;
 }
 
-static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
-{
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
-
-	return __ipmmu_domain_alloc(type);
-}
-
 static void ipmmu_domain_free(struct iommu_domain *io_domain)
 {
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
@@ -572,6 +569,9 @@ static int ipmmu_attach_device(struct io
 		dev_err(dev, "Can't attach IPMMU %s to domain on IPMMU %s\n",
 			dev_name(mmu->dev), dev_name(domain->mmu->dev));
 		ret = -EINVAL;
+	} else {
+			dev_info(dev, "Reusing IPMMU context %u\n",
+				 domain->context_id);
 	}
 
 	spin_unlock_irqrestore(&domain->lock, flags);
@@ -708,6 +708,7 @@ static int ipmmu_init_platform_device(st
 	archdata->mmu = mmu;
 	archdata->utlbs = utlbs;
 	archdata->num_utlbs = num_utlbs;
+	archdata->dev = dev;
 	dev->archdata.iommu = archdata;
 	return 0;
 
@@ -716,6 +717,16 @@ error:
 	return ret;
 }
 
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
+
+static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
+{
+	if (type != IOMMU_DOMAIN_UNMANAGED)
+		return NULL;
+
+	return __ipmmu_domain_alloc(type);
+}
+
 static int ipmmu_add_device(struct device *dev)
 {
 	struct ipmmu_vmsa_archdata *archdata;
@@ -823,6 +834,141 @@ static const struct iommu_ops ipmmu_ops
 	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
 };
 
+#endif /* !CONFIG_ARM && CONFIG_IOMMU_DMA */
+
+#ifdef CONFIG_IOMMU_DMA
+
+static DEFINE_SPINLOCK(ipmmu_slave_devices_lock);
+static LIST_HEAD(ipmmu_slave_devices);
+
+static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
+{
+	struct iommu_domain *io_domain = NULL;
+
+	switch (type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		io_domain = __ipmmu_domain_alloc(type);
+		break;
+
+	case IOMMU_DOMAIN_DMA:
+		io_domain = __ipmmu_domain_alloc(type);
+		if (io_domain)
+			iommu_get_dma_cookie(io_domain);
+		break;
+	}
+
+	return io_domain;
+}
+
+static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
+{
+	switch (io_domain->type) {
+	case IOMMU_DOMAIN_DMA:
+		iommu_put_dma_cookie(io_domain);
+		/* fall-through */
+	default:
+		ipmmu_domain_free(io_domain);
+		break;
+	}
+}
+
+static int ipmmu_add_device_dma(struct device *dev)
+{
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct iommu_group *group;
+
+	/* The device has been verified in xlate() */
+	if (!archdata)
+		return -ENODEV;
+
+	group = iommu_group_get_for_dev(dev);
+	if (IS_ERR(group))
+		return PTR_ERR(group);
+
+	spin_lock(&ipmmu_slave_devices_lock);
+	list_add(&archdata->list, &ipmmu_slave_devices);
+	spin_unlock(&ipmmu_slave_devices_lock);
+	return 0;
+}
+
+static void ipmmu_remove_device_dma(struct device *dev)
+{
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+
+	spin_lock(&ipmmu_slave_devices_lock);
+	list_del(&archdata->list);
+	spin_unlock(&ipmmu_slave_devices_lock);
+
+	iommu_group_remove_device(dev);
+}
+
+static struct device *ipmmu_find_sibling_device(struct device *dev)
+{
+	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
+	bool found = false;
+
+	spin_lock(&ipmmu_slave_devices_lock);
+
+	list_for_each_entry(sibling_archdata, &ipmmu_slave_devices, list) {
+		if (archdata == sibling_archdata)
+			continue;
+		if (sibling_archdata->mmu == archdata->mmu) {
+			found = true;
+			break;
+		}
+	}
+
+	spin_unlock(&ipmmu_slave_devices_lock);
+
+	return found ? sibling_archdata->dev : NULL;
+}
+
+static struct iommu_group *ipmmu_find_group_dma(struct device *dev)
+{
+	struct iommu_group *group;
+	struct device *sibling;
+
+	sibling = ipmmu_find_sibling_device(dev);
+	if (sibling)
+		group = iommu_group_get(sibling);
+	if (!sibling || IS_ERR(group))
+		group = generic_device_group(dev);
+
+	return group;
+}
+
+static int ipmmu_of_xlate_dma(struct device *dev,
+			      struct of_phandle_args *spec)
+{
+	/* If the IPMMU device is disabled in DT then return error
+	 * to make sure the of_iommu code does not install ops
+	 * even though the iommu device is disabled
+	 */
+	if (!of_device_is_available(spec->np))
+		return -ENODEV;
+
+	return ipmmu_init_platform_device(dev);
+}
+
+static const struct iommu_ops ipmmu_ops = {
+	.domain_alloc = ipmmu_domain_alloc_dma,
+	.domain_free = ipmmu_domain_free_dma,
+	.attach_dev = ipmmu_attach_device,
+	.detach_dev = ipmmu_detach_device,
+	.map = ipmmu_map,
+	.unmap = ipmmu_unmap,
+	.map_sg = default_iommu_map_sg,
+	.iova_to_phys = ipmmu_iova_to_phys,
+	.add_device = ipmmu_add_device_dma,
+	.remove_device = ipmmu_remove_device_dma,
+	.device_group = ipmmu_find_group_dma,
+	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
+	.of_xlate = ipmmu_of_xlate_dma,
+};
+
+#endif /* CONFIG_IOMMU_DMA */
+
 /* -----------------------------------------------------------------------------
  * Probe/remove and init
  */
@@ -912,7 +1058,9 @@ static int ipmmu_remove(struct platform_
 	list_del(&mmu->list);
 	spin_unlock(&ipmmu_devices_lock);
 
+#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
 	arm_iommu_release_mapping(mmu->mapping);
+#endif
 
 	ipmmu_device_reset(mmu);
 

Re: [PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops

[Robin Murphy]

On 07/03/17 03:17, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Introduce an alternative set of iommu_ops suitable for 64-bit ARM
> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
> Kconfig to depend on ARM or IOMMU_DMA. Initialize the device
> from ->xlate() when CONFIG_IOMMU_DMA=y.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  Changes since V6:
>  - Rolled in the following patches from "r8a7795 support V2":
>    [PATCH v2 04/11] iommu/ipmmu-vmsa: Reuse iommu groups
>    [PATCH v2 06/11] iommu/ipmmu-vmsa: Teach xlate() to skip disabled iommus
>  - Moved find_group() implementation to prevent warning on 32-bit ARM
>  - Rolled in the following patch from "IPMMU slave device whitelist V2":
>    [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
> 
>  drivers/iommu/Kconfig      |    1 
>  drivers/iommu/ipmmu-vmsa.c |  164 +++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 157 insertions(+), 8 deletions(-)
> 
> --- 0001/drivers/iommu/Kconfig
> +++ work/drivers/iommu/Kconfig	2017-03-06 18:42:42.000000000 +0900
> @@ -274,6 +274,7 @@ config EXYNOS_IOMMU_DEBUG
>  
>  config IPMMU_VMSA
>  	bool "Renesas VMSA-compatible IPMMU"
> +	depends on ARM || IOMMU_DMA
>  	depends on ARM_LPAE
>  	depends on ARCH_RENESAS || COMPILE_TEST
>  	select IOMMU_API
> --- 0009/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 19:22:27.700607110 +0900
> @@ -10,6 +10,7 @@
>  
>  #include <linux/bitmap.h>
>  #include <linux/delay.h>
> +#include <linux/dma-iommu.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
> @@ -22,8 +23,10 @@
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
>  
> +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
>  #include <asm/dma-iommu.h>
>  #include <asm/pgalloc.h>
> +#endif
>  
>  #include "io-pgtable.h"
>  
> @@ -57,6 +60,8 @@ struct ipmmu_vmsa_archdata {
>  	struct ipmmu_vmsa_device *mmu;
>  	unsigned int *utlbs;
>  	unsigned int num_utlbs;
> +	struct device *dev;
> +	struct list_head list;
>  };
>  
>  static DEFINE_SPINLOCK(ipmmu_devices_lock);
> @@ -522,14 +527,6 @@ static struct iommu_domain *__ipmmu_doma
>  	return &domain->io_domain;
>  }
>  
> -static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
> -{
> -	if (type != IOMMU_DOMAIN_UNMANAGED)
> -		return NULL;
> -
> -	return __ipmmu_domain_alloc(type);
> -}
> -
>  static void ipmmu_domain_free(struct iommu_domain *io_domain)
>  {
>  	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
> @@ -572,6 +569,9 @@ static int ipmmu_attach_device(struct io
>  		dev_err(dev, "Can't attach IPMMU %s to domain on IPMMU %s\n",
>  			dev_name(mmu->dev), dev_name(domain->mmu->dev));
>  		ret = -EINVAL;
> +	} else {
> +			dev_info(dev, "Reusing IPMMU context %u\n",
> +				 domain->context_id);

Indentation?

>  	}
>  
>  	spin_unlock_irqrestore(&domain->lock, flags);
> @@ -708,6 +708,7 @@ static int ipmmu_init_platform_device(st
>  	archdata->mmu = mmu;
>  	archdata->utlbs = utlbs;
>  	archdata->num_utlbs = num_utlbs;
> +	archdata->dev = dev;
>  	dev->archdata.iommu = archdata;
>  	return 0;
>  
> @@ -716,6 +717,16 @@ error:
>  	return ret;
>  }
>  
> +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
> +
> +static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
> +{
> +	if (type != IOMMU_DOMAIN_UNMANAGED)
> +		return NULL;
> +
> +	return __ipmmu_domain_alloc(type);
> +}
> +
>  static int ipmmu_add_device(struct device *dev)
>  {
>  	struct ipmmu_vmsa_archdata *archdata;
> @@ -823,6 +834,141 @@ static const struct iommu_ops ipmmu_ops
>  	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
>  };
>  
> +#endif /* !CONFIG_ARM && CONFIG_IOMMU_DMA */
> +
> +#ifdef CONFIG_IOMMU_DMA
> +
> +static DEFINE_SPINLOCK(ipmmu_slave_devices_lock);
> +static LIST_HEAD(ipmmu_slave_devices);
> +
> +static struct iommu_domain *ipmmu_domain_alloc_dma(unsigned type)
> +{
> +	struct iommu_domain *io_domain = NULL;
> +
> +	switch (type) {
> +	case IOMMU_DOMAIN_UNMANAGED:
> +		io_domain = __ipmmu_domain_alloc(type);
> +		break;
> +
> +	case IOMMU_DOMAIN_DMA:
> +		io_domain = __ipmmu_domain_alloc(type);
> +		if (io_domain)
> +			iommu_get_dma_cookie(io_domain);
> +		break;
> +	}
> +
> +	return io_domain;
> +}

I still think it would be tidier to put this logic straight into
__ipmmu_domain_alloc(), and use that directly as the callback for this
case. The ipmmu_domain_alloc() wrapper ensures that IOMMU_DOMAIN_DMA
can't be passed through in the legacy 32-bit case, and the cookie calls
are stubbed for !CONFIG_IOMMU_DMA so there are no build concerns.

> +static void ipmmu_domain_free_dma(struct iommu_domain *io_domain)
> +{
> +	switch (io_domain->type) {
> +	case IOMMU_DOMAIN_DMA:
> +		iommu_put_dma_cookie(io_domain);
> +		/* fall-through */
> +	default:
> +		ipmmu_domain_free(io_domain);
> +		break;
> +	}
> +}

And similarly. The day when 32-bit ARM gets cleaned up to use groups and
default domains properly creeps ever closer (the probe deferral series
looks on-track to finally get in this cycle), so the less fragmentation
and #ifdeffery to untangle at that point the better.

> +static int ipmmu_add_device_dma(struct device *dev)
> +{
> +	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct iommu_group *group;
> +
> +	/* The device has been verified in xlate() */
> +	if (!archdata)
> +		return -ENODEV;
> +
> +	group = iommu_group_get_for_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	spin_lock(&ipmmu_slave_devices_lock);
> +	list_add(&archdata->list, &ipmmu_slave_devices);
> +	spin_unlock(&ipmmu_slave_devices_lock);
> +	return 0;
> +}
> +
> +static void ipmmu_remove_device_dma(struct device *dev)
> +{
> +	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +
> +	spin_lock(&ipmmu_slave_devices_lock);
> +	list_del(&archdata->list);
> +	spin_unlock(&ipmmu_slave_devices_lock);
> +
> +	iommu_group_remove_device(dev);
> +}
> +
> +static struct device *ipmmu_find_sibling_device(struct device *dev)
> +{
> +	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
> +	bool found = false;
> +
> +	spin_lock(&ipmmu_slave_devices_lock);
> +
> +	list_for_each_entry(sibling_archdata, &ipmmu_slave_devices, list) {
> +		if (archdata == sibling_archdata)
> +			continue;
> +		if (sibling_archdata->mmu == archdata->mmu) {

So every master behind the same IPMMU gets put in the same group? In
that case, you don't need any of this machinery - you can simply keep
track of a group per IPMMU instance directly. See mtk_iommu.c for an
example.

> +			found = true;
> +			break;
> +		}
> +	}
> +
> +	spin_unlock(&ipmmu_slave_devices_lock);
> +
> +	return found ? sibling_archdata->dev : NULL;
> +}
> +
> +static struct iommu_group *ipmmu_find_group_dma(struct device *dev)
> +{
> +	struct iommu_group *group;
> +	struct device *sibling;
> +
> +	sibling = ipmmu_find_sibling_device(dev);
> +	if (sibling)
> +		group = iommu_group_get(sibling);
> +	if (!sibling || IS_ERR(group))
> +		group = generic_device_group(dev);
> +
> +	return group;
> +}
> +
> +static int ipmmu_of_xlate_dma(struct device *dev,
> +			      struct of_phandle_args *spec)
> +{
> +	/* If the IPMMU device is disabled in DT then return error
> +	 * to make sure the of_iommu code does not install ops
> +	 * even though the iommu device is disabled
> +	 */

If you're only calling iommu_device_register() from ipmmu_probe(), you
should never see that problem in the first place (because a disabled
node won't even get its platform device created, let alone probe a
driver). I guess this a leftover from when the init_fn once called
of_iommu_set_ops()?

Robin.

> +	if (!of_device_is_available(spec->np))
> +		return -ENODEV;
> +
> +	return ipmmu_init_platform_device(dev);
> +}
> +
> +static const struct iommu_ops ipmmu_ops = {
> +	.domain_alloc = ipmmu_domain_alloc_dma,
> +	.domain_free = ipmmu_domain_free_dma,
> +	.attach_dev = ipmmu_attach_device,
> +	.detach_dev = ipmmu_detach_device,
> +	.map = ipmmu_map,
> +	.unmap = ipmmu_unmap,
> +	.map_sg = default_iommu_map_sg,
> +	.iova_to_phys = ipmmu_iova_to_phys,
> +	.add_device = ipmmu_add_device_dma,
> +	.remove_device = ipmmu_remove_device_dma,
> +	.device_group = ipmmu_find_group_dma,
> +	.pgsize_bitmap = SZ_1G | SZ_2M | SZ_4K,
> +	.of_xlate = ipmmu_of_xlate_dma,
> +};
> +
> +#endif /* CONFIG_IOMMU_DMA */
> +
>  /* -----------------------------------------------------------------------------
>   * Probe/remove and init
>   */
> @@ -912,7 +1058,9 @@ static int ipmmu_remove(struct platform_
>  	list_del(&mmu->list);
>  	spin_unlock(&ipmmu_devices_lock);
>  
> +#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
>  	arm_iommu_release_mapping(mmu->mapping);
> +#endif
>  
>  	ipmmu_device_reset(mmu);
>  
> 

Re: [PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops

[Joerg Roedel]

On Tue, Mar 07, 2017 at 12:17:33PM +0900, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> 
> Introduce an alternative set of iommu_ops suitable for 64-bit ARM
> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
> Kconfig to depend on ARM or IOMMU_DMA. Initialize the device
> from ->xlate() when CONFIG_IOMMU_DMA=y.
> 
> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> ---
> 
>  Changes since V6:
>  - Rolled in the following patches from "r8a7795 support V2":
>    [PATCH v2 04/11] iommu/ipmmu-vmsa: Reuse iommu groups
>    [PATCH v2 06/11] iommu/ipmmu-vmsa: Teach xlate() to skip disabled iommus
>  - Moved find_group() implementation to prevent warning on 32-bit ARM
>  - Rolled in the following patch from "IPMMU slave device whitelist V2":
>    [PATCH/RFC v2 3/4] iommu/ipmmu-vmsa: Check devices in xlate()
> 
>  drivers/iommu/Kconfig      |    1 
>  drivers/iommu/ipmmu-vmsa.c |  164 +++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 157 insertions(+), 8 deletions(-)

Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

[PATCH v7 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Not all architectures have an iommu member in their archdata, so
use #ifdefs support build with COMPILE_TEST on any architecture.

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---

Changes since V6:
 - Updated patch to handle newly introduced functions in:
   [PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops

 drivers/iommu/ipmmu-vmsa.c |   43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

--- 0010/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 19:26:26.070607110 +0900
@@ -72,6 +72,25 @@ static struct ipmmu_vmsa_domain *to_vmsa
 	return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
 }
 
+#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
+static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
+{
+	return dev->archdata.iommu;
+}
+static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
+{
+	dev->archdata.iommu = p;
+}
+#else
+static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
+{
+	return NULL;
+}
+static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
+{
+}
+#endif
+
 #define TLB_LOOP_TIMEOUT		100	/* 100us */
 
 /* -----------------------------------------------------------------------------
@@ -543,7 +562,7 @@ static void ipmmu_domain_free(struct iom
 static int ipmmu_attach_device(struct iommu_domain *io_domain,
 			       struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
 	struct ipmmu_vmsa_device *mmu = archdata->mmu;
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 	unsigned long flags;
@@ -588,7 +607,7 @@ static int ipmmu_attach_device(struct io
 static void ipmmu_detach_device(struct iommu_domain *io_domain,
 				struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 	unsigned int i;
 
@@ -709,7 +728,7 @@ static int ipmmu_init_platform_device(st
 	archdata->utlbs = utlbs;
 	archdata->num_utlbs = num_utlbs;
 	archdata->dev = dev;
-	dev->archdata.iommu = archdata;
+	set_archdata(dev, archdata);
 	return 0;
 
 error:
@@ -729,12 +748,11 @@ static struct iommu_domain *ipmmu_domain
 
 static int ipmmu_add_device(struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata;
 	struct ipmmu_vmsa_device *mmu = NULL;
 	struct iommu_group *group;
 	int ret;
 
-	if (dev->archdata.iommu) {
+	if (to_archdata(dev)) {
 		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
 			 dev_name(dev));
 		return -EINVAL;
@@ -770,8 +788,7 @@ static int ipmmu_add_device(struct devic
 	 * - Make the mapping size configurable ? We currently use a 2GB mapping
 	 *   at a 1GB offset to ensure that NULL VAs will fault.
 	 */
-	archdata = dev->archdata.iommu;
-	mmu = archdata->mmu;
+	mmu = to_archdata(dev)->mmu;
 	if (!mmu->mapping) {
 		struct dma_iommu_mapping *mapping;
 
@@ -799,7 +816,7 @@ error:
 	if (mmu)
 		arm_iommu_release_mapping(mmu->mapping);
 
-	dev->archdata.iommu = NULL;
+	set_archdata(dev, NULL);
 
 	if (!IS_ERR_OR_NULL(group))
 		iommu_group_remove_device(dev);
@@ -809,7 +826,7 @@ error:
 
 static void ipmmu_remove_device(struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
 
 	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
@@ -817,7 +834,7 @@ static void ipmmu_remove_device(struct d
 	kfree(archdata->utlbs);
 	kfree(archdata);
 
-	dev->archdata.iommu = NULL;
+	set_archdata(dev, NULL);
 }
 
 static const struct iommu_ops ipmmu_ops = {
@@ -874,7 +891,7 @@ static void ipmmu_domain_free_dma(struct
 
 static int ipmmu_add_device_dma(struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
 	struct iommu_group *group;
 
 	/* The device has been verified in xlate() */
@@ -893,7 +910,7 @@ static int ipmmu_add_device_dma(struct d
 
 static void ipmmu_remove_device_dma(struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
 
 	spin_lock(&ipmmu_slave_devices_lock);
 	list_del(&archdata->list);
@@ -904,7 +921,7 @@ static void ipmmu_remove_device_dma(stru
 
 static struct device *ipmmu_find_sibling_device(struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
 	struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
 	bool found = false;
 

Re: [PATCH v7 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access

[Geert Uytterhoeven]

Hi Magnus,

On Tue, Mar 7, 2017 at 4:17 AM, Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
>
> Not all architectures have an iommu member in their archdata, so
> use #ifdefs support build with COMPILE_TEST on any architecture.

to support

> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

> --- 0010/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c     2017-03-06 19:26:26.070607110 +0900
> @@ -72,6 +72,25 @@ static struct ipmmu_vmsa_domain *to_vmsa
>         return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
>  }
>
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> +       return dev->archdata.iommu;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> +       dev->archdata.iommu = p;
> +}
> +#else

#else /* compile testing */

> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> +       return NULL;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> +}
> +#endif

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v7 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access

[Robin Murphy]

On 07/03/17 03:17, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> 
> Not all architectures have an iommu member in their archdata, so
> use #ifdefs support build with COMPILE_TEST on any architecture.

I have a feeling I might be repeating myself, but ipmmu_vmsa_archdata
looks to be trivially convertible to iommu_fwspec, which I strongly
encourage, not least because it would obviate bodges like this.

Robin.

> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> ---
> 
> Changes since V6:
>  - Updated patch to handle newly introduced functions in:
>    [PATCH v7 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops
> 
>  drivers/iommu/ipmmu-vmsa.c |   43 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> --- 0010/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c	2017-03-06 19:26:26.070607110 +0900
> @@ -72,6 +72,25 @@ static struct ipmmu_vmsa_domain *to_vmsa
>  	return container_of(dom, struct ipmmu_vmsa_domain, io_domain);
>  }
>  
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> +	return dev->archdata.iommu;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> +	dev->archdata.iommu = p;
> +}
> +#else
> +static struct ipmmu_vmsa_archdata *to_archdata(struct device *dev)
> +{
> +	return NULL;
> +}
> +static void set_archdata(struct device *dev, struct ipmmu_vmsa_archdata *p)
> +{
> +}
> +#endif
> +
>  #define TLB_LOOP_TIMEOUT		100	/* 100us */
>  
>  /* -----------------------------------------------------------------------------
> @@ -543,7 +562,7 @@ static void ipmmu_domain_free(struct iom
>  static int ipmmu_attach_device(struct iommu_domain *io_domain,
>  			       struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>  	struct ipmmu_vmsa_device *mmu = archdata->mmu;
>  	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
>  	unsigned long flags;
> @@ -588,7 +607,7 @@ static int ipmmu_attach_device(struct io
>  static void ipmmu_detach_device(struct iommu_domain *io_domain,
>  				struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>  	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
>  	unsigned int i;
>  
> @@ -709,7 +728,7 @@ static int ipmmu_init_platform_device(st
>  	archdata->utlbs = utlbs;
>  	archdata->num_utlbs = num_utlbs;
>  	archdata->dev = dev;
> -	dev->archdata.iommu = archdata;
> +	set_archdata(dev, archdata);
>  	return 0;
>  
>  error:
> @@ -729,12 +748,11 @@ static struct iommu_domain *ipmmu_domain
>  
>  static int ipmmu_add_device(struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata;
>  	struct ipmmu_vmsa_device *mmu = NULL;
>  	struct iommu_group *group;
>  	int ret;
>  
> -	if (dev->archdata.iommu) {
> +	if (to_archdata(dev)) {
>  		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
>  			 dev_name(dev));
>  		return -EINVAL;
> @@ -770,8 +788,7 @@ static int ipmmu_add_device(struct devic
>  	 * - Make the mapping size configurable ? We currently use a 2GB mapping
>  	 *   at a 1GB offset to ensure that NULL VAs will fault.
>  	 */
> -	archdata = dev->archdata.iommu;
> -	mmu = archdata->mmu;
> +	mmu = to_archdata(dev)->mmu;
>  	if (!mmu->mapping) {
>  		struct dma_iommu_mapping *mapping;
>  
> @@ -799,7 +816,7 @@ error:
>  	if (mmu)
>  		arm_iommu_release_mapping(mmu->mapping);
>  
> -	dev->archdata.iommu = NULL;
> +	set_archdata(dev, NULL);
>  
>  	if (!IS_ERR_OR_NULL(group))
>  		iommu_group_remove_device(dev);
> @@ -809,7 +826,7 @@ error:
>  
>  static void ipmmu_remove_device(struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>  
>  	arm_iommu_detach_device(dev);
>  	iommu_group_remove_device(dev);
> @@ -817,7 +834,7 @@ static void ipmmu_remove_device(struct d
>  	kfree(archdata->utlbs);
>  	kfree(archdata);
>  
> -	dev->archdata.iommu = NULL;
> +	set_archdata(dev, NULL);
>  }
>  
>  static const struct iommu_ops ipmmu_ops = {
> @@ -874,7 +891,7 @@ static void ipmmu_domain_free_dma(struct
>  
>  static int ipmmu_add_device_dma(struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>  	struct iommu_group *group;
>  
>  	/* The device has been verified in xlate() */
> @@ -893,7 +910,7 @@ static int ipmmu_add_device_dma(struct d
>  
>  static void ipmmu_remove_device_dma(struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>  
>  	spin_lock(&ipmmu_slave_devices_lock);
>  	list_del(&archdata->list);
> @@ -904,7 +921,7 @@ static void ipmmu_remove_device_dma(stru
>  
>  static struct device *ipmmu_find_sibling_device(struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_archdata *archdata = to_archdata(dev);
>  	struct ipmmu_vmsa_archdata *sibling_archdata = NULL;
>  	bool found = false;
>  
> 

Re: [PATCH v7 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access

[Magnus Damm]

Hi Robin,

On Wed, Mar 8, 2017 at 9:48 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> On 07/03/17 03:17, Magnus Damm wrote:
>> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
>>
>> Not all architectures have an iommu member in their archdata, so
>> use #ifdefs support build with COMPILE_TEST on any architecture.
>
> I have a feeling I might be repeating myself, but ipmmu_vmsa_archdata
> looks to be trivially convertible to iommu_fwspec, which I strongly
> encourage, not least because it would obviate bodges like this.

Yeah, I think it should be possible to use iommu_fwspec for this
purpose. The question is when to do it. =)

I actually looked into it recently, but then realised that for this to
work then due to code sharing I need to make use of iommu_fwspec on
both 32-bit and 64-bit ARM. So it requires rework of the existing
IPMMU for 32-bit ARM (including hairy legacy CONFIG_IOMMU_DMA=n code).
I was actually thinking of doing some rework of 32-bit ARM IPMMU code
anyway (I suspect iommu_device_* conversion caused breakage) and it
probably has to happen on top of current -next. I would also like to
start reducing burden of forward porting all these patches, and
stirring up the ground does not really help much there...

Cheers,

/ magnus

Re: [PATCH v7 06/07] iommu/ipmmu-vmsa: ARM and ARM64 archdata access

[Robin Murphy]

On 09/03/17 03:44, Magnus Damm wrote:
> Hi Robin,
> 
> On Wed, Mar 8, 2017 at 9:48 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 07/03/17 03:17, Magnus Damm wrote:
>>> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
>>>
>>> Not all architectures have an iommu member in their archdata, so
>>> use #ifdefs support build with COMPILE_TEST on any architecture.
>>
>> I have a feeling I might be repeating myself, but ipmmu_vmsa_archdata
>> looks to be trivially convertible to iommu_fwspec, which I strongly
>> encourage, not least because it would obviate bodges like this.
> 
> Yeah, I think it should be possible to use iommu_fwspec for this
> purpose. The question is when to do it. =)

I'd actually be inclined to do it *before* any other major changes, as
it would be pretty minimal given the current structure of the driver.
But then I'm free to wilfully ignore the burden of maintaining patch
stacks between both mainline and older trees ;)

> I actually looked into it recently, but then realised that for this to
> work then due to code sharing I need to make use of iommu_fwspec on
> both 32-bit and 64-bit ARM. So it requires rework of the existing
> IPMMU for 32-bit ARM (including hairy legacy CONFIG_IOMMU_DMA=n code).
> I was actually thinking of doing some rework of 32-bit ARM IPMMU code
> anyway (I suspect iommu_device_* conversion caused breakage) and it
> probably has to happen on top of current -next. I would also like to
> start reducing burden of forward porting all these patches, and
> stirring up the ground does not really help much there...

Note that iommu_fwspec can be used pretty much orthogonally to any of
the core DMA ops support, so it shouldn't be as invasive as you might
think. See 84672f192671 ("iommu/mediatek: Convert M4Uv1 to
iommu_fwspec") as an example of an archdata-to-fwspec conversion of a
driver which only supports 32-bit ARM, and notably borrows its master
handling directly from the the IPMMU driver.

Robin.

> 
> Cheers,
> 
> / magnus
> 

[PATCH v7 07/07] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

[Magnus Damm]

From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>

Neither the ARM page table code enabled by IOMMU_IO_PGTABLE_LPAE
nor the IPMMU_VMSA driver actually depends on ARM_LPAE, so get
rid of the dependency.

Tested with ipmmu-vmsa on r8a7794 ALT and a kernel config using:
 # CONFIG_ARM_LPAE is not set

Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---

 Changes since V6:
 - None

 drivers/iommu/Kconfig |    1 -
 1 file changed, 1 deletion(-)

--- 0011/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig	2017-03-06 18:50:49.260607110 +0900
@@ -275,7 +275,6 @@ config EXYNOS_IOMMU_DEBUG
 config IPMMU_VMSA
 	bool "Renesas VMSA-compatible IPMMU"
 	depends on ARM || IOMMU_DMA
-	depends on ARM_LPAE
 	depends on ARCH_RENESAS || COMPILE_TEST
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE