All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/2]  Fix dma mapping when the cache is coherent
@ 2016-04-12 15:31 Gregory CLEMENT
  2016-04-12 15:31   ` Gregory CLEMENT
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2016-04-12 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

These two patches fixes the dma mapping functions when the system is
cache coherent. The first one allows to fix an issue we have on Armada
375/38x with the PL310 that's why it is tagged for stable too.

Thanks,

Gregory

PS: the mailing list was missing in the recipient list of the v7. It
was a mistake and I realized it only now. If needed I can repost it
with the few exchange we had with Russell King.


Changelog
v7 -> v8:
 - Use a flag instead of a boolean for checking the coherency. It
   improve the readability of the code. Suggested by Russell King.
 - Consider that when coherency is set, it's for both L1 and L2 caches
 - Use the arm_dma_alloc_args struct to pass the coherency status when
   possible. Suggested by Rabin Vincent
 - Remove the Tested-by flag from Marcin because of the modifications
   around the L1 cache management in this series.

v6 -> v7:
 - Renamed is coherent by l2_coherent as suggested by Russell

v5 -> v6:
 - Rebased on v4.6-rc1

v4 -> v5
 - Keep the dmac_* function outside the !is_coherent case.

v3 -> v4:
 - Rebased on v4.3-rc1
 - Fix conflict with commit "21caf3a765b0 ARM: 8398/1: arm DMA: Fix
   allocation from CMA for coherent DMA"

v2 -> v3:

 - Fix comments in patch 1 as suggested by Catalin.
 - Fix build issues in patch 2 (by using the multi_v7_defconfig +
   CONFIG_ROCKCHIP_IOMMU).
 - Add the arm_coherent_iommu_mmap_attrs function.


Gregory CLEMENT (2):
  ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent
  ARM: dma-mapping: Fix the coherent case when iommu is used

 arch/arm/mm/dma-mapping.c | 144 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 105 insertions(+), 39 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v8 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent
  2016-04-12 15:31 [PATCH v8 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT
@ 2016-04-12 15:31   ` Gregory CLEMENT
  2016-04-12 15:31 ` [PATCH v8 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used Gregory CLEMENT
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2016-04-12 15:31 UTC (permalink / raw)
  To: Russell King - ARM Linux, Catalin Marinas
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Gregory CLEMENT, Thomas Petazzoni, linux-arm-kernel,
	Lior Amsalem, Nadav Haklai, Romain Perier, Omri Itach,
	Marcin Wojtas, Rabin Vincent, stable

When a L2 cache controller is used in a system that provides hardware
coherency, the entire outer cache operations are useless, and can be
skipped.  Moreover, on some systems, it is harmful as it causes
deadlocks between the Marvell coherency mechanism, the Marvell PCIe
controller and the Cortex-A9.

In the current kernel implementation, the outer cache flush range
operation is triggered by the dma_alloc function.
This operation can be take place during runtime and in some
circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x
SoCs.

This patch extends the __dma_clear_buffer() function to receive a
boolean argument related to the coherency of the system. The same
things is done for the calling functions.

Reported-by: Nadav Haklai <nadavh@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: <stable@vger.kernel.org> # v3.16+
---
 arch/arm/mm/dma-mapping.c | 62 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index deac58d5f1f7..97beffb8a687 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -49,6 +49,7 @@ struct arm_dma_alloc_args {
 	pgprot_t prot;
 	const void *caller;
 	bool want_vaddr;
+	int coherent_flag;
 };
 
 struct arm_dma_free_args {
@@ -59,6 +60,9 @@ struct arm_dma_free_args {
 	bool want_vaddr;
 };
 
+#define NORMAL	    0
+#define COHERENT    1
+
 struct arm_dma_allocator {
 	void *(*alloc)(struct arm_dma_alloc_args *args,
 		       struct page **ret_page);
@@ -274,7 +278,7 @@ static u64 get_coherent_dma_mask(struct device *dev)
 	return mask;
 }
 
-static void __dma_clear_buffer(struct page *page, size_t size)
+static void __dma_clear_buffer(struct page *page, size_t size, int coherent_flag)
 {
 	/*
 	 * Ensure that the allocated pages are zeroed, and that any data
@@ -286,17 +290,21 @@ static void __dma_clear_buffer(struct page *page, size_t size)
 		while (size > 0) {
 			void *ptr = kmap_atomic(page);
 			memset(ptr, 0, PAGE_SIZE);
-			dmac_flush_range(ptr, ptr + PAGE_SIZE);
+			if (coherent_flag != COHERENT)
+				dmac_flush_range(ptr, ptr + PAGE_SIZE);
 			kunmap_atomic(ptr);
 			page++;
 			size -= PAGE_SIZE;
 		}
-		outer_flush_range(base, end);
+		if (coherent_flag != COHERENT)
+			outer_flush_range(base, end);
 	} else {
 		void *ptr = page_address(page);
 		memset(ptr, 0, size);
-		dmac_flush_range(ptr, ptr + size);
-		outer_flush_range(__pa(ptr), __pa(ptr) + size);
+		if (coherent_flag != COHERENT) {
+			dmac_flush_range(ptr, ptr + size);
+			outer_flush_range(__pa(ptr), __pa(ptr) + size);
+		}
 	}
 }
 
@@ -304,7 +312,8 @@ static void __dma_clear_buffer(struct page *page, size_t size)
  * Allocate a DMA buffer for 'dev' of size 'size' using the
  * specified gfp mask.  Note that 'size' must be page aligned.
  */
-static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gfp)
+static struct page *__dma_alloc_buffer(struct device *dev, size_t size,
+				       gfp_t gfp, int coherent_flag)
 {
 	unsigned long order = get_order(size);
 	struct page *page, *p, *e;
@@ -320,7 +329,7 @@ static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gf
 	for (p = page + (size >> PAGE_SHIFT), e = page + (1 << order); p < e; p++)
 		__free_page(p);
 
-	__dma_clear_buffer(page, size);
+	__dma_clear_buffer(page, size, coherent_flag);
 
 	return page;
 }
@@ -342,7 +351,8 @@ static void __dma_free_buffer(struct page *page, size_t size)
 
 static void *__alloc_from_contiguous(struct device *dev, size_t size,
 				     pgprot_t prot, struct page **ret_page,
-				     const void *caller, bool want_vaddr);
+				     const void *caller, bool want_vaddr,
+				     int coherent_flag);
 
 static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp,
 				 pgprot_t prot, struct page **ret_page,
@@ -407,10 +417,13 @@ static int __init atomic_pool_init(void)
 	atomic_pool = gen_pool_create(PAGE_SHIFT, -1);
 	if (!atomic_pool)
 		goto out;
-
+	/*
+	 * The atomic pool is only used for non-coherent allocations
+	 * so we must pass NORMAL for coherent_flag.
+	 */
 	if (dev_get_cma_area(NULL))
 		ptr = __alloc_from_contiguous(NULL, atomic_pool_size, prot,
-					      &page, atomic_pool_init, true);
+				      &page, atomic_pool_init, true, NORMAL);
 	else
 		ptr = __alloc_remap_buffer(NULL, atomic_pool_size, gfp, prot,
 					   &page, atomic_pool_init, true);
@@ -524,7 +537,11 @@ static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp,
 {
 	struct page *page;
 	void *ptr = NULL;
-	page = __dma_alloc_buffer(dev, size, gfp);
+	/*
+	 * __alloc_remap_buffer is only called when the device is
+	 * non-coherent
+	 */
+	page = __dma_alloc_buffer(dev, size, gfp, NORMAL);
 	if (!page)
 		return NULL;
 	if (!want_vaddr)
@@ -579,7 +596,8 @@ static int __free_from_pool(void *start, size_t size)
 
 static void *__alloc_from_contiguous(struct device *dev, size_t size,
 				     pgprot_t prot, struct page **ret_page,
-				     const void *caller, bool want_vaddr)
+				     const void *caller, bool want_vaddr,
+				     int coherent_flag)
 {
 	unsigned long order = get_order(size);
 	size_t count = size >> PAGE_SHIFT;
@@ -590,7 +608,7 @@ static void *__alloc_from_contiguous(struct device *dev, size_t size,
 	if (!page)
 		return NULL;
 
-	__dma_clear_buffer(page, size);
+	__dma_clear_buffer(page, size, coherent_flag);
 
 	if (!want_vaddr)
 		goto out;
@@ -640,7 +658,7 @@ static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot)
 #define __get_dma_pgprot(attrs, prot)				__pgprot(0)
 #define __alloc_remap_buffer(dev, size, gfp, prot, ret, c, wv)	NULL
 #define __alloc_from_pool(size, ret_page)			NULL
-#define __alloc_from_contiguous(dev, size, prot, ret, c, wv)	NULL
+#define __alloc_from_contiguous(dev, size, prot, ret, c, wv, coherent_flag)	NULL
 #define __free_from_pool(cpu_addr, size)			do { } while (0)
 #define __free_from_contiguous(dev, page, cpu_addr, size, wv)	do { } while (0)
 #define __dma_free_remap(cpu_addr, size)			do { } while (0)
@@ -651,7 +669,8 @@ static void *__alloc_simple_buffer(struct device *dev, size_t size, gfp_t gfp,
 				   struct page **ret_page)
 {
 	struct page *page;
-	page = __dma_alloc_buffer(dev, size, gfp);
+	/* __alloc_simple_buffer is only called when the device is coherent */
+	page = __dma_alloc_buffer(dev, size, gfp, COHERENT);
 	if (!page)
 		return NULL;
 
@@ -681,7 +700,7 @@ static void *cma_allocator_alloc(struct arm_dma_alloc_args *args,
 {
 	return __alloc_from_contiguous(args->dev, args->size, args->prot,
 				       ret_page, args->caller,
-				       args->want_vaddr);
+				       args->want_vaddr, args->coherent_flag);
 }
 
 static void cma_allocator_free(struct arm_dma_free_args *args)
@@ -748,6 +767,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 		.prot = prot,
 		.caller = caller,
 		.want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs),
+		.coherent_flag = is_coherent ? COHERENT : NORMAL,
 	};
 
 #ifdef CONFIG_DMA_API_DEBUG
@@ -1264,7 +1284,8 @@ static inline void __free_iova(struct dma_iommu_mapping *mapping,
 static const int iommu_order_array[] = { 9, 8, 4, 0 };
 
 static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
-					  gfp_t gfp, struct dma_attrs *attrs)
+					  gfp_t gfp, struct dma_attrs *attrs,
+					  int coherent_flag)
 {
 	struct page **pages;
 	int count = size >> PAGE_SHIFT;
@@ -1288,7 +1309,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
 		if (!page)
 			goto error;
 
-		__dma_clear_buffer(page, size);
+		__dma_clear_buffer(page, size, coherent_flag);
 
 		for (i = 0; i < count; i++)
 			pages[i] = page + i;
@@ -1338,7 +1359,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
 				pages[i + j] = pages[i] + j;
 		}
 
-		__dma_clear_buffer(pages[i], PAGE_SIZE << order);
+		__dma_clear_buffer(pages[i], PAGE_SIZE << order, coherent_flag);
 		i += 1 << order;
 		count -= 1 << order;
 	}
@@ -1516,7 +1537,8 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	 */
 	gfp &= ~(__GFP_COMP);
 
-	pages = __iommu_alloc_buffer(dev, size, gfp, attrs);
+	/* For now always consider we are in a non-coherent case */
+	pages = __iommu_alloc_buffer(dev, size, gfp, attrs, NORMAL);
 	if (!pages)
 		return NULL;
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v8 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent
@ 2016-04-12 15:31   ` Gregory CLEMENT
  0 siblings, 0 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2016-04-12 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

