[PATCHv3 0/3] arm:dma-mapping Refactoring iommu dma-mapping code

[PATCHv3 0/3] arm:dma-mapping Refactoring iommu dma-mapping code

[ritesh.harjani at gmail.com]

From: Ritesh Harjani <ritesh.harjani@gmail.com>

Hi All, 

This patch series is to refactor iommu related common code from
arch/arm/dma-mapping.c to lib/iommu-helper.c based on the various
discussions with the maintainers/experts [1].

Currently the only user of the common lib/iommu-helper code will
be ARM & ARM64 but later various architecture might try to use this
iommu lib helper functions.

Major change of this refactoring depends on bringing out struct dma_iommu_mapping 
*mapping variable from arch/arm/include/asm/device.h to include/linux/device.h
and by moving out complete structure definition of dma_iommu_mapping to
inclue/linux/iommu-helper.h. These changes are now part of this patch series based
on Greg's(cc'd) comments[2].

There are 1/2 more function definitions which I can think of moving out, but
those can be done once this patch series is approved as those are not very
big changes.

Also, this has been tested and verified on ARM hardware with IOMMU support.

Changes v2 -> v3:
1. Included changes in device.h in this patch series[1/3] itself, as Greg
mentioned that there is no point in approving device.h changes without actual
refactoring changes gets approved [2].

So, I guess device.h changes should be fine but let Greg confirms it.


Links:
[1]: https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg03458.html 
[2]: https://lkml.org/lkml/2014/6/4/458


Ritesh Harjani (3):
  device.h: arm dma-iommu: Move out dma_iommu_mapping struct
  arm: dma-mapping: Refactor attach/detach, alloc/free func
  arm:dma-iommu: Move out complete func defs

 arch/arm/Kconfig                          |  46 +--
 arch/arm/include/asm/device.h             |   9 -
 arch/arm/include/asm/dma-iommu.h          |  16 +-
 arch/arm/mm/dma-mapping.c                 | 578 +++---------------------------
 drivers/gpu/drm/exynos/exynos_drm_iommu.c |  10 +-
 include/linux/device.h                    |   5 +
 include/linux/iommu-helper.h              |  61 ++++
 lib/iommu-helper.c                        | 562 +++++++++++++++++++++++++++++
 8 files changed, 710 insertions(+), 577 deletions(-)

-- 
1.8.1.3

[PATCHv3 1/3] device.h: arm dma-iommu: Move out dma_iommu_mapping struct

[ritesh.harjani at gmail.com]

From: Ritesh Harjani <ritesh.harjani@gmail.com>

This patch moves out *mapping pointer of dma_iommu_mapping
from arch/arm/include/asm/device.h to include/liunux/device.h

Also, it moves out complete structre definition of dma_iommu_mapping
to include/linux/iommu-helper.h

This is done since arm iommu's dma-mapping arch independent code,
needs to be moved out to lib/iommu-helper.c, this means
dma_iommu_mapping will be arch independent and later other archs
can make use of it.

Signed-off-by: Ritesh Harjani <ritesh.harjani@gmail.com>
---
 arch/arm/Kconfig                          |  4 ++++
 arch/arm/include/asm/device.h             |  9 ---------
 arch/arm/include/asm/dma-iommu.h          | 16 +---------------
 arch/arm/mm/dma-mapping.c                 | 20 ++++++++++----------
 drivers/gpu/drm/exynos/exynos_drm_iommu.c | 10 +++++-----
 include/linux/device.h                    |  5 +++++
 include/linux/iommu-helper.h              | 19 +++++++++++++++++++
 7 files changed, 44 insertions(+), 39 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index db3c541..e65042f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -92,6 +92,7 @@ config ARM_DMA_USE_IOMMU
 	bool
 	select ARM_HAS_SG_CHAIN
 	select NEED_SG_DMA_LENGTH
+	select DMA_USE_IOMMU_HELPER_MAPPING
 
 if ARM_DMA_USE_IOMMU
 
@@ -1911,6 +1912,9 @@ config SWIOTLB
 config IOMMU_HELPER
 	def_bool SWIOTLB
 
+config DMA_USE_IOMMU_HELPER_MAPPING
+	def_bool n
+
 config XEN_DOM0
 	def_bool y
 	depends on XEN
diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
index dc662fc..6e2cb0e 100644
--- a/arch/arm/include/asm/device.h
+++ b/arch/arm/include/asm/device.h
@@ -14,9 +14,6 @@ struct dev_archdata {
 #ifdef CONFIG_IOMMU_API
 	void *iommu; /* private IOMMU data */
 #endif
-#ifdef CONFIG_ARM_DMA_USE_IOMMU
-	struct dma_iommu_mapping	*mapping;
-#endif
 };
 
 struct omap_device;
@@ -27,10 +24,4 @@ struct pdev_archdata {
 #endif
 };
 
-#ifdef CONFIG_ARM_DMA_USE_IOMMU
-#define to_dma_iommu_mapping(dev) ((dev)->archdata.mapping)
-#else
-#define to_dma_iommu_mapping(dev) NULL
-#endif
-
 #endif
diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
index 8e3fcb9..50c010b 100644
--- a/arch/arm/include/asm/dma-iommu.h
+++ b/arch/arm/include/asm/dma-iommu.h
@@ -8,21 +8,7 @@
 #include <linux/dma-debug.h>
 #include <linux/kmemcheck.h>
 #include <linux/kref.h>
-
-struct dma_iommu_mapping {
-	/* iommu specific data */
-	struct iommu_domain	*domain;
-
-	unsigned long		**bitmaps;	/* array of bitmaps */
-	unsigned int		nr_bitmaps;	/* nr of elements in array */
-	unsigned int		extensions;
-	size_t			bitmap_size;	/* size of a single bitmap */
-	size_t			bits;		/* per bitmap */
-	dma_addr_t		base;
-
-	spinlock_t		lock;
-	struct kref		kref;
-};
+#include <linux/iommu-helper.h>
 
 struct dma_iommu_mapping *
 arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 3d43c41..b82561e 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1292,7 +1292,7 @@ err:
 static dma_addr_t
 __iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
 	dma_addr_t dma_addr, iova;
 	int i, ret = DMA_ERROR_CODE;
@@ -1328,7 +1328,7 @@ fail:
 
 static int __iommu_remove_mapping(struct device *dev, dma_addr_t iova, size_t size)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 
 	/*
 	 * add optional in-page offset from iova to size and align
@@ -1541,7 +1541,7 @@ static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
 			  enum dma_data_direction dir, struct dma_attrs *attrs,
 			  bool is_coherent)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 	dma_addr_t iova, iova_base;
 	int ret = 0;
 	unsigned int count;
@@ -1762,7 +1762,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
 	     unsigned long offset, size_t size, enum dma_data_direction dir,
 	     struct dma_attrs *attrs)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 	dma_addr_t dma_addr;
 	int ret, prot, len = PAGE_ALIGN(size + offset);
 
@@ -1815,7 +1815,7 @@ static void arm_coherent_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 		size_t size, enum dma_data_direction dir,
 		struct dma_attrs *attrs)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 	dma_addr_t iova = handle & PAGE_MASK;
 	int offset = handle & ~PAGE_MASK;
 	int len = PAGE_ALIGN(size + offset);
@@ -1840,7 +1840,7 @@ static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 		size_t size, enum dma_data_direction dir,
 		struct dma_attrs *attrs)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 	dma_addr_t iova = handle & PAGE_MASK;
 	struct page *page = phys_to_page(iommu_iova_to_phys(mapping->domain, iova));
 	int offset = handle & ~PAGE_MASK;
@@ -1859,7 +1859,7 @@ static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 static void arm_iommu_sync_single_for_cpu(struct device *dev,
 		dma_addr_t handle, size_t size, enum dma_data_direction dir)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 	dma_addr_t iova = handle & PAGE_MASK;
 	struct page *page = phys_to_page(iommu_iova_to_phys(mapping->domain, iova));
 	unsigned int offset = handle & ~PAGE_MASK;
@@ -1873,7 +1873,7 @@ static void arm_iommu_sync_single_for_cpu(struct device *dev,
 static void arm_iommu_sync_single_for_device(struct device *dev,
 		dma_addr_t handle, size_t size, enum dma_data_direction dir)
 {
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 	dma_addr_t iova = handle & PAGE_MASK;
 	struct page *page = phys_to_page(iommu_iova_to_phys(mapping->domain, iova));
 	unsigned int offset = handle & ~PAGE_MASK;
@@ -2045,7 +2045,7 @@ int arm_iommu_attach_device(struct device *dev,
 		return err;
 
 	kref_get(&mapping->kref);
-	dev->archdata.mapping = mapping;
+	dev->mapping = mapping;
 	set_dma_ops(dev, &iommu_ops);
 
 	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
@@ -2072,7 +2072,7 @@ void arm_iommu_detach_device(struct device *dev)
 
 	iommu_detach_device(mapping->domain, dev);
 	kref_put(&mapping->kref, release_iommu_mapping);
-	dev->archdata.mapping = NULL;
+	dev->mapping = NULL;
 	set_dma_ops(dev, NULL);
 
 	pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
index b32b291..9dbf369 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.c
@@ -49,7 +49,7 @@ int drm_create_iommu_mapping(struct drm_device *drm_dev)
 		goto error;
 
 	dma_set_max_seg_size(dev, 0xffffffffu);
-	dev->archdata.mapping = mapping;
+	dev->mapping = mapping;
 
 	return 0;
 error:
@@ -69,7 +69,7 @@ void drm_release_iommu_mapping(struct drm_device *drm_dev)
 {
 	struct device *dev = drm_dev->dev;
 
-	arm_iommu_release_mapping(dev->archdata.mapping);
+	arm_iommu_release_mapping(dev->mapping);
 }
 
 /*
@@ -87,7 +87,7 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 	struct device *dev = drm_dev->dev;
 	int ret;
 
-	if (!dev->archdata.mapping) {
+	if (!dev->mapping) {
 		DRM_ERROR("iommu_mapping is null.\n");
 		return -EFAULT;
 	}
@@ -100,7 +100,7 @@ int drm_iommu_attach_device(struct drm_device *drm_dev,
 
 	dma_set_max_seg_size(subdrv_dev, 0xffffffffu);
 
-	ret = arm_iommu_attach_device(subdrv_dev, dev->archdata.mapping);
+	ret = arm_iommu_attach_device(subdrv_dev, dev->mapping);
 	if (ret < 0) {
 		DRM_DEBUG_KMS("failed iommu attach.\n");
 		return ret;
@@ -133,7 +133,7 @@ void drm_iommu_detach_device(struct drm_device *drm_dev,
 				struct device *subdrv_dev)
 {
 	struct device *dev = drm_dev->dev;
-	struct dma_iommu_mapping *mapping = dev->archdata.mapping;
+	struct dma_iommu_mapping *mapping = dev->mapping;
 
 	if (!mapping || !mapping->domain)
 		return;
diff --git a/include/linux/device.h b/include/linux/device.h
index d1d1c05..2bb58c9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -761,6 +761,11 @@ struct device {
 	struct cma *cma_area;		/* contiguous memory area for dma
 					   allocations */
 #endif
