[PATCH 00/12] ARM/ARM64 MMU code consolidation, zeroing of DMA coherent memory

[PATCH 00/12] ARM/ARM64 MMU code consolidation, zeroing of DMA coherent memory

[Andrey Smirnov]

Everyone:

This series is a result of my attempt at changing the behaviour of
dma_alloc_coherent() to guarantee that memory it returns is zeroed
out. Mostly to avoid having to do that explicitly in driver code, but
also to match behaviour that that function has in Linux. While working
on that I noticed that there was a fair bit of MMU/DMA related code
between ARM/ARM64 that can be shared, so I created a number of patches
to do just that.

Feedback is welcome!

Thanks,
Andrey Smirnov

Andrey Smirnov (12):
  ARM: mmu: Drop custom virt_to_phys/phys_to_virt
  ARM: mmu: Simplify the use of dma_inv_range()
  ARM: mmu: Share code for dma_(un)map_single()
  ARM64: mmu: Use arch_remap_range() internaly
  ARM64: mmu: Merge create_sections() and map_region() together
  ARM: mmu: Share code for dma_free_coherent()
  ARM64: mmu: Invalidate memory before remapping as DMA coherent
  ARM: mmu: Share code for dma_alloc_coherent()
  ARM: mmu: Share code for dma_sync_single_for_cpu()
  ARM: mmu: Share sanity checking code in mmu_init()
  ARM: mmu: Share code for arm_mmu_not_initialized_error()
  ARM: mmu: Make sure DMA coherent memory is zeroed out

 arch/arm/cpu/Makefile     |  2 +-
 arch/arm/cpu/mmu-common.c | 82 +++++++++++++++++++++++++++++++++
 arch/arm/cpu/mmu-common.h | 20 ++++++++
 arch/arm/cpu/mmu.c        | 96 +++------------------------------------
 arch/arm/cpu/mmu.h        |  2 +
 arch/arm/cpu/mmu_64.c     | 96 ++++++---------------------------------
 arch/arm/cpu/mmu_64.h     |  2 +
 arch/arm/include/asm/io.h |  5 --
 8 files changed, 128 insertions(+), 177 deletions(-)
 create mode 100644 arch/arm/cpu/mmu-common.c
 create mode 100644 arch/arm/cpu/mmu-common.h

-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 01/12] ARM: mmu: Drop custom virt_to_phys/phys_to_virt

[Andrey Smirnov]

Neither ARM nor ARM64 define any address mapping functions that differ
from default provided for no-MMU configuration. Drop all the extra
code and just rely on functions provided in asm/io.h

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu.c        | 10 ----------
 arch/arm/cpu/mmu_64.c     | 10 ----------
 arch/arm/include/asm/io.h |  5 -----
 3 files changed, 25 deletions(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index fc48376f6..2b7b9e30a 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -524,16 +524,6 @@ void *dma_alloc_writecombine(size_t size, dma_addr_t *dma_handle)
 	return dma_alloc_map(size, dma_handle, ARCH_MAP_WRITECOMBINE);
 }
 
-unsigned long virt_to_phys(volatile void *virt)
-{
-	return (unsigned long)virt;
-}
-
-void *phys_to_virt(unsigned long phys)
-{
-	return (void *)phys;
-}
-
 void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
 {
 	size = PAGE_ALIGN(size);
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index 99ddd5a44..b6f30d1b3 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -252,16 +252,6 @@ void mmu_disable(void)
 	isb();
 }
 
-unsigned long virt_to_phys(volatile void *virt)
-{
-	return (unsigned long)virt;
-}
-
-void *phys_to_virt(unsigned long phys)
-{
-	return (void *)phys;
-}
-
 void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 {
 	void *ret;
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index d06ff8323..56db54634 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -69,10 +69,6 @@ extern void memset_io(volatile void __iomem *, int, size_t);
 #define setbits_8(addr, set) setbits(8, addr, set)
 #define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)
 
-#ifdef CONFIG_MMU
-void *phys_to_virt(unsigned long phys);
-unsigned long virt_to_phys(volatile void *virt);
-#else
 static inline void *phys_to_virt(unsigned long phys)
 {
 	return (void *)phys;
@@ -82,6 +78,5 @@ static inline unsigned long virt_to_phys(volatile void *mem)
 {
 	return (unsigned long)mem;
 }
-#endif
 
 #endif	/* __ASM_ARM_IO_H */
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 02/12] ARM: mmu: Simplify the use of dma_inv_range()

[Andrey Smirnov]

Simplify the use of dma_inv_range() by changing its signature to
accept pointer to start of the data and data size. This change allows
us to avoid a whole bunch of repetitive arithmetic currently done by
all of the callers.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 2b7b9e30a..390ccc581 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -118,8 +118,11 @@ static void dma_flush_range(void *ptr, size_t size)
 		outer_cache.flush_range(start, end);
 }
 
-static void dma_inv_range(unsigned long start, unsigned long end)
+static void dma_inv_range(void *ptr, size_t size)
 {
+	unsigned long start = (unsigned long)ptr;
+	unsigned long end = start + size;
+
 	if (outer_cache.inv_range)
 		outer_cache.inv_range(start, end);
 	__dma_inv_range(start, end);
@@ -507,7 +510,7 @@ static void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags)
 	if (dma_handle)
 		*dma_handle = (dma_addr_t)ret;
 
-	dma_inv_range((unsigned long)ret, (unsigned long)ret + size);
+	dma_inv_range(ret, size);
 
 	arch_remap_range(ret, size, flags);
 
@@ -536,7 +539,7 @@ void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
 			     enum dma_data_direction dir)
 {
 	if (dir != DMA_TO_DEVICE)
-		dma_inv_range(address, address + size);
+		dma_inv_range((void *)address, size);
 }
 
 void dma_sync_single_for_device(dma_addr_t address, size_t size,
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 03/12] ARM: mmu: Share code for dma_(un)map_single()

[Andrey Smirnov]

Both ARM and ARM64 define DMA mapping/unmapping functions that are
exactly the same. Introduce mmu-common.c and move the code there so it
can be shared.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/Makefile     |  2 +-
 arch/arm/cpu/mmu-common.c | 25 +++++++++++++++++++++++++
 arch/arm/cpu/mmu.c        | 16 ----------------
 arch/arm/cpu/mmu_64.c     | 16 ----------------
 4 files changed, 26 insertions(+), 33 deletions(-)
 create mode 100644 arch/arm/cpu/mmu-common.c

