Skip to contentSkip to conversation

arm:dma-mapping Refactoring iommu dma-mapping code

PATCH Superseded ritesh.harjani at gmail.com 4 participants 10 messages Jun 6, 2014 – Sep 1, 2014

Subsystems (6)
DRM drivers Arm (32-bit) Library code Arm Samsung Exynos +2 more subsystems
lore
Share & tools
lore

ritesh.harjani at gmail.com

author
#0 lore raw

v3 cover arm:dma-mapping Refactoring iommu dma-mapping code

From: Ritesh Harjani <[email protected]>

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(-)
Signature · 1 line
1.8.1.3
author
#1 lore raw

PATCH v3 1/3 device.h: arm dma-iommu: Move out dma_iommu_mapping struct

From: Ritesh Harjani <[email protected]>

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 <[email protected]>
---
 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(-)
arch/arm/Kconfig +4 -0
index db3c541..e65042f 100644
@@ -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
arch/arm/include/asm/device.h +0 -9
index dc662fc..6e2cb0e 100644
@@ -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
arch/arm/include/asm/dma-iommu.h +1 -15
index 8e3fcb9..50c010b 100644
@@ -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);
arch/arm/mm/dma-mapping.c +10 -10
index 3d43c41..b82561e 100644
@@ -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));
drivers/gpu/drm/exynos/exynos_drm_iommu.c +5 -5
index b32b291..9dbf369 100644
@@ -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;
include/linux/device.h +5 -0
index d1d1c05..2bb58c9 100644
@@ -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;
include/linux/iommu-helper.h +19 -0
index 86bdeff..961d8ef 100644
@@ -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)
Signature · 1 line
1.8.1.3
author
#2 lore raw

PATCH v3 2/3 arm: dma-mapping: Refactor attach/detach, alloc/free func

From: Ritesh Harjani <[email protected]>

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 <[email protected]>
---
 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(-)
arch/arm/mm/dma-mapping.c +19 -181
index b82561e..66cf96b 100644
@@ -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);
include/linux/iommu-helper.h +18 -0
index 961d8ef..09bcea3 100644
@@ -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
lib/iommu-helper.c +235 -0

244 lines not on this page · raw message

Signature · 1 line
1.8.1.3
author
#3 lore raw

PATCH v3 3/3 arm:dma-iommu: Move out complete func defs

From: Ritesh Harjani <[email protected]>

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 <[email protected]>
---
 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(-)
arch/arm/Kconfig +21 -21
index e65042f..a2cae78 100644
@@ -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
arch/arm/mm/dma-mapping.c +29 -343

480 lines not on this page · raw message

include/linux/iommu-helper.h +26 -2
index 09bcea3..685b786 100644
@@ -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
lib/iommu-helper.c +328 -1

353 lines not on this page · raw message

Signature · 1 line
1.8.1.3
#4 lore raw

PATCH v3 2/3 arm: dma-mapping: Refactor attach/detach, alloc/free func

In reply to ritesh.harjani at gmail.com
Quoted 10 lines · ritesh.harjani at gmail.com

From: Ritesh Harjani <[email protected]>

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).

Quoted 41 lines
include/linux/iommu-helper.h +18 -0
index 961d8ef..09bcea3 100644
@@ -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
lib/iommu-helper.c +0 -0
index c27e269..3664709 100644

lib/iommu-helper.c line 9

Quoted 4 lines
lib/iommu-helper.c +1 -0
@@ -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?

Quoted 36 lines
lib/iommu-helper.c +29 -0
+#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;
+

lib/iommu-helper.c again: · line 81

Quoted 10 lines
lib/iommu-helper.c +10 -0
+       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.

Quoted 4 lines
lib/iommu-helper.c +4 -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

lib/iommu-helper.c again: · line 162

Quoted 10 lines
lib/iommu-helper.c +10 -0
+ * @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

#5 lore raw

PATCH v3 2/3 arm: dma-mapping: Refactor attach/detach, alloc/free func

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.

Quoted 9 lines · Will Deacon
Quoted 8 lines · ritesh.harjani at gmail.com

From: Ritesh Harjani <[email protected]>

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

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

Quoted 5 lines
Quoted 2 lines

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!

Quoted 45 lines
Quoted 45 lines
include/linux/iommu-helper.h +18 -0
index 961d8ef..09bcea3 100644
@@ -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
lib/iommu-helper.c +0 -0
index c27e269..3664709 100644
@@ -6,6 +6,17 @@
 #include <linux/bitmap.h>
 #include <linux/bug.h>
Quoted 3 lines

lib/iommu-helper.c line 9

Quoted 1 line
lib/iommu-helper.c +1 -0
+#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.

Quoted 55 lines
Quoted 37 lines
lib/iommu-helper.c +29 -0
+#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;
+

lib/iommu-helper.c again: · line 81

Quoted 10 lines
lib/iommu-helper.c +10 -0
+       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.

Quoted 8 lines

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.

Quoted 20 lines
Quoted 5 lines
lib/iommu-helper.c +4 -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

lib/iommu-helper.c again: · line 162

Quoted 10 lines
lib/iommu-helper.c +10 -0
+ * @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.

Quoted 2 lines

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.

Quoted 7 lines

Will

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

Thanks
Ritesh

author
#6 lore raw

PATCH v3 2/3 arm: dma-mapping: Refactor attach/detach, alloc/free func

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

Quoted 187 lines · 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.

Quoted 9 lines · Will Deacon
Quoted 8 lines · ritesh.harjani at gmail.com

From: Ritesh Harjani <[email protected]>

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

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

Quoted 5 lines
Quoted 2 lines

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!

Quoted 45 lines
Quoted 45 lines
include/linux/iommu-helper.h +18 -0
index 961d8ef..09bcea3 100644
@@ -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
lib/iommu-helper.c +0 -0
index c27e269..3664709 100644
@@ -6,6 +6,17 @@
 #include <linux/bitmap.h>
 #include <linux/bug.h>
Quoted 3 lines

lib/iommu-helper.c line 9

Quoted 1 line
lib/iommu-helper.c +1 -0
+#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.

Quoted 55 lines
Quoted 37 lines
lib/iommu-helper.c +29 -0
+#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;
+

lib/iommu-helper.c again: · line 81

Quoted 10 lines
lib/iommu-helper.c +10 -0
+       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.

Quoted 8 lines

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.

Quoted 20 lines
Quoted 5 lines
lib/iommu-helper.c +4 -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

lib/iommu-helper.c again: · line 162

Quoted 10 lines
lib/iommu-helper.c +10 -0
+ * @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.

Quoted 2 lines

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.

Quoted 7 lines

Will

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

Thanks
Ritesh

author
#7 lore raw

v3 cover arm:dma-mapping Refactoring iommu dma-mapping code

In reply to ritesh.harjani at gmail.com

Hi Will/Catalin/Marek...

Any comments on this patch series ??

Thanks
Ritesh

Quoted 55 lines · On Fri, Jun 6, 2014 at 2:12 PM, <[email protected]> wrote:

From: Ritesh Harjani <[email protected]>

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

maintainer
#8 lore raw

v3 cover arm:dma-mapping Refactoring iommu dma-mapping code

Quoted 3 lines · Ritesh Harjani

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

author
#9 lore raw

v3 cover arm:dma-mapping Refactoring iommu dma-mapping code

Hi Greg,

Quoted 7 lines · Greg Kroah-Hartman
Quoted 3 lines · Ritesh Harjani

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.

Quoted 2 lines

greg k-h

Thanks
Ritesh