+
+#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
+	struct dma_iommu_mapping	*mapping;
+#endif
+
 	/* arch specific additions */
 	struct dev_archdata	archdata;
 
diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h
index 86bdeff..961d8ef 100644
--- a/include/linux/iommu-helper.h
+++ b/include/linux/iommu-helper.h
@@ -3,6 +3,25 @@
 
 #include <linux/kernel.h>
 
+#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
+struct dma_iommu_mapping {
+	/* iommu specific data */
+	struct iommu_domain	*domain;
+
+	unsigned long		**bitmaps;	/* array of bitmaps */
+	unsigned int		nr_bitmaps;	/* nr of elements in array */
+	unsigned int		extensions;
+	size_t			bitmap_size;	/* size of a single bitmap */
+	size_t			bits;		/* per bitmap */
+	dma_addr_t		base;
+
+	spinlock_t		lock;
+	struct kref		kref;
+};
+
+#define to_dma_iommu_mapping(dev) ((dev)->mapping)
+#endif
+
 static inline unsigned long iommu_device_max_index(unsigned long size,
 						   unsigned long offset,
 						   u64 dma_mask)
-- 
1.8.1.3

[PATCHv3 2/3] arm: dma-mapping: Refactor attach/detach, alloc/free func

[ritesh.harjani at gmail.com]

From: Ritesh Harjani <ritesh.harjani@gmail.com>

Refactor following function calls to lib/iommu-helper.c

1.
arm_iommu_attach/detach device function calls.
arm_iommu_init/release_mapping function calls.

2. iommu_alloc/free_buffer can be moved out from
arm/dma-mapping.c to lib/iommu_helper.c

Signed-off-by: Ritesh Harjani <ritesh.harjani@gmail.com>
---
 arch/arm/mm/dma-mapping.c    | 200 ++++--------------------------------
 include/linux/iommu-helper.h |  18 ++++
 lib/iommu-helper.c           | 235 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+), 181 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index b82561e..66cf96b 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1161,98 +1161,6 @@ static inline void __free_iova(struct dma_iommu_mapping *mapping,
 	spin_unlock_irqrestore(&mapping->lock, flags);
 }
 
-static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
-					  gfp_t gfp, struct dma_attrs *attrs)
-{
-	struct page **pages;
-	int count = size >> PAGE_SHIFT;
-	int array_size = count * sizeof(struct page *);
-	int i = 0;
-
-	if (array_size <= PAGE_SIZE)
-		pages = kzalloc(array_size, gfp);
-	else
-		pages = vzalloc(array_size);
-	if (!pages)
-		return NULL;
-
-	if (dma_get_attr(DMA_ATTR_FORCE_CONTIGUOUS, attrs))
-	{
-		unsigned long order = get_order(size);
-		struct page *page;
-
-		page = dma_alloc_from_contiguous(dev, count, order);
-		if (!page)
-			goto error;
-
-		__dma_clear_buffer(page, size);
-
-		for (i = 0; i < count; i++)
-			pages[i] = page + i;
-
-		return pages;
-	}
-
-	/*
-	 * IOMMU can map any pages, so himem can also be used here
-	 */
-	gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
-
-	while (count) {
-		int j, order = __fls(count);
-
-		pages[i] = alloc_pages(gfp, order);
-		while (!pages[i] && order)
-			pages[i] = alloc_pages(gfp, --order);
-		if (!pages[i])
-			goto error;
-
-		if (order) {
-			split_page(pages[i], order);
-			j = 1 << order;
-			while (--j)
-				pages[i + j] = pages[i] + j;
-		}
-
-		__dma_clear_buffer(pages[i], PAGE_SIZE << order);
-		i += 1 << order;
-		count -= 1 << order;
-	}
-
-	return pages;
-error:
-	while (i--)
-		if (pages[i])
-			__free_pages(pages[i], 0);
-	if (array_size <= PAGE_SIZE)
-		kfree(pages);
-	else
-		vfree(pages);
-	return NULL;
-}
-
-static int __iommu_free_buffer(struct device *dev, struct page **pages,
-			       size_t size, struct dma_attrs *attrs)
-{
-	int count = size >> PAGE_SHIFT;
-	int array_size = count * sizeof(struct page *);
-	int i;
-
-	if (dma_get_attr(DMA_ATTR_FORCE_CONTIGUOUS, attrs)) {
-		dma_release_from_contiguous(dev, pages[0], count);
-	} else {
-		for (i = 0; i < count; i++)
-			if (pages[i])
-				__free_pages(pages[i], 0);
-	}
-
-	if (array_size <= PAGE_SIZE)
-		kfree(pages);
-	else
-		vfree(pages);
-	return 0;
-}
-
 /*
  * Create a CPU mapping for a specified pages
  */
@@ -1417,7 +1325,8 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	 */
 	gfp &= ~(__GFP_COMP);
 
-	pages = __iommu_alloc_buffer(dev, size, gfp, attrs);
+	pages = iommu_helper_alloc_buffer(dev, size, gfp, attrs,
+					__dma_clear_buffer);
 	if (!pages)
 		return NULL;
 
@@ -1438,7 +1347,7 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 err_mapping:
 	__iommu_remove_mapping(dev, *handle, size);
 err_buffer:
-	__iommu_free_buffer(dev, pages, size, attrs);
+	iommu_helper_free_buffer(dev, pages, size, attrs);
 	return NULL;
 }
 
@@ -1495,7 +1404,7 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
 	}
 
 	__iommu_remove_mapping(dev, handle, size);
-	__iommu_free_buffer(dev, pages, size, attrs);
+	iommu_helper_free_buffer(dev, pages, size, attrs);
 }
 
 static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