diff --git a/arch/arm/cpu/Makefile b/arch/arm/cpu/Makefile
index a35db435c..8e1af8bf8 100644
--- a/arch/arm/cpu/Makefile
+++ b/arch/arm/cpu/Makefile
@@ -1,7 +1,7 @@
 obj-y += cpu.o
 
 obj-$(CONFIG_ARM_EXCEPTIONS) += exceptions$(S64).o interrupts$(S64).o
-obj-$(CONFIG_MMU) += mmu$(S64).o
+obj-$(CONFIG_MMU) += mmu$(S64).o mmu-common.o
 lwl-y += lowlevel$(S64).o
 obj-pbl-$(CONFIG_MMU) += mmu-early$(S64).o
 obj-pbl-$(CONFIG_CPU_32v7) += hyp.o
diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
new file mode 100644
index 000000000..5d5812974
--- /dev/null
+++ b/arch/arm/cpu/mmu-common.c
@@ -0,0 +1,25 @@
+
+#define pr_fmt(fmt)	"mmu: " fmt
+
+#include <common.h>
+#include <dma-dir.h>
+#include <dma.h>
+#include <mmu.h>
+
+#include "mmu.h"
+
+dma_addr_t dma_map_single(struct device_d *dev, void *ptr, size_t size,
+			  enum dma_data_direction dir)
+{
+	unsigned long addr = (unsigned long)ptr;
+
+	dma_sync_single_for_device(addr, size, dir);
+
+	return addr;
+}
+
+void dma_unmap_single(struct device_d *dev, dma_addr_t addr, size_t size,
+		      enum dma_data_direction dir)
+{
+	dma_sync_single_for_cpu(addr, size, dir);
+}
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 390ccc581..f134e8bb3 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -555,19 +555,3 @@ void dma_sync_single_for_device(dma_addr_t address, size_t size,
 			outer_cache.clean_range(address, address + size);
 	}
 }
-
-dma_addr_t dma_map_single(struct device_d *dev, void *ptr, size_t size,
-			  enum dma_data_direction dir)
-{
-	unsigned long addr = (unsigned long)ptr;
-
-	dma_sync_single_for_device(addr, size, dir);
-
-	return addr;
-}
-
-void dma_unmap_single(struct device_d *dev, dma_addr_t addr, size_t size,
-		      enum dma_data_direction dir)
-{
-	dma_sync_single_for_cpu(addr, size, dir);
-}
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index b6f30d1b3..e2dd5b4d8 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -291,19 +291,3 @@ void dma_sync_single_for_device(dma_addr_t address, size_t size,
 	else
 		v8_flush_dcache_range(address, address + size - 1);
 }
-
-dma_addr_t dma_map_single(struct device_d *dev, void *ptr, size_t size,
-			  enum dma_data_direction dir)
-{
-	unsigned long addr = (unsigned long)ptr;
-
-	dma_sync_single_for_device(addr, size, dir);
-
-	return addr;
-}
-
-void dma_unmap_single(struct device_d *dev, dma_addr_t addr, size_t size,
-		      enum dma_data_direction dir)
-{
-	dma_sync_single_for_cpu(addr, size, dir);
-}
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 04/12] ARM64: mmu: Use arch_remap_range() internaly

[Andrey Smirnov]

Instead of calling map_region() explicitly, call arch_regmap_range()
instead to simplify the code. This also ensures that tlb_invalidate()
gets called when dma_free_coherent() is invoked.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu_64.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index e2dd5b4d8..d7c2542d1 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -261,8 +261,7 @@ void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 	if (dma_handle)
 		*dma_handle = (dma_addr_t)ret;
 
-	map_region((unsigned long)ret, (unsigned long)ret, size, UNCACHED_MEM);
-	tlb_invalidate();
+	arch_remap_range(ret, size, MAP_UNCACHED);
 
 	return ret;
 }
@@ -270,8 +269,7 @@ void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
 {
 	size = PAGE_ALIGN(size);
-
-	map_region((unsigned long)mem, (unsigned long)mem, size, CACHED_MEM);
+	arch_remap_range(ret, size, MAP_CACHED);
 
 	free(mem);
 }
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 05/12] ARM64: mmu: Merge create_sections() and map_region() together

[Andrey Smirnov]

Since map_region() is never called without being followed by
tlb_invalidate(), merge it with create_sections() to simplify the
code.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu_64.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index d7c2542d1..ad94ff14d 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -119,7 +119,8 @@ static void split_block(uint64_t *pte, int level)
 	set_table(pte, new_table);
 }
 
-static void map_region(uint64_t virt, uint64_t phys, uint64_t size, uint64_t attr)
+static void create_sections(uint64_t virt, uint64_t phys, uint64_t size,
+			    uint64_t attr)
 {
 	uint64_t block_size;
 	uint64_t block_shift;
@@ -162,11 +163,7 @@ static void map_region(uint64_t virt, uint64_t phys, uint64_t size, uint64_t att
 		}
 
 	}
-}
 
-static void create_sections(uint64_t virt, uint64_t phys, uint64_t size, uint64_t flags)
-{
-	map_region(virt, phys, size, flags);
 	tlb_invalidate();
 }
 
@@ -183,9 +180,8 @@ int arch_remap_range(void *_start, size_t size, unsigned flags)
 		return -EINVAL;
 	}
 
-	map_region((uint64_t)_start, (uint64_t)_start, (uint64_t)size, flags);
-	tlb_invalidate();
-
+	create_sections((uint64_t)_start, (uint64_t)_start, (uint64_t)size,
+			flags);
 	return 0;
 }
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 06/12] ARM: mmu: Share code for dma_free_coherent()

[Andrey Smirnov]

Now that AArch64 version is calling arch_remap_range() it is identical
to ARM version in mmu.c. Move the definition to mmu-common.c to avoid
duplication.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu-common.c | 8 ++++++++
 arch/arm/cpu/mmu.c        | 8 --------
 arch/arm/cpu/mmu_64.c     | 8 --------
 3 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
index 5d5812974..65cc786e1 100644
--- a/arch/arm/cpu/mmu-common.c
+++ b/arch/arm/cpu/mmu-common.c
@@ -23,3 +23,11 @@ void dma_unmap_single(struct device_d *dev, dma_addr_t addr, size_t size,
 {
 	dma_sync_single_for_cpu(addr, size, dir);
 }
+
+void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
+{
+	size = PAGE_ALIGN(size);
+	arch_remap_range(mem, size, MAP_CACHED);
+
+	free(mem);
+}
\ No newline at end of file
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index f134e8bb3..70ab5ebb5 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -527,14 +527,6 @@ void *dma_alloc_writecombine(size_t size, dma_addr_t *dma_handle)
 	return dma_alloc_map(size, dma_handle, ARCH_MAP_WRITECOMBINE);
 }
 
