Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1292084 > unrolled thread

[PATCH 00/06] iommu/ipmmu-vmsa: IPMMU multi-arch update

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/06] iommu/ipmmu-vmsa: IPMMU multi-arch update Magnus Damm <magnus.damm@gmail.com> - 2015-12-15 13:10 +0100
    [PATCH 06/06] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency Magnus Damm <magnus.damm@gmail.com> - 2015-12-15 13:10 +0100
      Re: [PATCH 06/06] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2015-12-29 01:30 +0100
    [PATCH 05/06] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code Magnus Damm <magnus.damm@gmail.com> - 2015-12-15 13:10 +0100
      Re: [PATCH 05/06] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2015-12-29 01:30 +0100
    [PATCH 02/06] iommu/ipmmu-vmsa: Convert to dev_data Magnus Damm <magnus.damm@gmail.com> - 2015-12-15 13:10 +0100
      Re: [PATCH 02/06] iommu/ipmmu-vmsa: Convert to dev_data Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2015-12-29 01:10 +0100
    [PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context Magnus Damm <magnus.damm@gmail.com> - 2015-12-15 13:10 +0100
      Re: [PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2015-12-29 01:20 +0100
    [PATCH 01/06] iommu/ipmmu-vmsa: Remove platform data handling Magnus Damm <magnus.damm@gmail.com> - 2015-12-15 13:10 +0100
      Re: [PATCH 01/06] iommu/ipmmu-vmsa: Remove platform data handling Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2015-12-29 01:10 +0100
    [PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function Magnus Damm <magnus.damm@gmail.com> - 2015-12-15 13:10 +0100
      Re: [PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function Geert Uytterhoeven <geert@linux-m68k.org> - 2015-12-15 16:50 +0100
        Re: [PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2015-12-29 01:10 +0100

#1292084 — [PATCH 00/06] iommu/ipmmu-vmsa: IPMMU multi-arch update

iommu/ipmmu-vmsa: IPMMU multi-arch update

[PATCH 01/06] iommu/ipmmu-vmsa: Remove platform data handling
[PATCH 02/06] iommu/ipmmu-vmsa: Convert to dev_data
[PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function
[PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context
[PATCH 05/06] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code
[PATCH 06/06] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

These patches update the IPMMU driver with a few minor changes
to support multiple architectures. With these patches applied
the driver is known to compile without issues on 32-bit ARM,
64-bit ARM and x86_64.

With the series applied the r8a7790 IPMMU still gets invoked
when the modetest utility is used to display a test pattern
on the VGA port driven by the DU device.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 Developed on top of renesas-drivers-2015-12-08-v4.4-rc4

 drivers/iommu/Kconfig      |    1 
 drivers/iommu/ipmmu-vmsa.c |  224 +++++++++++++++++++++++++++-----------------
 2 files changed, 139 insertions(+), 86 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1292087 — [PATCH 06/06] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

From: Magnus Damm <damm+renesas@opensource.se>

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@opensource.se>
---

 This time the result also compiles on x86. Need to be
 applied as last patch in the following series:
 [PATCH 00/06] iommu/ipmmu-vmsa: IPMMU multi-arch update

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

--- 0001/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig	2015-10-18 14:58:09.080513000 +0900
@@ -324,7 +324,6 @@ config SHMOBILE_IOMMU_L1SIZE
 
 config IPMMU_VMSA
 	bool "Renesas VMSA-compatible IPMMU"
-	depends on ARM_LPAE
 	depends on ARCH_SHMOBILE || COMPILE_TEST
 	select IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298810 — Re: [PATCH 06/06] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

Hi Magnus,

Thank you for the patch.

On Tuesday 15 December 2015 21:03:08 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> 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@opensource.se>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
>  This time the result also compiles on x86. Need to be
>  applied as last patch in the following series:
>  [PATCH 00/06] iommu/ipmmu-vmsa: IPMMU multi-arch update
> 
>  drivers/iommu/Kconfig |    1 -
>  1 file changed, 1 deletion(-)
> 
> --- 0001/drivers/iommu/Kconfig
> +++ work/drivers/iommu/Kconfig	2015-10-18 14:58:09.080513000 +0900
> @@ -324,7 +324,6 @@ config SHMOBILE_IOMMU_L1SIZE
> 
>  config IPMMU_VMSA
>  	bool "Renesas VMSA-compatible IPMMU"
> -	depends on ARM_LPAE
>  	depends on ARCH_SHMOBILE || COMPILE_TEST
>  	select IOMMU_API
>  	select IOMMU_IO_PGTABLE_LPAE

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292088 — [PATCH 05/06] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code

From: Magnus Damm <damm+renesas@opensource.se>

Make the driver compile on more than just 32-bit ARM
by breaking out and wrapping ARM specific functions
in #ifdefs. Needed to be able to use the driver on
other architectures.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

 drivers/iommu/ipmmu-vmsa.c |   94 +++++++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 32 deletions(-)

--- 0007/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 13:20:26.580513000 +0900
@@ -22,8 +22,10 @@
 #include <linux/sizes.h>
 #include <linux/slab.h>
 
+#ifdef CONFIG_ARM
 #include <asm/dma-iommu.h>
 #include <asm/pgalloc.h>
+#endif
 
 #include "io-pgtable.h"
 
@@ -38,7 +40,9 @@ struct ipmmu_vmsa_device {
 	DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
 	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
+#ifdef CONFIG_ARM
 	struct dma_iommu_mapping *mapping;
+#endif
 };
 
 struct ipmmu_vmsa_domain {
@@ -621,6 +625,60 @@ static int ipmmu_find_utlbs(struct ipmmu
 	return 0;
 }
 
+#ifdef CONFIG_ARM
+static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device *mmu)
+{
+	int ret;
+
+	/*
+	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
+	 * VAs. This will allocate a corresponding IOMMU domain.
+	 *
+	 * TODO:
+	 * - Create one mapping per context (TLB).
+	 * - Make the mapping size configurable ? We currently use a 2GB mapping
+	 *   at a 1GB offset to ensure that NULL VAs will fault.
+	 */
+	if (!mmu->mapping) {
+		struct dma_iommu_mapping *mapping;
+
+		mapping = arm_iommu_create_mapping(&platform_bus_type,
+						   SZ_1G, SZ_2G);
+		if (IS_ERR(mapping)) {
+			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
+			return PTR_ERR(mapping);
+		}
+
+		mmu->mapping = mapping;
+	}
+
+	/* Attach the ARM VA mapping to the device. */
+	ret = arm_iommu_attach_device(dev, mmu->mapping);
+	if (ret < 0) {
+		dev_err(dev, "Failed to attach device to VA mapping\n");
+		arm_iommu_release_mapping(mmu->mapping);
+	}
+
+	return ret;
+}
+static inline void ipmmu_detach(struct device *dev)
+{
+	arm_iommu_detach_device(dev);
+}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu)
+{
+	arm_iommu_release_mapping(mmu->mapping);
+}
+#else
+static inline int ipmmu_map_attach(struct device *dev,
+				   struct ipmmu_vmsa_device *mmu)
+{
+	return 0;
+}
+static inline void ipmmu_detach(struct device *dev) {}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu) {}
+#endif
+
 static int ipmmu_add_device(struct device *dev)
 {
 	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
@@ -701,41 +759,13 @@ static int ipmmu_add_device(struct devic
 	dev_data->num_utlbs = num_utlbs;
 	set_dev_data(dev, dev_data);
 
-	/*
-	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
-	 * VAs. This will allocate a corresponding IOMMU domain.
-	 *
-	 * TODO:
-	 * - Create one mapping per context (TLB).
-	 * - Make the mapping size configurable ? We currently use a 2GB mapping
-	 *   at a 1GB offset to ensure that NULL VAs will fault.
-	 */
-	if (!mmu->mapping) {
-		struct dma_iommu_mapping *mapping;
-
-		mapping = arm_iommu_create_mapping(&platform_bus_type,
-						   SZ_1G, SZ_2G);
-		if (IS_ERR(mapping)) {
-			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
-			ret = PTR_ERR(mapping);
-			goto error;
-		}
-
-		mmu->mapping = mapping;
-	}
-
-	/* Attach the ARM VA mapping to the device. */
-	ret = arm_iommu_attach_device(dev, mmu->mapping);
-	if (ret < 0) {
-		dev_err(dev, "Failed to attach device to VA mapping\n");
+	ret = ipmmu_map_attach(dev, mmu);
+	if (ret < 0)
 		goto error;
-	}
 
 	return 0;
 
 error:
-	arm_iommu_release_mapping(mmu->mapping);
-
 	kfree(dev_data);
 	kfree(utlbs);
 
@@ -751,7 +781,7 @@ static void ipmmu_remove_device(struct d
 {
 	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
 
-	arm_iommu_detach_device(dev);
+	ipmmu_detach(dev);
 	iommu_group_remove_device(dev);
 
 	kfree(dev_data->utlbs);
@@ -862,7 +892,7 @@ static int ipmmu_remove(struct platform_
 	list_del(&mmu->list);
 	spin_unlock(&ipmmu_devices_lock);
 
-	arm_iommu_release_mapping(mmu->mapping);
+	ipmmu_release_mapping(mmu);
 
 	ipmmu_device_reset(mmu);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298812 — Re: [PATCH 05/06] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code

Hi Magnus,

Thank you for the patch.

On Tuesday 15 December 2015 21:02:58 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Make the driver compile on more than just 32-bit ARM
> by breaking out and wrapping ARM specific functions
> in #ifdefs. Needed to be able to use the driver on
> other architectures.

The only other architecture where we need to use the driver is ARM64, and I'm 
not sure this is the right fix to correctly support ARM and ARM64. I believe 
we should instead improve the ARM implementation to get rid of 
arm_iommu_create_mapping() and arm_iommu_attach_device() the same way that 
ARM64 did.

> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
> drivers/iommu/ipmmu-vmsa.c |   94 ++++++++++++++++++++++++++---------------
> 1 file changed, 62 insertions(+), 32 deletions(-)
> 
> --- 0007/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 13:20:26.580513000 +0900
> @@ -22,8 +22,10 @@
>  #include <linux/sizes.h>
>  #include <linux/slab.h>
> 
> +#ifdef CONFIG_ARM
>  #include <asm/dma-iommu.h>
>  #include <asm/pgalloc.h>
> +#endif
> 
>  #include "io-pgtable.h"
> 
> @@ -38,7 +40,9 @@ struct ipmmu_vmsa_device {
>  	DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
>  	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
> 
> +#ifdef CONFIG_ARM
>  	struct dma_iommu_mapping *mapping;
> +#endif
>  };
> 
>  struct ipmmu_vmsa_domain {
> @@ -621,6 +625,60 @@ static int ipmmu_find_utlbs(struct ipmmu
>  	return 0;
>  }
> 
> +#ifdef CONFIG_ARM
> +static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device
> *mmu)
> +{
> +	int ret;
> +
> +	/*
> +	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
> +	 * VAs. This will allocate a corresponding IOMMU domain.
> +	 *
> +	 * TODO:
> +	 * - Create one mapping per context (TLB).
> +	 * - Make the mapping size configurable ? We currently use a 2GB mapping
> +	 *   at a 1GB offset to ensure that NULL VAs will fault.
> +	 */
> +	if (!mmu->mapping) {
> +		struct dma_iommu_mapping *mapping;
> +
> +		mapping = arm_iommu_create_mapping(&platform_bus_type,
> +						   SZ_1G, SZ_2G);
> +		if (IS_ERR(mapping)) {
> +			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
> +			return PTR_ERR(mapping);
> +		}
> +
> +		mmu->mapping = mapping;
> +	}
> +
> +	/* Attach the ARM VA mapping to the device. */
> +	ret = arm_iommu_attach_device(dev, mmu->mapping);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to attach device to VA mapping\n");
> +		arm_iommu_release_mapping(mmu->mapping);
> +	}
> +
> +	return ret;
> +}
> +static inline void ipmmu_detach(struct device *dev)
> +{
> +	arm_iommu_detach_device(dev);
> +}
> +static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu)
> +{
> +	arm_iommu_release_mapping(mmu->mapping);
> +}
> +#else
> +static inline int ipmmu_map_attach(struct device *dev,
> +				   struct ipmmu_vmsa_device *mmu)
> +{
> +	return 0;
> +}
> +static inline void ipmmu_detach(struct device *dev) {}
> +static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu) {}
> +#endif
> +
>  static int ipmmu_add_device(struct device *dev)
>  {
>  	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> @@ -701,41 +759,13 @@ static int ipmmu_add_device(struct devic
>  	dev_data->num_utlbs = num_utlbs;
>  	set_dev_data(dev, dev_data);
> 
> -	/*
> -	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
> -	 * VAs. This will allocate a corresponding IOMMU domain.
> -	 *
> -	 * TODO:
> -	 * - Create one mapping per context (TLB).
> -	 * - Make the mapping size configurable ? We currently use a 2GB mapping
> -	 *   at a 1GB offset to ensure that NULL VAs will fault.
> -	 */
> -	if (!mmu->mapping) {
> -		struct dma_iommu_mapping *mapping;
> -
> -		mapping = arm_iommu_create_mapping(&platform_bus_type,
> -						   SZ_1G, SZ_2G);
> -		if (IS_ERR(mapping)) {
> -			dev_err(mmu->dev, "failed to create ARM IOMMU mapping\n");
> -			ret = PTR_ERR(mapping);
> -			goto error;
> -		}
> -
> -		mmu->mapping = mapping;
> -	}
> -
> -	/* Attach the ARM VA mapping to the device. */
> -	ret = arm_iommu_attach_device(dev, mmu->mapping);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to attach device to VA mapping\n");
> +	ret = ipmmu_map_attach(dev, mmu);
> +	if (ret < 0)
>  		goto error;
> -	}
> 
>  	return 0;
> 
>  error:
> -	arm_iommu_release_mapping(mmu->mapping);
> -
>  	kfree(dev_data);
>  	kfree(utlbs);
> 
> @@ -751,7 +781,7 @@ static void ipmmu_remove_device(struct d
>  {
>  	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> 
> -	arm_iommu_detach_device(dev);
> +	ipmmu_detach(dev);
>  	iommu_group_remove_device(dev);
> 
>  	kfree(dev_data->utlbs);
> @@ -862,7 +892,7 @@ static int ipmmu_remove(struct platform_
>  	list_del(&mmu->list);
>  	spin_unlock(&ipmmu_devices_lock);
> 
> -	arm_iommu_release_mapping(mmu->mapping);
> +	ipmmu_release_mapping(mmu);
> 
>  	ipmmu_device_reset(mmu);

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292089 — [PATCH 02/06] iommu/ipmmu-vmsa: Convert to dev_data

From: Magnus Damm <damm+renesas@opensource.se>

Rename ipmmu_vmsa_archdata to ipmmu_vmsa_dev_data to avoid
confusion when using the driver on multiple architectures.

The data now stored in ipmmu_vmsa_dev_data is used to point
various on-chip devices to the actual IPMMU instances.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

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

--- 0003/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 13:16:27.160513000 +0900
@@ -47,7 +47,7 @@ struct ipmmu_vmsa_domain {
 	spinlock_t lock;			/* Protects mappings */
 };
 
-struct ipmmu_vmsa_archdata {
+struct ipmmu_vmsa_dev_data {
 	struct ipmmu_vmsa_device *mmu;
 	unsigned int *utlbs;
 	unsigned int num_utlbs;
@@ -182,6 +182,20 @@ static struct ipmmu_vmsa_domain *to_vmsa
 #define IMUASID_ASID0_SHIFT		0
 
 /* -----------------------------------------------------------------------------
+ * Consumer device side private data handling
+ */
+
+static struct ipmmu_vmsa_dev_data *get_dev_data(struct device *dev)
+{
+	return dev->archdata.iommu;
+}
+
+static void set_dev_data(struct device *dev, struct ipmmu_vmsa_dev_data *data)
+{
+	dev->archdata.iommu = data;
+}
+
+/* -----------------------------------------------------------------------------
  * Read/Write Access
  */
 
@@ -485,8 +499,8 @@ 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_device *mmu = archdata->mmu;
+	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
+	struct ipmmu_vmsa_device *mmu = dev_data->mmu;
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 	unsigned long flags;
 	unsigned int i;
@@ -518,8 +532,8 @@ static int ipmmu_attach_device(struct io
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < archdata->num_utlbs; ++i)
-		ipmmu_utlb_enable(domain, archdata->utlbs[i]);
+	for (i = 0; i < dev_data->num_utlbs; ++i)
+		ipmmu_utlb_enable(domain, dev_data->utlbs[i]);
 
 	return 0;
 }
@@ -527,12 +541,12 @@ 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_dev_data *dev_data = get_dev_data(dev);
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 	unsigned int i;
 
-	for (i = 0; i < archdata->num_utlbs; ++i)
-		ipmmu_utlb_disable(domain, archdata->utlbs[i]);
+	for (i = 0; i < dev_data->num_utlbs; ++i)
+		ipmmu_utlb_disable(domain, dev_data->utlbs[i]);
 
 	/*
 	 * TODO: Optimize by disabling the context when no device is attached.
@@ -595,7 +609,7 @@ static int ipmmu_find_utlbs(struct ipmmu
 
 static int ipmmu_add_device(struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata;
+	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
 	struct ipmmu_vmsa_device *mmu;
 	struct iommu_group *group = NULL;
 	unsigned int *utlbs;
@@ -603,7 +617,7 @@ static int ipmmu_add_device(struct devic
 	int num_utlbs;
 	int ret = -ENODEV;
 
-	if (dev->archdata.iommu) {
+	if (dev_data) {
 		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
 			 dev_name(dev));
 		return -EINVAL;
@@ -662,16 +676,16 @@ static int ipmmu_add_device(struct devic
 		goto error;
 	}
 
-	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
-	if (!archdata) {
+	dev_data = kzalloc(sizeof(*dev_data), GFP_KERNEL);
+	if (!dev_data) {
 		ret = -ENOMEM;
 		goto error;
 	}
 
-	archdata->mmu = mmu;
-	archdata->utlbs = utlbs;
-	archdata->num_utlbs = num_utlbs;
-	dev->archdata.iommu = archdata;
+	dev_data->mmu = mmu;
+	dev_data->utlbs = utlbs;
+	dev_data->num_utlbs = num_utlbs;
+	set_dev_data(dev, dev_data);
 
 	/*
 	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
@@ -708,10 +722,10 @@ static int ipmmu_add_device(struct devic
 error:
 	arm_iommu_release_mapping(mmu->mapping);
 
-	kfree(dev->archdata.iommu);
+	kfree(dev_data);
 	kfree(utlbs);
 
-	dev->archdata.iommu = NULL;
+	set_dev_data(dev, NULL);
 
 	if (!IS_ERR_OR_NULL(group))
 		iommu_group_remove_device(dev);
@@ -721,15 +735,15 @@ error:
 
 static void ipmmu_remove_device(struct device *dev)
 {
-	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
+	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
 
 	arm_iommu_detach_device(dev);
 	iommu_group_remove_device(dev);
 
-	kfree(archdata->utlbs);
-	kfree(archdata);
+	kfree(dev_data->utlbs);
+	kfree(dev_data);
 
-	dev->archdata.iommu = NULL;
+	set_dev_data(dev, NULL);
 }
 
 static const struct iommu_ops ipmmu_ops = {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298801 — Re: [PATCH 02/06] iommu/ipmmu-vmsa: Convert to dev_data

Hi Magnus,

Thank you for the patch.

On Tuesday 15 December 2015 21:02:30 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Rename ipmmu_vmsa_archdata to ipmmu_vmsa_dev_data to avoid
> confusion when using the driver on multiple architectures.

I'm sorry but this patches makes the driver more confusing to me. archdata is 
a well established concept for IOMMU on ARM, I don't see how hiding help would 
help.

> The data now stored in ipmmu_vmsa_dev_data is used to point
> various on-chip devices to the actual IPMMU instances.
> 
> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  drivers/iommu/ipmmu-vmsa.c |   58 +++++++++++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 22 deletions(-)
> 
> --- 0003/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 13:16:27.160513000 +0900
> @@ -47,7 +47,7 @@ struct ipmmu_vmsa_domain {
>  	spinlock_t lock;			/* Protects mappings */
>  };
> 
> -struct ipmmu_vmsa_archdata {
> +struct ipmmu_vmsa_dev_data {
>  	struct ipmmu_vmsa_device *mmu;
>  	unsigned int *utlbs;
>  	unsigned int num_utlbs;
> @@ -182,6 +182,20 @@ static struct ipmmu_vmsa_domain *to_vmsa
>  #define IMUASID_ASID0_SHIFT		0
> 
>  /* ------------------------------------------------------------------------
> + * Consumer device side private data handling
> + */
> +
> +static struct ipmmu_vmsa_dev_data *get_dev_data(struct device *dev)
> +{
> +	return dev->archdata.iommu;
> +}
> +
> +static void set_dev_data(struct device *dev, struct ipmmu_vmsa_dev_data
> *data)
> +{
> +	dev->archdata.iommu = data;
> +}
> +
> +/* ------------------------------------------------------------------------
>   * Read/Write Access
>   */
> 
> @@ -485,8 +499,8 @@ 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_device *mmu = archdata->mmu;
> +	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> +	struct ipmmu_vmsa_device *mmu = dev_data->mmu;
>  	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
>  	unsigned long flags;
>  	unsigned int i;
> @@ -518,8 +532,8 @@ static int ipmmu_attach_device(struct io
>  	if (ret < 0)
>  		return ret;
> 
> -	for (i = 0; i < archdata->num_utlbs; ++i)
> -		ipmmu_utlb_enable(domain, archdata->utlbs[i]);
> +	for (i = 0; i < dev_data->num_utlbs; ++i)
> +		ipmmu_utlb_enable(domain, dev_data->utlbs[i]);
> 
>  	return 0;
>  }
> @@ -527,12 +541,12 @@ 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_dev_data *dev_data = get_dev_data(dev);
>  	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
>  	unsigned int i;
> 
> -	for (i = 0; i < archdata->num_utlbs; ++i)
> -		ipmmu_utlb_disable(domain, archdata->utlbs[i]);
> +	for (i = 0; i < dev_data->num_utlbs; ++i)
> +		ipmmu_utlb_disable(domain, dev_data->utlbs[i]);
> 
>  	/*
>  	 * TODO: Optimize by disabling the context when no device is attached.
> @@ -595,7 +609,7 @@ static int ipmmu_find_utlbs(struct ipmmu
> 
>  static int ipmmu_add_device(struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata;
> +	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
>  	struct ipmmu_vmsa_device *mmu;
>  	struct iommu_group *group = NULL;
>  	unsigned int *utlbs;
> @@ -603,7 +617,7 @@ static int ipmmu_add_device(struct devic
>  	int num_utlbs;
>  	int ret = -ENODEV;
> 
> -	if (dev->archdata.iommu) {
> +	if (dev_data) {
>  		dev_warn(dev, "IOMMU driver already assigned to device %s\n",
>  			 dev_name(dev));
>  		return -EINVAL;
> @@ -662,16 +676,16 @@ static int ipmmu_add_device(struct devic
>  		goto error;
>  	}
> 
> -	archdata = kzalloc(sizeof(*archdata), GFP_KERNEL);
> -	if (!archdata) {
> +	dev_data = kzalloc(sizeof(*dev_data), GFP_KERNEL);
> +	if (!dev_data) {
>  		ret = -ENOMEM;
>  		goto error;
>  	}
> 
> -	archdata->mmu = mmu;
> -	archdata->utlbs = utlbs;
> -	archdata->num_utlbs = num_utlbs;
> -	dev->archdata.iommu = archdata;
> +	dev_data->mmu = mmu;
> +	dev_data->utlbs = utlbs;
> +	dev_data->num_utlbs = num_utlbs;
> +	set_dev_data(dev, dev_data);
> 
>  	/*
>  	 * Create the ARM mapping, used by the ARM DMA mapping core to allocate
> @@ -708,10 +722,10 @@ static int ipmmu_add_device(struct devic
>  error:
>  	arm_iommu_release_mapping(mmu->mapping);
> 
> -	kfree(dev->archdata.iommu);
> +	kfree(dev_data);
>  	kfree(utlbs);
> 
> -	dev->archdata.iommu = NULL;
> +	set_dev_data(dev, NULL);
> 
>  	if (!IS_ERR_OR_NULL(group))
>  		iommu_group_remove_device(dev);
> @@ -721,15 +735,15 @@ error:
> 
>  static void ipmmu_remove_device(struct device *dev)
>  {
> -	struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
> +	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> 
>  	arm_iommu_detach_device(dev);
>  	iommu_group_remove_device(dev);
> 
> -	kfree(archdata->utlbs);
> -	kfree(archdata);
> +	kfree(dev_data->utlbs);
> +	kfree(dev_data);
> 
> -	dev->archdata.iommu = NULL;
> +	set_dev_data(dev, NULL);
>  }
> 
>  static const struct iommu_ops ipmmu_ops = {

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292090 — [PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

From: Magnus Damm <damm+renesas@opensource.se>

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

At this point the number of contexts are still limited.

The purpose of this patch is to remove the use of the
ARM specific mapping variable from ipmmu_irq(). 

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

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

--- 0007/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 13:14:35.540513000 +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,16 @@
 
 #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;
+	DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
 	struct dma_iommu_mapping *mapping;
 };
@@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
 static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 {
 	phys_addr_t ttbr;
+	int ret;
 
 	/*
 	 * Allocate the page table operations.
@@ -348,10 +354,16 @@ 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 = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
+	if (ret < 0) {
+		free_io_pgtable_ops(domain->iop);
+		return ret;
+	}
+
+	domain->context_id = ret;
+	domain->mmu->domains[ret] = domain;
 
 	/* TTBR0 */
 	ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
 
 static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
 {
+	bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
+
 	/*
 	 * Disable the context. Flush the TLB as required when modifying the
 	 * context registers.
@@ -460,16 +474,16 @@ 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;
-
-	if (!mmu->mapping)
-		return IRQ_NONE;
+	irqreturn_t status = IRQ_NONE;
+	unsigned int k;
 
-	io_domain = mmu->mapping->domain;
-	domain = to_vmsa_domain(io_domain);
+	/* Check interrupts for all active contexts */
+	for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
+	     k < IPMMU_CTX_MAX && status == IRQ_NONE;
+	     k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))
+		status = ipmmu_domain_irq(mmu->domains[k]);
 
-	return ipmmu_domain_irq(domain);
+	return status;
 }
 
 /* -----------------------------------------------------------------------------
@@ -788,6 +802,7 @@ static int ipmmu_probe(struct platform_d
 
 	mmu->dev = &pdev->dev;
 	mmu->num_utlbs = 32;
+	bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
 
 	/* Map I/O memory and request IRQ. */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298808 — Re: [PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

Hi Magnus,

Thank you for the patch.

On Tuesday 15 December 2015 21:02:49 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> Introduce a bitmap for context handing and convert the
> interrupt routine to go handle all registered contexts.
> 
> At this point the number of contexts are still limited.

That's all nice, but without seeing support for multiple contexts it's hard to 
tell if the implementation is correct for multiple context purpose.

> The purpose of this patch is to remove the use of the
> ARM specific mapping variable from ipmmu_irq().

Why do you want to do that ?

> Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
> ---
> 
>  drivers/iommu/ipmmu-vmsa.c |   37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
> 
> --- 0007/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 13:14:35.540513000 +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,16 @@
> 
>  #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;
> +	DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);

We have up to 4 context on Gen2 and 8 on Gen3, a bitmap might be slightly 
overkill.

> +	struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
> 
>  	struct dma_iommu_mapping *mapping;
>  };
> @@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
>  static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
>  {
>  	phys_addr_t ttbr;
> +	int ret;
> 
>  	/*
>  	 * Allocate the page table operations.
> @@ -348,10 +354,16 @@ static int ipmmu_domain_init_context(str
>  		return -EINVAL;
> 
>  	/*
> -	 * TODO: When adding support for multiple contexts, find an unused
> -	 * context.
> +	 * Find an unused context.

We need to support multiple devices per context or we will very soon run out 
of contexts. How to pick a proper context is a topic that needs to be 
researched, I believe IOMMU groups might come into play.

>  	 */
> -	domain->context_id = 0;
> +	ret = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
> +	if (ret < 0) {
> +		free_io_pgtable_ops(domain->iop);
> +		return ret;
> +	}
> +
> +	domain->context_id = ret;
> +	domain->mmu->domains[ret] = domain;

This requires locking to protect against races with the interrupt handler.

> 
>  	/* TTBR0 */
>  	ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
> @@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
> 
>  static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
>  {
> +	bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
> +
>  	/*
>  	 * Disable the context. Flush the TLB as required when modifying the
>  	 * context registers.
> @@ -460,16 +474,16 @@ 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;
> -
> -	if (!mmu->mapping)
> -		return IRQ_NONE;
> +	irqreturn_t status = IRQ_NONE;
> +	unsigned int k;

i is a perfectly fine loop counter :-)

> -	io_domain = mmu->mapping->domain;
> -	domain = to_vmsa_domain(io_domain);
> +	/* Check interrupts for all active contexts */
> +	for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
> +	     k < IPMMU_CTX_MAX && status == IRQ_NONE;
> +	     k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))

You can just loop over mmu->domains and skip NULL entries.

> +		status = ipmmu_domain_irq(mmu->domains[k]);

Only the status of the last domain is taken into account.

> -	return ipmmu_domain_irq(domain);
> +	return status;
>  }
> 
>  /* ------------------------------------------------------------------------
> @@ -788,6 +802,7 @@ static int ipmmu_probe(struct platform_d
> 
>  	mmu->dev = &pdev->dev;
>  	mmu->num_utlbs = 32;
> +	bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
> 
>  	/* Map I/O memory and request IRQ. */
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292091 — [PATCH 01/06] iommu/ipmmu-vmsa: Remove platform data handling

From: Magnus Damm <damm+renesas@opensource.se>

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@opensource.se>
---

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

--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 11:35:00.490513000 +0900
@@ -766,11 +766,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");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298805 — Re: [PATCH 01/06] iommu/ipmmu-vmsa: Remove platform data handling

Hi Magnus,

Thank you for the patch.

On Tuesday 15 December 2015 21:02:21 Magnus Damm wrote:
> From: Magnus Damm <damm+renesas@opensource.se>
> 
> 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@opensource.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
>  drivers/iommu/ipmmu-vmsa.c |    5 -----
>  1 file changed, 5 deletions(-)
> 
> --- 0001/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 11:35:00.490513000 +0900
> @@ -766,11 +766,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");

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292092 — [PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function

From: Magnus Damm <damm+renesas@opensource.se>

Introduce the function ipmmu_utlb_ctrl() that shares code
to enable or disable utlbs.

Signed-off-by: Magnus Damm <damm+renesas@opensource.se>
---

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

--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c	2015-12-15 13:17:40.580513000 +0900
@@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm
 static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
 			       unsigned int utlb)
 {
-	struct ipmmu_vmsa_device *mmu = domain->mmu;
+	ipmmu_write(domain->mmu, IMUCTR(utlb), 0);
+}
+
+static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain,
+			    void (*fn)(struct ipmmu_vmsa_domain *,
+				       unsigned int utlb), struct device *dev)
+{
+	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
+	unsigned int i;
 
-	ipmmu_write(mmu, IMUCTR(utlb), 0);
+	for (i = 0; i < dev_data->num_utlbs; ++i)
+		fn(domain, dev_data->utlbs[i]);
 }
 
 static void ipmmu_tlb_flush_all(void *cookie)
@@ -503,7 +512,6 @@ static int ipmmu_attach_device(struct io
 	struct ipmmu_vmsa_device *mmu = dev_data->mmu;
 	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
 	unsigned long flags;
-	unsigned int i;
 	int ret = 0;
 
 	if (!mmu) {
@@ -529,24 +537,16 @@ static int ipmmu_attach_device(struct io
 
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-	if (ret < 0)
-		return ret;
+	if (!ret)
+		ipmmu_utlb_ctrl(domain, ipmmu_utlb_enable, dev);
 
-	for (i = 0; i < dev_data->num_utlbs; ++i)
-		ipmmu_utlb_enable(domain, dev_data->utlbs[i]);
-
-	return 0;
+	return ret;
 }
 
 static void ipmmu_detach_device(struct iommu_domain *io_domain,
 				struct device *dev)
 {
-	struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
-	struct ipmmu_vmsa_domain *domain = to_vmsa_domain(io_domain);
-	unsigned int i;
-
-	for (i = 0; i < dev_data->num_utlbs; ++i)
-		ipmmu_utlb_disable(domain, dev_data->utlbs[i]);
+	ipmmu_utlb_ctrl(to_vmsa_domain(io_domain), ipmmu_utlb_disable, dev);
 
 	/*
 	 * TODO: Optimize by disabling the context when no device is attached.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292300 — Re: [PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function

Hi Magnus,

On Tue, Dec 15, 2015 at 1:02 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> --- 0004/drivers/iommu/ipmmu-vmsa.c
> +++ work/drivers/iommu/ipmmu-vmsa.c     2015-12-15 13:17:40.580513000 +0900
> @@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm
>  static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
>                                unsigned int utlb)
>  {
> -       struct ipmmu_vmsa_device *mmu = domain->mmu;
> +       ipmmu_write(domain->mmu, IMUCTR(utlb), 0);
> +}
> +
> +static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain,
> +                           void (*fn)(struct ipmmu_vmsa_domain *,
> +                                      unsigned int utlb), struct device *dev)
> +{
> +       struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> +       unsigned int i;
>
> -       ipmmu_write(mmu, IMUCTR(utlb), 0);
> +       for (i = 0; i < dev_data->num_utlbs; ++i)
> +               fn(domain, dev_data->utlbs[i]);
>  }

Unless you have further plans with the "fn" parameter, I would simply pass
a bool enable/disable flag instead of a function pointer.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298803 — Re: [PATCH 03/06] iommu/ipmmu-vmsa: Break out utlb control function

Hi Magnus,

On Tuesday 15 December 2015 16:48:42 Geert Uytterhoeven wrote:
> On Tue, Dec 15, 2015 at 1:02 PM, Magnus Damm <magnus.damm@gmail.com> wrote:
> > --- 0004/drivers/iommu/ipmmu-vmsa.c
> > +++ work/drivers/iommu/ipmmu-vmsa.c     2015-12-15 13:17:40.580513000
> > +0900
> > @@ -279,9 +279,18 @@ static void ipmmu_utlb_enable(struct ipm
> >  static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
> >                                unsigned int utlb)
> >  {
> > -       struct ipmmu_vmsa_device *mmu = domain->mmu;
> > +       ipmmu_write(domain->mmu, IMUCTR(utlb), 0);
> > +}
> > +
> > +static void ipmmu_utlb_ctrl(struct ipmmu_vmsa_domain *domain,
> > +                           void (*fn)(struct ipmmu_vmsa_domain *,
> > +                                      unsigned int utlb), struct device
> > *dev)
> > +{
> > +       struct ipmmu_vmsa_dev_data *dev_data = get_dev_data(dev);
> > +       unsigned int i;
> > 
> > -       ipmmu_write(mmu, IMUCTR(utlb), 0);
> > +       for (i = 0; i < dev_data->num_utlbs; ++i)
> > +               fn(domain, dev_data->utlbs[i]);
> > 
> >  }
> 
> Unless you have further plans with the "fn" parameter, I would simply pass
> a bool enable/disable flag instead of a function pointer.

I agree with Geert. What's your plan here ?

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web