@@ -1924,7 +1833,8 @@ struct dma_map_ops iommu_coherent_ops = {
  * @base: start address of the valid IO address space
  * @size: maximum size of the valid IO address space
  *
- * Creates a mapping structure which holds information about used/unused
+ * Calls for lib/iommu-helper function which creates a mapping
+ * structure which holds information about used/unused
  * IO address ranges, which is required to perform memory allocation and
  * mapping with IOMMU aware functions.
  *
@@ -1934,71 +1844,10 @@ struct dma_map_ops iommu_coherent_ops = {
 struct dma_iommu_mapping *
 arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
 {
-	unsigned int bits = size >> PAGE_SHIFT;
-	unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
-	struct dma_iommu_mapping *mapping;
-	int extensions = 1;
-	int err = -ENOMEM;
-
-	if (!bitmap_size)
-		return ERR_PTR(-EINVAL);
-
-	if (bitmap_size > PAGE_SIZE) {
-		extensions = bitmap_size / PAGE_SIZE;
-		bitmap_size = PAGE_SIZE;
-	}
-
-	mapping = kzalloc(sizeof(struct dma_iommu_mapping), GFP_KERNEL);
-	if (!mapping)
-		goto err;
-
-	mapping->bitmap_size = bitmap_size;
-	mapping->bitmaps = kzalloc(extensions * sizeof(unsigned long *),
-				GFP_KERNEL);
-	if (!mapping->bitmaps)
-		goto err2;
-
-	mapping->bitmaps[0] = kzalloc(bitmap_size, GFP_KERNEL);
-	if (!mapping->bitmaps[0])
-		goto err3;
-
-	mapping->nr_bitmaps = 1;
-	mapping->extensions = extensions;
-	mapping->base = base;
-	mapping->bits = BITS_PER_BYTE * bitmap_size;
-
-	spin_lock_init(&mapping->lock);
-
-	mapping->domain = iommu_domain_alloc(bus);
-	if (!mapping->domain)
-		goto err4;
-
-	kref_init(&mapping->kref);
-	return mapping;
-err4:
-	kfree(mapping->bitmaps[0]);
-err3:
-	kfree(mapping->bitmaps);
-err2:
-	kfree(mapping);
-err:
-	return ERR_PTR(err);
+	return iommu_helper_init_mapping(bus, base, size);
 }
 EXPORT_SYMBOL_GPL(arm_iommu_create_mapping);
 
-static void release_iommu_mapping(struct kref *kref)
-{
-	int i;
-	struct dma_iommu_mapping *mapping =
-		container_of(kref, struct dma_iommu_mapping, kref);
-
-	iommu_domain_free(mapping->domain);
-	for (i = 0; i < mapping->nr_bitmaps; i++)
-		kfree(mapping->bitmaps[i]);
-	kfree(mapping->bitmaps);
-	kfree(mapping);
-}
-
 static int extend_iommu_mapping(struct dma_iommu_mapping *mapping)
 {
 	int next_bitmap;
@@ -2019,8 +1868,7 @@ static int extend_iommu_mapping(struct dma_iommu_mapping *mapping)
 
 void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping)
 {
-	if (mapping)
-		kref_put(&mapping->kref, release_iommu_mapping);
+	iommu_helper_release_mapping(mapping);
 }
 EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
 
@@ -2030,8 +1878,9 @@ EXPORT_SYMBOL_GPL(arm_iommu_release_mapping);
  * @mapping: io address space mapping structure (returned from
  *	arm_iommu_create_mapping)
  *
- * Attaches specified io address space mapping to the provided device,
- * this replaces the dma operations (dma_map_ops pointer) with the
+ * Calls for lib/iommu-helper which attaches specified io
+ * address space mapping to the provided device, this
+ * replaces the dma operations (dma_map_ops pointer) with the
  * IOMMU aware version. More than one client might be attached to
  * the same io address space mapping.
  */
@@ -2040,13 +1889,12 @@ int arm_iommu_attach_device(struct device *dev,
 {
 	int err;
 
-	err = iommu_attach_device(mapping->domain, dev);
-	if (err)
-		return err;
+	err = iommu_helper_attach_device(dev, mapping);
 
-	kref_get(&mapping->kref);
-	dev->mapping = mapping;
-	set_dma_ops(dev, &iommu_ops);
+	if (!err)
+		set_dma_ops(dev, &iommu_ops);
+	else
+		return err;
 
 	pr_debug("Attached IOMMU controller to %s device.\n", dev_name(dev));
 	return 0;
@@ -2057,24 +1905,14 @@ EXPORT_SYMBOL_GPL(arm_iommu_attach_device);
  * arm_iommu_detach_device
  * @dev: valid struct device pointer
  *
- * Detaches the provided device from a previously attached map.
+ * Calls for lib/iommu-helper which detaches the provided
+ * device from a previously attached map.
  * This voids the dma operations (dma_map_ops pointer)
  */
 void arm_iommu_detach_device(struct device *dev)
 {
-	struct dma_iommu_mapping *mapping;
-
-	mapping = to_dma_iommu_mapping(dev);
-	if (!mapping) {
-		dev_warn(dev, "Not attached\n");
-		return;
-	}
-
-	iommu_detach_device(mapping->domain, dev);
-	kref_put(&mapping->kref, release_iommu_mapping);
-	dev->mapping = NULL;
+	iommu_helper_detach_device(dev);
 	set_dma_ops(dev, NULL);
-
 	pr_debug("Detached IOMMU controller from %s device.\n", dev_name(dev));
 }
 EXPORT_SYMBOL_GPL(arm_iommu_detach_device);
diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h
index 961d8ef..09bcea3 100644
--- a/include/linux/iommu-helper.h
+++ b/include/linux/iommu-helper.h
@@ -2,6 +2,7 @@
 #define _LINUX_IOMMU_HELPER_H
 
 #include <linux/kernel.h>
+#include <linux/dma-attrs.h>
 
 #ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
 struct dma_iommu_mapping {
@@ -19,6 +20,23 @@ struct dma_iommu_mapping {
 	struct kref		kref;
 };
 
+extern struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
+					  gfp_t gfp, struct dma_attrs *attrs,
+			void (*arch_clear_buffer_cb)(struct page*, size_t));
+
+extern int iommu_helper_free_buffer(struct device *dev, struct page **pages,
+			       size_t size, struct dma_attrs *attrs);
+
+extern void iommu_helper_detach_device(struct device *dev);
+
+extern void iommu_helper_release_mapping(struct dma_iommu_mapping *mapping);
+
+extern int iommu_helper_attach_device(struct device *dev,
+			    struct dma_iommu_mapping *mapping);
+
+extern struct dma_iommu_mapping *
+iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
+
 #define to_dma_iommu_mapping(dev) ((dev)->mapping)
 #endif
 
diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
index c27e269..3664709 100644
--- a/lib/iommu-helper.c
+++ b/lib/iommu-helper.c
@@ -6,6 +6,17 @@
 #include <linux/bitmap.h>
 #include <linux/bug.h>
 
+#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
+#include <linux/iommu.h>
+#include <linux/device.h>
+#include <linux/iommu-helper.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/errno.h>
+#include <linux/dma-contiguous.h>
+#include <linux/mm.h>
+#endif
+
 int iommu_is_span_boundary(unsigned int index, unsigned int nr,
 			   unsigned long shift,
 			   unsigned long boundary_size)
@@ -39,3 +50,227 @@ again:
 	return -1;
 }
 EXPORT_SYMBOL(iommu_area_alloc);
+
+#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
+
+struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
+					  gfp_t gfp, struct dma_attrs *attrs,
+			void (*arch_clear_buffer_cb)(struct page*, size_t))
+{
+	struct page **pages;
+	int count = size >> PAGE_SHIFT;
+	int array_size = count * sizeof(struct page *);
+	int i = 0;
+
+	if (array_size <= PAGE_SIZE)
+		pages = kzalloc(array_size, gfp);
+	else
+		pages = vzalloc(array_size);
+	if (!pages)
+		return NULL;
+
+	if (dma_get_attr(DMA_ATTR_FORCE_CONTIGUOUS, attrs)) {
+		unsigned long order = get_order(size);
+		struct page *page;
+
+		page = dma_alloc_from_contiguous(dev, count, order);
+		if (!page)
+			goto error;
+
+		if (arch_clear_buffer_cb)
+			arch_clear_buffer_cb(page, size);
+
+		for (i = 0; i < count; i++)
+			pages[i] = page + i;
+
+		return pages;
+	}
+
+	/*
+	 * IOMMU can map any pages, so himem can also be used here
+	 */
+	gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
+
+	while (count) {
+		int j, order = __fls(count);
+
+		pages[i] = alloc_pages(gfp, order);
+		while (!pages[i] && order)
+			pages[i] = alloc_pages(gfp, --order);
+		if (!pages[i])
+			goto error;
+
+		if (order) {
+			split_page(pages[i], order);
+			j = 1 << order;
+			while (--j)
+				pages[i + j] = pages[i] + j;
+		}
+		if (arch_clear_buffer_cb)
+			arch_clear_buffer_cb(pages[i], PAGE_SIZE << order);
+		i += 1 << order;
+		count -= 1 << order;
+	}
+
+	return pages;
+error:
+	while (i--)
+		if (pages[i])
+			__free_pages(pages[i], 0);
+	if (array_size <= PAGE_SIZE)
+		kfree(pages);
+	else
+		vfree(pages);
+	return NULL;
+}
+
+int iommu_helper_free_buffer(struct device *dev, struct page **pages,
+			       size_t size, struct dma_attrs *attrs)
+{
+	int count = size >> PAGE_SHIFT;
+	int array_size = count * sizeof(struct page *);
+	int i;
+
+	if (dma_get_attr(DMA_ATTR_FORCE_CONTIGUOUS, attrs)) {
+		dma_release_from_contiguous(dev, pages[0], count);
+	} else {
+		for (i = 0; i < count; i++)
+			if (pages[i])
+				__free_pages(pages[i], 0);
+	}
+
+	if (array_size <= PAGE_SIZE)
+		kfree(pages);
+	else
+		vfree(pages);
+	return 0;
+}
+
+/**
+ * iommu_helper_init_mapping
+ * @bus: pointer to the bus holding the client device (for IOMMU calls)
+ * @base: start address of the valid IO address space
+ * @size: maximum size of the valid IO address space
+ *
+ * Creates a mapping structure which holds information about used/unused
+ * IO address ranges, which is required to perform memory allocation and
+ * mapping with IOMMU aware functions.
+ *
+ */
+
+struct dma_iommu_mapping *
+iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
+{
+	unsigned int bits = size >> PAGE_SHIFT;
+	unsigned int bitmap_size = BITS_TO_LONGS(bits) * sizeof(long);
+	struct dma_iommu_mapping *mapping;
+	int extensions = 1;
+	int err = -ENOMEM;
+
+	if (!bitmap_size)
+		return ERR_PTR(-EINVAL);
+
+	if (bitmap_size > PAGE_SIZE) {
+		extensions = bitmap_size / PAGE_SIZE;
+		bitmap_size = PAGE_SIZE;
+	}
+
+	mapping = kzalloc(sizeof(struct dma_iommu_mapping), GFP_KERNEL);
+	if (!mapping)
+		goto err;
+
+	mapping->bitmap_size = bitmap_size;
+	mapping->bitmaps = kzalloc(extensions * sizeof(unsigned long *),
+				GFP_KERNEL);
+	if (!mapping->bitmaps)
+		goto err2;
+
+	mapping->bitmaps[0] = kzalloc(bitmap_size, GFP_KERNEL);
+	if (!mapping->bitmaps[0])
+		goto err3;
+
+	mapping->nr_bitmaps = 1;
+	mapping->extensions = extensions;
+	mapping->base = base;
+	mapping->bits = BITS_PER_BYTE * bitmap_size;
+
+	spin_lock_init(&mapping->lock);
+
+	mapping->domain = iommu_domain_alloc(bus);
+	if (!mapping->domain)
+		goto err4;
+
+	kref_init(&mapping->kref);
+	return mapping;
+err4:
+	kfree(mapping->bitmaps[0]);
+err3:
+	kfree(mapping->bitmaps);
+err2:
+	kfree(mapping);
+err:
+	return ERR_PTR(err);
+}
+
+static void release_iommu_mapping(struct kref *kref)
+{
+	int i;
+	struct dma_iommu_mapping *mapping =
+		container_of(kref, struct dma_iommu_mapping, kref);
+
+	iommu_domain_free(mapping->domain);
+	for (i = 0; i < mapping->nr_bitmaps; i++)
+		kfree(mapping->bitmaps[i]);
+	kfree(mapping->bitmaps);
+	kfree(mapping);
+}
+
+
+void iommu_helper_release_mapping(struct dma_iommu_mapping *mapping)
+{
+	if (mapping)
+		kref_put(&mapping->kref, release_iommu_mapping);
+}
+
+/**
+ * iommu_helper_detach_device
+ * @dev: valid struct device pointer
+ *
+ * Detaches the provided device from a previously attached map.
+ */
+void iommu_helper_detach_device(struct device *dev)
+{
+	struct dma_iommu_mapping *mapping;
+
+	mapping = to_dma_iommu_mapping(dev);
+	if (!mapping) {
+		dev_warn(dev, "Not attached\n");
+		return;
+	}
+
+	iommu_detach_device(mapping->domain, dev);
+	kref_put(&mapping->kref, release_iommu_mapping);
+	dev->mapping = NULL;
+}
+
+/**
+ * iommu_helper_attach_device
+ * @dev: valid struct device pointer
+ * @mapping: io address space mapping structure
+ *
+ * Attaches specified io address space mapping to the provided device.
+ */
+int iommu_helper_attach_device(struct device *dev,
+			    struct dma_iommu_mapping *mapping)
+{
+	int err;
+
+	err = iommu_attach_device(mapping->domain, dev);
+	if (err)
+		return err;
+
+	kref_get(&mapping->kref);
+	dev->mapping = mapping;
+	return 0;
+}
+#endif
-- 
1.8.1.3

[PATCHv3 3/3] arm:dma-iommu: Move out complete func defs

[ritesh.harjani at gmail.com]

From: Ritesh Harjani <ritesh.harjani@gmail.com>

Move out complete function definitions from
arch/arm/dma-mapping to lib/iommu-helper

1. Moved out iova alloc/free routine and make them
statically defined.

2. Moved out complete function definitions which calls
alloc/free_iova routine to lib/iommu-helper.c

3. Seperated out cache maintainance from iommu_map/unmap
function routine, to be called from within arch/arm/dma-mapping.c

Signed-off-by: Ritesh Harjani <ritesh.harjani@gmail.com>
---
 arch/arm/Kconfig             |  42 ++---
 arch/arm/mm/dma-mapping.c    | 372 ++++---------------------------------------
 include/linux/iommu-helper.h |  28 +++-
 lib/iommu-helper.c           | 329 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 404 insertions(+), 367 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e65042f..a2cae78 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -94,27 +94,6 @@ config ARM_DMA_USE_IOMMU
 	select NEED_SG_DMA_LENGTH
 	select DMA_USE_IOMMU_HELPER_MAPPING
 
-if ARM_DMA_USE_IOMMU
-
-config ARM_DMA_IOMMU_ALIGNMENT
-	int "Maximum PAGE_SIZE order of alignment for DMA IOMMU buffers"
-	range 4 9
-	default 8
-	help
-	  DMA mapping framework by default aligns all buffers to the smallest
-	  PAGE_SIZE order which is greater than or equal to the requested buffer
-	  size. This works well for buffers up to a few hundreds kilobytes, but
-	  for larger buffers it just a waste of address space. Drivers which has
-	  relatively small addressing window (like 64Mib) might run out of
-	  virtual space with just a few allocations.
-
-	  With this parameter you can specify the maximum PAGE_SIZE order for
-	  DMA IOMMU buffers. Larger buffers will be aligned only to this
-	  specified order. The order is expressed as a power of two multiplied
-	  by the PAGE_SIZE.
-
-endif
-
 config MIGHT_HAVE_PCI
 	bool
 
@@ -1915,6 +1894,27 @@ config IOMMU_HELPER
 config DMA_USE_IOMMU_HELPER_MAPPING
 	def_bool n
 
+if DMA_USE_IOMMU_HELPER_MAPPING
+
+config DMA_IOMMU_ALIGNMENT
+	int "Maximum PAGE_SIZE order of alignment for DMA IOMMU buffers"
+	range 4 9
+	default 8
+	help
+	  DMA mapping framework by default aligns all buffers to the smallest
+	  PAGE_SIZE order which is greater than or equal to the requested buffer
+	  size. This works well for buffers up to a few hundreds kilobytes, but
+	  for larger buffers it just a waste of address space. Drivers which has
+	  relatively small addressing window (like 64Mib) might run out of
+	  virtual space with just a few allocations.
+
+	  With this parameter you can specify the maximum PAGE_SIZE order for
+	  DMA IOMMU buffers. Larger buffers will be aligned only to this
+	  specified order. The order is expressed as a power of two multiplied
+	  by the PAGE_SIZE.
+
+endif
+
 config XEN_DOM0
 	def_bool y
 	depends on XEN
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 66cf96b..d2192d4 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1066,101 +1066,6 @@ fs_initcall(dma_debug_do_init);
 
 /* IOMMU */
 
-static int extend_iommu_mapping(struct dma_iommu_mapping *mapping);
-
-static inline dma_addr_t __alloc_iova(struct dma_iommu_mapping *mapping,
-				      size_t size)
-{
-	unsigned int order = get_order(size);
-	unsigned int align = 0;
-	unsigned int count, start;
-	size_t mapping_size = mapping->bits << PAGE_SHIFT;
-	unsigned long flags;
-	dma_addr_t iova;
-	int i;
-
-	if (order > CONFIG_ARM_DMA_IOMMU_ALIGNMENT)
-		order = CONFIG_ARM_DMA_IOMMU_ALIGNMENT;
-
-	count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-	align = (1 << order) - 1;
-
-	spin_lock_irqsave(&mapping->lock, flags);
-	for (i = 0; i < mapping->nr_bitmaps; i++) {
-		start = bitmap_find_next_zero_area(mapping->bitmaps[i],
-				mapping->bits, 0, count, align);
-
-		if (start > mapping->bits)
-			continue;
-
-		bitmap_set(mapping->bitmaps[i], start, count);
-		break;
-	}
-
-	/*
-	 * No unused range found. Try to extend the existing mapping
-	 * and perform a second attempt to reserve an IO virtual
-	 * address range of size bytes.
-	 */
-	if (i == mapping->nr_bitmaps) {
-		if (extend_iommu_mapping(mapping)) {
-			spin_unlock_irqrestore(&mapping->lock, flags);
-			return DMA_ERROR_CODE;
-		}
-
-		start = bitmap_find_next_zero_area(mapping->bitmaps[i],
-				mapping->bits, 0, count, align);
-
-		if (start > mapping->bits) {
-			spin_unlock_irqrestore(&mapping->lock, flags);
-			return DMA_ERROR_CODE;
-		}
-
-		bitmap_set(mapping->bitmaps[i], start, count);
-	}
-	spin_unlock_irqrestore(&mapping->lock, flags);
-
-	iova = mapping->base + (mapping_size * i);
-	iova += start << PAGE_SHIFT;
-
-	return iova;
-}
-
-static inline void __free_iova(struct dma_iommu_mapping *mapping,
-			       dma_addr_t addr, size_t size)
-{
-	unsigned int start, count;
-	size_t mapping_size = mapping->bits << PAGE_SHIFT;
-	unsigned long flags;
-	dma_addr_t bitmap_base;
-	u32 bitmap_index;
-
-	if (!size)
-		return;
-
-	bitmap_index = (u32) (addr - mapping->base) / (u32) mapping_size;
-	BUG_ON(addr < mapping->base || bitmap_index > mapping->extensions);
-
-	bitmap_base = mapping->base + mapping_size * bitmap_index;
-
-	start = (addr - bitmap_base) >>	PAGE_SHIFT;
-
-	if (addr + size > bitmap_base + mapping_size) {
-		/*
-		 * The address range to be freed reaches into the iova
-		 * range of the next bitmap. This should not happen as
-		 * we don't allow this in __alloc_iova (at the
-		 * moment).
-		 */
-		BUG();
-	} else
-		count = size >> PAGE_SHIFT;
-
-	spin_lock_irqsave(&mapping->lock, flags);
-	bitmap_clear(mapping->bitmaps[bitmap_index], start, count);
-	spin_unlock_irqrestore(&mapping->lock, flags);
-}
-
 /*
  * Create a CPU mapping for a specified pages
  */
@@ -1194,62 +1099,6 @@ err:
 	return NULL;
 }
 
-/*
- * Create a mapping in device IO address space for specified pages
- */
-static dma_addr_t
-__iommu_create_mapping(struct device *dev, struct page **pages, size_t size)
-{
-	struct dma_iommu_mapping *mapping = dev->mapping;
-	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-	dma_addr_t dma_addr, iova;
-	int i, ret = DMA_ERROR_CODE;
-
-	dma_addr = __alloc_iova(mapping, size);
-	if (dma_addr == DMA_ERROR_CODE)
-		return dma_addr;
-
-	iova = dma_addr;
-	for (i = 0; i < count; ) {
-		unsigned int next_pfn = page_to_pfn(pages[i]) + 1;
-		phys_addr_t phys = page_to_phys(pages[i]);
-		unsigned int len, j;
-
-		for (j = i + 1; j < count; j++, next_pfn++)
-			if (page_to_pfn(pages[j]) != next_pfn)
-				break;
-
-		len = (j - i) << PAGE_SHIFT;
-		ret = iommu_map(mapping->domain, iova, phys, len,
-				IOMMU_READ|IOMMU_WRITE);
-		if (ret < 0)
-			goto fail;
-		iova += len;
-		i = j;
-	}
-	return dma_addr;
-fail:
-	iommu_unmap(mapping->domain, dma_addr, iova-dma_addr);
-	__free_iova(mapping, dma_addr, size);
-	return DMA_ERROR_CODE;
-}
-
-static int __iommu_remove_mapping(struct device *dev, dma_addr_t iova, size_t size)
-{
-	struct dma_iommu_mapping *mapping = dev->mapping;
-
-	/*
-	 * add optional in-page offset from iova to size and align
-	 * result to page size
-	 */
-	size = PAGE_ALIGN((iova & ~PAGE_MASK) + size);
-	iova &= PAGE_MASK;
-
-	iommu_unmap(mapping->domain, iova, size);
-	__free_iova(mapping, iova, size);
-	return 0;
-}
-
 static struct page **__atomic_get_pages(void *addr)
 {
 	struct dma_pool *pool = &atomic_pool;
@@ -1285,7 +1134,7 @@ static void *__iommu_alloc_atomic(struct device *dev, size_t size,
 	if (!addr)
 		return NULL;
 
-	*handle = __iommu_create_mapping(dev, &page, size);
+	*handle = iommu_helper_create_mapping(dev, &page, size);
 	if (*handle == DMA_ERROR_CODE)
 		goto err_mapping;
 
@@ -1299,7 +1148,7 @@ err_mapping:
 static void __iommu_free_atomic(struct device *dev, void *cpu_addr,
 				dma_addr_t handle, size_t size)
 {
-	__iommu_remove_mapping(dev, handle, size);
+	iommu_helper_remove_mapping(dev, handle, size);
 	__free_from_pool(cpu_addr, size);
 }
 
@@ -1330,7 +1179,7 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	if (!pages)
 		return NULL;
 
-	*handle = __iommu_create_mapping(dev, pages, size);
+	*handle = iommu_helper_create_mapping(dev, pages, size);
 	if (*handle == DMA_ERROR_CODE)
 		goto err_buffer;
 
@@ -1345,7 +1194,7 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	return addr;
 
 err_mapping:
-	__iommu_remove_mapping(dev, *handle, size);
+	iommu_helper_remove_mapping(dev, *handle, size);
 err_buffer:
 	iommu_helper_free_buffer(dev, pages, size, attrs);
 	return NULL;
@@ -1403,7 +1252,7 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
 		vunmap(cpu_addr);
 	}
 
-	__iommu_remove_mapping(dev, handle, size);
+	iommu_helper_remove_mapping(dev, handle, size);
 	iommu_helper_free_buffer(dev, pages, size, attrs);
 }
 
@@ -1421,120 +1270,6 @@ static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
 					 GFP_KERNEL);
 }
 