-void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
-{
-	size = PAGE_ALIGN(size);
-	arch_remap_range(mem, size, MAP_CACHED);
-
-	free(mem);
-}
-
 void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
 			     enum dma_data_direction dir)
 {
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index ad94ff14d..a7186eda4 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -262,14 +262,6 @@ void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 	return ret;
 }
 
-void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
-{
-	size = PAGE_ALIGN(size);
-	arch_remap_range(ret, size, MAP_CACHED);
-
-	free(mem);
-}
-
 void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
                              enum dma_data_direction dir)
 {
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 07/12] ARM64: mmu: Invalidate memory before remapping as DMA coherent

[Andrey Smirnov]

Although there are known problems caused by this, it seems prudent to
invalidate the region of memory we are about remap as
uncached. Additionaliy this matches how dma_alloc_coherent() is
implemented on ARM.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu_64.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index a7186eda4..1ee6a3b8c 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -248,6 +248,14 @@ void mmu_disable(void)
 	isb();
 }
 
+static void dma_inv_range(void *ptr, size_t size)
+{
+	unsigned long start = (unsigned long)ptr;
+	unsigned long end = start + size - 1;
+
+	v8_inv_dcache_range(start, end);
+}
+
 void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 {
 	void *ret;
@@ -257,6 +265,8 @@ void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 	if (dma_handle)
 		*dma_handle = (dma_addr_t)ret;
 
+	dma_inv_range(ret, size);
+
 	arch_remap_range(ret, size, MAP_UNCACHED);
 
 	return ret;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 08/12] ARM: mmu: Share code for dma_alloc_coherent()

[Andrey Smirnov]

Both ARM and ARM64 implement almost identical algorithms in
dma_alloc_coherent(). Move the code to mmu-common.c, so it can be
shared.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu-common.c | 21 +++++++++++++++++++++
 arch/arm/cpu/mmu-common.h |  7 +++++++
 arch/arm/cpu/mmu.c        | 23 +----------------------
 arch/arm/cpu/mmu.h        |  2 ++
 arch/arm/cpu/mmu_64.c     | 18 +-----------------
 arch/arm/cpu/mmu_64.h     |  2 ++
 6 files changed, 34 insertions(+), 39 deletions(-)
 create mode 100644 arch/arm/cpu/mmu-common.h

diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
index 65cc786e1..8c7d61447 100644
--- a/arch/arm/cpu/mmu-common.c
+++ b/arch/arm/cpu/mmu-common.c
@@ -24,6 +24,27 @@ void dma_unmap_single(struct device_d *dev, dma_addr_t addr, size_t size,
 	dma_sync_single_for_cpu(addr, size, dir);
 }
 
+void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags)
+{
+	void *ret;
+
+	size = PAGE_ALIGN(size);
+	ret = xmemalign(PAGE_SIZE, size);
+	if (dma_handle)
+		*dma_handle = (dma_addr_t)ret;
+
+	dma_inv_range(ret, size);
+
+	arch_remap_range(ret, size, flags);
+
+	return ret;
+}
+
+void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
+{
+	return dma_alloc_map(size, dma_handle, MAP_UNCACHED);
+}
+
 void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
 {
 	size = PAGE_ALIGN(size);
diff --git a/arch/arm/cpu/mmu-common.h b/arch/arm/cpu/mmu-common.h
new file mode 100644
index 000000000..e8689ac31
--- /dev/null
+++ b/arch/arm/cpu/mmu-common.h
@@ -0,0 +1,7 @@
+#ifndef __ARM_MMU_COMMON_H
+#define __ARM_MMU_COMMON_H
+
+void dma_inv_range(void *ptr, size_t size);
+void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags);
+
+#endif
\ No newline at end of file
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 70ab5ebb5..35ac00a8f 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -118,7 +118,7 @@ static void dma_flush_range(void *ptr, size_t size)
 		outer_cache.flush_range(start, end);
 }
 
-static void dma_inv_range(void *ptr, size_t size)
+void dma_inv_range(void *ptr, size_t size)
 {
 	unsigned long start = (unsigned long)ptr;
 	unsigned long end = start + size;
@@ -501,27 +501,6 @@ void mmu_disable(void)
 	__mmu_cache_off();
 }
 
-static void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags)
-{
-	void *ret;
-
-	size = PAGE_ALIGN(size);
-	ret = xmemalign(PAGE_SIZE, size);
-	if (dma_handle)
-		*dma_handle = (dma_addr_t)ret;
-
-	dma_inv_range(ret, size);
-
-	arch_remap_range(ret, size, flags);
-
-	return ret;
-}
-
-void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
-{
-	return dma_alloc_map(size, dma_handle, MAP_UNCACHED);
-}
-
 void *dma_alloc_writecombine(size_t size, dma_addr_t *dma_handle)
 {
 	return dma_alloc_map(size, dma_handle, ARCH_MAP_WRITECOMBINE);
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 2e425e092..338728aac 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -4,6 +4,8 @@
 #include <asm/pgtable.h>
 #include <linux/sizes.h>
 
+#include "mmu-common.h"
+
 #define PGDIR_SHIFT	20
 #define PGDIR_SIZE	(1UL << PGDIR_SHIFT)
 
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index 1ee6a3b8c..ed4aa00a8 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -248,7 +248,7 @@ void mmu_disable(void)
 	isb();
 }
 
-static void dma_inv_range(void *ptr, size_t size)
+void dma_inv_range(void *ptr, size_t size)
 {
 	unsigned long start = (unsigned long)ptr;
 	unsigned long end = start + size - 1;
@@ -256,22 +256,6 @@ static void dma_inv_range(void *ptr, size_t size)
 	v8_inv_dcache_range(start, end);
 }
 
