linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
@ 2017-01-13 11:07 Geert Uytterhoeven
  2017-01-13 11:32 ` Robin Murphy
  0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-01-13 11:07 UTC (permalink / raw)
  To: Joerg Roedel, Catalin Marinas, Will Deacon, Robin Murphy
  Cc: Laurent Pinchart, Marek Szyprowski, Magnus Damm, iommu,
	linux-arm-kernel, linux-renesas-soc, linux-kernel,
	Geert Uytterhoeven

Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
This allows to allocate physically contiguous DMA buffers on arm64
systems with an IOMMU.

Note that as this uses the CMA allocator, setting this attribute has a
runtime-dependency on CONFIG_DMA_CMA, just like on arm32.

For arm64 systems using swiotlb, no changes are needed to support the
allocation of physically contiguous DMA buffers:
  - swiotlb always uses physically contiguous buffers (up to
    IO_TLB_SEGSIZE = 128 pages),
  - arm64's __dma_alloc_coherent() already calls
    dma_alloc_from_contiguous() when CMA is available.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/mm/dma-mapping.c |  4 ++--
 drivers/iommu/dma-iommu.c   | 44 ++++++++++++++++++++++++++++++++++----------
 include/linux/dma-iommu.h   |  2 +-
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 1d7d5d2881db7c19..5fba14a21163e41f 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -589,7 +589,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
 		addr = dma_common_pages_remap(pages, size, VM_USERMAP, prot,
 					      __builtin_return_address(0));
 		if (!addr)
-			iommu_dma_free(dev, pages, iosize, handle);
+			iommu_dma_free(dev, pages, iosize, handle, attrs);
 	} else {
 		struct page *page;
 		/*
@@ -642,7 +642,7 @@ static void __iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
 
 		if (WARN_ON(!area || !area->pages))
 			return;
-		iommu_dma_free(dev, area->pages, iosize, &handle);
+		iommu_dma_free(dev, area->pages, iosize, &handle, attrs);
 		dma_common_free_remap(cpu_addr, size, VM_USERMAP);
 	} else {
 		iommu_dma_unmap_page(dev, handle, iosize, 0, 0);
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 2db0d641cf4505b5..b991e8dcbbbb35c5 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -30,6 +30,7 @@
 #include <linux/pci.h>
 #include <linux/scatterlist.h>
 #include <linux/vmalloc.h>
+#include <linux/dma-contiguous.h>
 
 struct iommu_dma_msi_page {
 	struct list_head	list;
@@ -238,15 +239,21 @@ static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t dma_addr)
 	__free_iova(iovad, iova);
 }
 
-static void __iommu_dma_free_pages(struct page **pages, int count)
+static void __iommu_dma_free_pages(struct device *dev, struct page **pages,
+				   int count, unsigned long attrs)
 {
-	while (count--)
-		__free_page(pages[count]);
+	if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
+		dma_release_from_contiguous(dev, pages[0], count);
+	} else {
+		while (count--)
+			__free_page(pages[count]);
+	}
 	kvfree(pages);
 }
 
-static struct page **__iommu_dma_alloc_pages(unsigned int count,
-		unsigned long order_mask, gfp_t gfp)
+static struct page **__iommu_dma_alloc_pages(struct device *dev,
+		unsigned int count, unsigned long order_mask, gfp_t gfp,
+		unsigned long attrs)
 {
 	struct page **pages;
 	unsigned int i = 0, array_size = count * sizeof(*pages);
@@ -265,6 +272,20 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
 	/* IOMMU can map any pages, so himem can also be used here */
 	gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
 
+	if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
+		int order = get_order(count << PAGE_SHIFT);
+		struct page *page;
+
+		page = dma_alloc_from_contiguous(dev, count, order);
+		if (!page)
+			return NULL;
+
+		while (count--)
+			pages[i++] = page++;
+
+		return pages;
+	}
+
 	while (count) {
 		struct page *page = NULL;
 		unsigned int order_size;
@@ -294,7 +315,7 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
 			__free_pages(page, order);
 		}
 		if (!page) {
-			__iommu_dma_free_pages(pages, i);
+			__iommu_dma_free_pages(dev, pages, i, attrs);
 			return NULL;
 		}
 		count -= order_size;
@@ -310,15 +331,17 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
  * @pages: Array of buffer pages as returned by iommu_dma_alloc()
  * @size: Size of buffer in bytes
  * @handle: DMA address of buffer
+ * @attrs: DMA attributes used for allocation of this buffer
  *
  * Frees both the pages associated with the buffer, and the array
  * describing them
  */
 void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
-		dma_addr_t *handle)
+		dma_addr_t *handle, unsigned long attrs)
 {
 	__iommu_dma_unmap(iommu_get_domain_for_dev(dev), *handle);
-	__iommu_dma_free_pages(pages, PAGE_ALIGN(size) >> PAGE_SHIFT);
+	__iommu_dma_free_pages(dev, pages, PAGE_ALIGN(size) >> PAGE_SHIFT,
+			       attrs);
 	*handle = DMA_ERROR_CODE;
 }
 
@@ -365,7 +388,8 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
 		alloc_sizes = min_size;
 
 	count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-	pages = __iommu_dma_alloc_pages(count, alloc_sizes >> PAGE_SHIFT, gfp);
+	pages = __iommu_dma_alloc_pages(dev, count, alloc_sizes >> PAGE_SHIFT,
+					gfp, attrs);
 	if (!pages)
 		return NULL;
 
@@ -403,7 +427,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
 out_free_iova:
 	__free_iova(iovad, iova);
 out_free_pages:
-	__iommu_dma_free_pages(pages, count);
+	__iommu_dma_free_pages(dev, pages, count, attrs);
 	return NULL;
 }
 
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 7f7e9a7e3839966c..35fa6b7f9bac9b35 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -44,7 +44,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
 		unsigned long attrs, int prot, dma_addr_t *handle,
 		void (*flush_page)(struct device *, const void *, phys_addr_t));
 void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
-		dma_addr_t *handle);
+		dma_addr_t *handle, unsigned long attrs);
 
 int iommu_dma_mmap(struct page **pages, size_t size, struct vm_area_struct *vma);
 
-- 
1.9.1

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

* Re: [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
  2017-01-13 11:07 [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS Geert Uytterhoeven
@ 2017-01-13 11:32 ` Robin Murphy
  2017-01-13 11:59   ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Robin Murphy @ 2017-01-13 11:32 UTC (permalink / raw)
  To: Geert Uytterhoeven, Joerg Roedel, Catalin Marinas, Will Deacon
  Cc: Laurent Pinchart, Marek Szyprowski, Magnus Damm, iommu,
	linux-arm-kernel, linux-renesas-soc, linux-kernel