-static int __dma_direction_to_prot(enum dma_data_direction dir)
-{
-	int prot;
-
-	switch (dir) {
-	case DMA_BIDIRECTIONAL:
-		prot = IOMMU_READ | IOMMU_WRITE;
-		break;
-	case DMA_TO_DEVICE:
-		prot = IOMMU_READ;
-		break;
-	case DMA_FROM_DEVICE:
-		prot = IOMMU_WRITE;
-		break;
-	default:
-		prot = 0;
-	}
-
-	return prot;
-}
-
-/*
- * Map a part of the scatter-gather list into contiguous io address space
- */
-static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
-			  size_t size, dma_addr_t *handle,
-			  enum dma_data_direction dir, struct dma_attrs *attrs,
-			  bool is_coherent)
-{
-	struct dma_iommu_mapping *mapping = dev->mapping;
-	dma_addr_t iova, iova_base;
-	int ret = 0;
-	unsigned int count;
-	struct scatterlist *s;
-	int prot;
-
-	size = PAGE_ALIGN(size);
-	*handle = DMA_ERROR_CODE;
-
-	iova_base = iova = __alloc_iova(mapping, size);
-	if (iova == DMA_ERROR_CODE)
-		return -ENOMEM;
-
-	for (count = 0, s = sg; count < (size >> PAGE_SHIFT); s = sg_next(s)) {
-		phys_addr_t phys = page_to_phys(sg_page(s));
-		unsigned int len = PAGE_ALIGN(s->offset + s->length);
-
-		if (!is_coherent &&
-			!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
-			__dma_page_cpu_to_dev(sg_page(s), s->offset, s->length, dir);
-
-		prot = __dma_direction_to_prot(dir);
-
-		ret = iommu_map(mapping->domain, iova, phys, len, prot);
-		if (ret < 0)
-			goto fail;
-		count += len >> PAGE_SHIFT;
-		iova += len;
-	}
-	*handle = iova_base;
-
-	return 0;
-fail:
-	iommu_unmap(mapping->domain, iova_base, count * PAGE_SIZE);
-	__free_iova(mapping, iova_base, size);
-	return ret;
-}
-
-static int __iommu_map_sg(struct device *dev, struct scatterlist *sg, int nents,
-		     enum dma_data_direction dir, struct dma_attrs *attrs,
-		     bool is_coherent)
-{
-	struct scatterlist *s = sg, *dma = sg, *start = sg;
-	int i, count = 0;
-	unsigned int offset = s->offset;
-	unsigned int size = s->offset + s->length;
-	unsigned int max = dma_get_max_seg_size(dev);
-
-	for (i = 1; i < nents; i++) {
-		s = sg_next(s);
-
-		s->dma_address = DMA_ERROR_CODE;
-		s->dma_length = 0;
-
-		if (s->offset || (size & ~PAGE_MASK) || size + s->length > max) {
-			if (__map_sg_chunk(dev, start, size, &dma->dma_address,
-			    dir, attrs, is_coherent) < 0)
-				goto bad_mapping;
-
-			dma->dma_address += offset;
-			dma->dma_length = size - offset;
-
-			size = offset = s->offset;
-			start = s;
-			dma = sg_next(dma);
-			count += 1;
-		}
-		size += s->length;
-	}
-	if (__map_sg_chunk(dev, start, size, &dma->dma_address, dir, attrs,
-		is_coherent) < 0)
-		goto bad_mapping;
-
-	dma->dma_address += offset;
-	dma->dma_length = size - offset;
-
-	return count+1;
-
-bad_mapping:
-	for_each_sg(sg, s, count, i)
-		__iommu_remove_mapping(dev, sg_dma_address(s), sg_dma_len(s));
-	return 0;
-}
-
 /**
  * arm_coherent_iommu_map_sg - map a set of SG buffers for streaming mode DMA
  * @dev: valid struct device pointer
@@ -1550,7 +1285,7 @@ bad_mapping:
 int arm_coherent_iommu_map_sg(struct device *dev, struct scatterlist *sg,
 		int nents, enum dma_data_direction dir, struct dma_attrs *attrs)
 {
-	return __iommu_map_sg(dev, sg, nents, dir, attrs, true);
+	return iommu_helper_map_sg(dev, sg, nents, dir, attrs);
 }
 
 /**
@@ -1568,25 +1303,15 @@ int arm_coherent_iommu_map_sg(struct device *dev, struct scatterlist *sg,
 int arm_iommu_map_sg(struct device *dev, struct scatterlist *sg,
 		int nents, enum dma_data_direction dir, struct dma_attrs *attrs)
 {
-	return __iommu_map_sg(dev, sg, nents, dir, attrs, false);
-}
-
-static void __iommu_unmap_sg(struct device *dev, struct scatterlist *sg,
-		int nents, enum dma_data_direction dir, struct dma_attrs *attrs,
-		bool is_coherent)
-{
 	struct scatterlist *s;
-	int i;
-
-	for_each_sg(sg, s, nents, i) {
-		if (sg_dma_len(s))
-			__iommu_remove_mapping(dev, sg_dma_address(s),
-					       sg_dma_len(s));
-		if (!is_coherent &&
-		    !dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
-			__dma_page_dev_to_cpu(sg_page(s), s->offset,
-					      s->length, dir);
+	int i, ret;
+	ret = iommu_helper_map_sg(dev, sg, nents, dir, attrs);
+	if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs)) {
+		for_each_sg(sg, s, ret, i)
+			__dma_page_cpu_to_dev(sg_page(s), s->offset,
+					s->length, dir);
 	}
+	return ret;
 }
 
 /**
@@ -1602,7 +1327,7 @@ static void __iommu_unmap_sg(struct device *dev, struct scatterlist *sg,
 void arm_coherent_iommu_unmap_sg(struct device *dev, struct scatterlist *sg,
 		int nents, enum dma_data_direction dir, struct dma_attrs *attrs)
 {
-	__iommu_unmap_sg(dev, sg, nents, dir, attrs, true);
+	iommu_helper_unmap_sg(dev, sg, nents, dir, attrs);
 }
 
 /**
@@ -1618,7 +1343,16 @@ void arm_coherent_iommu_unmap_sg(struct device *dev, struct scatterlist *sg,
 void arm_iommu_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
 			enum dma_data_direction dir, struct dma_attrs *attrs)
 {
-	__iommu_unmap_sg(dev, sg, nents, dir, attrs, false);
+	struct scatterlist *s;
+	int i;
+
+	iommu_helper_unmap_sg(dev, sg, nents, dir, attrs);
+
+	for_each_sg(sg, s, nents, i) {
+		if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
+			__dma_page_dev_to_cpu(sg_page(s), s->offset,
+					      s->length, dir);
+	}
 }
 
 /**
@@ -1671,24 +1405,7 @@ static dma_addr_t arm_coherent_iommu_map_page(struct device *dev, struct page *p
 	     unsigned long offset, size_t size, enum dma_data_direction dir,
 	     struct dma_attrs *attrs)
 {
-	struct dma_iommu_mapping *mapping = dev->mapping;
-	dma_addr_t dma_addr;
-	int ret, prot, len = PAGE_ALIGN(size + offset);
-
-	dma_addr = __alloc_iova(mapping, len);
-	if (dma_addr == DMA_ERROR_CODE)
-		return dma_addr;
-
-	prot = __dma_direction_to_prot(dir);
-
-	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
-	if (ret < 0)
-		goto fail;
-
-	return dma_addr + offset;
-fail:
-	__free_iova(mapping, dma_addr, len);
-	return DMA_ERROR_CODE;
+	return iommu_helper_map_page(dev, page, offset, size, dir);
 }
 
 /**
@@ -1708,7 +1425,7 @@ static dma_addr_t arm_iommu_map_page(struct device *dev, struct page *page,
 	if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
 		__dma_page_cpu_to_dev(page, offset, size, dir);
 
-	return arm_coherent_iommu_map_page(dev, page, offset, size, dir, attrs);
+	return iommu_helper_map_page(dev, page, offset, size, dir);
 }
 
 /**
@@ -1724,16 +1441,7 @@ static void arm_coherent_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 		size_t size, enum dma_data_direction dir,
 		struct dma_attrs *attrs)
 {
-	struct dma_iommu_mapping *mapping = dev->mapping;
-	dma_addr_t iova = handle & PAGE_MASK;
-	int offset = handle & ~PAGE_MASK;
-	int len = PAGE_ALIGN(size + offset);
-
-	if (!iova)
-		return;
-
-	iommu_unmap(mapping->domain, iova, len);
-	__free_iova(mapping, iova, len);
+	iommu_helper_unmap_page(dev, handle, size, dir);
 }
 
 /**
@@ -1753,16 +1461,12 @@ static void arm_iommu_unmap_page(struct device *dev, dma_addr_t handle,
 	dma_addr_t iova = handle & PAGE_MASK;
 	struct page *page = phys_to_page(iommu_iova_to_phys(mapping->domain, iova));
 	int offset = handle & ~PAGE_MASK;
-	int len = PAGE_ALIGN(size + offset);
-
-	if (!iova)
-		return;
 
 	if (!dma_get_attr(DMA_ATTR_SKIP_CPU_SYNC, attrs))
 		__dma_page_dev_to_cpu(page, offset, size, dir);
 
-	iommu_unmap(mapping->domain, iova, len);
-	__free_iova(mapping, iova, len);
+	iommu_helper_unmap_page(dev, handle, size, dir);
+
 }
 
 static void arm_iommu_sync_single_for_cpu(struct device *dev,
@@ -1848,24 +1552,6 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
 }
 EXPORT_SYMBOL_GPL(arm_iommu_create_mapping);
 
-static int extend_iommu_mapping(struct dma_iommu_mapping *mapping)
-{
-	int next_bitmap;
-
-	if (mapping->nr_bitmaps > mapping->extensions)
-		return -EINVAL;
-
-	next_bitmap = mapping->nr_bitmaps;
-	mapping->bitmaps[next_bitmap] = kzalloc(mapping->bitmap_size,
-						GFP_ATOMIC);
-	if (!mapping->bitmaps[next_bitmap])
-		return -ENOMEM;
-
-	mapping->nr_bitmaps++;
-
-	return 0;
-}
-
 void arm_iommu_release_mapping(struct dma_iommu_mapping *mapping)
 {
 	iommu_helper_release_mapping(mapping);
diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h
index 09bcea3..685b786 100644
--- a/include/linux/iommu-helper.h
+++ b/include/linux/iommu-helper.h
@@ -5,6 +5,12 @@
 #include <linux/dma-attrs.h>
 
 #ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
+#include <linux/mm_types.h>
+#include <linux/dma-debug.h>
+#include <linux/kmemcheck.h>
+#include <linux/kref.h>
+#include <linux/dma-mapping.h>
+
 struct dma_iommu_mapping {
 	/* iommu specific data */
 	struct iommu_domain	*domain;
@@ -20,6 +26,25 @@ struct dma_iommu_mapping {
 	struct kref		kref;
 };
 
+extern dma_addr_t iommu_helper_create_mapping(struct device *dev, struct page **pages,
+					size_t size);
+
+extern int iommu_helper_remove_mapping(struct device *dev, dma_addr_t iova,
+				size_t size);
+
+extern dma_addr_t iommu_helper_map_page(struct device *dev, struct page *page,
+		unsigned long offset, size_t size, enum dma_data_direction dir);
+
+extern void iommu_helper_unmap_page(struct device *dev, dma_addr_t handle,
+		size_t size, enum dma_data_direction dir);
+
+extern int iommu_helper_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+		     enum dma_data_direction dir, struct dma_attrs *attrs);
+
+extern void iommu_helper_unmap_sg(struct device *dev, struct scatterlist *sg,
+		int nents, enum dma_data_direction dir,
+		struct dma_attrs *attrs);
+
 extern struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
 					  gfp_t gfp, struct dma_attrs *attrs,
 			void (*arch_clear_buffer_cb)(struct page*, size_t));