-void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
-{
-	void *ret;
-
-	size = PAGE_ALIGN(size);
-	ret = xmemalign(PAGE_SIZE, size);
-	if (dma_handle)
-		*dma_handle = (dma_addr_t)ret;
-
-	dma_inv_range(ret, size);
-
-	arch_remap_range(ret, size, MAP_UNCACHED);
-
-	return ret;
-}
-
 void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
                              enum dma_data_direction dir)
 {
diff --git a/arch/arm/cpu/mmu_64.h b/arch/arm/cpu/mmu_64.h
index 2cbe72062..e2e125686 100644
--- a/arch/arm/cpu/mmu_64.h
+++ b/arch/arm/cpu/mmu_64.h
@@ -1,4 +1,6 @@
 
+#include "mmu-common.h"
+
 #define CACHED_MEM      (PTE_BLOCK_MEMTYPE(MT_NORMAL) | \
 			 PTE_BLOCK_OUTER_SHARE | \
 			 PTE_BLOCK_AF)
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 09/12] ARM: mmu: Share code for dma_sync_single_for_cpu()

[Andrey Smirnov]

Both ARM and ARM64 have identical code for
dma_sync_single_for_cpu(). Move it to mmu-common.c so it can be shared.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu-common.c | 8 ++++++++
 arch/arm/cpu/mmu.c        | 7 -------
 arch/arm/cpu/mmu_64.c     | 7 -------
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
index 8c7d61447..a7d3b5b11 100644
--- a/arch/arm/cpu/mmu-common.c
+++ b/arch/arm/cpu/mmu-common.c
@@ -8,6 +8,14 @@
 
 #include "mmu.h"
 
+
+void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
+			     enum dma_data_direction dir)
+{
+	if (dir != DMA_TO_DEVICE)
+		dma_inv_range((void *)address, size);
+}
+
 dma_addr_t dma_map_single(struct device_d *dev, void *ptr, size_t size,
 			  enum dma_data_direction dir)
 {
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 35ac00a8f..97c459ff0 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -506,13 +506,6 @@ void *dma_alloc_writecombine(size_t size, dma_addr_t *dma_handle)
 	return dma_alloc_map(size, dma_handle, ARCH_MAP_WRITECOMBINE);
 }
 
-void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
-			     enum dma_data_direction dir)
-{
-	if (dir != DMA_TO_DEVICE)
-		dma_inv_range((void *)address, size);
-}
-
 void dma_sync_single_for_device(dma_addr_t address, size_t size,
 				enum dma_data_direction dir)
 {
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index ed4aa00a8..2cb62370e 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -256,13 +256,6 @@ void dma_inv_range(void *ptr, size_t size)
 	v8_inv_dcache_range(start, end);
 }
 
-void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
-                             enum dma_data_direction dir)
-{
-	if (dir != DMA_TO_DEVICE)
-		v8_inv_dcache_range(address, address + size - 1);
-}
-
 void dma_sync_single_for_device(dma_addr_t address, size_t size,
                                 enum dma_data_direction dir)
 {
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 10/12] ARM: mmu: Share sanity checking code in mmu_init()

[Andrey Smirnov]

Share sanity checking code in mmu_init() as well as code to detect if
MMU is on or not on both ARM and ARM64.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu-common.c | 23 +++++++++++++++++++++--
 arch/arm/cpu/mmu-common.h |  1 +
 arch/arm/cpu/mmu.c        | 16 ++--------------
 arch/arm/cpu/mmu_64.c     | 16 ++--------------
 4 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
index a7d3b5b11..c5b24bff8 100644
--- a/arch/arm/cpu/mmu-common.c
+++ b/arch/arm/cpu/mmu-common.c
@@ -2,10 +2,12 @@
 #define pr_fmt(fmt)	"mmu: " fmt
 
 #include <common.h>
+#include <init.h>
 #include <dma-dir.h>
 #include <dma.h>
 #include <mmu.h>
-
+#include <asm/system.h>
+#include <memory.h>
 #include "mmu.h"
 
 
@@ -59,4 +61,21 @@ void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
 	arch_remap_range(mem, size, MAP_CACHED);
 
 	free(mem);
-}
\ No newline at end of file
+}
+
+static int mmu_init(void)
+{
+	if (list_empty(&memory_banks))
+		/*
+		 * If you see this it means you have no memory registered.
+		 * This can be done either with arm_add_mem_device() in an
+		 * initcall prior to mmu_initcall or via devicetree in the
+		 * memory node.
+		 */
+		panic("MMU: No memory bank found! Cannot continue\n");
+
+	__mmu_init(get_cr() & CR_M);
+
+	return 0;
+}
+mmu_initcall(mmu_init);
\ No newline at end of file
diff --git a/arch/arm/cpu/mmu-common.h b/arch/arm/cpu/mmu-common.h
index e8689ac31..37eef3ef2 100644
--- a/arch/arm/cpu/mmu-common.h
+++ b/arch/arm/cpu/mmu-common.h
@@ -3,5 +3,6 @@
 
 void dma_inv_range(void *ptr, size_t size);
 void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags);
+void __mmu_init(bool mmu_on);
 
 #endif
\ No newline at end of file
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 97c459ff0..ba1c3e007 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -412,19 +412,10 @@ static void vectors_init(void)
 /*
  * Prepare MMU for usage enable it.
  */
