Re: [PATCH 05/18] ARM/dma-mapping: Switch to iommu_dma_ops

2020-09-28 Thread Marek Szyprowski
Hi Robin,

On 20.08.2020 17:08, Robin Murphy wrote:
> With the IOMMU ops now looking much the same shape as iommu_dma_ops,
> switch them out in favour of the iommu-dma library, currently enhanced
> with temporary workarounds that allow it to also sit underneath the
> arch-specific API. With that in place, we can now start converting the
> remaining IOMMU drivers and consumers to work with IOMMU API default
> domains instead.
>
> Signed-off-by: Robin Murphy 

I've played a bit longer with this and found that reading the kernel 
virtual address of the buffers allocated via dma_alloc_attrs() from 
dma-iommu ops gives trashes from time to time. It took me a while to 
debug this...

Your conversion misses adding arch_dma_prep_coherent() to arch/arm, so 
the buffers are cleared by the mm allocator, but the caches are NOT 
flushed for the newly allocated buffers.

This fixes the issue:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index fec3e59215b8..8b60bcc5b14f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2,6 +2,7 @@
  config ARM
     bool
     default y
+   select ARCH_HAS_DMA_PREP_COHERENT
     select ARCH_32BIT_OFF_T
     select ARCH_HAS_BINFMT_FLAT
     select ARCH_HAS_DEBUG_VIRTUAL if MMU
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ff6c4962161a..6954681b73da 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -266,6 +266,20 @@ static void __dma_clear_buffer(struct page *page, 
size_t size, int coherent_flag
     }
  }

+void arch_dma_prep_coherent(struct page *page, size_t size)
+{
+
+   if (PageHighMem(page)) {
+   phys_addr_t base = __pfn_to_phys(page_to_pfn(page));
+   phys_addr_t end = base + size;
+   outer_flush_range(base, end);
+   } else {
+   void *ptr = page_address(page);
+   dmac_flush_range(ptr, ptr + size);
+   outer_flush_range(__pa(ptr), __pa(ptr) + size);
+   }
+}
+
  /*
   * Allocate a DMA buffer for 'dev' of size 'size' using the
   * specified gfp mask.  Note that 'size' must be page aligned.

I also wonder if it would be better to use per-arch __dma_clear_buffer() 
instead of setting __GFP_ZERO unconditionally in dma-iommu.c. This 
should be faster on ARM with highmem...

 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R Institute Poland

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 05/18] ARM/dma-mapping: Switch to iommu_dma_ops

2020-08-20 Thread Robin Murphy
With the IOMMU ops now looking much the same shape as iommu_dma_ops,
switch them out in favour of the iommu-dma library, currently enhanced
with temporary workarounds that allow it to also sit underneath the
arch-specific API. With that in place, we can now start converting the
remaining IOMMU drivers and consumers to work with IOMMU API default
domains instead.

Signed-off-by: Robin Murphy 
---
 arch/arm/Kconfig |  24 +-
 arch/arm/include/asm/dma-iommu.h |   8 -
 arch/arm/mm/dma-mapping.c| 887 +--
 drivers/iommu/Kconfig|   8 -
 drivers/media/platform/Kconfig   |   1 -
 5 files changed, 22 insertions(+), 906 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b91273f9fd43..79406fe5cd6b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -133,31 +133,11 @@ config ARM_HAS_SG_CHAIN
bool
 
 config ARM_DMA_USE_IOMMU
-   bool
+   def_bool IOMMU_SUPPORT
select ARM_HAS_SG_CHAIN
+   select IOMMU_DMA
select NEED_SG_DMA_LENGTH
 
-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 SYS_SUPPORTS_APM_EMULATION
bool
 
diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h
index 86405cc81385..f39cfa509fe4 100644
--- a/arch/arm/include/asm/dma-iommu.h
+++ b/arch/arm/include/asm/dma-iommu.h
@@ -13,14 +13,6 @@ struct dma_iommu_mapping {
/* iommu specific data */
struct iommu_domain *domain;
 
-   unsigned long   **bitmaps;  /* array of bitmaps */
-   unsigned intnr_bitmaps; /* nr of elements in array */
-   unsigned intextensions;
-   size_t  bitmap_size;/* size of a single bitmap */
-   size_t  bits;   /* per bitmap */
-   dma_addr_t  base;
-
-   spinlock_t  lock;
struct kref kref;
 };
 
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 0537c97cebe1..0f69ede44cd7 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1074,812 +1075,9 @@ static const struct dma_map_ops 
*arm_get_dma_map_ops(bool coherent)
 
 #ifdef CONFIG_ARM_DMA_USE_IOMMU
 
-static int __dma_info_to_prot(enum dma_data_direction dir, unsigned long attrs)
-{
-   int prot = 0;
-
-   if (attrs & DMA_ATTR_PRIVILEGED)
-   prot |= IOMMU_PRIV;
-
-   switch (dir) {
-   case DMA_BIDIRECTIONAL:
-   return prot | IOMMU_READ | IOMMU_WRITE;
-   case DMA_TO_DEVICE:
-   return prot | IOMMU_READ;
-   case DMA_FROM_DEVICE:
-   return prot | IOMMU_WRITE;
-   default:
-   return prot;
-   }
-}
-
-/* IOMMU */
-
-static int extend_iommu_mapping(struct dma_iommu_mapping *mapping);
-
-static inline dma_addr_t __alloc_iova(struct dma_iommu_mapping *mapping,
- size_t size)
-{
-   unsigned int order = get_order(size);
-   unsigned int align = 0;
-   unsigned int count, start;
-   size_t mapping_size = mapping->bits << PAGE_SHIFT;
-   unsigned long flags;
-   dma_addr_t iova;
-   int i;
-
-   if (order > CONFIG_ARM_DMA_IOMMU_ALIGNMENT)
-   order = CONFIG_ARM_DMA_IOMMU_ALIGNMENT;
-
-   count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-   align = (1 << order) - 1;
-
-   spin_lock_irqsave(>lock, flags);
-   for (i = 0; i < mapping->nr_bitmaps; i++) {
-   start = bitmap_find_next_zero_area(mapping->bitmaps[i],
-   mapping->bits, 0, count, align);
-
-   if (start > mapping->bits)
-   continue;
-
-   bitmap_set(mapping->bitmaps[i], start, count);
-   break;
-   }
-
-   /*
-* No unused range found. Try to extend the existing mapping
-* and perform a second attempt to reserve an IO virtual
-* address range of size bytes.
-*/
-   if (i == mapping->nr_bitmaps) {
-   if