When a L2 cache controller is used in a system that provides hardware
coherency, the entire outer cache operations are useless, and can be
skipped.  Moreover, on some systems, it is harmful as it causes
deadlocks between the Marvell coherency mechanism, the Marvell PCIe
controller and the Cortex-A9.

In the current kernel implementation, the outer cache flush range
operation is triggered by the dma_alloc function.
This operation can be take place during runtime and in some
circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x
SoCs.

This patch extends the __dma_clear_buffer() function to receive a
boolean argument related to the coherency of the system. The same
things is done for the calling functions.

Reported-by: Nadav Haklai <nadavh@marvell.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: <stable@vger.kernel.org> # v3.16+
---
 arch/arm/mm/dma-mapping.c | 62 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index deac58d5f1f7..97beffb8a687 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -49,6 +49,7 @@ struct arm_dma_alloc_args {
 	pgprot_t prot;
 	const void *caller;
 	bool want_vaddr;
+	int coherent_flag;
 };
 
 struct arm_dma_free_args {
@@ -59,6 +60,9 @@ struct arm_dma_free_args {
 	bool want_vaddr;
 };
 
+#define NORMAL	    0
+#define COHERENT    1
+
 struct arm_dma_allocator {
 	void *(*alloc)(struct arm_dma_alloc_args *args,
 		       struct page **ret_page);
@@ -274,7 +278,7 @@ static u64 get_coherent_dma_mask(struct device *dev)
 	return mask;
 }
 
-static void __dma_clear_buffer(struct page *page, size_t size)
+static void __dma_clear_buffer(struct page *page, size_t size, int coherent_flag)
 {
 	/*
 	 * Ensure that the allocated pages are zeroed, and that any data
@@ -286,17 +290,21 @@ static void __dma_clear_buffer(struct page *page, size_t size)
 		while (size > 0) {
 			void *ptr = kmap_atomic(page);
 			memset(ptr, 0, PAGE_SIZE);
-			dmac_flush_range(ptr, ptr + PAGE_SIZE);
+			if (coherent_flag != COHERENT)
+				dmac_flush_range(ptr, ptr + PAGE_SIZE);
 			kunmap_atomic(ptr);
 			page++;
 			size -= PAGE_SIZE;
 		}
-		outer_flush_range(base, end);
+		if (coherent_flag != COHERENT)
+			outer_flush_range(base, end);
 	} else {
 		void *ptr = page_address(page);
 		memset(ptr, 0, size);
-		dmac_flush_range(ptr, ptr + size);
-		outer_flush_range(__pa(ptr), __pa(ptr) + size);
+		if (coherent_flag != COHERENT) {
+			dmac_flush_range(ptr, ptr + size);
+			outer_flush_range(__pa(ptr), __pa(ptr) + size);
+		}
 	}
 }
 
@@ -304,7 +312,8 @@ static void __dma_clear_buffer(struct page *page, size_t size)
  * Allocate a DMA buffer for 'dev' of size 'size' using the
  * specified gfp mask.  Note that 'size' must be page aligned.
  */
-static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gfp)
+static struct page *__dma_alloc_buffer(struct device *dev, size_t size,
+				       gfp_t gfp, int coherent_flag)
 {
 	unsigned long order = get_order(size);
 	struct page *page, *p, *e;
@@ -320,7 +329,7 @@ static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gf
 	for (p = page + (size >> PAGE_SHIFT), e = page + (1 << order); p < e; p++)
 		__free_page(p);
 
-	__dma_clear_buffer(page, size);
+	__dma_clear_buffer(page, size, coherent_flag);
 
 	return page;
 }
@@ -342,7 +351,8 @@ static void __dma_free_buffer(struct page *page, size_t size)
 
 static void *__alloc_from_contiguous(struct device *dev, size_t size,
 				     pgprot_t prot, struct page **ret_page,
-				     const void *caller, bool want_vaddr);
+				     const void *caller, bool want_vaddr,
+				     int coherent_flag);
 
 static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp,
 				 pgprot_t prot, struct page **ret_page,