-static int mmu_init(void)
+void __mmu_init(bool mmu_on)
 {
 	struct memory_bank *bank;
 
-	if (list_empty(&memory_banks))
-		/*
-		 * If you see this it means you have no memory registered.
-		 * This can be done either with arm_add_mem_device() in an
-		 * initcall prior to mmu_initcall or via devicetree in the
-		 * memory node.
-		 */
-		panic("MMU: No memory bank found! Cannot continue\n");
-
 	arm_set_cache_functions();
 
 	if (cpu_architecture() >= CPU_ARCH_ARMv7) {
@@ -439,7 +430,7 @@ static int mmu_init(void)
 		pte_flags_uncached = PTE_FLAGS_UNCACHED_V4;
 	}
 
-	if (get_cr() & CR_M) {
+	if (mmu_on) {
 		/*
 		 * Early MMU code has already enabled the MMU. We assume a
 		 * flat 1:1 section mapping in this case.
@@ -483,10 +474,7 @@ static int mmu_init(void)
 	}
 
 	__mmu_cache_on();
-
-	return 0;
 }
-mmu_initcall(mmu_init);
 
 /*
  * Clean and invalide caches, disable MMU
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index 2cb62370e..7f8a8f249 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -194,21 +194,12 @@ static void mmu_enable(void)
 /*
  * Prepare MMU for usage enable it.
  */
-static int mmu_init(void)
+void __mmu_init(bool mmu_on)
 {
 	struct memory_bank *bank;
 	unsigned int el;
 
-	if (list_empty(&memory_banks))
-		/*
-		 * If you see this it means you have no memory registered.
-		 * This can be done either with arm_add_mem_device() in an
-		 * initcall prior to mmu_initcall or via devicetree in the
-		 * memory node.
-		 */
-		panic("MMU: No memory bank found! Cannot continue\n");
-
-	if (get_cr() & CR_M)
+	if (mmu_on)
 		mmu_disable();
 
 	ttb = create_table();
@@ -228,10 +219,7 @@ static int mmu_init(void)
 	create_sections(0x0, 0x0, 0x1000, 0x0);
 
 	mmu_enable();
-
-	return 0;
 }
-mmu_initcall(mmu_init);
 
 void mmu_disable(void)
 {
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 11/12] ARM: mmu: Share code for arm_mmu_not_initialized_error()

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu-common.h | 11 +++++++++++
 arch/arm/cpu/mmu.c        | 11 -----------
 arch/arm/cpu/mmu_64.c     | 11 -----------
 3 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/arch/arm/cpu/mmu-common.h b/arch/arm/cpu/mmu-common.h
index 37eef3ef2..679799c3c 100644
--- a/arch/arm/cpu/mmu-common.h
+++ b/arch/arm/cpu/mmu-common.h
@@ -5,4 +5,15 @@ void dma_inv_range(void *ptr, size_t size);
 void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags);
 void __mmu_init(bool mmu_on);
 
+static inline void arm_mmu_not_initialized_error(void)
+{
+	/*
+	 * This means:
+	 * - one of the MMU functions like dma_alloc_coherent
+	 *   or remap_range is called too early, before the MMU is initialized
+	 * - Or the MMU initialization has failed earlier
+	 */
+	panic("MMU not initialized\n");
+}
+
 #endif
\ No newline at end of file
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index ba1c3e007..9e261a0be 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -75,17 +75,6 @@ static uint32_t pgd_flags_wc;
 
 #define PTE_MASK ((1 << 12) - 1)
 
-static void arm_mmu_not_initialized_error(void)
-{
-	/*
-	 * This means:
-	 * - one of the MMU functions like dma_alloc_coherent
-	 *   or remap_range is called too early, before the MMU is initialized
-	 * - Or the MMU initialization has failed earlier
-	 */
-	panic("MMU not initialized\n");
-}
-
 static bool pgd_type_table(u32 pgd)
 {
 	return (pgd & PMD_TYPE_MASK) == PMD_TYPE_TABLE;
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index 7f8a8f249..a3074b10d 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -37,17 +37,6 @@
 
 static uint64_t *ttb;
 
-static void arm_mmu_not_initialized_error(void)
-{
-	/*
-	 * This means:
-	 * - one of the MMU functions like dma_alloc_coherent
-	 *   or remap_range is called too early, before the MMU is initialized
-	 * - Or the MMU initialization has failed earlier
-	 */
-	panic("MMU not initialized\n");
-}
-
 static void set_table(uint64_t *pt, uint64_t *table_addr)
 {
 	uint64_t val;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 12/12] ARM: mmu: Make sure DMA coherent memory is zeroed out

[Andrey Smirnov]

In order to avoid passing random/junky values to DMA/HW as well as to
allow simplifying memory initialization in individual drivers, chagnge
dma_alloc_coherent() to guarantee that memory it returs is properly
zeroed out.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu-common.c | 3 ++-
 arch/arm/cpu/mmu-common.h | 1 +
 arch/arm/cpu/mmu.c        | 2 +-
 arch/arm/cpu/mmu_64.c     | 8 ++++++++
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
index c5b24bff8..aeefbb2da 100644
--- a/arch/arm/cpu/mmu-common.c
+++ b/arch/arm/cpu/mmu-common.c
@@ -43,7 +43,8 @@ void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags)
 	if (dma_handle)
 		*dma_handle = (dma_addr_t)ret;
 
-	dma_inv_range(ret, size);
+	memset(ret, 0, size);
+	dma_flush_range(ret, size);
 
 	arch_remap_range(ret, size, flags);
 
diff --git a/arch/arm/cpu/mmu-common.h b/arch/arm/cpu/mmu-common.h
index 679799c3c..0a33b138e 100644
--- a/arch/arm/cpu/mmu-common.h
+++ b/arch/arm/cpu/mmu-common.h
@@ -2,6 +2,7 @@
 #define __ARM_MMU_COMMON_H
 
 void dma_inv_range(void *ptr, size_t size);
+void dma_flush_range(void *ptr, size_t size);
 void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags);
 void __mmu_init(bool mmu_on);
 
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 9e261a0be..29816ad56 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -97,7 +97,7 @@ static u32 *find_pte(unsigned long adr)
 	return &table[(adr >> PAGE_SHIFT) & 0xff];
 }
 
-static void dma_flush_range(void *ptr, size_t size)
+void dma_flush_range(void *ptr, size_t size)
 {
 	unsigned long start = (unsigned long)ptr;
 	unsigned long end = start + size;
diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index a3074b10d..b45a69661 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -233,6 +233,14 @@ void dma_inv_range(void *ptr, size_t size)
 	v8_inv_dcache_range(start, end);
 }
 
+void dma_flush_range(void *ptr, size_t size)
+{
+	unsigned long start = (unsigned long)ptr;
+	unsigned long end = start + size - 1;
+
+	v8_flush_dcache_range(start, end);
+}
+
 void dma_sync_single_for_device(dma_addr_t address, size_t size,
                                 enum dma_data_direction dir)
 {
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 08/12] ARM: mmu: Share code for dma_alloc_coherent()

[Sam Ravnborg]

Hi Andrey.

On Wed, Jan 16, 2019 at 10:38:36PM -0800, Andrey Smirnov wrote:
> Both ARM and ARM64 implement almost identical algorithms in
> dma_alloc_coherent(). Move the code to mmu-common.c, so it can be
> shared.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/mmu-common.c | 21 +++++++++++++++++++++
>  arch/arm/cpu/mmu-common.h |  7 +++++++
We already have rch/arm/cpu/mmu.h, wait is the benefit of the extra header file?
In other words, suggest to move this to mmu.h

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 09/12] ARM: mmu: Share code for dma_sync_single_for_cpu()

[Sam Ravnborg]

Hi Andrey.