On 13/01/17 11:07, Geert Uytterhoeven wrote:
> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
> This allows to allocate physically contiguous DMA buffers on arm64
> systems with an IOMMU.

Can anyone explain what this attribute is actually used for? I've never
quite figured it out.

> Note that as this uses the CMA allocator, setting this attribute has a
> runtime-dependency on CONFIG_DMA_CMA, just like on arm32.
> 
> For arm64 systems using swiotlb, no changes are needed to support the
> allocation of physically contiguous DMA buffers:
>   - swiotlb always uses physically contiguous buffers (up to
>     IO_TLB_SEGSIZE = 128 pages),
>   - arm64's __dma_alloc_coherent() already calls
>     dma_alloc_from_contiguous() when CMA is available.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/mm/dma-mapping.c |  4 ++--
>  drivers/iommu/dma-iommu.c   | 44 ++++++++++++++++++++++++++++++++++----------
>  include/linux/dma-iommu.h   |  2 +-
>  3 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 1d7d5d2881db7c19..5fba14a21163e41f 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -589,7 +589,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size,
>  		addr = dma_common_pages_remap(pages, size, VM_USERMAP, prot,
>  					      __builtin_return_address(0));
>  		if (!addr)
> -			iommu_dma_free(dev, pages, iosize, handle);
> +			iommu_dma_free(dev, pages, iosize, handle, attrs);
>  	} else {
>  		struct page *page;
>  		/*
> @@ -642,7 +642,7 @@ static void __iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr,
>  
>  		if (WARN_ON(!area || !area->pages))
>  			return;
> -		iommu_dma_free(dev, area->pages, iosize, &handle);
> +		iommu_dma_free(dev, area->pages, iosize, &handle, attrs);
>  		dma_common_free_remap(cpu_addr, size, VM_USERMAP);
>  	} else {
>  		iommu_dma_unmap_page(dev, handle, iosize, 0, 0);
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 2db0d641cf4505b5..b991e8dcbbbb35c5 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -30,6 +30,7 @@
>  #include <linux/pci.h>
>  #include <linux/scatterlist.h>
>  #include <linux/vmalloc.h>
> +#include <linux/dma-contiguous.h>
>  
>  struct iommu_dma_msi_page {
>  	struct list_head	list;
> @@ -238,15 +239,21 @@ static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t dma_addr)
>  	__free_iova(iovad, iova);
>  }
>  
> -static void __iommu_dma_free_pages(struct page **pages, int count)
> +static void __iommu_dma_free_pages(struct device *dev, struct page **pages,
> +				   int count, unsigned long attrs)
>  {
> -	while (count--)
> -		__free_page(pages[count]);
> +	if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
> +		dma_release_from_contiguous(dev, pages[0], count);
> +	} else {
> +		while (count--)
> +			__free_page(pages[count]);
> +	}
>  	kvfree(pages);
>  }
>  
> -static struct page **__iommu_dma_alloc_pages(unsigned int count,
> -		unsigned long order_mask, gfp_t gfp)
> +static struct page **__iommu_dma_alloc_pages(struct device *dev,
> +		unsigned int count, unsigned long order_mask, gfp_t gfp,
> +		unsigned long attrs)
>  {
>  	struct page **pages;
>  	unsigned int i = 0, array_size = count * sizeof(*pages);
> @@ -265,6 +272,20 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
>  	/* IOMMU can map any pages, so himem can also be used here */
>  	gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
>  
> +	if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
> +		int order = get_order(count << PAGE_SHIFT);
> +		struct page *page;
> +
> +		page = dma_alloc_from_contiguous(dev, count, order);
> +		if (!page)
> +			return NULL;
> +
> +		while (count--)
> +			pages[i++] = page++;
> +
> +		return pages;
> +	}
> +

This is really yuck. Plus it's entirely pointless to go through the
whole page array/scatterlist dance when we know the buffer is going to
be physically contiguous - it should just be allocate, map, done. I'd
much rather see standalone iommu_dma_{alloc,free}_contiguous()
functions, and let the arch code handle dispatching appropriately.

Robin.

>  	while (count) {
>  		struct page *page = NULL;
>  		unsigned int order_size;
> @@ -294,7 +315,7 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
>  			__free_pages(page, order);
>  		}
>  		if (!page) {
> -			__iommu_dma_free_pages(pages, i);
> +			__iommu_dma_free_pages(dev, pages, i, attrs);
>  			return NULL;
>  		}
>  		count -= order_size;
> @@ -310,15 +331,17 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
>   * @pages: Array of buffer pages as returned by iommu_dma_alloc()
>   * @size: Size of buffer in bytes
>   * @handle: DMA address of buffer
> + * @attrs: DMA attributes used for allocation of this buffer
>   *
>   * Frees both the pages associated with the buffer, and the array
>   * describing them
>   */
>  void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
> -		dma_addr_t *handle)
> +		dma_addr_t *handle, unsigned long attrs)
>  {
>  	__iommu_dma_unmap(iommu_get_domain_for_dev(dev), *handle);
> -	__iommu_dma_free_pages(pages, PAGE_ALIGN(size) >> PAGE_SHIFT);
> +	__iommu_dma_free_pages(dev, pages, PAGE_ALIGN(size) >> PAGE_SHIFT,
> +			       attrs);
>  	*handle = DMA_ERROR_CODE;
>  }
>  
> @@ -365,7 +388,8 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
>  		alloc_sizes = min_size;
>  
>  	count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> -	pages = __iommu_dma_alloc_pages(count, alloc_sizes >> PAGE_SHIFT, gfp);
> +	pages = __iommu_dma_alloc_pages(dev, count, alloc_sizes >> PAGE_SHIFT,
> +					gfp, attrs);
>  	if (!pages)
>  		return NULL;
>  
> @@ -403,7 +427,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
>  out_free_iova:
>  	__free_iova(iovad, iova);
>  out_free_pages:
> -	__iommu_dma_free_pages(pages, count);
> +	__iommu_dma_free_pages(dev, pages, count, attrs);
>  	return NULL;
>  }
>  
> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> index 7f7e9a7e3839966c..35fa6b7f9bac9b35 100644
> --- a/include/linux/dma-iommu.h
> +++ b/include/linux/dma-iommu.h
> @@ -44,7 +44,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t size, gfp_t gfp,
>  		unsigned long attrs, int prot, dma_addr_t *handle,
>  		void (*flush_page)(struct device *, const void *, phys_addr_t));
>  void iommu_dma_free(struct device *dev, struct page **pages, size_t size,
> -		dma_addr_t *handle);
> +		dma_addr_t *handle, unsigned long attrs);
>  
>  int iommu_dma_mmap(struct page **pages, size_t size, struct vm_area_struct *vma);
>  
> 

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

* Re: [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
  2017-01-13 11:32 ` Robin Murphy
@ 2017-01-13 11:59   ` Geert Uytterhoeven
  2017-01-13 12:17     ` Robin Murphy
  0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-01-13 11:59 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Geert Uytterhoeven, Joerg Roedel, Catalin Marinas, Will Deacon,
	Laurent Pinchart, Marek Szyprowski, Magnus Damm, iommu,
	linux-arm-kernel, Linux-Renesas, linux-kernel

Hi Robin,

On Fri, Jan 13, 2017 at 12:32 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 13/01/17 11:07, Geert Uytterhoeven wrote:
>> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
>> This allows to allocate physically contiguous DMA buffers on arm64
>> systems with an IOMMU.
>
> Can anyone explain what this attribute is actually used for? I've never
> quite figured it out.

My understanding is that DMA_ATTR_FORCE_CONTIGUOUS is needed when using
an IOMMU but wanting the buffers to be both contiguous in IOVA space and
physically contiguous to allow passing to devices without IOMMU.

Main users are graphic and remote processors.

>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c

>> @@ -265,6 +272,20 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
>>       /* IOMMU can map any pages, so himem can also be used here */
>>       gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
>>
>> +     if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
>> +             int order = get_order(count << PAGE_SHIFT);
>> +             struct page *page;
>> +
>> +             page = dma_alloc_from_contiguous(dev, count, order);
>> +             if (!page)
>> +                     return NULL;
>> +
>> +             while (count--)
>> +                     pages[i++] = page++;
>> +
>> +             return pages;
>> +     }
>> +
>
> This is really yuck. Plus it's entirely pointless to go through the
> whole page array/scatterlist dance when we know the buffer is going to
> be physically contiguous - it should just be allocate, map, done. I'd
> much rather see standalone iommu_dma_{alloc,free}_contiguous()
> functions, and let the arch code handle dispatching appropriately.

Fair enough.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

* Re: [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
  2017-01-13 11:59   ` Geert Uytterhoeven
@ 2017-01-13 12:17     ` Robin Murphy
  2017-01-13 12:36       ` Geert Uytterhoeven
  2017-01-15 21:12       ` Laurent Pinchart
  0 siblings, 2 replies; 6+ messages in thread
From: Robin Murphy @ 2017-01-13 12:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Joerg Roedel, Catalin Marinas, Will Deacon,
	Laurent Pinchart, Marek Szyprowski, Magnus Damm, iommu,
	linux-arm-kernel, Linux-Renesas, linux-kernel

On 13/01/17 11:59, Geert Uytterhoeven wrote:
> Hi Robin,
> 
> On Fri, Jan 13, 2017 at 12:32 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 13/01/17 11:07, Geert Uytterhoeven wrote:
>>> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
>>> This allows to allocate physically contiguous DMA buffers on arm64
>>> systems with an IOMMU.
>>
>> Can anyone explain what this attribute is actually used for? I've never
>> quite figured it out.
> 
> My understanding is that DMA_ATTR_FORCE_CONTIGUOUS is needed when using
> an IOMMU but wanting the buffers to be both contiguous in IOVA space and
> physically contiguous to allow passing to devices without IOMMU.
> 
> Main users are graphic and remote processors.

Sure, I assumed it must be to do with buffer sharing, but the systems
I'm aware of which have IOMMUs in their media subsystems tend to have
them in front of every IP block involved, so I was curious as to what
bit of non-IOMMU hardware wanted to play too. The lone in-tree use in
the Exynos DRM driver was never very revealing, and the new one I see in
the Qualcomm PIL driver frankly looks redundant to me.

Robin.

>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
> 
>>> @@ -265,6 +272,20 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count,
>>>       /* IOMMU can map any pages, so himem can also be used here */
>>>       gfp |= __GFP_NOWARN | __GFP_HIGHMEM;
>>>
>>> +     if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
>>> +             int order = get_order(count << PAGE_SHIFT);
>>> +             struct page *page;
>>> +
>>> +             page = dma_alloc_from_contiguous(dev, count, order);
>>> +             if (!page)
>>> +                     return NULL;
>>> +
>>> +             while (count--)
>>> +                     pages[i++] = page++;
>>> +
>>> +             return pages;
>>> +     }
>>> +
>>
>> This is really yuck. Plus it's entirely pointless to go through the
>> whole page array/scatterlist dance when we know the buffer is going to
>> be physically contiguous - it should just be allocate, map, done. I'd
>> much rather see standalone iommu_dma_{alloc,free}_contiguous()
>> functions, and let the arch code handle dispatching appropriately.
> 
> Fair enough.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

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

* Re: [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
  2017-01-13 12:17     ` Robin Murphy
@ 2017-01-13 12:36       ` Geert Uytterhoeven
  2017-01-15 21:12       ` Laurent Pinchart
  1 sibling, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-01-13 12:36 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Geert Uytterhoeven, Joerg Roedel, Catalin Marinas, Will Deacon,
	Laurent Pinchart, Marek Szyprowski, Magnus Damm, iommu,
	linux-arm-kernel, Linux-Renesas, linux-kernel

Hi Robin,

On Fri, Jan 13, 2017 at 1:17 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 13/01/17 11:59, Geert Uytterhoeven wrote:
>> On Fri, Jan 13, 2017 at 12:32 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>>> On 13/01/17 11:07, Geert Uytterhoeven wrote:
>>>> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
>>>> This allows to allocate physically contiguous DMA buffers on arm64
>>>> systems with an IOMMU.
>>>
>>> Can anyone explain what this attribute is actually used for? I've never
>>> quite figured it out.
>>
>> My understanding is that DMA_ATTR_FORCE_CONTIGUOUS is needed when using
>> an IOMMU but wanting the buffers to be both contiguous in IOVA space and
>> physically contiguous to allow passing to devices without IOMMU.
>>
>> Main users are graphic and remote processors.
>
> Sure, I assumed it must be to do with buffer sharing, but the systems
> I'm aware of which have IOMMUs in their media subsystems tend to have
> them in front of every IP block involved, so I was curious as to what
> bit of non-IOMMU hardware wanted to play too. The lone in-tree use in
> the Exynos DRM driver was never very revealing, and the new one I see in
> the Qualcomm PIL driver frankly looks redundant to me.

I'll let the GPU-literate people comment on that...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

* Re: [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS
  2017-01-13 12:17     ` Robin Murphy
  2017-01-13 12:36       ` Geert Uytterhoeven
@ 2017-01-15 21:12       ` Laurent Pinchart
  1 sibling, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2017-01-15 21:12 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Geert Uytterhoeven, Geert Uytterhoeven, Joerg Roedel,
	Catalin Marinas, Will Deacon, Marek Szyprowski, Magnus Damm,
	iommu, linux-arm-kernel, Linux-Renesas, linux-kernel

Hi Robin,

On Friday 13 Jan 2017 12:17:24 Robin Murphy wrote:
> On 13/01/17 11:59, Geert Uytterhoeven wrote:
> > On Fri, Jan 13, 2017 at 12:32 PM, Robin Murphy wrote:
> >> On 13/01/17 11:07, Geert Uytterhoeven wrote:
> >>> Add support for DMA_ATTR_FORCE_CONTIGUOUS to the generic IOMMU DMA code.
> >>> This allows to allocate physically contiguous DMA buffers on arm64
> >>> systems with an IOMMU.
> >> 
> >> Can anyone explain what this attribute is actually used for? I've never
> >> quite figured it out.
> > 
> > My understanding is that DMA_ATTR_FORCE_CONTIGUOUS is needed when using
> > an IOMMU but wanting the buffers to be both contiguous in IOVA space and
> > physically contiguous to allow passing to devices without IOMMU.
> > 
> > Main users are graphic and remote processors.
> 
> Sure, I assumed it must be to do with buffer sharing, but the systems
> I'm aware of which have IOMMUs in their media subsystems tend to have
> them in front of every IP block involved, so I was curious as to what
> bit of non-IOMMU hardware wanted to play too. The lone in-tree use in
> the Exynos DRM driver was never very revealing, and the new one I see in
> the Qualcomm PIL driver frankly looks redundant to me.

If two (or more) devices with different memory requirements are involved in 
buffer sharing, we need to either allocate and export buffers from the device 
with the strictest requirements, or to implement a central buffer allocator. 
In any case, I don't think DMA_ATTR_FORCE_CONTIGUOUS is the right solution to 
that problem.

Forcing contiguous allocation can however help with performance optimization, 
as mapping physically contiguous memory through IOMMUs can make use of larger 
page sizes.

-- 
Regards,

Laurent Pinchart

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

end of thread, other threads:[~2017-01-15 21:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 11:07 [PATCH] iommu/dma: Add support for DMA_ATTR_FORCE_CONTIGUOUS Geert Uytterhoeven
2017-01-13 11:32 ` Robin Murphy
2017-01-13 11:59   ` Geert Uytterhoeven
2017-01-13 12:17     ` Robin Murphy
2017-01-13 12:36       ` Geert Uytterhoeven
2017-01-15 21:12       ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).