@@ -407,10 +417,13 @@ static int __init atomic_pool_init(void)
 	atomic_pool = gen_pool_create(PAGE_SHIFT, -1);
 	if (!atomic_pool)
 		goto out;
-
+	/*
+	 * The atomic pool is only used for non-coherent allocations
+	 * so we must pass NORMAL for coherent_flag.
+	 */
 	if (dev_get_cma_area(NULL))
 		ptr = __alloc_from_contiguous(NULL, atomic_pool_size, prot,
-					      &page, atomic_pool_init, true);
+				      &page, atomic_pool_init, true, NORMAL);
 	else
 		ptr = __alloc_remap_buffer(NULL, atomic_pool_size, gfp, prot,
 					   &page, atomic_pool_init, true);
@@ -524,7 +537,11 @@ static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp,
 {
 	struct page *page;
 	void *ptr = NULL;
-	page = __dma_alloc_buffer(dev, size, gfp);
+	/*
+	 * __alloc_remap_buffer is only called when the device is
+	 * non-coherent
+	 */
+	page = __dma_alloc_buffer(dev, size, gfp, NORMAL);
 	if (!page)
 		return NULL;
 	if (!want_vaddr)
@@ -579,7 +596,8 @@ static int __free_from_pool(void *start, size_t size)
 
 static void *__alloc_from_contiguous(struct device *dev, size_t size,
 				     pgprot_t prot, struct page **ret_page,
-				     const void *caller, bool want_vaddr)
+				     const void *caller, bool want_vaddr,
+				     int coherent_flag)
 {
 	unsigned long order = get_order(size);
 	size_t count = size >> PAGE_SHIFT;
@@ -590,7 +608,7 @@ static void *__alloc_from_contiguous(struct device *dev, size_t size,
 	if (!page)
 		return NULL;
 
-	__dma_clear_buffer(page, size);
+	__dma_clear_buffer(page, size, coherent_flag);
 
 	if (!want_vaddr)
 		goto out;
@@ -640,7 +658,7 @@ static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot)
 #define __get_dma_pgprot(attrs, prot)				__pgprot(0)
 #define __alloc_remap_buffer(dev, size, gfp, prot, ret, c, wv)	NULL
 #define __alloc_from_pool(size, ret_page)			NULL
-#define __alloc_from_contiguous(dev, size, prot, ret, c, wv)	NULL
+#define __alloc_from_contiguous(dev, size, prot, ret, c, wv, coherent_flag)	NULL
 #define __free_from_pool(cpu_addr, size)			do { } while (0)
 #define __free_from_contiguous(dev, page, cpu_addr, size, wv)	do { } while (0)
 #define __dma_free_remap(cpu_addr, size)			do { } while (0)
@@ -651,7 +669,8 @@ static void *__alloc_simple_buffer(struct device *dev, size_t size, gfp_t gfp,
 				   struct page **ret_page)
 {
 	struct page *page;
-	page = __dma_alloc_buffer(dev, size, gfp);
+	/* __alloc_simple_buffer is only called when the device is coherent */
+	page = __dma_alloc_buffer(dev, size, gfp, COHERENT);
 	if (!page)
 		return NULL;
 
@@ -681,7 +700,7 @@ static void *cma_allocator_alloc(struct arm_dma_alloc_args *args,
 {
 	return __alloc_from_contiguous(args->dev, args->size, args->prot,
 				       ret_page, args->caller,
-				       args->want_vaddr);
+				       args->want_vaddr, args->coherent_flag);
 }
 
 static void cma_allocator_free(struct arm_dma_free_args *args)
@@ -748,6 +767,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
 		.prot = prot,
 		.caller = caller,
 		.want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs),
+		.coherent_flag = is_coherent ? COHERENT : NORMAL,
 	};
 
 #ifdef CONFIG_DMA_API_DEBUG
@@ -1264,7 +1284,8 @@ static inline void __free_iova(struct dma_iommu_mapping *mapping,
 static const int iommu_order_array[] = { 9, 8, 4, 0 };
 
 static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
-					  gfp_t gfp, struct dma_attrs *attrs)
+					  gfp_t gfp, struct dma_attrs *attrs,
+					  int coherent_flag)
 {
 	struct page **pages;
 	int count = size >> PAGE_SHIFT;
@@ -1288,7 +1309,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
 		if (!page)
 			goto error;
 
-		__dma_clear_buffer(page, size);
+		__dma_clear_buffer(page, size, coherent_flag);
 
 		for (i = 0; i < count; i++)
 			pages[i] = page + i;
@@ -1338,7 +1359,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
 				pages[i + j] = pages[i] + j;
 		}
 
-		__dma_clear_buffer(pages[i], PAGE_SIZE << order);
+		__dma_clear_buffer(pages[i], PAGE_SIZE << order, coherent_flag);
 		i += 1 << order;
 		count -= 1 << order;
 	}
@@ -1516,7 +1537,8 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	 */
 	gfp &= ~(__GFP_COMP);
 
-	pages = __iommu_alloc_buffer(dev, size, gfp, attrs);
+	/* For now always consider we are in a non-coherent case */
+	pages = __iommu_alloc_buffer(dev, size, gfp, attrs, NORMAL);
 	if (!pages)
 		return NULL;
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v8 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used
  2016-04-12 15:31 [PATCH v8 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT
  2016-04-12 15:31   ` Gregory CLEMENT
@ 2016-04-12 15:31 ` Gregory CLEMENT
  2016-04-16 15:27 ` [PATCH v8 0/2] Fix dma mapping when the cache is coherent Marcin Wojtas
  2016-06-16 14:14 ` Gregory CLEMENT
  3 siblings, 0 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2016-04-12 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

When doing dma allocation with IOMMU the __iommu_alloc_atomic() was
used even when the system was coherent. However, this function
allocates from a non-cacheable pool, which is fine when the device is
not cache coherent but won't work as expected in the device is cache
coherent. Indeed, the CPU and device must access the memory using the
same cacheability attributes.

Moreover when the devices are coherent, the mmap call must not change
the pg_prot flags in the vma struct. The arm_coherent_iommu_mmap_attrs
has been updated in the same way that it was done for the arm_dma_mmap
in commit 55af8a91640d ("ARM: 8387/1: arm/mm/dma-mapping.c: Add
arm_coherent_dma_mmap").

Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/mm/dma-mapping.c | 86 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 65 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 97beffb8a687..bc7bbef264de 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1487,13 +1487,16 @@ static struct page **__iommu_get_pages(void *cpu_addr, struct dma_attrs *attrs)
 	return NULL;
 }
 
-static void *__iommu_alloc_atomic(struct device *dev, size_t size,
-				  dma_addr_t *handle)
+static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp,
+				  dma_addr_t *handle, int coherent_flag)
 {
 	struct page *page;
 	void *addr;
 
-	addr = __alloc_from_pool(size, &page);
+	if (coherent_flag  == COHERENT)
+		addr = __alloc_simple_buffer(dev, size, gfp, &page);
+	else
+		addr = __alloc_from_pool(size, &page);
 	if (!addr)
 		return NULL;
 
@@ -1509,14 +1512,18 @@ err_mapping:
 }
 
 static void __iommu_free_atomic(struct device *dev, void *cpu_addr,
-				dma_addr_t handle, size_t size)
+			dma_addr_t handle, size_t size, int coherent_flag)
 {
 	__iommu_remove_mapping(dev, handle, size);
-	__free_from_pool(cpu_addr, size);
+	if (coherent_flag == COHERENT)
+		__dma_free_buffer(virt_to_page(cpu_addr), size);
+	else
+		__free_from_pool(cpu_addr, size);
 }
 
-static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
-	    dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs)
+static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size,
+	    dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs,
+	    int coherent_flag)
 {
 	pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL);
 	struct page **pages;
@@ -1525,8 +1532,9 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	*handle = DMA_ERROR_CODE;
 	size = PAGE_ALIGN(size);
 
-	if (!gfpflags_allow_blocking(gfp))
-		return __iommu_alloc_atomic(dev, size, handle);
+	if (coherent_flag  == COHERENT || !gfpflags_allow_blocking(gfp))
+		return __iommu_alloc_simple(dev, size, gfp, handle,
+					    coherent_flag);
 
 	/*
 	 * Following is a work-around (a.k.a. hack) to prevent pages
@@ -1537,8 +1545,7 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
 	 */
 	gfp &= ~(__GFP_COMP);
 
-	/* For now always consider we are in a non-coherent case */
-	pages = __iommu_alloc_buffer(dev, size, gfp, attrs, NORMAL);
+	pages = __iommu_alloc_buffer(dev, size, gfp, attrs, coherent_flag);
 	if (!pages)
 		return NULL;
 
@@ -1563,7 +1570,19 @@ err_buffer:
 	return NULL;
 }
 
-static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
+static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
+		    dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs)
+{
+	return __arm_iommu_alloc_attrs(dev, size, handle, gfp, attrs, NORMAL);
+}
+
+static void *arm_coherent_iommu_alloc_attrs(struct device *dev, size_t size,
+		    dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs)
+{
+	return __arm_iommu_alloc_attrs(dev, size, handle, gfp, attrs, COHERENT);
+}
+
+static int __arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
 		    void *cpu_addr, dma_addr_t dma_addr, size_t size,
 		    struct dma_attrs *attrs)
 {
@@ -1573,8 +1592,6 @@ static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
 	unsigned long nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
 	unsigned long off = vma->vm_pgoff;
 
-	vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
-
 	if (!pages)
 		return -ENXIO;
 
@@ -1595,19 +1612,34 @@ static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
 
 	return 0;
 }
+static int arm_iommu_mmap_attrs(struct device *dev,
+		struct vm_area_struct *vma, void *cpu_addr,
+		dma_addr_t dma_addr, size_t size, struct dma_attrs *attrs)
+{
+	vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
+
+	return __arm_iommu_mmap_attrs(dev, vma, cpu_addr, dma_addr, size, attrs);
+}
+
+static int arm_coherent_iommu_mmap_attrs(struct device *dev,
+		struct vm_area_struct *vma, void *cpu_addr,
+		dma_addr_t dma_addr, size_t size, struct dma_attrs *attrs)
+{
+	return __arm_iommu_mmap_attrs(dev, vma, cpu_addr, dma_addr, size, attrs);
+}
 
 /*
  * free a page as defined by the above mapping.
  * Must not be called with IRQs disabled.
  */
-void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
-			  dma_addr_t handle, struct dma_attrs *attrs)
+void __arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
+	dma_addr_t handle, struct dma_attrs *attrs, int coherent_flag)
 {
 	struct page **pages;
 	size = PAGE_ALIGN(size);
 
-	if (__in_atomic_pool(cpu_addr, size)) {
-		__iommu_free_atomic(dev, cpu_addr, handle, size);
+	if (coherent_flag == COHERENT || __in_atomic_pool(cpu_addr, size)) {
+		__iommu_free_atomic(dev, cpu_addr, handle, size, coherent_flag);
 		return;
 	}
 
@@ -1626,6 +1658,18 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
 	__iommu_free_buffer(dev, pages, size, attrs);
 }
 
+void arm_iommu_free_attrs(struct device *dev, size_t size,
+		    void *cpu_addr, dma_addr_t handle, struct dma_attrs *attrs)
+{
+	__arm_iommu_free_attrs(dev, size, cpu_addr, handle, attrs, NORMAL);
+}
+
+void arm_coherent_iommu_free_attrs(struct device *dev, size_t size,
+		    void *cpu_addr, dma_addr_t handle, struct dma_attrs *attrs)
+{
+	__arm_iommu_free_attrs(dev, size, cpu_addr, handle, attrs, COHERENT);
+}
+
 static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt,
 				 void *cpu_addr, dma_addr_t dma_addr,
 				 size_t size, struct dma_attrs *attrs)
@@ -2032,9 +2076,9 @@ struct dma_map_ops iommu_ops = {
 };
 
 struct dma_map_ops iommu_coherent_ops = {
-	.alloc		= arm_iommu_alloc_attrs,
-	.free		= arm_iommu_free_attrs,
-	.mmap		= arm_iommu_mmap_attrs,
+	.alloc		= arm_coherent_iommu_alloc_attrs,
+	.free		= arm_coherent_iommu_free_attrs,
+	.mmap		= arm_coherent_iommu_mmap_attrs,
 	.get_sgtable	= arm_iommu_get_sgtable,
 
 	.map_page	= arm_coherent_iommu_map_page,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v8 0/2] Fix dma mapping when the cache is coherent
  2016-04-12 15:31 [PATCH v8 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT
  2016-04-12 15:31   ` Gregory CLEMENT
  2016-04-12 15:31 ` [PATCH v8 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used Gregory CLEMENT
@ 2016-04-16 15:27 ` Marcin Wojtas
  2016-04-20 23:02   ` Gregory CLEMENT
  2016-06-16 14:14 ` Gregory CLEMENT
  3 siblings, 1 reply; 7+ messages in thread
From: Marcin Wojtas @ 2016-04-16 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Gregory,

You can re-add my tested-by. Hang is no longer a problem with your
patch during my test of neta.

Best regards,
Marcin

2016-04-12 17:31 GMT+02:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> Hi,
>
> These two patches fixes the dma mapping functions when the system is
> cache coherent. The first one allows to fix an issue we have on Armada
> 375/38x with the PL310 that's why it is tagged for stable too.
>
> Thanks,
>
> Gregory
>
> PS: the mailing list was missing in the recipient list of the v7. It
> was a mistake and I realized it only now. If needed I can repost it
> with the few exchange we had with Russell King.
>
>
> Changelog
> v7 -> v8:
>  - Use a flag instead of a boolean for checking the coherency. It
>    improve the readability of the code. Suggested by Russell King.
>  - Consider that when coherency is set, it's for both L1 and L2 caches
>  - Use the arm_dma_alloc_args struct to pass the coherency status when
>    possible. Suggested by Rabin Vincent
>  - Remove the Tested-by flag from Marcin because of the modifications
>    around the L1 cache management in this series.
>
> v6 -> v7:
>  - Renamed is coherent by l2_coherent as suggested by Russell
>
> v5 -> v6:
>  - Rebased on v4.6-rc1
>
> v4 -> v5
>  - Keep the dmac_* function outside the !is_coherent case.
>
> v3 -> v4:
>  - Rebased on v4.3-rc1
>  - Fix conflict with commit "21caf3a765b0 ARM: 8398/1: arm DMA: Fix
>    allocation from CMA for coherent DMA"
>
> v2 -> v3:
>
>  - Fix comments in patch 1 as suggested by Catalin.
>  - Fix build issues in patch 2 (by using the multi_v7_defconfig +
>    CONFIG_ROCKCHIP_IOMMU).
>  - Add the arm_coherent_iommu_mmap_attrs function.
>
>
> Gregory CLEMENT (2):
>   ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent
>   ARM: dma-mapping: Fix the coherent case when iommu is used
>
>  arch/arm/mm/dma-mapping.c | 144 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 105 insertions(+), 39 deletions(-)
>
> --
> 2.5.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v8 0/2] Fix dma mapping when the cache is coherent
  2016-04-16 15:27 ` [PATCH v8 0/2] Fix dma mapping when the cache is coherent Marcin Wojtas
@ 2016-04-20 23:02   ` Gregory CLEMENT
  0 siblings, 0 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2016-04-20 23:02 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Marcin,
 
 On sam., avril 16 2016, Marcin Wojtas <mw@semihalf.com> wrote:

> Hi Gregory,
>
> You can re-add my tested-by. Hang is no longer a problem with your
> patch during my test of neta.

The patch was already submitted in the Russell King's Patch Tracking
System, so I can't add the tested-by on them, but I am sure it will help
these pacthes to be merged.

Thanks,

Gregory

>
> Best regards,
> Marcin
>
> 2016-04-12 17:31 GMT+02:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
>> Hi,
>>
>> These two patches fixes the dma mapping functions when the system is
>> cache coherent. The first one allows to fix an issue we have on Armada
>> 375/38x with the PL310 that's why it is tagged for stable too.
>>
>> Thanks,
>>
>> Gregory
>>
>> PS: the mailing list was missing in the recipient list of the v7. It
>> was a mistake and I realized it only now. If needed I can repost it
>> with the few exchange we had with Russell King.
>>
>>
>> Changelog
>> v7 -> v8:
>>  - Use a flag instead of a boolean for checking the coherency. It
>>    improve the readability of the code. Suggested by Russell King.
>>  - Consider that when coherency is set, it's for both L1 and L2 caches
>>  - Use the arm_dma_alloc_args struct to pass the coherency status when
>>    possible. Suggested by Rabin Vincent
>>  - Remove the Tested-by flag from Marcin because of the modifications
>>    around the L1 cache management in this series.
>>
>> v6 -> v7:
>>  - Renamed is coherent by l2_coherent as suggested by Russell
>>
>> v5 -> v6:
>>  - Rebased on v4.6-rc1
>>
>> v4 -> v5
>>  - Keep the dmac_* function outside the !is_coherent case.
>>
>> v3 -> v4:
>>  - Rebased on v4.3-rc1
>>  - Fix conflict with commit "21caf3a765b0 ARM: 8398/1: arm DMA: Fix
>>    allocation from CMA for coherent DMA"
>>
>> v2 -> v3:
>>
>>  - Fix comments in patch 1 as suggested by Catalin.
>>  - Fix build issues in patch 2 (by using the multi_v7_defconfig +
>>    CONFIG_ROCKCHIP_IOMMU).
>>  - Add the arm_coherent_iommu_mmap_attrs function.
>>
>>
>> Gregory CLEMENT (2):
>>   ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent
>>   ARM: dma-mapping: Fix the coherent case when iommu is used
>>
>>  arch/arm/mm/dma-mapping.c | 144 +++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 105 insertions(+), 39 deletions(-)
>>
>> --
>> 2.5.0
>>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v8 0/2]  Fix dma mapping when the cache is coherent
  2016-04-12 15:31 [PATCH v8 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT
                   ` (2 preceding siblings ...)
  2016-04-16 15:27 ` [PATCH v8 0/2] Fix dma mapping when the cache is coherent Marcin Wojtas
@ 2016-06-16 14:14 ` Gregory CLEMENT
  3 siblings, 0 replies; 7+ messages in thread
From: Gregory CLEMENT @ 2016-06-16 14:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,
 
 On mar., avril 12 2016, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> Hi,
>
> These two patches fixes the dma mapping functions when the system is
> cache coherent. The first one allows to fix an issue we have on Armada
> 375/38x with the PL310 that's why it is tagged for stable too.
>
> Thanks,
[..]
> Gregory CLEMENT (2):
>   ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent
>   ARM: dma-mapping: Fix the coherent case when iommu is used

I submit these two patches to your patch system 2 months ago:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8561/3
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8561/4

but they were not merged yet. Did you expect more change?
What can I do to help them to be merged?

I have just tested and these two patches still appply, as is, on the
v4.7-rc3 so there is no need to rebase them.

Thanks,

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-06-16 14:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-12 15:31 [PATCH v8 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT
2016-04-12 15:31 ` [PATCH v8 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C " Gregory CLEMENT
2016-04-12 15:31   ` Gregory CLEMENT
2016-04-12 15:31 ` [PATCH v8 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used Gregory CLEMENT
2016-04-16 15:27 ` [PATCH v8 0/2] Fix dma mapping when the cache is coherent Marcin Wojtas
2016-04-20 23:02   ` Gregory CLEMENT
2016-06-16 14:14 ` Gregory CLEMENT

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.