On Wed, Jan 16, 2019 at 10:38:37PM -0800, Andrey Smirnov wrote:
> Both ARM and ARM64 have identical code for
> dma_sync_single_for_cpu(). Move it to mmu-common.c so it can be shared.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/mmu-common.c | 8 ++++++++
>  arch/arm/cpu/mmu.c        | 7 -------
>  arch/arm/cpu/mmu_64.c     | 7 -------
>  3 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
> index 8c7d61447..a7d3b5b11 100644
> --- a/arch/arm/cpu/mmu-common.c
> +++ b/arch/arm/cpu/mmu-common.c
> @@ -8,6 +8,14 @@
>  
>  #include "mmu.h"
>  
> +
> +void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
> +			     enum dma_data_direction dir)
> +{
> +	if (dir != DMA_TO_DEVICE)
> +		dma_inv_range((void *)address, size);
> +}

I cannot see how this will work?!?
We are on a v8 architecture for a 64bit variant.
But in cache.c we have only:

DEFINE_CPU_FNS(v4)
DEFINE_CPU_FNS(v5)
DEFINE_CPU_FNS(v6)
DEFINE_CPU_FNS(v7)

So I do not see how we call v8_inv_dcache_range() here.
Do I miss something?

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 12/12] ARM: mmu: Make sure DMA coherent memory is zeroed out

[Sam Ravnborg]

Hi Andrey.

On Wed, Jan 16, 2019 at 10:38:40PM -0800, Andrey Smirnov wrote:
> In order to avoid passing random/junky values to DMA/HW as well as to
> allow simplifying memory initialization in individual drivers, chagnge
chagnge => change

> dma_alloc_coherent() to guarantee that memory it returs is properly
returs => returns
> zeroed out.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/mmu-common.c | 3 ++-
>  arch/arm/cpu/mmu-common.h | 1 +
>  arch/arm/cpu/mmu.c        | 2 +-
>  arch/arm/cpu/mmu_64.c     | 8 ++++++++
>  4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
> index c5b24bff8..aeefbb2da 100644
> --- a/arch/arm/cpu/mmu-common.c
> +++ b/arch/arm/cpu/mmu-common.c
> @@ -43,7 +43,8 @@ void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags)
>  	if (dma_handle)
>  		*dma_handle = (dma_addr_t)ret;
>  
> -	dma_inv_range(ret, size);
> +	memset(ret, 0, size);
> +	dma_flush_range(ret, size);
>  
>  	arch_remap_range(ret, size, flags);
>  
> diff --git a/arch/arm/cpu/mmu-common.h b/arch/arm/cpu/mmu-common.h
> index 679799c3c..0a33b138e 100644
> --- a/arch/arm/cpu/mmu-common.h
> +++ b/arch/arm/cpu/mmu-common.h
> @@ -2,6 +2,7 @@
>  #define __ARM_MMU_COMMON_H
>  
>  void dma_inv_range(void *ptr, size_t size);
> +void dma_flush_range(void *ptr, size_t size);
>  void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags);
>  void __mmu_init(bool mmu_on);
>  
> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> index 9e261a0be..29816ad56 100644
> --- a/arch/arm/cpu/mmu.c
> +++ b/arch/arm/cpu/mmu.c
> @@ -97,7 +97,7 @@ static u32 *find_pte(unsigned long adr)
>  	return &table[(adr >> PAGE_SHIFT) & 0xff];
>  }
>  
> -static void dma_flush_range(void *ptr, size_t size)
> +void dma_flush_range(void *ptr, size_t size)
>  {
>  	unsigned long start = (unsigned long)ptr;
>  	unsigned long end = start + size;
> diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
> index a3074b10d..b45a69661 100644
> --- a/arch/arm/cpu/mmu_64.c
> +++ b/arch/arm/cpu/mmu_64.c
> @@ -233,6 +233,14 @@ void dma_inv_range(void *ptr, size_t size)
>  	v8_inv_dcache_range(start, end);
>  }
>  
> +void dma_flush_range(void *ptr, size_t size)
> +{
> +	unsigned long start = (unsigned long)ptr;
> +	unsigned long end = start + size - 1;
> +
> +	v8_flush_dcache_range(start, end);
> +}
We should use the existing indirection with struct cache_fns
here, then we could share the code.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 00/12] ARM/ARM64 MMU code consolidation, zeroing of DMA coherent memory

[Sam Ravnborg]

Hi Andrey.

> Everyone:
> 
> This series is a result of my attempt at changing the behaviour of
> dma_alloc_coherent() to guarantee that memory it returns is zeroed
> out. Mostly to avoid having to do that explicitly in driver code, but
> also to match behaviour that that function has in Linux. While working
> on that I noticed that there was a fair bit of MMU/DMA related code
> between ARM/ARM64 that can be shared, so I created a number of patches
> to do just that.
> 
> Feedback is welcome!

I read all the patches, and due to the nice small incremental patches
you do (well done!) I could follow them all.
A few comments and for the rest you can add my "Reviewed-by:" tag.


> Andrey Smirnov (12):
>   ARM: mmu: Drop custom virt_to_phys/phys_to_virt
>   ARM: mmu: Simplify the use of dma_inv_range()
>   ARM: mmu: Share code for dma_(un)map_single()
>   ARM64: mmu: Use arch_remap_range() internaly
internally

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 09/12] ARM: mmu: Share code for dma_sync_single_for_cpu()

[Andrey Smirnov]

On Thu, Jan 17, 2019 at 1:28 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Andrey.
>
> On Wed, Jan 16, 2019 at 10:38:37PM -0800, Andrey Smirnov wrote:
> > Both ARM and ARM64 have identical code for
> > dma_sync_single_for_cpu(). Move it to mmu-common.c so it can be shared.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  arch/arm/cpu/mmu-common.c | 8 ++++++++
> >  arch/arm/cpu/mmu.c        | 7 -------
> >  arch/arm/cpu/mmu_64.c     | 7 -------
> >  3 files changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
> > index 8c7d61447..a7d3b5b11 100644
> > --- a/arch/arm/cpu/mmu-common.c
> > +++ b/arch/arm/cpu/mmu-common.c
> > @@ -8,6 +8,14 @@
> >
> >  #include "mmu.h"
> >
> > +
> > +void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
> > +                          enum dma_data_direction dir)
> > +{
> > +     if (dir != DMA_TO_DEVICE)
> > +             dma_inv_range((void *)address, size);
> > +}
>
> I cannot see how this will work?!?
> We are on a v8 architecture for a 64bit variant.
> But in cache.c we have only:
>
> DEFINE_CPU_FNS(v4)
> DEFINE_CPU_FNS(v5)
> DEFINE_CPU_FNS(v6)
> DEFINE_CPU_FNS(v7)
>