@@ -29,14 +54,13 @@ extern int iommu_helper_free_buffer(struct device *dev, struct page **pages,
 
 extern void iommu_helper_detach_device(struct device *dev);
 
-extern void iommu_helper_release_mapping(struct dma_iommu_mapping *mapping);
-
 extern int iommu_helper_attach_device(struct device *dev,
 			    struct dma_iommu_mapping *mapping);
 
 extern struct dma_iommu_mapping *
 iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
 
+extern void iommu_helper_release_mapping(struct dma_iommu_mapping *mapping);
 #define to_dma_iommu_mapping(dev) ((dev)->mapping)
 #endif
 
diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
index 3664709..320d885 100644
--- a/lib/iommu-helper.c
+++ b/lib/iommu-helper.c
@@ -8,13 +8,14 @@
 
 #ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
 #include <linux/iommu.h>
-#include <linux/device.h>
 #include <linux/iommu-helper.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/errno.h>
 #include <linux/dma-contiguous.h>
 #include <linux/mm.h>
+
+#include <asm/dma-mapping.h>
 #endif
 
 int iommu_is_span_boundary(unsigned int index, unsigned int nr,
@@ -53,6 +54,195 @@ EXPORT_SYMBOL(iommu_area_alloc);
 
 #ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
 
+/* IOMMU */
+static int extend_iommu_mapping(struct dma_iommu_mapping *mapping)
+{
+	int next_bitmap;
+
+	if (mapping->nr_bitmaps > mapping->extensions)
+		return -EINVAL;
+
+	next_bitmap = mapping->nr_bitmaps;
+	mapping->bitmaps[next_bitmap] = kzalloc(mapping->bitmap_size,
+						GFP_ATOMIC);
+	if (!mapping->bitmaps[next_bitmap])
+		return -ENOMEM;
+
+	mapping->nr_bitmaps++;
+
+	return 0;
+}
+
+static int __dma_direction_to_prot(enum dma_data_direction dir)
+{
+	int prot;
+
+	switch (dir) {
+	case DMA_BIDIRECTIONAL:
+		prot = IOMMU_READ | IOMMU_WRITE;
+		break;
+	case DMA_TO_DEVICE:
+		prot = IOMMU_READ;
+		break;
+	case DMA_FROM_DEVICE:
+		prot = IOMMU_WRITE;
+		break;
+	default:
+		prot = 0;
+	}
+
+	return prot;
+}
+
+static inline dma_addr_t __alloc_iova(struct dma_iommu_mapping *mapping,
+				      size_t size)
+{
+	unsigned int order = get_order(size);
+	unsigned int align = 0;
+	unsigned int count, start;
+	size_t mapping_size = mapping->bits << PAGE_SHIFT;
+	unsigned long flags;
+	dma_addr_t iova;
+	int i;
+
+	if (order > CONFIG_DMA_IOMMU_ALIGNMENT)
+		order = CONFIG_DMA_IOMMU_ALIGNMENT;
+
+	count = PAGE_ALIGN(size) >> PAGE_SHIFT;
+	align = (1 << order) - 1;
+
+	spin_lock_irqsave(&mapping->lock, flags);
+	for (i = 0; i < mapping->nr_bitmaps; i++) {
+		start = bitmap_find_next_zero_area(mapping->bitmaps[i],
+				mapping->bits, 0, count, align);
+
+		if (start > mapping->bits)
+			continue;
+
+		bitmap_set(mapping->bitmaps[i], start, count);
+		break;
+	}
+
+	/*
+	 * No unused range found. Try to extend the existing mapping
+	 * and perform a second attempt to reserve an IO virtual
+	 * address range of size bytes.
+	 */
+	if (i == mapping->nr_bitmaps) {
+		if (extend_iommu_mapping(mapping)) {
+			spin_unlock_irqrestore(&mapping->lock, flags);
+			return DMA_ERROR_CODE;
+		}
+
+		start = bitmap_find_next_zero_area(mapping->bitmaps[i],
+				mapping->bits, 0, count, align);
+
+		if (start > mapping->bits) {
+			spin_unlock_irqrestore(&mapping->lock, flags);
+			return DMA_ERROR_CODE;
+		}
+
+		bitmap_set(mapping->bitmaps[i], start, count);
+	}
+	spin_unlock_irqrestore(&mapping->lock, flags);
+
+	iova = mapping->base + (mapping_size * i);
+	iova += start << PAGE_SHIFT;
+
+	return iova;
+}
+
+static inline void __free_iova(struct dma_iommu_mapping *mapping,
+			       dma_addr_t addr, size_t size)
+{
+	unsigned int start, count;
+	size_t mapping_size = mapping->bits << PAGE_SHIFT;
+	unsigned long flags;
+	dma_addr_t bitmap_base;
+	u32 bitmap_index;
+
+	if (!size)
+		return;
+
+	bitmap_index = (u32) (addr - mapping->base) / (u32) mapping_size;
+	BUG_ON(addr < mapping->base || bitmap_index > mapping->extensions);
+
+	bitmap_base = mapping->base + mapping_size * bitmap_index;
+
+	start = (addr - bitmap_base) >>	PAGE_SHIFT;
+
+	if (addr + size > bitmap_base + mapping_size) {
+		/*
+		 * The address range to be freed reaches into the iova
+		 * range of the next bitmap. This should not happen as
+		 * we don't allow this in __alloc_iova (at the
+		 * moment).
+		 */
+		BUG();
+	} else
+		count = size >> PAGE_SHIFT;
+
+	spin_lock_irqsave(&mapping->lock, flags);
+	bitmap_clear(mapping->bitmaps[bitmap_index], start, count);
+	spin_unlock_irqrestore(&mapping->lock, flags);
+}
+
+/*
+ * Create a mapping in device IO address space for specified pages
+ */
+dma_addr_t
+iommu_helper_create_mapping(struct device *dev, struct page **pages, size_t size)
+{
+	struct dma_iommu_mapping *mapping = dev->mapping;
+	unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT;
+	dma_addr_t dma_addr, iova;
+	int i, ret = DMA_ERROR_CODE;
+
+	dma_addr = __alloc_iova(mapping, size);
+	if (dma_addr == DMA_ERROR_CODE)
+		return dma_addr;
+
+	iova = dma_addr;
+	for (i = 0; i < count; ) {
+		unsigned int next_pfn = page_to_pfn(pages[i]) + 1;
+		phys_addr_t phys = page_to_phys(pages[i]);
+		unsigned int len, j;
+
+		for (j = i + 1; j < count; j++, next_pfn++)
+			if (page_to_pfn(pages[j]) != next_pfn)
+				break;
+
+		len = (j - i) << PAGE_SHIFT;
+		ret = iommu_map(mapping->domain, iova, phys, len,
+				IOMMU_READ|IOMMU_WRITE);
+		if (ret < 0)
+			goto fail;
+		iova += len;
+		i = j;
+	}
+	return dma_addr;
+fail:
+	iommu_unmap(mapping->domain, dma_addr, iova-dma_addr);
+	__free_iova(mapping, dma_addr, size);
+	return DMA_ERROR_CODE;
+}
+
+int iommu_helper_remove_mapping(struct device *dev, dma_addr_t iova, size_t size)
+{
+	struct dma_iommu_mapping *mapping = dev->mapping;
+
+	/*
+	 * add optional in-page offset from iova to size and align
+	 * result to page size
+	 */
+	size = PAGE_ALIGN((iova & ~PAGE_MASK) + size);
+	iova &= PAGE_MASK;
+
+	iommu_unmap(mapping->domain, iova, size);
+	__free_iova(mapping, iova, size);
+	return 0;
+}
+
 struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
 					  gfp_t gfp, struct dma_attrs *attrs,
 			void (*arch_clear_buffer_cb)(struct page*, size_t))
@@ -146,6 +336,143 @@ int iommu_helper_free_buffer(struct device *dev, struct page **pages,
 	return 0;
 }
 
+/*
+ * Map a part of the scatter-gather list into contiguous io address space
+ */
+static int __map_sg_chunk(struct device *dev, struct scatterlist *sg,
+			  size_t size, dma_addr_t *handle,
+			  enum dma_data_direction dir, struct dma_attrs *attrs)
+{
+	struct dma_iommu_mapping *mapping = dev->mapping;
+	dma_addr_t iova, iova_base;
+	int ret = 0;
+	unsigned int count;
+	struct scatterlist *s;
+	int prot;
+
+	size = PAGE_ALIGN(size);
+	*handle = DMA_ERROR_CODE;
+
+	iova_base = iova = __alloc_iova(mapping, size);
+	if (iova == DMA_ERROR_CODE)
+		return -ENOMEM;
+
+	for (count = 0, s = sg; count < (size >> PAGE_SHIFT); s = sg_next(s)) {
+		phys_addr_t phys = page_to_phys(sg_page(s));
+		unsigned int len = PAGE_ALIGN(s->offset + s->length);
+
+		prot = __dma_direction_to_prot(dir);
+
+		ret = iommu_map(mapping->domain, iova, phys, len, prot);
+		if (ret < 0)
+			goto fail;
+		count += len >> PAGE_SHIFT;
+		iova += len;
+	}
+	*handle = iova_base;
+
+	return 0;
+fail:
+	iommu_unmap(mapping->domain, iova_base, count * PAGE_SIZE);
+	__free_iova(mapping, iova_base, size);
+	return ret;
+}
+
+int iommu_helper_map_sg(struct device *dev, struct scatterlist *sg, int nents,
+		     enum dma_data_direction dir, struct dma_attrs *attrs)
+{
+	struct scatterlist *s = sg, *dma = sg, *start = sg;
+	int i, count = 0;
+	unsigned int offset = s->offset;
+	unsigned int size = s->offset + s->length;
+	unsigned int max = dma_get_max_seg_size(dev);
+
+	for (i = 1; i < nents; i++) {
+		s = sg_next(s);
+
+		s->dma_address = DMA_ERROR_CODE;
+		s->dma_length = 0;
+
+		if (s->offset || (size & ~PAGE_MASK) || size + s->length > max) {
+			if (__map_sg_chunk(dev, start, size, &dma->dma_address,
+			    dir, attrs) < 0)
+				goto bad_mapping;
+
+			dma->dma_address += offset;
+			dma->dma_length = size - offset;
+
+			size = offset = s->offset;
+			start = s;
+			dma = sg_next(dma);
+			count += 1;
+		}
+		size += s->length;
+	}
+	if (__map_sg_chunk(dev, start, size, &dma->dma_address, dir, attrs) < 0)
+		goto bad_mapping;
+
+	dma->dma_address += offset;
+	dma->dma_length = size - offset;
+
+	return count+1;
+
+bad_mapping:
+	for_each_sg(sg, s, count, i)
+		iommu_helper_remove_mapping(dev, sg_dma_address(s), sg_dma_len(s));
+	return 0;
+}
+
+void iommu_helper_unmap_sg(struct device *dev, struct scatterlist *sg,
+		int nents, enum dma_data_direction dir, struct dma_attrs *attrs)
+{
+	struct scatterlist *s;
+	int i;
+
+	for_each_sg(sg, s, nents, i) {
+		if (sg_dma_len(s))
+			iommu_helper_remove_mapping(dev, sg_dma_address(s),
+					       sg_dma_len(s));
+}
+}
+
+dma_addr_t iommu_helper_map_page(struct device *dev, struct page *page,
+		unsigned long offset, size_t size, enum dma_data_direction dir)
+{
+	struct dma_iommu_mapping *mapping = dev->mapping;
+	dma_addr_t dma_addr;
+	int ret, prot, len = PAGE_ALIGN(size + offset);
+
+	dma_addr = __alloc_iova(mapping, len);
+	if (dma_addr == DMA_ERROR_CODE)
+		return dma_addr;
+
+	prot = __dma_direction_to_prot(dir);
+
+	ret = iommu_map(mapping->domain, dma_addr, page_to_phys(page), len, prot);
+	if (ret < 0)
+		goto fail;
+
+	return dma_addr + offset;
+fail:
+	__free_iova(mapping, dma_addr, len);
+	return DMA_ERROR_CODE;
+}
+
+void iommu_helper_unmap_page(struct device *dev, dma_addr_t handle,
+		size_t size, enum dma_data_direction dir)
+{
+	struct dma_iommu_mapping *mapping = dev->mapping;
+	dma_addr_t iova = handle & PAGE_MASK;
+	int offset = handle & ~PAGE_MASK;
+	int len = PAGE_ALIGN(size + offset);
+
+	if (!iova)
+		return;
+
+	iommu_unmap(mapping->domain, iova, len);
+	__free_iova(mapping, iova, len);
+}
+
 /**
  * iommu_helper_init_mapping
  * @bus: pointer to the bus holding the client device (for IOMMU calls)
-- 
1.8.1.3

[PATCHv3 0/3] arm:dma-mapping Refactoring iommu dma-mapping code

[Ritesh Harjani]

Hi Will/Catalin/Marek...

Any comments on this patch series ??

Thanks
Ritesh


On Fri, Jun 6, 2014 at 2:12 PM,  <ritesh.harjani@gmail.com> wrote:
> From: Ritesh Harjani <ritesh.harjani@gmail.com>
>
> Hi All,
>
> This patch series is to refactor iommu related common code from
> arch/arm/dma-mapping.c to lib/iommu-helper.c based on the various
> discussions with the maintainers/experts [1].
>
> Currently the only user of the common lib/iommu-helper code will
> be ARM & ARM64 but later various architecture might try to use this
> iommu lib helper functions.
>
> Major change of this refactoring depends on bringing out struct dma_iommu_mapping
> *mapping variable from arch/arm/include/asm/device.h to include/linux/device.h
> and by moving out complete structure definition of dma_iommu_mapping to
> inclue/linux/iommu-helper.h. These changes are now part of this patch series based
> on Greg's(cc'd) comments[2].
>
> There are 1/2 more function definitions which I can think of moving out, but
> those can be done once this patch series is approved as those are not very
> big changes.
>
> Also, this has been tested and verified on ARM hardware with IOMMU support.
>
> Changes v2 -> v3:
> 1. Included changes in device.h in this patch series[1/3] itself, as Greg
> mentioned that there is no point in approving device.h changes without actual
> refactoring changes gets approved [2].
>
> So, I guess device.h changes should be fine but let Greg confirms it.
>
>
> Links:
> [1]: https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg03458.html
> [2]: https://lkml.org/lkml/2014/6/4/458
>
>
> Ritesh Harjani (3):
>   device.h: arm dma-iommu: Move out dma_iommu_mapping struct
>   arm: dma-mapping: Refactor attach/detach, alloc/free func
>   arm:dma-iommu: Move out complete func defs
>
>  arch/arm/Kconfig                          |  46 +--
>  arch/arm/include/asm/device.h             |   9 -
>  arch/arm/include/asm/dma-iommu.h          |  16 +-
>  arch/arm/mm/dma-mapping.c                 | 578 +++---------------------------
>  drivers/gpu/drm/exynos/exynos_drm_iommu.c |  10 +-
>  include/linux/device.h                    |   5 +
>  include/linux/iommu-helper.h              |  61 ++++
>  lib/iommu-helper.c                        | 562 +++++++++++++++++++++++++++++
>  8 files changed, 710 insertions(+), 577 deletions(-)
>
> --
> 1.8.1.3
>

[PATCHv3 0/3] arm:dma-mapping Refactoring iommu dma-mapping code

[Greg Kroah-Hartman]

On Wed, Jun 11, 2014 at 08:27:54AM +0530, Ritesh Harjani wrote:
> Hi Will/Catalin/Marek...
> 
> Any comments on this patch series ??

It's the middle of the merge window, we can't do anything right now and
are all swamped with other work, give people some time...

greg k-h

[PATCHv3 0/3] arm:dma-mapping Refactoring iommu dma-mapping code

[Ritesh Harjani]

Hi Greg,

On Wed, Jun 11, 2014 at 8:53 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Jun 11, 2014 at 08:27:54AM +0530, Ritesh Harjani wrote:
>> Hi Will/Catalin/Marek...
>>
>> Any comments on this patch series ??
>
> It's the middle of the merge window, we can't do anything right now and
> are all swamped with other work, give people some time...

Sure, thanks for the reply.

>
> greg k-h


Thanks
Ritesh

[PATCHv3 2/3] arm: dma-mapping: Refactor attach/detach, alloc/free func

[Will Deacon]

On Fri, Jun 06, 2014 at 09:42:40AM +0100, ritesh.harjani at gmail.com wrote:
> From: Ritesh Harjani <ritesh.harjani@gmail.com>
> 
> Refactor following function calls to lib/iommu-helper.c
> 
> 1.
> arm_iommu_attach/detach device function calls.
> arm_iommu_init/release_mapping function calls.
> 
> 2. iommu_alloc/free_buffer can be moved out from
> arm/dma-mapping.c to lib/iommu_helper.c

Moves like this are always difficult to review, so I'm just going to look at
the additions (i.e. the new helpers).

> diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h
> index 961d8ef..09bcea3 100644
> --- a/include/linux/iommu-helper.h
> +++ b/include/linux/iommu-helper.h
> @@ -2,6 +2,7 @@
>  #define _LINUX_IOMMU_HELPER_H
> 
>  #include <linux/kernel.h>
> +#include <linux/dma-attrs.h>
> 
>  #ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
>  struct dma_iommu_mapping {
> @@ -19,6 +20,23 @@ struct dma_iommu_mapping {
>         struct kref             kref;
>  };
> 
> +extern struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
> +                                         gfp_t gfp, struct dma_attrs *attrs,
> +                       void (*arch_clear_buffer_cb)(struct page*, size_t));
> +
> +extern int iommu_helper_free_buffer(struct device *dev, struct page **pages,
> +                              size_t size, struct dma_attrs *attrs);
> +
> +extern void iommu_helper_detach_device(struct device *dev);
> +
> +extern void iommu_helper_release_mapping(struct dma_iommu_mapping *mapping);
> +
> +extern int iommu_helper_attach_device(struct device *dev,
> +                           struct dma_iommu_mapping *mapping);
> +
> +extern struct dma_iommu_mapping *
> +iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
> +
>  #define to_dma_iommu_mapping(dev) ((dev)->mapping)
>  #endif
> 
> diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
> index c27e269..3664709 100644
> --- a/lib/iommu-helper.c
> +++ b/lib/iommu-helper.c
> @@ -6,6 +6,17 @@
>  #include <linux/bitmap.h>
>  #include <linux/bug.h>
> 
> +#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING

Why isn't this dependency simply part of the Makefile rules?

> +#include <linux/iommu.h>
> +#include <linux/device.h>
> +#include <linux/iommu-helper.h>
> +#include <linux/slab.h>
> +#include <linux/vmalloc.h>
> +#include <linux/errno.h>
> +#include <linux/dma-contiguous.h>
> +#include <linux/mm.h>
> +#endif
> +
>  int iommu_is_span_boundary(unsigned int index, unsigned int nr,
>                            unsigned long shift,
>                            unsigned long boundary_size)
> @@ -39,3 +50,227 @@ again:
>         return -1;
>  }
>  EXPORT_SYMBOL(iommu_area_alloc);
> +
> +#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
> +
> +struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
> +                                         gfp_t gfp, struct dma_attrs *attrs,
> +                       void (*arch_clear_buffer_cb)(struct page*, size_t))
> +{
> +       struct page **pages;
> +       int count = size >> PAGE_SHIFT;
> +       int array_size = count * sizeof(struct page *);
> +       int i = 0;
> +
> +       if (array_size <= PAGE_SIZE)
> +               pages = kzalloc(array_size, gfp);
> +       else
> +               pages = vzalloc(array_size);
> +       if (!pages)
> +               return NULL;
> +
> +       if (dma_get_attr(DMA_ATTR_FORCE_CONTIGUOUS, attrs)) {
> +               unsigned long order = get_order(size);
> +               struct page *page;
> +
> +               page = dma_alloc_from_contiguous(dev, count, order);
> +               if (!page)
> +                       goto error;
> +
> +               if (arch_clear_buffer_cb)
> +                       arch_clear_buffer_cb(page, size);

Actually, clearing a buffer is always a memset of zero -- the arch-specific
part is about cache maintenance. The fly in the ointment here is flushing
highmem pages when you have an outer (physically addressed cache), since we
want to do the outer-cache maintenance outside of the kmap_atomic region as
a large block, but the inner-cache maintenance by VA with the highmem
mapping.

Given that this is only called on the alloc path, is this actually a
fastpath for anybody? If not, we could move the outer-cache flushing into
the loop and simply have a arch_flush_buffer_cb instead, which would flush
l1 and l2 in turn.

That way, architectures that don't have highmem and don't require cache
maintenance needn't supply a callback at all. Failure to supply a callback
with your current code means that the buffer is full of junk.

> +/**
> + * iommu_helper_init_mapping
> + * @bus: pointer to the bus holding the client device (for IOMMU calls)
> + * @base: start address of the valid IO address space
> + * @size: maximum size of the valid IO address space
> + *
> + * Creates a mapping structure which holds information about used/unused
> + * IO address ranges, which is required to perform memory allocation and
> + * mapping with IOMMU aware functions.
> + *
> + */
> +
> +struct dma_iommu_mapping *
> +iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size)

I spoke to Arnd on IRC the other day about this, and ultimately this
function can plug into of_dma_configure to place each iommu_group into a
separate iommu_mapping automatically.

Can you put a dummy version of it in the header file, so it returns an error
when !CONFIG_DMA_USE_IOMMU_HELPER_MAPPING?

Will

[PATCHv3 2/3] arm: dma-mapping: Refactor attach/detach, alloc/free func

[Ritesh Harjani]

Hi Will,

Thanks for taking time to review the code. Due to some activity going
at my place, I might take some time
to reply with patches addressing the problem you have noted down.

But please find my answers inline.


On Fri, Jun 27, 2014 at 4:46 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Jun 06, 2014 at 09:42:40AM +0100, ritesh.harjani at gmail.com wrote:
>> From: Ritesh Harjani <ritesh.harjani@gmail.com>
>>
>> Refactor following function calls to lib/iommu-helper.c
>>
>> 1.
>> arm_iommu_attach/detach device function calls.
>> arm_iommu_init/release_mapping function calls.
>>
>> 2. iommu_alloc/free_buffer can be moved out from
>> arm/dma-mapping.c to lib/iommu_helper.c
>
> Moves like this are always difficult to review, so I'm just going to look at
> the additions (i.e. the new helpers).
Ok, thanks!

>
>> diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h
>> index 961d8ef..09bcea3 100644
>> --- a/include/linux/iommu-helper.h
>> +++ b/include/linux/iommu-helper.h
>> @@ -2,6 +2,7 @@
>>  #define _LINUX_IOMMU_HELPER_H
>>
>>  #include <linux/kernel.h>
>> +#include <linux/dma-attrs.h>
>>
>>  #ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
>>  struct dma_iommu_mapping {
>> @@ -19,6 +20,23 @@ struct dma_iommu_mapping {
>>         struct kref             kref;
>>  };
>>
>> +extern struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
>> +                                         gfp_t gfp, struct dma_attrs *attrs,
>> +                       void (*arch_clear_buffer_cb)(struct page*, size_t));
>> +
>> +extern int iommu_helper_free_buffer(struct device *dev, struct page **pages,
>> +                              size_t size, struct dma_attrs *attrs);
>> +
>> +extern void iommu_helper_detach_device(struct device *dev);
>> +
>> +extern void iommu_helper_release_mapping(struct dma_iommu_mapping *mapping);
>> +
>> +extern int iommu_helper_attach_device(struct device *dev,
>> +                           struct dma_iommu_mapping *mapping);
>> +
>> +extern struct dma_iommu_mapping *
>> +iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
>> +
>>  #define to_dma_iommu_mapping(dev) ((dev)->mapping)
>>  #endif
>>
>> diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
>> index c27e269..3664709 100644
>> --- a/lib/iommu-helper.c
>> +++ b/lib/iommu-helper.c
>> @@ -6,6 +6,17 @@
>>  #include <linux/bitmap.h>
>>  #include <linux/bug.h>
>>
>> +#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
>
> Why isn't this dependency simply part of the Makefile rules?

There are already current users of iommu-helper.c which calls for
iommu_area_alloc APIs.
Putting this config as part of Makefile without other users(except
ARM/ARM64) using this, I felt not very relevant at this point of time.
Moreover, we will have to see all the current users of iommu-helper
and add the config option with them as well, which again I felt not to
do at this point of time.

>
>> +#include <linux/iommu.h>
>> +#include <linux/device.h>
>> +#include <linux/iommu-helper.h>
>> +#include <linux/slab.h>
>> +#include <linux/vmalloc.h>
>> +#include <linux/errno.h>
>> +#include <linux/dma-contiguous.h>
>> +#include <linux/mm.h>
>> +#endif
>> +
>>  int iommu_is_span_boundary(unsigned int index, unsigned int nr,
>>                            unsigned long shift,
>>                            unsigned long boundary_size)
>> @@ -39,3 +50,227 @@ again:
>>         return -1;
>>  }
>>  EXPORT_SYMBOL(iommu_area_alloc);
>> +
>> +#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
>> +
>> +struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
>> +                                         gfp_t gfp, struct dma_attrs *attrs,
>> +                       void (*arch_clear_buffer_cb)(struct page*, size_t))
>> +{
>> +       struct page **pages;
>> +       int count = size >> PAGE_SHIFT;
>> +       int array_size = count * sizeof(struct page *);
>> +       int i = 0;
>> +
>> +       if (array_size <= PAGE_SIZE)
>> +               pages = kzalloc(array_size, gfp);
>> +       else
>> +               pages = vzalloc(array_size);
>> +       if (!pages)
>> +               return NULL;
>> +
>> +       if (dma_get_attr(DMA_ATTR_FORCE_CONTIGUOUS, attrs)) {
>> +               unsigned long order = get_order(size);
>> +               struct page *page;
>> +
>> +               page = dma_alloc_from_contiguous(dev, count, order);
>> +               if (!page)
>> +                       goto error;
>> +
>> +               if (arch_clear_buffer_cb)
>> +                       arch_clear_buffer_cb(page, size);
>
> Actually, clearing a buffer is always a memset of zero -- the arch-specific
> part is about cache maintenance. The fly in the ointment here is flushing
> highmem pages when you have an outer (physically addressed cache), since we
> want to do the outer-cache maintenance outside of the kmap_atomic region as
> a large block, but the inner-cache maintenance by VA with the highmem
> mapping.
>
> Given that this is only called on the alloc path, is this actually a
> fastpath for anybody? If not, we could move the outer-cache flushing into
> the loop and simply have a arch_flush_buffer_cb instead, which would flush
> l1 and l2 in turn.
>
> That way, architectures that don't have highmem and don't require cache
> maintenance needn't supply a callback at all. Failure to supply a callback
> with your current code means that the buffer is full of junk.

Sure, you are correct here. Let me again take a look at this and get
back to you.

>
>> +/**
>> + * iommu_helper_init_mapping
>> + * @bus: pointer to the bus holding the client device (for IOMMU calls)
>> + * @base: start address of the valid IO address space
>> + * @size: maximum size of the valid IO address space
>> + *
>> + * Creates a mapping structure which holds information about used/unused
>> + * IO address ranges, which is required to perform memory allocation and
>> + * mapping with IOMMU aware functions.
>> + *
>> + */
>> +
>> +struct dma_iommu_mapping *
>> +iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
>
> I spoke to Arnd on IRC the other day about this, and ultimately this
> function can plug into of_dma_configure to place each iommu_group into a
> separate iommu_mapping automatically.
>
> Can you put a dummy version of it in the header file, so it returns an error
> when !CONFIG_DMA_USE_IOMMU_HELPER_MAPPING?

OK sure.

>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



Thanks
Ritesh

[PATCHv3 2/3] arm: dma-mapping: Refactor attach/detach, alloc/free func

[Ritesh Harjani]

Hi All,

As I wrote before in previous mail, that I might take some time to
reply with new patch series, as there were some issues in my company.
But currently, I am no more left with any hardware support to test the
patch series or to make further changes on this as of now.

Anyways there have been lot of changes in files in this patch, so I
would say it would be better if someone else can take this up and
start afresh.

Thanks & Regards
Ritesh

On Mon, Jun 30, 2014 at 3:49 PM, Ritesh Harjani <ritesh.list@gmail.com> wrote:
> Hi Will,
>
> Thanks for taking time to review the code. Due to some activity going
> at my place, I might take some time
> to reply with patches addressing the problem you have noted down.
>
> But please find my answers inline.
>
>
> On Fri, Jun 27, 2014 at 4:46 PM, Will Deacon <will.deacon@arm.com> wrote:
>> On Fri, Jun 06, 2014 at 09:42:40AM +0100, ritesh.harjani at gmail.com wrote:
>>> From: Ritesh Harjani <ritesh.harjani@gmail.com>
>>>
>>> Refactor following function calls to lib/iommu-helper.c
>>>
>>> 1.
>>> arm_iommu_attach/detach device function calls.
>>> arm_iommu_init/release_mapping function calls.
>>>
>>> 2. iommu_alloc/free_buffer can be moved out from
>>> arm/dma-mapping.c to lib/iommu_helper.c
>>
>> Moves like this are always difficult to review, so I'm just going to look at
>> the additions (i.e. the new helpers).
> Ok, thanks!
>
>>
>>> diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h
>>> index 961d8ef..09bcea3 100644
>>> --- a/include/linux/iommu-helper.h
>>> +++ b/include/linux/iommu-helper.h
>>> @@ -2,6 +2,7 @@
>>>  #define _LINUX_IOMMU_HELPER_H
>>>
>>>  #include <linux/kernel.h>
>>> +#include <linux/dma-attrs.h>
>>>
>>>  #ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
>>>  struct dma_iommu_mapping {
>>> @@ -19,6 +20,23 @@ struct dma_iommu_mapping {
>>>         struct kref             kref;
>>>  };
>>>
>>> +extern struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
>>> +                                         gfp_t gfp, struct dma_attrs *attrs,
>>> +                       void (*arch_clear_buffer_cb)(struct page*, size_t));
>>> +
>>> +extern int iommu_helper_free_buffer(struct device *dev, struct page **pages,
>>> +                              size_t size, struct dma_attrs *attrs);
>>> +
>>> +extern void iommu_helper_detach_device(struct device *dev);
>>> +
>>> +extern void iommu_helper_release_mapping(struct dma_iommu_mapping *mapping);
>>> +
>>> +extern int iommu_helper_attach_device(struct device *dev,
>>> +                           struct dma_iommu_mapping *mapping);
>>> +
>>> +extern struct dma_iommu_mapping *
>>> +iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size);
>>> +
>>>  #define to_dma_iommu_mapping(dev) ((dev)->mapping)
>>>  #endif
>>>
>>> diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
>>> index c27e269..3664709 100644
>>> --- a/lib/iommu-helper.c
>>> +++ b/lib/iommu-helper.c
>>> @@ -6,6 +6,17 @@
>>>  #include <linux/bitmap.h>
>>>  #include <linux/bug.h>
>>>
>>> +#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
>>
>> Why isn't this dependency simply part of the Makefile rules?
>
> There are already current users of iommu-helper.c which calls for
> iommu_area_alloc APIs.
> Putting this config as part of Makefile without other users(except
> ARM/ARM64) using this, I felt not very relevant at this point of time.
> Moreover, we will have to see all the current users of iommu-helper
> and add the config option with them as well, which again I felt not to
> do at this point of time.
>
>>
>>> +#include <linux/iommu.h>
>>> +#include <linux/device.h>
>>> +#include <linux/iommu-helper.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/vmalloc.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/dma-contiguous.h>
>>> +#include <linux/mm.h>
>>> +#endif
>>> +
>>>  int iommu_is_span_boundary(unsigned int index, unsigned int nr,
>>>                            unsigned long shift,
>>>                            unsigned long boundary_size)
>>> @@ -39,3 +50,227 @@ again:
>>>         return -1;
>>>  }
>>>  EXPORT_SYMBOL(iommu_area_alloc);
>>> +
>>> +#ifdef CONFIG_DMA_USE_IOMMU_HELPER_MAPPING
>>> +
>>> +struct page **iommu_helper_alloc_buffer(struct device *dev, size_t size,
>>> +                                         gfp_t gfp, struct dma_attrs *attrs,
>>> +                       void (*arch_clear_buffer_cb)(struct page*, size_t))
>>> +{
>>> +       struct page **pages;
>>> +       int count = size >> PAGE_SHIFT;
>>> +       int array_size = count * sizeof(struct page *);
>>> +       int i = 0;
>>> +
>>> +       if (array_size <= PAGE_SIZE)
>>> +               pages = kzalloc(array_size, gfp);
>>> +       else
>>> +               pages = vzalloc(array_size);
>>> +       if (!pages)
>>> +               return NULL;
>>> +
>>> +       if (dma_get_attr(DMA_ATTR_FORCE_CONTIGUOUS, attrs)) {
>>> +               unsigned long order = get_order(size);
>>> +               struct page *page;
>>> +
>>> +               page = dma_alloc_from_contiguous(dev, count, order);
>>> +               if (!page)
>>> +                       goto error;
>>> +
>>> +               if (arch_clear_buffer_cb)
>>> +                       arch_clear_buffer_cb(page, size);
>>
>> Actually, clearing a buffer is always a memset of zero -- the arch-specific
>> part is about cache maintenance. The fly in the ointment here is flushing
>> highmem pages when you have an outer (physically addressed cache), since we
>> want to do the outer-cache maintenance outside of the kmap_atomic region as
>> a large block, but the inner-cache maintenance by VA with the highmem
>> mapping.
>>
>> Given that this is only called on the alloc path, is this actually a
>> fastpath for anybody? If not, we could move the outer-cache flushing into
>> the loop and simply have a arch_flush_buffer_cb instead, which would flush
>> l1 and l2 in turn.
>>
>> That way, architectures that don't have highmem and don't require cache
>> maintenance needn't supply a callback at all. Failure to supply a callback
>> with your current code means that the buffer is full of junk.
>
> Sure, you are correct here. Let me again take a look at this and get
> back to you.
>
>>
>>> +/**
>>> + * iommu_helper_init_mapping
>>> + * @bus: pointer to the bus holding the client device (for IOMMU calls)
>>> + * @base: start address of the valid IO address space
>>> + * @size: maximum size of the valid IO address space
>>> + *
>>> + * Creates a mapping structure which holds information about used/unused
>>> + * IO address ranges, which is required to perform memory allocation and
>>> + * mapping with IOMMU aware functions.
>>> + *
>>> + */
>>> +
>>> +struct dma_iommu_mapping *
>>> +iommu_helper_init_mapping(struct bus_type *bus, dma_addr_t base, size_t size)
>>
>> I spoke to Arnd on IRC the other day about this, and ultimately this
>> function can plug into of_dma_configure to place each iommu_group into a
>> separate iommu_mapping automatically.
>>
>> Can you put a dummy version of it in the header file, so it returns an error
>> when !CONFIG_DMA_USE_IOMMU_HELPER_MAPPING?
>
> OK sure.
>
>>
>> Will
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> Thanks
> Ritesh