Re: [PATCH v5] arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU

2017-04-20 Thread Catalin Marinas
Catching up with these threads, so replying to a patch I already
applied.

On Tue, Mar 07, 2017 at 06:43:32PM +0100, Geert Uytterhoeven wrote:
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -584,20 +584,7 @@ static void *__iommu_alloc_attrs(struct device *dev, 
> size_t size,
>*/
>   gfp |= __GFP_ZERO;
>  
> - if (gfpflags_allow_blocking(gfp)) {
> - struct page **pages;
> - pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> -
> - pages = iommu_dma_alloc(dev, iosize, gfp, attrs, ioprot,
> - handle, flush_page);
> - if (!pages)
> - return NULL;
> -
> - addr = dma_common_pages_remap(pages, size, VM_USERMAP, prot,
> -   __builtin_return_address(0));
> - if (!addr)
> - iommu_dma_free(dev, pages, iosize, handle);
> - } else {
> + if (!gfpflags_allow_blocking(gfp)) {
>   struct page *page;
>   /*
>* In atomic context we can't remap anything, so we'll only
> @@ -621,6 +608,45 @@ static void *__iommu_alloc_attrs(struct device *dev, 
> size_t size,
>   __free_from_pool(addr, size);
>   addr = NULL;
>   }
> + } else if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
> + pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
> + struct page *page;
> +
> + page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT,
> +  get_order(size), gfp);
> + if (!page)
> + return NULL;
> +
> + *handle = iommu_dma_map_page(dev, page, 0, iosize, ioprot);
> + if (iommu_dma_mapping_error(dev, *handle)) {
> + dma_release_from_contiguous(dev, page,
> + size >> PAGE_SHIFT);
> + return NULL;
> + }
> + if (!coherent)
> + __dma_flush_area(page_to_virt(page), iosize);
> +
> + addr = dma_common_contiguous_remap(page, size, VM_USERMAP,
> +prot,
> +__builtin_return_address(0));

Do we need to call dma_common_pages_remap() if the allocation is
coherent? In the __dma_alloc() case we don't do it but simply use
page_address(page) as returned by __dma_alloc_coherent().

(note that my comment is not meant to fix the issue reported by Andrzej
Hajda but I just spotted it)

-- 
Catalin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5] arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU

2017-03-21 Thread Catalin Marinas
On Tue, Mar 07, 2017 at 06:43:32PM +0100, Geert Uytterhoeven wrote:
> Add support for allocating physically contiguous DMA buffers on arm64
> systems with an IOMMU.  This can be useful when two or more devices
> with different memory requirements are involved in buffer sharing.
> 
> Note that as this uses the CMA allocator, setting the
> DMA_ATTR_FORCE_CONTIGUOUS 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 
> Acked-by: Laurent Pinchart 
> Reviewed-by: Robin Murphy 

Queued for 4.12. Thanks.

-- 
Catalin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v5] arm64: Add support for DMA_ATTR_FORCE_CONTIGUOUS to IOMMU

2017-03-07 Thread Geert Uytterhoeven
Add support for allocating physically contiguous DMA buffers on arm64
systems with an IOMMU.  This can be useful when two or more devices
with different memory requirements are involved in buffer sharing.

Note that as this uses the CMA allocator, setting the
DMA_ATTR_FORCE_CONTIGUOUS 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 
Acked-by: Laurent Pinchart 
Reviewed-by: Robin Murphy 
---
v5:
  - Add Reviewed-by,
  - Pass GFP flags to dma_alloc_from_contiguous(), cfr. commit
712c604dcdf818629 ("mm: wire up GFP flag passing in
dma_alloc_from_contiguous"),

v4:
  - Replace dma_to_phys()/phys_to_page() by vmalloc_to_page(), to pass
the correct page pointer to dma_release_from_contiguous().
Note that the latter doesn't scream when passed a wrong pointer, but
just returns false.  While this makes life easier for callers who
may want to call another deallocator, it makes it harder catching
bugs.

v3:
  - Add Acked-by,
  - Update comment to "one of _4_ things",
  - Call dma_alloc_from_contiguous() and iommu_dma_map_page() directly,
as suggested by Robin Murphy,

v2:
  - New, handle dispatching in the arch (arm64) code, as requested by
Robin Murphy.
---
 arch/arm64/mm/dma-mapping.c | 63 ++---
 1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 81cdb2e844ed9fe8..f7b54019ef55378d 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -584,20 +584,7 @@ static void *__iommu_alloc_attrs(struct device *dev, 
size_t size,
 */
gfp |= __GFP_ZERO;
 
-   if (gfpflags_allow_blocking(gfp)) {
-   struct page **pages;
-   pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
-
-   pages = iommu_dma_alloc(dev, iosize, gfp, attrs, ioprot,
-   handle, flush_page);
-   if (!pages)
-   return NULL;
-
-   addr = dma_common_pages_remap(pages, size, VM_USERMAP, prot,
- __builtin_return_address(0));
-   if (!addr)
-   iommu_dma_free(dev, pages, iosize, handle);
-   } else {
+   if (!gfpflags_allow_blocking(gfp)) {
struct page *page;
/*
 * In atomic context we can't remap anything, so we'll only
@@ -621,6 +608,45 @@ static void *__iommu_alloc_attrs(struct device *dev, 
size_t size,
__free_from_pool(addr, size);
addr = NULL;
}
+   } else if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) {
+   pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
+   struct page *page;
+
+   page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT,
+get_order(size), gfp);
+   if (!page)
+   return NULL;
+
+   *handle = iommu_dma_map_page(dev, page, 0, iosize, ioprot);
+   if (iommu_dma_mapping_error(dev, *handle)) {
+   dma_release_from_contiguous(dev, page,
+   size >> PAGE_SHIFT);
+   return NULL;
+   }
+   if (!coherent)
+   __dma_flush_area(page_to_virt(page), iosize);
+
+   addr = dma_common_contiguous_remap(page, size, VM_USERMAP,
+  prot,
+  __builtin_return_address(0));
+   if (!addr) {
+   iommu_dma_unmap_page(dev, *handle, iosize, 0, attrs);
+   dma_release_from_contiguous(dev, page,
+   size >> PAGE_SHIFT);
+   }
+   } else {
+   pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent);
+   struct page **pages;
+
+   pages = iommu_dma_alloc(dev, iosize, gfp, attrs, ioprot,
+   handle, flush_page);
+   if (!pages)
+   return NULL;
+
+   addr = dma_common_pages_remap(pages, size, VM_USERMAP, prot,
+ __builtin_return_address(0));
+   if (!addr)
+   iommu_dma_free(dev, pages, iosize, handle);
}
return addr;
 }
@@ -632,7 +658,8 @@ static void __iommu_free_attrs(s