Cache.c isn't going to be built on AArch64, so it doesn't really
matter what's in it. V8_* cache functions are defined in cache-armv8.S
and there's no analog of  DEFINE_CPU_FNS() on AArch64.

> So I do not see how we call v8_inv_dcache_range() here.
> Do I miss something?
>

On AArch64 dma_inv_range() is defined as a wrapper around
v8_inv_dcache_range() in "ARM64: mmu: Invalidate memory before
remapping as DMA coherent" and exported to be available to other
compilation units in "ARM: mmu: Share code for dma_alloc_coherent()".
You can also see a pretty concise summary of it in this diff in this
bit:

diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
index ed4aa00a8..2cb62370e 100644
--- a/arch/arm/cpu/mmu_64.c
+++ b/arch/arm/cpu/mmu_64.c
@@ -256,13 +256,6 @@ void dma_inv_range(void *ptr, size_t size)
        v8_inv_dcache_range(start, end);
 }

Hope this clarifies things.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 08/12] ARM: mmu: Share code for dma_alloc_coherent()

[Andrey Smirnov]

On Thu, Jan 17, 2019 at 1:23 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Andrey.
>
> On Wed, Jan 16, 2019 at 10:38:36PM -0800, Andrey Smirnov wrote:
> > Both ARM and ARM64 implement almost identical algorithms in
> > dma_alloc_coherent(). Move the code to mmu-common.c, so it can be
> > shared.
> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  arch/arm/cpu/mmu-common.c | 21 +++++++++++++++++++++
> >  arch/arm/cpu/mmu-common.h |  7 +++++++
> We already have rch/arm/cpu/mmu.h, wait is the benefit of the extra header file?

Arch/arm/cpu/mmu.h is a 32-bit specific file that is not used on
AArch64, which has its own counterpart in arch/arm/cpu/mmu_64.h.
Mmu-common.h contains the bits that are shared between the two
architectures.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 12/12] ARM: mmu: Make sure DMA coherent memory is zeroed out

[Andrey Smirnov]

On Thu, Jan 17, 2019 at 1:36 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Andrey.
>
> On Wed, Jan 16, 2019 at 10:38:40PM -0800, Andrey Smirnov wrote:
> > In order to avoid passing random/junky values to DMA/HW as well as to
> > allow simplifying memory initialization in individual drivers, chagnge
> chagnge => change
>
> > dma_alloc_coherent() to guarantee that memory it returs is properly
> returs => returns
> > zeroed out.

Ugh, will fix the typos.

> >
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  arch/arm/cpu/mmu-common.c | 3 ++-
> >  arch/arm/cpu/mmu-common.h | 1 +
> >  arch/arm/cpu/mmu.c        | 2 +-
> >  arch/arm/cpu/mmu_64.c     | 8 ++++++++
> >  4 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
> > index c5b24bff8..aeefbb2da 100644
> > --- a/arch/arm/cpu/mmu-common.c
> > +++ b/arch/arm/cpu/mmu-common.c
> > @@ -43,7 +43,8 @@ void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags)
> >       if (dma_handle)
> >               *dma_handle = (dma_addr_t)ret;
> >
> > -     dma_inv_range(ret, size);
> > +     memset(ret, 0, size);
> > +     dma_flush_range(ret, size);
> >
> >       arch_remap_range(ret, size, flags);
> >
> > diff --git a/arch/arm/cpu/mmu-common.h b/arch/arm/cpu/mmu-common.h
> > index 679799c3c..0a33b138e 100644
> > --- a/arch/arm/cpu/mmu-common.h
> > +++ b/arch/arm/cpu/mmu-common.h
> > @@ -2,6 +2,7 @@
> >  #define __ARM_MMU_COMMON_H
> >
> >  void dma_inv_range(void *ptr, size_t size);
> > +void dma_flush_range(void *ptr, size_t size);
> >  void *dma_alloc_map(size_t size, dma_addr_t *dma_handle, unsigned flags);
> >  void __mmu_init(bool mmu_on);
> >
> > diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> > index 9e261a0be..29816ad56 100644
> > --- a/arch/arm/cpu/mmu.c
> > +++ b/arch/arm/cpu/mmu.c
> > @@ -97,7 +97,7 @@ static u32 *find_pte(unsigned long adr)
> >       return &table[(adr >> PAGE_SHIFT) & 0xff];
> >  }
> >
> > -static void dma_flush_range(void *ptr, size_t size)
> > +void dma_flush_range(void *ptr, size_t size)
> >  {
> >       unsigned long start = (unsigned long)ptr;
> >       unsigned long end = start + size;
> > diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
> > index a3074b10d..b45a69661 100644
> > --- a/arch/arm/cpu/mmu_64.c
> > +++ b/arch/arm/cpu/mmu_64.c
> > @@ -233,6 +233,14 @@ void dma_inv_range(void *ptr, size_t size)
> >       v8_inv_dcache_range(start, end);
> >  }
> >
> > +void dma_flush_range(void *ptr, size_t size)
> > +{
> > +     unsigned long start = (unsigned long)ptr;
> > +     unsigned long end = start + size - 1;
> > +
> > +     v8_flush_dcache_range(start, end);
> > +}
> We should use the existing indirection with struct cache_fns
> here, then we could share the code.

AArch64 used to use struct cache_fns, but that approach was decided
against in 4b57aae26c0ada3139ccb1011bdcbd88dc7e1a91 ("ARM: Create own
cache.c file for aarch64") by Sascha. My interpretation of it is that
adding struct cache_fns back would not be particularly welcome, but I
am more that happy to do it if told otherwise.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 00/12] ARM/ARM64 MMU code consolidation, zeroing of DMA coherent memory

[Andrey Smirnov]

On Thu, Jan 17, 2019 at 1:40 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Andrey.
>
> > Everyone:
> >
> > This series is a result of my attempt at changing the behaviour of
> > dma_alloc_coherent() to guarantee that memory it returns is zeroed
> > out. Mostly to avoid having to do that explicitly in driver code, but
> > also to match behaviour that that function has in Linux. While working
> > on that I noticed that there was a fair bit of MMU/DMA related code
> > between ARM/ARM64 that can be shared, so I created a number of patches
> > to do just that.
> >
> > Feedback is welcome!
>
> I read all the patches, and due to the nice small incremental patches
> you do (well done!) I could follow them all.
> A few comments and for the rest you can add my "Reviewed-by:" tag.
>
>
> > Andrey Smirnov (12):
> >   ARM: mmu: Drop custom virt_to_phys/phys_to_virt
> >   ARM: mmu: Simplify the use of dma_inv_range()
> >   ARM: mmu: Share code for dma_(un)map_single()
> >   ARM64: mmu: Use arch_remap_range() internaly
> internally
>

Will fix in v2.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 09/12] ARM: mmu: Share code for dma_sync_single_for_cpu()

[Sam Ravnborg]

Hi Andrey.

On Thu, Jan 17, 2019 at 01:50:31PM -0800, Andrey Smirnov wrote:
> On Thu, Jan 17, 2019 at 1:28 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Andrey.
> >
> > On Wed, Jan 16, 2019 at 10:38:37PM -0800, Andrey Smirnov wrote:
> > > Both ARM and ARM64 have identical code for
> > > dma_sync_single_for_cpu(). Move it to mmu-common.c so it can be shared.
> > >
> > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > > ---
> > >  arch/arm/cpu/mmu-common.c | 8 ++++++++
> > >  arch/arm/cpu/mmu.c        | 7 -------
> > >  arch/arm/cpu/mmu_64.c     | 7 -------
> > >  3 files changed, 8 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/arch/arm/cpu/mmu-common.c b/arch/arm/cpu/mmu-common.c
> > > index 8c7d61447..a7d3b5b11 100644
> > > --- a/arch/arm/cpu/mmu-common.c
> > > +++ b/arch/arm/cpu/mmu-common.c
> > > @@ -8,6 +8,14 @@
> > >
> > >  #include "mmu.h"
> > >
> > > +
> > > +void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
> > > +                          enum dma_data_direction dir)
> > > +{
> > > +     if (dir != DMA_TO_DEVICE)
> > > +             dma_inv_range((void *)address, size);
> > > +}
> >
> > I cannot see how this will work?!?
> > We are on a v8 architecture for a 64bit variant.
> > But in cache.c we have only:
> >
> > DEFINE_CPU_FNS(v4)
> > DEFINE_CPU_FNS(v5)
> > DEFINE_CPU_FNS(v6)
> > DEFINE_CPU_FNS(v7)
> >
> 
> Cache.c isn't going to be built on AArch64, so it doesn't really
> matter what's in it. V8_* cache functions are defined in cache-armv8.S
> and there's no analog of  DEFINE_CPU_FNS() on AArch64.
> 
> > So I do not see how we call v8_inv_dcache_range() here.
> > Do I miss something?
> >
> 
> On AArch64 dma_inv_range() is defined as a wrapper around
> v8_inv_dcache_range() in "ARM64: mmu: Invalidate memory before
> remapping as DMA coherent" and exported to be available to other
> compilation units in "ARM: mmu: Share code for dma_alloc_coherent()".
> You can also see a pretty concise summary of it in this diff in this
> bit:
> 
> diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
> index ed4aa00a8..2cb62370e 100644
> --- a/arch/arm/cpu/mmu_64.c
> +++ b/arch/arm/cpu/mmu_64.c
> @@ -256,13 +256,6 @@ void dma_inv_range(void *ptr, size_t size)
>         v8_inv_dcache_range(start, end);
>  }
> 
> Hope this clarifies things.
Indeed, this was what I missed.
Thanks for explaining.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 08/12] ARM: mmu: Share code for dma_alloc_coherent()

[Sam Ravnborg]

On Thu, Jan 17, 2019 at 01:54:46PM -0800, Andrey Smirnov wrote:
> On Thu, Jan 17, 2019 at 1:23 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Andrey.
> >
> > On Wed, Jan 16, 2019 at 10:38:36PM -0800, Andrey Smirnov wrote:
> > > Both ARM and ARM64 implement almost identical algorithms in
> > > dma_alloc_coherent(). Move the code to mmu-common.c, so it can be
> > > shared.
> > >
> > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > > ---
> > >  arch/arm/cpu/mmu-common.c | 21 +++++++++++++++++++++
> > >  arch/arm/cpu/mmu-common.h |  7 +++++++
> > We already have rch/arm/cpu/mmu.h, wait is the benefit of the extra header file?
> 
> Arch/arm/cpu/mmu.h is a 32-bit specific file that is not used on
> AArch64, which has its own counterpart in arch/arm/cpu/mmu_64.h.
> Mmu-common.h contains the bits that are shared between the two
> architectures.

I have only looked at 32bit so far, so missed that mmu.h was only 32bit.
With this in mind mmu-common.h makes good sense.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 00/12] ARM/ARM64 MMU code consolidation, zeroing of DMA coherent memory

[Sam Ravnborg]

On Thu, Jan 17, 2019 at 10:40:39PM +0100, Sam Ravnborg wrote:
> Hi Andrey.
> 
> > Everyone:
> > 
> > This series is a result of my attempt at changing the behaviour of
> > dma_alloc_coherent() to guarantee that memory it returns is zeroed
> > out. Mostly to avoid having to do that explicitly in driver code, but
> > also to match behaviour that that function has in Linux. While working
> > on that I noticed that there was a fair bit of MMU/DMA related code
> > between ARM/ARM64 that can be shared, so I created a number of patches
> > to do just that.
> > 
> > Feedback is welcome!
> 
> I read all the patches, and due to the nice small incremental patches
> you do (well done!) I could follow them all.
> A few comments and for the rest you can add my "Reviewed-by:" tag.
You can extend this to all patches now.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 12/12] ARM: mmu: Make sure DMA coherent memory is zeroed out

[Sam Ravnborg]

> > > +     unsigned long end = start + size - 1;
> > > +
> > > +     v8_flush_dcache_range(start, end);
> > > +}
> > We should use the existing indirection with struct cache_fns
> > here, then we could share the code.
> 
> AArch64 used to use struct cache_fns, but that approach was decided
> against in 4b57aae26c0ada3139ccb1011bdcbd88dc7e1a91 ("ARM: Create own
> cache.c file for aarch64") by Sascha. My interpretation of it is that
> adding struct cache_fns back would not be particularly welcome, but I
> am more that happy to do it if told otherwise.
To be clear - my comments was based on the wrogn assumption
that we had cache_fns for 64 bit.
As long as we do not see a v9 (do not follow ARM) then I see no
reason for adding the extra indirection / complexity.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox