[PATCH v2 00/28] ARM MMU code improvements and on-demand PTE allocation

[PATCH v2 00/28] ARM MMU code improvements and on-demand PTE allocation

[Andrey Smirnov]

Everyone:

With exeception of patch 27/28, all patches in this series are just
small improvements(IMHO) that I made while working with ARM MMU code.

Patch 27/28, OTOH, brings a bit of a change to how MMU code allocates
PTEs, postponing that until the users of DMA API actually request
that.

Feedback is welcome!

Changes since [v1]:

	- create_sections() reworked to keep all of its arguments
          32-bits

	- Fixed compliation error in 9/28

	- Small style fix for 26/28

	- Collected Reviewed-by's from Lucas

[v1] http://lists.infradead.org/pipermail/barebox/2018-May/032816.html

Andrey Smirnov (28):
  ARM: mmu: Remove unused ARM_VECTORS_SIZE
  ARM: mmu: Make use of IS_ALIGNED in arm_mmu_remap_sdram()
  ARM: mmu: Use ALIGN and ALIGN_DOWN in map_cachable()
  ARM: mmu: Introduce set_ttbr()
  ARM: mmu: Introduce set_domain()
  ARM: mmu: Share code for create_sections()
  ARM: mmu: Separate index and address in create_sections()
  sizes.h: Sync with Linux 4.16
  ARM: mmu: Specify size in bytes in create_sections()
  ARM: mmu: Share code for initial flat mapping creation
  ARM: mmu: Share PMD_SECT_DEF_CACHED
  ARM: mmu: Drop needless shifting in map_io_sections()
  ARM: mmu: Replace hardcoded shifts with pgd_index() from Linux
  ARM: mmu: Trivial simplification in arm_mmu_remap_sdram()
  ARM: mmu: Replace various SZ_1M with PGDIR_SIZE
  ARM: mmu: Use PAGE_SIZE when specifying size of one page
  ARM: mmu: Define and use PTRS_PER_PTE
  ARM: mmu: Use PAGE_SIZE instead of magic right shift by 12
  ARM: mmu: Use xmemalign in arm_create_pte()
  ARM: mmu: Use xmemalign in mmu_init()
  ARM: mmu: Share code between dma_alloc_*() functions
  ARM: mmu: Pass PTE flags a parameter to arm_create_pte()
  ARM: mmu: Make sure that address is 1M aligned in arm_create_pte()
  ARM: mmu: Use find_pte() to find PTE in create_vector_table()
  ARM: mmu: Use dma_inv_range() in dma_sync_single_for_cpu()
  ARM: mmu: Simplify the use of dma_flush_range()
  ARM: mmu: Implement on-demand PTE allocation
  ARM: mmu: Introduce ARM_TTB_SIZE

 arch/arm/boards/phytec-som-imx6/lowlevel.c |   1 +
 arch/arm/cpu/mmu-early.c                   |  31 +-
 arch/arm/cpu/mmu.c                         | 321 ++++++++++-----------
 arch/arm/cpu/mmu.h                         |  53 ++++
 arch/arm/include/asm/barebox-arm.h         |   8 +-
 include/linux/sizes.h                      |   4 +
 6 files changed, 221 insertions(+), 197 deletions(-)

-- 
2.17.0


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

[PATCH v2 01/28] ARM: mmu: Remove unused ARM_VECTORS_SIZE

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 6ccd5893b..9472013ca 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -270,12 +270,6 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 
 	return 0;
 }
-/*
- * We have 8 exception vectors and the table consists of absolute
- * jumps, so we need 8 * 4 bytes for the instructions and another
- * 8 * 4 bytes for the addresses.
- */
-#define ARM_VECTORS_SIZE	(sizeof(u32) * 8 * 2)
 
 #define ARM_HIGH_VECTORS	0xffff0000
 #define ARM_LOW_VECTORS		0x0
-- 
2.17.0


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

[PATCH v2 02/28] ARM: mmu: Make use of IS_ALIGNED in arm_mmu_remap_sdram()

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 9472013ca..c70e9e782 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -241,7 +241,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 	 * We replace each 1MiB section in this range with second level page
 	 * tables, therefore we must have 1Mib aligment here.
 	 */
-	if ((phys & (SZ_1M - 1)) || (bank->size & (SZ_1M - 1)))
+	if (!IS_ALIGNED(phys, SZ_1M) || !IS_ALIGNED(bank->size, SZ_1M))
 		return -EINVAL;
 
 	ptes = xmemalign(PAGE_SIZE, num_ptes * sizeof(u32));
-- 
2.17.0


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

[PATCH v2 03/28] ARM: mmu: Use ALIGN and ALIGN_DOWN in map_cachable()

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
index 70cb5fe31..fc14d5cac 100644
--- a/arch/arm/cpu/mmu-early.c
+++ b/arch/arm/cpu/mmu-early.c
@@ -23,8 +23,8 @@ static void create_sections(unsigned long addr, int size_m, unsigned int flags)
 
 static void map_cachable(unsigned long start, unsigned long size)
 {
-	start &= ~(SZ_1M - 1);
-	size = (size + (SZ_1M - 1)) & ~(SZ_1M - 1);
+	start = ALIGN_DOWN(start, SZ_1M);
+	size  = ALIGN(size, SZ_1M);
 
 	create_sections(start, size >> 20, PMD_SECT_AP_WRITE |
 			PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
-- 
2.17.0


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

[PATCH v2 04/28] ARM: mmu: Introduce set_ttbr()

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
index fc14d5cac..79fc3080a 100644
--- a/arch/arm/cpu/mmu-early.c
+++ b/arch/arm/cpu/mmu-early.c
@@ -39,8 +39,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
 
 	arm_set_cache_functions();
 
-	/* Set the ttb register */
-	asm volatile ("mcr  p15,0,%0,c2,c0,0" : : "r"(ttb) /*:*/);
+	set_ttbr(ttb);
 
 	/* Set the Domain Access Control Register */
 	i = 0x3;
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index c70e9e782..28732b795 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -471,8 +471,7 @@ static int mmu_init(void)
 
 	pr_debug("ttb: 0x%p\n", ttb);
 
-	/* Set the ttb register */
-	asm volatile ("mcr  p15,0,%0,c2,c0,0" : : "r"(ttb) /*:*/);
+	set_ttbr(ttb);
 
 	/* Set the Domain Access Control Register */
 	i = 0x3;
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 79ebc80d7..5dcf4b53a 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -11,4 +11,9 @@ static inline void __mmu_cache_off(void) {}
 static inline void __mmu_cache_flush(void) {}
 #endif
 
+static inline void set_ttbr(void *ttb)
+{
+	asm volatile ("mcr  p15,0,%0,c2,c0,0" : : "r"(ttb) /*:*/);
+}
+
 #endif /* __ARM_MMU_H */
-- 
2.17.0


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

[PATCH v2 05/28] ARM: mmu: Introduce set_domain()

[Andrey Smirnov]

Port set_domain() form Linux kernel and share it between regular and
early MMU code to avoid duplication.

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

diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
index 79fc3080a..f75cc7e4a 100644
--- a/arch/arm/cpu/mmu-early.c
+++ b/arch/arm/cpu/mmu-early.c
@@ -33,17 +33,12 @@ static void map_cachable(unsigned long start, unsigned long size)
 void mmu_early_enable(unsigned long membase, unsigned long memsize,
 		      unsigned long _ttb)
 {
-	int i;
-
 	ttb = (uint32_t *)_ttb;
 
 	arm_set_cache_functions();
 
 	set_ttbr(ttb);
-
-	/* Set the Domain Access Control Register */
-	i = 0x3;
-	asm volatile ("mcr  p15,0,%0,c3,c0,0" : : "r"(i) /*:*/);
+	set_domain(DOMAIN_MANAGER);
 
 	create_sections(0, 4096, PMD_SECT_AP_WRITE |
 			PMD_SECT_AP_READ | PMD_TYPE_SECT);
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 28732b795..7e087d08f 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -422,7 +422,6 @@ static void vectors_init(void)
 static int mmu_init(void)
 {
 	struct memory_bank *bank;
-	int i;
 
 	if (list_empty(&memory_banks))
 		/*
@@ -472,10 +471,7 @@ static int mmu_init(void)
 	pr_debug("ttb: 0x%p\n", ttb);
 
 	set_ttbr(ttb);
-
-	/* Set the Domain Access Control Register */
-	i = 0x3;
-	asm volatile ("mcr  p15,0,%0,c3,c0,0" : : "r"(i) /*:*/);
+	set_domain(DOMAIN_MANAGER);
 
 	/* create a flat mapping using 1MiB sections */
 	create_sections(0, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 5dcf4b53a..e71ff8e9a 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -16,4 +16,13 @@ static inline void set_ttbr(void *ttb)
 	asm volatile ("mcr  p15,0,%0,c2,c0,0" : : "r"(ttb) /*:*/);
 }
 
+#define DOMAIN_MANAGER	3
+
+static inline void set_domain(unsigned val)
+{
+	/* Set the Domain Access Control Register */
+	asm volatile ("mcr  p15,0,%0,c3,c0,0" : : "r"(val) /*:*/);
+}
+
+
 #endif /* __ARM_MMU_H */
-- 
2.17.0


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

[PATCH v2 06/28] ARM: mmu: Share code for create_sections()

[Andrey Smirnov]

Regular MMU code never creates anything but 1:1 mapping, and barring
that plus the call to __mmu_cache_flush(), early MMU code version of
the function is pretty much identical. To avoid code duplication, move
it to mmu.h and convert both regular and early MMU code to use it.

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

diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
index f75cc7e4a..70ece0d2f 100644
--- a/arch/arm/cpu/mmu-early.c
+++ b/arch/arm/cpu/mmu-early.c
@@ -11,22 +11,12 @@
 
 static uint32_t *ttb;
 
-static void create_sections(unsigned long addr, int size_m, unsigned int flags)
-{
-	int i;
-
-	addr >>= 20;
-
-	for (i = size_m; i > 0; i--, addr++)
-		ttb[addr] = (addr << 20) | flags;
-}
-
 static void map_cachable(unsigned long start, unsigned long size)
 {
 	start = ALIGN_DOWN(start, SZ_1M);
 	size  = ALIGN(size, SZ_1M);
 
-	create_sections(start, size >> 20, PMD_SECT_AP_WRITE |
+	create_sections(ttb, start, size >> 20, PMD_SECT_AP_WRITE |
 			PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
 }
 
@@ -40,7 +30,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
 	set_ttbr(ttb);
 	set_domain(DOMAIN_MANAGER);
 
-	create_sections(0, 4096, PMD_SECT_AP_WRITE |
+	create_sections(ttb, 0, 4096, PMD_SECT_AP_WRITE |
 			PMD_SECT_AP_READ | PMD_TYPE_SECT);
 
 	map_cachable(membase, memsize);
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 7e087d08f..0c367e47c 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -37,21 +37,7 @@
 #define PMD_SECT_DEF_UNCACHED (PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | PMD_TYPE_SECT)
 #define PMD_SECT_DEF_CACHED (PMD_SECT_WB | PMD_SECT_DEF_UNCACHED)
 
-static unsigned long *ttb;
-
-static void create_sections(unsigned long virt, unsigned long phys, int size_m,
-		unsigned int flags)
-{
-	int i;
-
-	phys >>= 20;
-	virt >>= 20;
-
-	for (i = size_m; i > 0; i--, virt++, phys++)
-		ttb[virt] = (phys << 20) | flags;
-
-	__mmu_cache_flush();
-}
+static uint32_t *ttb;
 
 /*
  * Do it the simple way for now and invalidate the entire
@@ -474,8 +460,9 @@ static int mmu_init(void)
 	set_domain(DOMAIN_MANAGER);
 
 	/* create a flat mapping using 1MiB sections */
-	create_sections(0, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
+	create_sections(ttb, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
 			PMD_TYPE_SECT);
+	__mmu_cache_flush();
 
 	vectors_init();
 
@@ -484,9 +471,11 @@ static int mmu_init(void)
 	 * This is to speed up the generation of 2nd level page tables
 	 * below
 	 */
-	for_each_memory_bank(bank)
-		create_sections(bank->start, bank->start, bank->size >> 20,
+	for_each_memory_bank(bank) {
+		create_sections(ttb, bank->start, bank->size >> 20,
 				PMD_SECT_DEF_CACHED);
+		__mmu_cache_flush();
+	}
 
 	__mmu_cache_on();
 
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index e71ff8e9a..59f72049f 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -24,5 +24,13 @@ static inline void set_domain(unsigned val)
 	asm volatile ("mcr  p15,0,%0,c3,c0,0" : : "r"(val) /*:*/);
 }
 
+static inline void
+create_sections(uint32_t *ttb, unsigned long addr,
+		int size_m, unsigned int flags)
+{
+	for (addr >>= 20; addr < size_m; addr++)
+		ttb[addr] = (addr << 20) | flags;
+}
+
 
 #endif /* __ARM_MMU_H */
-- 
2.17.0


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

[PATCH v2 07/28] ARM: mmu: Separate index and address in create_sections()

[Andrey Smirnov]

Both TTB index and address used to fill that entry are derived from
the same variable 'addr' which requires shifting right and left by 20
and somewhat confusing.

Split the counter used to iterate over elements of  TTB into a
separate variable to make this code a bit easier to read.

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

diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 59f72049f..d71cd7e38 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -1,6 +1,8 @@
 #ifndef __ARM_MMU_H
 #define __ARM_MMU_H
 
+#include <linux/sizes.h>
+
 #ifdef CONFIG_MMU
 void __mmu_cache_on(void);
 void __mmu_cache_off(void);
@@ -28,8 +30,12 @@ static inline void
 create_sections(uint32_t *ttb, unsigned long addr,
 		int size_m, unsigned int flags)
 {
-	for (addr >>= 20; addr < size_m; addr++)
-		ttb[addr] = (addr << 20) | flags;
+	unsigned long ttb_start = addr >> 20;
+	unsigned long ttb_end   = ttb_start + size_m;
+	unsigned int i;
+
+	for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
+		ttb[i] = addr | flags;
 }
 
 
-- 
2.17.0


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

[PATCH v2 08/28] sizes.h: Sync with Linux 4.16

[Andrey Smirnov]

Sync with Linux 4.16 to get SZ_4G.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/boards/phytec-som-imx6/lowlevel.c | 1 +
 include/linux/sizes.h                      | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/arch/arm/boards/phytec-som-imx6/lowlevel.c b/arch/arm/boards/phytec-som-imx6/lowlevel.c
index f9d70c745..05f918f6c 100644
--- a/arch/arm/boards/phytec-som-imx6/lowlevel.c
+++ b/arch/arm/boards/phytec-som-imx6/lowlevel.c
@@ -38,6 +38,7 @@ static inline void setup_uart(void)
 	putc_ll('>');
 }
 
+#undef SZ_4G
 #define SZ_4G 0xEFFFFFF8
 
 BAREBOX_IMD_TAG_STRING(physom_mx6_memsize_SZ_128M, IMD_TYPE_PARAMETER, "memsize=128", 0);
diff --git a/include/linux/sizes.h b/include/linux/sizes.h
index ce3e8150c..fbde0bc7e 100644
--- a/include/linux/sizes.h
+++ b/include/linux/sizes.h
@@ -8,6 +8,8 @@
 #ifndef __LINUX_SIZES_H__
 #define __LINUX_SIZES_H__
 
+#include <linux/const.h>
+
 #define SZ_1				0x00000001
 #define SZ_2				0x00000002
 #define SZ_4				0x00000004
@@ -44,4 +46,6 @@
 #define SZ_1G				0x40000000
 #define SZ_2G				0x80000000
 
+#define SZ_4G				_AC(0x100000000, ULL)
+
 #endif /* __LINUX_SIZES_H__ */
-- 
2.17.0


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

[PATCH v2 09/28] ARM: mmu: Specify size in bytes in create_sections()

[Andrey Smirnov]

Seeing

        create_sections(ttb, 0, PAGE_SIZE, ...);

as the code the creates initial flat 4 GiB mapping is a bit less
intuitive then

        create_sections(ttb, 0, SZ_4G, ...);

so, for the sake of clarification, convert create_sections() to accept
size in bytes and do bytes -> MiB converstion as a part of the
function.

NOTE: To keep all of the arguments of create_sections() 32-bit:

    - Move all of the real code into a helper function accepting first
      and last addresses of the region (e.g passing 0 and U32_MAX
      means all 4GiB of address space)

    - Convert create_section() into a macro that does necessary size
      -> last addres conversion under the hood to preserve original API

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

diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
index 70ece0d2f..136b33c3a 100644
--- a/arch/arm/cpu/mmu-early.c
+++ b/arch/arm/cpu/mmu-early.c
@@ -16,7 +16,7 @@ static void map_cachable(unsigned long start, unsigned long size)
 	start = ALIGN_DOWN(start, SZ_1M);
 	size  = ALIGN(size, SZ_1M);
 
-	create_sections(ttb, start, size >> 20, PMD_SECT_AP_WRITE |
+	create_sections(ttb, start, size, PMD_SECT_AP_WRITE |
 			PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
 }
 
@@ -30,7 +30,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
 	set_ttbr(ttb);
 	set_domain(DOMAIN_MANAGER);
 
-	create_sections(ttb, 0, 4096, PMD_SECT_AP_WRITE |
+	create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE |
 			PMD_SECT_AP_READ | PMD_TYPE_SECT);
 
 	map_cachable(membase, memsize);
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 0c367e47c..f02c99f65 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -460,7 +460,7 @@ static int mmu_init(void)
 	set_domain(DOMAIN_MANAGER);
 
 	/* create a flat mapping using 1MiB sections */
-	create_sections(ttb, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
+	create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
 			PMD_TYPE_SECT);
 	__mmu_cache_flush();
 
@@ -472,7 +472,7 @@ static int mmu_init(void)
 	 * below
 	 */
 	for_each_memory_bank(bank) {
-		create_sections(ttb, bank->start, bank->size >> 20,
+		create_sections(ttb, bank->start, bank->size,
 				PMD_SECT_DEF_CACHED);
 		__mmu_cache_flush();
 	}
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index d71cd7e38..52689359a 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -27,16 +27,26 @@ static inline void set_domain(unsigned val)
 }
 
 static inline void
-create_sections(uint32_t *ttb, unsigned long addr,
-		int size_m, unsigned int flags)
+__create_sections(uint32_t *ttb, unsigned long first,
+		  unsigned long last, unsigned int flags)
 {
-	unsigned long ttb_start = addr >> 20;
-	unsigned long ttb_end   = ttb_start + size_m;
-	unsigned int i;
+	unsigned long ttb_start = first >> 20;
+	unsigned long ttb_end   = (last >> 20) + 1;
+	unsigned int i, addr;
 
-	for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
+	for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
 		ttb[i] = addr | flags;
 }
 
+#define create_sections(ttb, addr, size, flags)				\
+	({								\
+		typeof(addr) __addr = addr;				\
+		typeof(size) __size = size;				\
+		/* Check for overflow */				\
+		BUG_ON(__addr > ULONG_MAX - __size + 1);		\
+		__create_sections(ttb, __addr, __addr + (size)  - 1,	\
+				  flags);				\
+	})
+
 
 #endif /* __ARM_MMU_H */
-- 
2.17.0


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

[PATCH v2 10/28] ARM: mmu: Share code for initial flat mapping creation

[Andrey Smirnov]

Code creating inital 4GiB flat mapping is identical between mmu.c and
mmu-early.c, so move it to mmu.h and share.

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

diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
index 136b33c3a..1ff48d1da 100644
--- a/arch/arm/cpu/mmu-early.c
+++ b/arch/arm/cpu/mmu-early.c
@@ -5,7 +5,6 @@
 #include <asm/memory.h>
 #include <asm/system.h>
 #include <asm/cache.h>
-#include <asm/pgtable.h>
 
 #include "mmu.h"
 
@@ -30,8 +29,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
 	set_ttbr(ttb);
 	set_domain(DOMAIN_MANAGER);
 
-	create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE |
-			PMD_SECT_AP_READ | PMD_TYPE_SECT);
+	create_flat_mapping(ttb);
 
 	map_cachable(membase, memsize);
 
diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index f02c99f65..5600cfbe2 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -27,14 +27,12 @@
 #include <asm/barebox-arm.h>
 #include <asm/system.h>
 #include <asm/cache.h>
-#include <asm/pgtable.h>
 #include <memory.h>
 #include <asm/system_info.h>
 #include <asm/sections.h>
 
 #include "mmu.h"
 
-#define PMD_SECT_DEF_UNCACHED (PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | PMD_TYPE_SECT)
 #define PMD_SECT_DEF_CACHED (PMD_SECT_WB | PMD_SECT_DEF_UNCACHED)
 
 static uint32_t *ttb;
@@ -459,9 +457,7 @@ static int mmu_init(void)
 	set_ttbr(ttb);
 	set_domain(DOMAIN_MANAGER);
 
-	/* create a flat mapping using 1MiB sections */
-	create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
-			PMD_TYPE_SECT);
+	create_flat_mapping(ttb);
 	__mmu_cache_flush();
 
 	vectors_init();
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 52689359a..87dd17710 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -1,6 +1,7 @@
 #ifndef __ARM_MMU_H
 #define __ARM_MMU_H
 
+#include <asm/pgtable.h>
 #include <linux/sizes.h>
 
 #ifdef CONFIG_MMU
@@ -49,4 +50,12 @@ __create_sections(uint32_t *ttb, unsigned long first,
 	})
 
 
+#define PMD_SECT_DEF_UNCACHED (PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | PMD_TYPE_SECT)
+
+static inline void create_flat_mapping(uint32_t *ttb)
+{
+	/* create a flat mapping using 1MiB sections */
+	create_sections(ttb, 0, SZ_4G, PMD_SECT_DEF_UNCACHED);
+}
+
 #endif /* __ARM_MMU_H */
-- 
2.17.0


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

[PATCH v2 11/28] ARM: mmu: Share PMD_SECT_DEF_CACHED

[Andrey Smirnov]

Share PMD_SECT_DEF_CACHED between mmu.c and mmu-early.c.

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

diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
index 1ff48d1da..18df78370 100644
--- a/arch/arm/cpu/mmu-early.c
+++ b/arch/arm/cpu/mmu-early.c
@@ -15,8 +15,7 @@ static void map_cachable(unsigned long start, unsigned long size)
 	start = ALIGN_DOWN(start, SZ_1M);
 	size  = ALIGN(size, SZ_1M);
 
-	create_sections(ttb, start, size, PMD_SECT_AP_WRITE |
-			PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
+	create_sections(ttb, start, size, PMD_SECT_DEF_CACHED);
 }
 
 void mmu_early_enable(unsigned long membase, unsigned long memsize,
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 87dd17710..04aa81732 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -51,6 +51,7 @@ __create_sections(uint32_t *ttb, unsigned long first,
 
 
 #define PMD_SECT_DEF_UNCACHED (PMD_SECT_AP_WRITE | PMD_SECT_AP_READ | PMD_TYPE_SECT)
+#define PMD_SECT_DEF_CACHED (PMD_SECT_WB | PMD_SECT_DEF_UNCACHED)
 
 static inline void create_flat_mapping(uint32_t *ttb)
 {
-- 
2.17.0


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

[PATCH v2 12/28] ARM: mmu: Drop needless shifting in map_io_sections()

[Andrey Smirnov]

Instead of shifting phys right by 20 and then again left by the same
amount, just convert the code to expect it to be in unit of bytes all
the time.

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 5600cfbe2..40bef016b 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -196,9 +196,8 @@ void *map_io_sections(unsigned long phys, void *_start, size_t size)
 {
 	unsigned long start = (unsigned long)_start, sec;
 
-	phys >>= 20;
-	for (sec = start; sec < start + size; sec += (1 << 20))
-		ttb[sec >> 20] = (phys++ << 20) | PMD_SECT_DEF_UNCACHED;
+	for (sec = start; sec < start + size; sec += (1 << 20), phys += SZ_1M)
+		ttb[sec >> 20] = phys | PMD_SECT_DEF_UNCACHED;
 
 	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
 	tlb_invalidate();
-- 
2.17.0


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

[PATCH v2 13/28] ARM: mmu: Replace hardcoded shifts with pgd_index() from Linux

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 40bef016b..e0365eec1 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -96,7 +96,7 @@ static u32 *arm_create_pte(unsigned long virt)
 	if (!ttb)
 		arm_mmu_not_initialized_error();
 
-	ttb[virt >> 20] = (unsigned long)table | PMD_TYPE_TABLE;
+	ttb[pgd_index(virt)] = (unsigned long)table | PMD_TYPE_TABLE;
 
 	for (i = 0; i < 256; i++) {
 		table[i] = virt | PTE_TYPE_SMALL | pte_flags_uncached;
@@ -113,7 +113,7 @@ static u32 *find_pte(unsigned long adr)
 	if (!ttb)
 		arm_mmu_not_initialized_error();
 
-	if ((ttb[adr >> 20] & PMD_TYPE_MASK) != PMD_TYPE_TABLE) {
+	if ((ttb[pgd_index(adr)] & PMD_TYPE_MASK) != PMD_TYPE_TABLE) {
 		struct memory_bank *bank;
 		int i = 0;
 
@@ -132,7 +132,7 @@ static u32 *find_pte(unsigned long adr)
 	}
 
 	/* find the coarse page table base address */
-	table = (u32 *)(ttb[adr >> 20] & ~0x3ff);
+	table = (u32 *)(ttb[pgd_index(adr)] & ~0x3ff);
 
 	/* find second level descriptor */
 	return &table[(adr >> PAGE_SHIFT) & 0xff];
@@ -197,7 +197,7 @@ void *map_io_sections(unsigned long phys, void *_start, size_t size)
 	unsigned long start = (unsigned long)_start, sec;
 
 	for (sec = start; sec < start + size; sec += (1 << 20), phys += SZ_1M)
-		ttb[sec >> 20] = phys | PMD_SECT_DEF_UNCACHED;
+		ttb[pgd_index(sec)] = phys | PMD_SECT_DEF_UNCACHED;
 
 	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
 	tlb_invalidate();
@@ -211,8 +211,8 @@ void *map_io_sections(unsigned long phys, void *_start, size_t size)
 static int arm_mmu_remap_sdram(struct memory_bank *bank)
 {
 	unsigned long phys = (unsigned long)bank->start;
-	unsigned long ttb_start = phys >> 20;
-	unsigned long ttb_end = (phys >> 20) + (bank->size >> 20);
+	unsigned long ttb_start = pgd_index(phys);
+	unsigned long ttb_end = pgd_index(phys) + pgd_index(bank->size);
 	unsigned long num_ptes = bank->size >> 12;
 	int i, pte;
 	u32 *ptes;
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 04aa81732..13a10c7b7 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -4,6 +4,10 @@
 #include <asm/pgtable.h>
 #include <linux/sizes.h>
 
+#define PGDIR_SHIFT	20
+
+#define pgd_index(addr)		((addr) >> PGDIR_SHIFT)
+
 #ifdef CONFIG_MMU
 void __mmu_cache_on(void);
 void __mmu_cache_off(void);
@@ -31,8 +35,8 @@ static inline void
 __create_sections(uint32_t *ttb, unsigned long first,
 		  unsigned long last, unsigned int flags)
 {
-	unsigned long ttb_start = first >> 20;
-	unsigned long ttb_end   = (last >> 20) + 1;
+	unsigned long ttb_start = pgd_index(first);
+	unsigned long ttb_end   = pgd_index(last) + 1;
 	unsigned int i, addr;
 
 	for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
-- 
2.17.0


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

[PATCH v2 14/28] ARM: mmu: Trivial simplification in arm_mmu_remap_sdram()

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index e0365eec1..d8fb3afa4 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -212,7 +212,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 {
 	unsigned long phys = (unsigned long)bank->start;
 	unsigned long ttb_start = pgd_index(phys);
-	unsigned long ttb_end = pgd_index(phys) + pgd_index(bank->size);
+	unsigned long ttb_end = ttb_start + pgd_index(bank->size);
 	unsigned long num_ptes = bank->size >> 12;
 	int i, pte;
 	u32 *ptes;
-- 
2.17.0


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

[PATCH v2 15/28] ARM: mmu: Replace various SZ_1M with PGDIR_SIZE

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index d8fb3afa4..228fef433 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -196,7 +196,7 @@ void *map_io_sections(unsigned long phys, void *_start, size_t size)
 {
 	unsigned long start = (unsigned long)_start, sec;
 
-	for (sec = start; sec < start + size; sec += (1 << 20), phys += SZ_1M)
+	for (sec = start; sec < start + size; sec += PGDIR_SIZE, phys += PGDIR_SIZE)
 		ttb[pgd_index(sec)] = phys | PMD_SECT_DEF_UNCACHED;
 
 	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
@@ -224,7 +224,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 	 * We replace each 1MiB section in this range with second level page
 	 * tables, therefore we must have 1Mib aligment here.
 	 */
-	if (!IS_ALIGNED(phys, SZ_1M) || !IS_ALIGNED(bank->size, SZ_1M))
+	if (!IS_ALIGNED(phys, PGDIR_SIZE) || !IS_ALIGNED(bank->size, PGDIR_SIZE))
 		return -EINVAL;
 
 	ptes = xmemalign(PAGE_SIZE, num_ptes * sizeof(u32));
@@ -289,8 +289,8 @@ static void create_vector_table(unsigned long adr)
 		vectors = xmemalign(PAGE_SIZE, PAGE_SIZE);
 		pr_debug("Creating vector table, virt = 0x%p, phys = 0x%08lx\n",
 			 vectors, adr);
-		exc = arm_create_pte(ALIGN_DOWN(adr, SZ_1M));
-		idx = (adr & (SZ_1M - 1)) >> PAGE_SHIFT;
+		exc = arm_create_pte(ALIGN_DOWN(adr, PGDIR_SIZE));
+		idx = (adr & (PGDIR_SIZE - 1)) >> PAGE_SHIFT;
 		exc[idx] = (u32)vectors | PTE_TYPE_SMALL | pte_flags_cached;
 	}
 
diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
index 13a10c7b7..67cc670f0 100644
--- a/arch/arm/cpu/mmu.h
+++ b/arch/arm/cpu/mmu.h
@@ -5,6 +5,7 @@
 #include <linux/sizes.h>
 
 #define PGDIR_SHIFT	20
+#define PGDIR_SIZE	(1UL << PGDIR_SHIFT)
 
 #define pgd_index(addr)		((addr) >> PGDIR_SHIFT)
 
@@ -39,7 +40,7 @@ __create_sections(uint32_t *ttb, unsigned long first,
 	unsigned long ttb_end   = pgd_index(last) + 1;
 	unsigned int i, addr;
 
-	for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
+	for (i = ttb_start, addr = first; i < ttb_end; i++, addr += PGDIR_SIZE)
 		ttb[i] = addr | flags;
 }
 
-- 
2.17.0


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

[PATCH v2 16/28] ARM: mmu: Use PAGE_SIZE when specifying size of one page

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 228fef433..b4cc474d8 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -271,7 +271,7 @@ static void create_vector_table(unsigned long adr)
 	u32 *exc;
 	int idx;
 
-	vectors_sdram = request_sdram_region("vector table", adr, SZ_4K);
+	vectors_sdram = request_sdram_region("vector table", adr, PAGE_SIZE);
 	if (vectors_sdram) {
 		/*
 		 * The vector table address is inside the SDRAM physical
@@ -352,7 +352,7 @@ static void create_zero_page(void)
 	struct resource *zero_sdram;
 	u32 *zero;
 
-	zero_sdram = request_sdram_region("zero page", 0x0, SZ_4K);
+	zero_sdram = request_sdram_region("zero page", 0x0, PAGE_SIZE);
 	if (zero_sdram) {
 		/*
 		 * Here we would need to set the second level page table
-- 
2.17.0


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

[PATCH v2 17/28] ARM: mmu: Define and use PTRS_PER_PTE

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index b4cc474d8..d70516f9c 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -34,6 +34,7 @@
 #include "mmu.h"
 
 #define PMD_SECT_DEF_CACHED (PMD_SECT_WB | PMD_SECT_DEF_UNCACHED)
+#define PTRS_PER_PTE		(PGDIR_SIZE / PAGE_SIZE)
 
 static uint32_t *ttb;
 
@@ -91,14 +92,15 @@ static u32 *arm_create_pte(unsigned long virt)
 	u32 *table;
 	int i;
 
-	table = memalign(0x400, 0x400);
+	table = memalign(PTRS_PER_PTE * sizeof(u32),
+			 PTRS_PER_PTE * sizeof(u32));
 
 	if (!ttb)
 		arm_mmu_not_initialized_error();
 
 	ttb[pgd_index(virt)] = (unsigned long)table | PMD_TYPE_TABLE;
 
-	for (i = 0; i < 256; i++) {
+	for (i = 0; i < PTRS_PER_PTE; i++) {
 		table[i] = virt | PTE_TYPE_SMALL | pte_flags_uncached;
 		virt += PAGE_SIZE;
 	}
@@ -242,7 +244,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 	for (i = ttb_start; i < ttb_end; i++) {
 		ttb[i] = (unsigned long)(&ptes[pte]) | PMD_TYPE_TABLE |
 			(0 << 4);
-		pte += 256;
+		pte += PTRS_PER_PTE;
 	}
 
 	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
-- 
2.17.0


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

[PATCH v2 18/28] ARM: mmu: Use PAGE_SIZE instead of magic right shift by 12

[Andrey Smirnov]

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index d70516f9c..314867282 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -215,7 +215,7 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 	unsigned long phys = (unsigned long)bank->start;
 	unsigned long ttb_start = pgd_index(phys);
 	unsigned long ttb_end = ttb_start + pgd_index(bank->size);
-	unsigned long num_ptes = bank->size >> 12;
+	unsigned long num_ptes = bank->size / PAGE_SIZE;
 	int i, pte;
 	u32 *ptes;
 
-- 
2.17.0


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

[PATCH v2 19/28] ARM: mmu: Use xmemalign in arm_create_pte()

[Andrey Smirnov]

We don't handle the OOM case in that code, so using xmemalign seems
like a better option.

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 314867282..42341a7a3 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -92,8 +92,8 @@ static u32 *arm_create_pte(unsigned long virt)
 	u32 *table;
 	int i;
 
-	table = memalign(PTRS_PER_PTE * sizeof(u32),
-			 PTRS_PER_PTE * sizeof(u32));
+	table = xmemalign(PTRS_PER_PTE * sizeof(u32),
+			  PTRS_PER_PTE * sizeof(u32));
 
 	if (!ttb)
 		arm_mmu_not_initialized_error();
-- 
2.17.0


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

[PATCH v2 20/28] ARM: mmu: Use xmemalign in mmu_init()

[Andrey Smirnov]

We don't handle OOM case in that code, so using xmemalign seems like a
better option.

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 42341a7a3..5e8d439bf 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -450,7 +450,7 @@ static int mmu_init(void)
 			pr_crit("Critical Error: Can't request SDRAM region for ttb at %p\n",
 					ttb);
 	} else {
-		ttb = memalign(0x10000, 0x4000);
+		ttb = xmemalign(0x10000, 0x4000);
 	}
 
 	pr_debug("ttb: 0x%p\n", ttb);
-- 
2.17.0


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

[PATCH v2 21/28] ARM: mmu: Share code between dma_alloc_*() functions

[Andrey Smirnov]

Code of dma_alloc_coherent() and dma_alloc_writecombine() is almost
identical with exception of the flags passed to undelying call to
__remap_range(). Move commong code into a shared subroutine and
convert both functions to use it.

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 5e8d439bf..339ab8e82 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -500,7 +500,7 @@ void mmu_disable(void)
 	__mmu_cache_off();
 }
 
-void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
+static void *dma_alloc(size_t size, dma_addr_t *dma_handle, uint32_t pte_flags)
 {
 	void *ret;
 
@@ -511,25 +511,19 @@ void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 
 	dma_inv_range((unsigned long)ret, (unsigned long)ret + size);
 
-	__remap_range(ret, size, pte_flags_uncached);
+	__remap_range(ret, size, pte_flags);
 
 	return ret;
 }
 
-void *dma_alloc_writecombine(size_t size, dma_addr_t *dma_handle)
+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((unsigned long)ret, (unsigned long)ret + size);
-
-	__remap_range(ret, size, pte_flags_wc);
+	return dma_alloc(size, dma_handle, pte_flags_uncached);
+}
 
-	return ret;
+void *dma_alloc_writecombine(size_t size, dma_addr_t *dma_handle)
+{
+	return dma_alloc(size, dma_handle, pte_flags_wc);
 }
 
 unsigned long virt_to_phys(volatile void *virt)
-- 
2.17.0


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

[PATCH v2 22/28] ARM: mmu: Pass PTE flags a parameter to arm_create_pte()

[Andrey Smirnov]

In order to make it possible to use this functions in contexts where
creating a new PTE of uncached pages in not appropriate, pass PTE
flags a parameter to arm_create_pte() and fix all of the current users
as necessary.

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 339ab8e82..eeab0a749 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -87,7 +87,7 @@ static void arm_mmu_not_initialized_error(void)
  * We initially create a flat uncached mapping on it.
  * Not yet exported, but may be later if someone finds use for it.
  */
-static u32 *arm_create_pte(unsigned long virt)
+static u32 *arm_create_pte(unsigned long virt, uint32_t flags)
 {
 	u32 *table;
 	int i;
@@ -101,7 +101,7 @@ static u32 *arm_create_pte(unsigned long virt)
 	ttb[pgd_index(virt)] = (unsigned long)table | PMD_TYPE_TABLE;
 
 	for (i = 0; i < PTRS_PER_PTE; i++) {
-		table[i] = virt | PTE_TYPE_SMALL | pte_flags_uncached;
+		table[i] = virt | PTE_TYPE_SMALL | flags;
 		virt += PAGE_SIZE;
 	}
 
@@ -291,7 +291,8 @@ static void create_vector_table(unsigned long adr)
 		vectors = xmemalign(PAGE_SIZE, PAGE_SIZE);
 		pr_debug("Creating vector table, virt = 0x%p, phys = 0x%08lx\n",
 			 vectors, adr);
-		exc = arm_create_pte(ALIGN_DOWN(adr, PGDIR_SIZE));
+		exc = arm_create_pte(ALIGN_DOWN(adr, PGDIR_SIZE),
+				     pte_flags_uncached);
 		idx = (adr & (PGDIR_SIZE - 1)) >> PAGE_SHIFT;
 		exc[idx] = (u32)vectors | PTE_TYPE_SMALL | pte_flags_cached;
 	}
@@ -362,7 +363,7 @@ static void create_zero_page(void)
 		 */
 		pr_debug("zero page is in SDRAM area, currently not supported\n");
 	} else {
-		zero = arm_create_pte(0x0);
+		zero = arm_create_pte(0x0, pte_flags_uncached);
 		zero[0] = 0;
 		pr_debug("Created zero page\n");
 	}
-- 
2.17.0


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

[PATCH v2 23/28] ARM: mmu: Make sure that address is 1M aligned in arm_create_pte()

[Andrey Smirnov]

If address passed arm_create_pte() is not 1M (PGDIR_SIZE) aligned,
page table that is created will end up having unexpected mapping
offset, breaking "1:1 mapping" assumption and leading to bugs that are
not immediately obvious in their nature.

To prevent this and because all of the callers already do said
alignement in-place, move the alignment code to be a part of
are_create_pte().

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index eeab0a749..94388f11a 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -92,6 +92,8 @@ static u32 *arm_create_pte(unsigned long virt, uint32_t flags)
 	u32 *table;
 	int i;
 
+	virt = ALIGN_DOWN(virt, PGDIR_SIZE);
+
 	table = xmemalign(PTRS_PER_PTE * sizeof(u32),
 			  PTRS_PER_PTE * sizeof(u32));
 
@@ -291,8 +293,7 @@ static void create_vector_table(unsigned long adr)
 		vectors = xmemalign(PAGE_SIZE, PAGE_SIZE);
 		pr_debug("Creating vector table, virt = 0x%p, phys = 0x%08lx\n",
 			 vectors, adr);
-		exc = arm_create_pte(ALIGN_DOWN(adr, PGDIR_SIZE),
-				     pte_flags_uncached);
+		exc = arm_create_pte(adr, pte_flags_uncached);
 		idx = (adr & (PGDIR_SIZE - 1)) >> PAGE_SHIFT;
 		exc[idx] = (u32)vectors | PTE_TYPE_SMALL | pte_flags_cached;
 	}
-- 
2.17.0


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

[PATCH v2 24/28] ARM: mmu: Use find_pte() to find PTE in create_vector_table()

[Andrey Smirnov]

There's already a function that implement necessary arithemtic to find
offset within page table for a given address, so make use of it
instead of re-implementing it again.

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 94388f11a..ceb854ac8 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -272,8 +272,7 @@ static void create_vector_table(unsigned long adr)
 {
 	struct resource *vectors_sdram;
 	void *vectors;
-	u32 *exc;
-	int idx;
+	u32 *pte;
 
 	vectors_sdram = request_sdram_region("vector table", adr, PAGE_SIZE);
 	if (vectors_sdram) {
@@ -293,9 +292,9 @@ static void create_vector_table(unsigned long adr)
 		vectors = xmemalign(PAGE_SIZE, PAGE_SIZE);
 		pr_debug("Creating vector table, virt = 0x%p, phys = 0x%08lx\n",
 			 vectors, adr);
-		exc = arm_create_pte(adr, pte_flags_uncached);
-		idx = (adr & (PGDIR_SIZE - 1)) >> PAGE_SHIFT;
-		exc[idx] = (u32)vectors | PTE_TYPE_SMALL | pte_flags_cached;
+		arm_create_pte(adr, pte_flags_uncached);
+		pte = find_pte(adr);
+		*pte = (u32)vectors | PTE_TYPE_SMALL | pte_flags_cached;
 	}
 
 	arm_fixup_vectors();
-- 
2.17.0


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

[PATCH v2 25/28] ARM: mmu: Use dma_inv_range() in dma_sync_single_for_cpu()

[Andrey Smirnov]

The code in the if () statement is identical to already existing
dma_inv_rand(). Use it instead.

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index ceb854ac8..b14a869af 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -548,11 +548,8 @@ void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
 void dma_sync_single_for_cpu(dma_addr_t address, size_t size,
 			     enum dma_data_direction dir)
 {
-	if (dir != DMA_TO_DEVICE) {
-		if (outer_cache.inv_range)
-			outer_cache.inv_range(address, address + size);
-		__dma_inv_range(address, address + size);
-	}
+	if (dir != DMA_TO_DEVICE)
+		dma_inv_range(address, address + size);
 }
 
 void dma_sync_single_for_device(dma_addr_t address, size_t size,
-- 
2.17.0


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

[PATCH v2 26/28] ARM: mmu: Simplify the use of dma_flush_range()

[Andrey Smirnov]

Simplify the use of dma_flush_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.

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index b14a869af..30f321691 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -142,8 +142,11 @@ static u32 *find_pte(unsigned long adr)
 	return &table[(adr >> PAGE_SHIFT) & 0xff];
 }
 
-static void dma_flush_range(unsigned long start, unsigned long end)
+static void dma_flush_range(void *ptr, size_t size)
 {
+	unsigned long start = (unsigned long)ptr;
+	unsigned long end = start + size;
+
 	__dma_flush_range(start, end);
 	if (outer_cache.flush_range)
 		outer_cache.flush_range(start, end);
@@ -170,9 +173,7 @@ static int __remap_range(void *_start, size_t size, u32 pte_flags)
 		p[i] |= pte_flags | PTE_TYPE_SMALL;
 	}
 
-	dma_flush_range((unsigned long)p,
-			(unsigned long)p + numentries * sizeof(u32));
-
+	dma_flush_range(p, numentries * sizeof(u32));
 	tlb_invalidate();
 
 	return 0;
@@ -203,7 +204,7 @@ void *map_io_sections(unsigned long phys, void *_start, size_t size)
 	for (sec = start; sec < start + size; sec += PGDIR_SIZE, phys += PGDIR_SIZE)
 		ttb[pgd_index(sec)] = phys | PMD_SECT_DEF_UNCACHED;
 
-	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
+	dma_flush_range(ttb, 0x4000);
 	tlb_invalidate();
 	return _start;
 }
@@ -249,9 +250,8 @@ static int arm_mmu_remap_sdram(struct memory_bank *bank)
 		pte += PTRS_PER_PTE;
 	}
 
-	dma_flush_range((unsigned long)ttb, (unsigned long)ttb + 0x4000);
-	dma_flush_range((unsigned long)ptes,
-			(unsigned long)ptes + num_ptes * sizeof(u32));
+	dma_flush_range(ttb, 0x4000);
+	dma_flush_range(ptes, num_ptes * sizeof(u32));
 
 	tlb_invalidate();
 
-- 
2.17.0


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

[PATCH v2 27/28] ARM: mmu: Implement on-demand PTE allocation

[Andrey Smirnov]

Allocating PTEs for every 4K page corresponding to SDRAM upfront costs
us quite a bit of memory: 1KB per 1MB or RAM. This is far from being a
deal-breaker for majority of use-cases, but for builds where amount of
free memory is in hundres of KBs* it becomes a real hurdle for being
able to use MMU (which also means no L1 cache).

Given how we really only need PTEs for a very few regions of memory
dedicated from DMA buffers (Ethernet, USB, etc), changing MMU code to
do on-demand section splitting can allow us to save significant amount
of memory without any functionality loss.

Below is a very trivial comparison of memory usages on start before
and after this patch is applied.

Before:

barebox@ZII VF610 Development Board, Rev B:/ meminfo
used: 1271584
free: 265553032

After:

barebox@ZII VF610 Development Board, Rev B:/ meminfo
used: 795276
free: 266024448

Tested on:

       - VF610 Tower Board,
       - VF610 ZII Development Board (Rev. C)
       - i.MX51 Babbage Board
       - i.MX7 SabreSD Board
       - i.MX6 ZII RDU2 Board
       - AT91SAM9X5-EK Board

* One example of such use-case is memory testing while running purely
  out of SRAM

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

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index 30f321691..fdb919eb8 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -35,6 +35,7 @@
 
 #define PMD_SECT_DEF_CACHED (PMD_SECT_WB | PMD_SECT_DEF_UNCACHED)
 #define PTRS_PER_PTE		(PGDIR_SIZE / PAGE_SIZE)
+#define ARCH_MAP_WRITECOMBINE	((unsigned)-1)
 
 static uint32_t *ttb;
 
@@ -60,6 +61,7 @@ static inline void tlb_invalidate(void)
 #define PTE_FLAGS_UNCACHED_V7 (0)
 #define PTE_FLAGS_CACHED_V4 (PTE_SMALL_AP_UNO_SRW | PTE_BUFFERABLE | PTE_CACHEABLE)
 #define PTE_FLAGS_UNCACHED_V4 PTE_SMALL_AP_UNO_SRW
+#define PGD_FLAGS_WC_V7 PMD_SECT_TEX(1)
 
 /*
  * PTE flags to set cached and uncached areas.
@@ -68,6 +70,7 @@ static inline void tlb_invalidate(void)
 static uint32_t pte_flags_cached;
 static uint32_t pte_flags_wc;
 static uint32_t pte_flags_uncached;
+static uint32_t pgd_flags_wc;
 
 #define PTE_MASK ((1 << 12) - 1)
 
@@ -110,6 +113,11 @@ static u32 *arm_create_pte(unsigned long virt, uint32_t flags)
 	return table;
 }
 
+static bool pgd_type_table(u32 pgd)
+{
+	return (pgd & PMD_TYPE_MASK) == PMD_TYPE_TABLE;
+}
+
 static u32 *find_pte(unsigned long adr)
 {
 	u32 *table;
@@ -117,23 +125,8 @@ static u32 *find_pte(unsigned long adr)
 	if (!ttb)
 		arm_mmu_not_initialized_error();
 
-	if ((ttb[pgd_index(adr)] & PMD_TYPE_MASK) != PMD_TYPE_TABLE) {
-		struct memory_bank *bank;
-		int i = 0;
-
-		/*
-		 * This should only be called for page mapped memory inside our
-		 * memory banks. It's a bug to call it with section mapped memory
-		 * locations.
-		 */
-		pr_crit("%s: TTB for address 0x%08lx is not of type table\n",
-				__func__, adr);
-		pr_crit("Memory banks:\n");
-		for_each_memory_bank(bank)
-			pr_crit("#%d 0x%08lx - 0x%08lx\n", i, bank->start,
-					bank->start + bank->size - 1);
-		BUG();
-	}
+	if (!pgd_type_table(ttb[pgd_index(adr)]))
+		return NULL;
 
 	/* find the coarse page table base address */
 	table = (u32 *)(ttb[pgd_index(adr)] & ~0x3ff);
@@ -159,42 +152,115 @@ static void dma_inv_range(unsigned long start, unsigned long end)
 	__dma_inv_range(start, end);
 }
 
-static int __remap_range(void *_start, size_t size, u32 pte_flags)
-{
-	unsigned long start = (unsigned long)_start;
-	u32 *p;
-	int numentries, i;
-
-	numentries = size >> PAGE_SHIFT;
-	p = find_pte(start);
-
-	for (i = 0; i < numentries; i++) {
-		p[i] &= ~PTE_MASK;
-		p[i] |= pte_flags | PTE_TYPE_SMALL;
-	}
-
-	dma_flush_range(p, numentries * sizeof(u32));
-	tlb_invalidate();
-
-	return 0;
-}
-
 int arch_remap_range(void *start, size_t size, unsigned flags)
 {
+	u32 addr = (u32)start;
 	u32 pte_flags;
+	u32 pgd_flags;
+
+	BUG_ON(!IS_ALIGNED(addr, PAGE_SIZE));
 
 	switch (flags) {
 	case MAP_CACHED:
 		pte_flags = pte_flags_cached;
+		pgd_flags = PMD_SECT_DEF_CACHED;
 		break;
 	case MAP_UNCACHED:
 		pte_flags = pte_flags_uncached;
+		pgd_flags = PMD_SECT_DEF_UNCACHED;
+		break;
+	case ARCH_MAP_WRITECOMBINE:
+		pte_flags = pte_flags_wc;
+		pgd_flags = pgd_flags_wc;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	return __remap_range(start, size, pte_flags);
+	while (size) {
+		const bool pgdir_size_aligned = IS_ALIGNED(addr, PGDIR_SIZE);
+		u32 *pgd = (u32 *)&ttb[pgd_index(addr)];
+		size_t chunk;
+
+		if (size >= PGDIR_SIZE && pgdir_size_aligned &&
+		    !pgd_type_table(*pgd)) {
+			/*
+			 * TODO: Add code to discard a page table and
+			 * replace it with a section
+			 */
+			chunk = PGDIR_SIZE;
+			*pgd = addr | pgd_flags;
+			dma_flush_range(pgd, sizeof(*pgd));
+		} else {
+			unsigned int num_ptes;
+			u32 *table = NULL;
+			unsigned int i;
+			u32 *pte;
+			/*
+			 * We only want to cover pages up until next
+			 * section boundary in case there we would
+			 * have an opportunity to re-map the whole
+			 * section (say if we got here becasue address
+			 * was not aligned on PGDIR_SIZE boundary)
+			 */
+			chunk = pgdir_size_aligned ?
+				PGDIR_SIZE : ALIGN(addr, PGDIR_SIZE) - addr;
+			/*
+			 * At the same time we want to make sure that
+			 * we don't go on remapping past requested
+			 * size in case that is less that the distance
+			 * to next PGDIR_SIZE boundary.
+			 */
+			chunk = min(chunk, size);
+			num_ptes = chunk / PAGE_SIZE;
+
+			pte = find_pte(addr);
+			if (!pte) {
+				/*
+				 * If PTE is not found it means that
+				 * we needs to split this section and
+				 * create a new page table for it
+				 *
+				 * NOTE: Here we assume that section
+				 * we just split was mapped as cached
+				 */
+				table = arm_create_pte(addr, pte_flags_cached);
+				pte = find_pte(addr);
+				BUG_ON(!pte);
+				/*
+				 * We just converted split this
+				 * section and modified it's Level 1
+				 * descriptor, so we mark it to be
+				 * flushed
+				 */
+				dma_flush_range(pgd, sizeof(*pgd));
+			}
+
+			for (i = 0; i < num_ptes; i++) {
+				pte[i] &= ~PTE_MASK;
+				pte[i] |= pte_flags | PTE_TYPE_SMALL;
+			}
+
+			if (table) {
+				/*
+				 * If we just created a new page
+				 * table, the whole table would have
+				 * to be flushed, not just PTEs that
+				 * we touched when re-mapping.
+				 */
+				pte = table;
+				num_ptes = PTRS_PER_PTE;
+			}
+
+			dma_flush_range(pte, num_ptes * sizeof(u32));
+		}
+
+		addr += chunk;
+		size -= chunk;
+	}
+
+	tlb_invalidate();
+	return 0;
 }
 
 void *map_io_sections(unsigned long phys, void *_start, size_t size)
@@ -209,55 +275,6 @@ void *map_io_sections(unsigned long phys, void *_start, size_t size)
 	return _start;
 }
 
-/*
- * remap the memory bank described by mem cachable and
- * bufferable
- */
-static int arm_mmu_remap_sdram(struct memory_bank *bank)
-{
-	unsigned long phys = (unsigned long)bank->start;
-	unsigned long ttb_start = pgd_index(phys);
-	unsigned long ttb_end = ttb_start + pgd_index(bank->size);
-	unsigned long num_ptes = bank->size / PAGE_SIZE;
-	int i, pte;
-	u32 *ptes;
-
-	pr_debug("remapping SDRAM from 0x%08lx (size 0x%08lx)\n",
-			phys, bank->size);
-
-	/*
-	 * We replace each 1MiB section in this range with second level page
-	 * tables, therefore we must have 1Mib aligment here.
-	 */
-	if (!IS_ALIGNED(phys, PGDIR_SIZE) || !IS_ALIGNED(bank->size, PGDIR_SIZE))
-		return -EINVAL;
-
-	ptes = xmemalign(PAGE_SIZE, num_ptes * sizeof(u32));
-
-	pr_debug("ptes: 0x%p ttb_start: 0x%08lx ttb_end: 0x%08lx\n",
-			ptes, ttb_start, ttb_end);
-
-	for (i = 0; i < num_ptes; i++) {
-		ptes[i] = (phys + i * PAGE_SIZE) | PTE_TYPE_SMALL |
-			pte_flags_cached;
-	}
-
-	pte = 0;
-
-	for (i = ttb_start; i < ttb_end; i++) {
-		ttb[i] = (unsigned long)(&ptes[pte]) | PMD_TYPE_TABLE |
-			(0 << 4);
-		pte += PTRS_PER_PTE;
-	}
-
-	dma_flush_range(ttb, 0x4000);
-	dma_flush_range(ptes, num_ptes * sizeof(u32));
-
-	tlb_invalidate();
-
-	return 0;
-}
-
 #define ARM_HIGH_VECTORS	0xffff0000
 #define ARM_LOW_VECTORS		0x0
 
@@ -423,10 +440,12 @@ static int mmu_init(void)
 	if (cpu_architecture() >= CPU_ARCH_ARMv7) {
 		pte_flags_cached = PTE_FLAGS_CACHED_V7;
 		pte_flags_wc = PTE_FLAGS_WC_V7;
+		pgd_flags_wc = PGD_FLAGS_WC_V7;
 		pte_flags_uncached = PTE_FLAGS_UNCACHED_V7;
 	} else {
 		pte_flags_cached = PTE_FLAGS_CACHED_V4;
 		pte_flags_wc = PTE_FLAGS_UNCACHED_V4;
+		pgd_flags_wc = PMD_SECT_DEF_UNCACHED;
 		pte_flags_uncached = PTE_FLAGS_UNCACHED_V4;
 	}
 
@@ -477,13 +496,6 @@ static int mmu_init(void)
 
 	__mmu_cache_on();
 
-	/*
-	 * Now that we have the MMU and caches on remap sdram again using
-	 * page tables
-	 */
-	for_each_memory_bank(bank)
-		arm_mmu_remap_sdram(bank);
-
 	return 0;
 }
 mmu_initcall(mmu_init);
@@ -501,7 +513,7 @@ void mmu_disable(void)
 	__mmu_cache_off();
 }
 
-static void *dma_alloc(size_t size, dma_addr_t *dma_handle, uint32_t pte_flags)
+static void *dma_alloc(size_t size, dma_addr_t *dma_handle, unsigned flags)
 {
 	void *ret;
 
@@ -512,19 +524,19 @@ static void *dma_alloc(size_t size, dma_addr_t *dma_handle, uint32_t pte_flags)
 
 	dma_inv_range((unsigned long)ret, (unsigned long)ret + size);
 
-	__remap_range(ret, size, pte_flags);
+	arch_remap_range(ret, size, flags);
 
 	return ret;
 }
 
 void *dma_alloc_coherent(size_t size, dma_addr_t *dma_handle)
 {
-	return dma_alloc(size, dma_handle, pte_flags_uncached);
+	return dma_alloc(size, dma_handle, MAP_UNCACHED);
 }
 
 void *dma_alloc_writecombine(size_t size, dma_addr_t *dma_handle)
 {
-	return dma_alloc(size, dma_handle, pte_flags_wc);
+	return dma_alloc(size, dma_handle, ARCH_MAP_WRITECOMBINE);
 }
 
 unsigned long virt_to_phys(volatile void *virt)
@@ -540,7 +552,7 @@ void *phys_to_virt(unsigned long phys)
 void dma_free_coherent(void *mem, dma_addr_t dma_handle, size_t size)
 {
 	size = PAGE_ALIGN(size);
-	__remap_range(mem, size, pte_flags_cached);
+	arch_remap_range(mem, size, MAP_CACHED);
 
 	free(mem);
 }
-- 
2.17.0


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

[PATCH v2 28/28] ARM: mmu: Introduce ARM_TTB_SIZE

[Andrey Smirnov]

Commit 1c33aacf8a247ab45814b43ac0ca903677afffae ("ARM: use memalign to
allocate page table"), reasonalby changed TTB allocation size from
SZ_32K to SZ_16K (TTB's real size), but it also changed alignment from
SZ_16K to SZ_64K for unclear reasons.

Reading various TTBR related ARM documentation it seems that worst
case alignment for it is 16KiB (bits [0, 13 - N] must be zero) which
also matches early TTB allocation code.

Since both early and regular MMU code has to share this paramter,
introduce ARM_TTB_SIZE and use it in both cases for both size and
alignment.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/cpu/mmu.c                 | 2 +-
 arch/arm/include/asm/barebox-arm.h | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
index fdb919eb8..0fe1efae1 100644
--- a/arch/arm/cpu/mmu.c
+++ b/arch/arm/cpu/mmu.c
@@ -470,7 +470,7 @@ static int mmu_init(void)
 			pr_crit("Critical Error: Can't request SDRAM region for ttb at %p\n",
 					ttb);
 	} else {
-		ttb = xmemalign(0x10000, 0x4000);
+		ttb = xmemalign(ARM_TTB_SIZE, ARM_TTB_SIZE);
 	}
 
 	pr_debug("ttb: 0x%p\n", ttb);
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index f55c8bb0c..d97f68c54 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -32,6 +32,12 @@
 #include <linux/compiler.h>
 #include <asm/barebox-arm-head.h>
 
+/*
+ * We have a 4GiB address space split into 1MiB sections, with each
+ * section header taking 4 bytes
+ */
+#define ARM_TTB_SIZE	(SZ_4G / SZ_1M * sizeof(u32))
+
 unsigned long get_runtime_offset(void);
 
 /* global_variable_offset() - Access global variables when not running at link address
@@ -117,7 +123,7 @@ static inline unsigned long arm_mem_ttb(unsigned long membase,
 					unsigned long endmem)
 {
 	endmem = arm_mem_stack(membase, endmem);
-	endmem = ALIGN_DOWN(endmem, SZ_16K) - SZ_16K;
+	endmem = ALIGN_DOWN(endmem, ARM_TTB_SIZE) - ARM_TTB_SIZE;
 
 	return endmem;
 }
-- 
2.17.0


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

Re: [PATCH v2 06/28] ARM: mmu: Share code for create_sections()

[Sam Ravnborg]

Hi Andrey

On Wed, May 16, 2018 at 01:00:14PM -0700, Andrey Smirnov wrote:
> Regular MMU code never creates anything but 1:1 mapping, and barring
> that plus the call to __mmu_cache_flush(), early MMU code version of
> the function is pretty much identical. To avoid code duplication, move
> it to mmu.h and convert both regular and early MMU code to use it.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/mmu-early.c | 14 ++------------
>  arch/arm/cpu/mmu.c       | 25 +++++++------------------
>  arch/arm/cpu/mmu.h       |  8 ++++++++
>  3 files changed, 17 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
> index f75cc7e4a..70ece0d2f 100644
> --- a/arch/arm/cpu/mmu-early.c
> +++ b/arch/arm/cpu/mmu-early.c
> @@ -11,22 +11,12 @@
>  
>  static uint32_t *ttb;
>  
> -static void create_sections(unsigned long addr, int size_m, unsigned int flags)
> -{
> -	int i;
> -
> -	addr >>= 20;
> -
> -	for (i = size_m; i > 0; i--, addr++)
> -		ttb[addr] = (addr << 20) | flags;
> -}
This iterates in the for loop size_m times.

> -
> -static void create_sections(unsigned long virt, unsigned long phys, int size_m,
> -		unsigned int flags)
> -{
> -	int i;
> -
> -	phys >>= 20;
> -	virt >>= 20;
> -
> -	for (i = size_m; i > 0; i--, virt++, phys++)
> -		ttb[virt] = (phys << 20) | flags;
> -
> -	__mmu_cache_flush();
> -}
likewise

> +++ b/arch/arm/cpu/mmu.h
> @@ -24,5 +24,13 @@ static inline void set_domain(unsigned val)
>  	asm volatile ("mcr  p15,0,%0,c3,c0,0" : : "r"(val) /*:*/);
>  }
>  
> +static inline void
> +create_sections(uint32_t *ttb, unsigned long addr,
> +		int size_m, unsigned int flags)
> +{
> +	for (addr >>= 20; addr < size_m; addr++)
> +		ttb[addr] = (addr << 20) | flags;
> +}

But this iterates in the for loop while addr >> 20 is less than size_m.
I cannot see from the code nor the changelog if this is an intentional change

(I only stumbled over this while browsing the patch, no testing done)

	Sam

> +
>  
>  #endif /* __ARM_MMU_H */
> -- 
> 2.17.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

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

Re: [PATCH v2 07/28] ARM: mmu: Separate index and address in create_sections()

[Sam Ravnborg]

On Wed, May 16, 2018 at 01:00:15PM -0700, Andrey Smirnov wrote:
> Both TTB index and address used to fill that entry are derived from
> the same variable 'addr' which requires shifting right and left by 20
> and somewhat confusing.
> 
> Split the counter used to iterate over elements of  TTB into a
> separate variable to make this code a bit easier to read.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/mmu.h | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
> index 59f72049f..d71cd7e38 100644
> --- a/arch/arm/cpu/mmu.h
> +++ b/arch/arm/cpu/mmu.h
> @@ -1,6 +1,8 @@
>  #ifndef __ARM_MMU_H
>  #define __ARM_MMU_H
>  
> +#include <linux/sizes.h>
> +
>  #ifdef CONFIG_MMU
>  void __mmu_cache_on(void);
>  void __mmu_cache_off(void);
> @@ -28,8 +30,12 @@ static inline void
>  create_sections(uint32_t *ttb, unsigned long addr,
>  		int size_m, unsigned int flags)
>  {
> -	for (addr >>= 20; addr < size_m; addr++)
> -		ttb[addr] = (addr << 20) | flags;
> +	unsigned long ttb_start = addr >> 20;
> +	unsigned long ttb_end   = ttb_start + size_m;
> +	unsigned int i;
> +
> +	for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
> +		ttb[i] = addr | flags;
>  }

Much more readable - thanks.
Of personal taste I would have put the assignment of addr inside the
for loop to IMO increase readability.

	Sam

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

Re: [PATCH v2 06/28] ARM: mmu: Share code for create_sections()

[Andrey Smirnov]

On Wed, May 16, 2018 at 1:49 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> Hi Andrey
>
> On Wed, May 16, 2018 at 01:00:14PM -0700, Andrey Smirnov wrote:
>> Regular MMU code never creates anything but 1:1 mapping, and barring
>> that plus the call to __mmu_cache_flush(), early MMU code version of
>> the function is pretty much identical. To avoid code duplication, move
>> it to mmu.h and convert both regular and early MMU code to use it.
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  arch/arm/cpu/mmu-early.c | 14 ++------------
>>  arch/arm/cpu/mmu.c       | 25 +++++++------------------
>>  arch/arm/cpu/mmu.h       |  8 ++++++++
>>  3 files changed, 17 insertions(+), 30 deletions(-)
>>
>> diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
>> index f75cc7e4a..70ece0d2f 100644
>> --- a/arch/arm/cpu/mmu-early.c
>> +++ b/arch/arm/cpu/mmu-early.c
>> @@ -11,22 +11,12 @@
>>
>>  static uint32_t *ttb;
>>
>> -static void create_sections(unsigned long addr, int size_m, unsigned int flags)
>> -{
>> -     int i;
>> -
>> -     addr >>= 20;
>> -
>> -     for (i = size_m; i > 0; i--, addr++)
>> -             ttb[addr] = (addr << 20) | flags;
>> -}
> This iterates in the for loop size_m times.
>
>> -
>> -static void create_sections(unsigned long virt, unsigned long phys, int size_m,
>> -             unsigned int flags)
>> -{
>> -     int i;
>> -
>> -     phys >>= 20;
>> -     virt >>= 20;
>> -
>> -     for (i = size_m; i > 0; i--, virt++, phys++)
>> -             ttb[virt] = (phys << 20) | flags;
>> -
>> -     __mmu_cache_flush();
>> -}
> likewise
>
>> +++ b/arch/arm/cpu/mmu.h
>> @@ -24,5 +24,13 @@ static inline void set_domain(unsigned val)
>>       asm volatile ("mcr  p15,0,%0,c3,c0,0" : : "r"(val) /*:*/);
>>  }
>>
>> +static inline void
>> +create_sections(uint32_t *ttb, unsigned long addr,
>> +             int size_m, unsigned int flags)
>> +{
>> +     for (addr >>= 20; addr < size_m; addr++)
>> +             ttb[addr] = (addr << 20) | flags;
>> +}
>
> But this iterates in the for loop while addr >> 20 is less than size_m.
> I cannot see from the code nor the changelog if this is an intentional change
>
> (I only stumbled over this while browsing the patch, no testing done)
>

Good catch! Not intentional at all. Will fix in v3.

Thanks,
Andrey Smirnov

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

Re: [PATCH v2 07/28] ARM: mmu: Separate index and address in create_sections()

[Andrey Smirnov]

On Wed, May 16, 2018 at 1:52 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> On Wed, May 16, 2018 at 01:00:15PM -0700, Andrey Smirnov wrote:
>> Both TTB index and address used to fill that entry are derived from
>> the same variable 'addr' which requires shifting right and left by 20
>> and somewhat confusing.
>>
>> Split the counter used to iterate over elements of  TTB into a
>> separate variable to make this code a bit easier to read.
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  arch/arm/cpu/mmu.h | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
>> index 59f72049f..d71cd7e38 100644
>> --- a/arch/arm/cpu/mmu.h
>> +++ b/arch/arm/cpu/mmu.h
>> @@ -1,6 +1,8 @@
>>  #ifndef __ARM_MMU_H
>>  #define __ARM_MMU_H
>>
>> +#include <linux/sizes.h>
>> +
>>  #ifdef CONFIG_MMU
>>  void __mmu_cache_on(void);
>>  void __mmu_cache_off(void);
>> @@ -28,8 +30,12 @@ static inline void
>>  create_sections(uint32_t *ttb, unsigned long addr,
>>               int size_m, unsigned int flags)
>>  {
>> -     for (addr >>= 20; addr < size_m; addr++)
>> -             ttb[addr] = (addr << 20) | flags;
>> +     unsigned long ttb_start = addr >> 20;
>> +     unsigned long ttb_end   = ttb_start + size_m;
>> +     unsigned int i;
>> +
>> +     for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
>> +             ttb[i] = addr | flags;
>>  }
>
> Much more readable - thanks.
> Of personal taste I would have put the assignment of addr inside the
> for loop to IMO increase readability.
>

Sure, I'll do that in v3.

Thanks,
Andrey Smrinov

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

Re: [PATCH v2 09/28] ARM: mmu: Specify size in bytes in create_sections()

[Sascha Hauer]

On Wed, May 16, 2018 at 01:00:17PM -0700, Andrey Smirnov wrote:
> Seeing
> 
>         create_sections(ttb, 0, PAGE_SIZE, ...);
> 
> as the code the creates initial flat 4 GiB mapping is a bit less
> intuitive then
> 
>         create_sections(ttb, 0, SZ_4G, ...);
> 
> so, for the sake of clarification, convert create_sections() to accept
> size in bytes and do bytes -> MiB converstion as a part of the
> function.
> 
> NOTE: To keep all of the arguments of create_sections() 32-bit:
> 
>     - Move all of the real code into a helper function accepting first
>       and last addresses of the region (e.g passing 0 and U32_MAX
>       means all 4GiB of address space)
> 
>     - Convert create_section() into a macro that does necessary size
>       -> last addres conversion under the hood to preserve original API
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  arch/arm/cpu/mmu-early.c |  4 ++--
>  arch/arm/cpu/mmu.c       |  4 ++--
>  arch/arm/cpu/mmu.h       | 22 ++++++++++++++++------
>  3 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
> index 70ece0d2f..136b33c3a 100644
> --- a/arch/arm/cpu/mmu-early.c
> +++ b/arch/arm/cpu/mmu-early.c
> @@ -16,7 +16,7 @@ static void map_cachable(unsigned long start, unsigned long size)
>  	start = ALIGN_DOWN(start, SZ_1M);
>  	size  = ALIGN(size, SZ_1M);
>  
> -	create_sections(ttb, start, size >> 20, PMD_SECT_AP_WRITE |
> +	create_sections(ttb, start, size, PMD_SECT_AP_WRITE |
>  			PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
>  }
>  
> @@ -30,7 +30,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
>  	set_ttbr(ttb);
>  	set_domain(DOMAIN_MANAGER);
>  
> -	create_sections(ttb, 0, 4096, PMD_SECT_AP_WRITE |
> +	create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE |
>  			PMD_SECT_AP_READ | PMD_TYPE_SECT);
>  
>  	map_cachable(membase, memsize);
> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> index 0c367e47c..f02c99f65 100644
> --- a/arch/arm/cpu/mmu.c
> +++ b/arch/arm/cpu/mmu.c
> @@ -460,7 +460,7 @@ static int mmu_init(void)
>  	set_domain(DOMAIN_MANAGER);
>  
>  	/* create a flat mapping using 1MiB sections */
> -	create_sections(ttb, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
> +	create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
>  			PMD_TYPE_SECT);
>  	__mmu_cache_flush();
>  
> @@ -472,7 +472,7 @@ static int mmu_init(void)
>  	 * below
>  	 */
>  	for_each_memory_bank(bank) {
> -		create_sections(ttb, bank->start, bank->size >> 20,
> +		create_sections(ttb, bank->start, bank->size,
>  				PMD_SECT_DEF_CACHED);
>  		__mmu_cache_flush();
>  	}
> diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
> index d71cd7e38..52689359a 100644
> --- a/arch/arm/cpu/mmu.h
> +++ b/arch/arm/cpu/mmu.h
> @@ -27,16 +27,26 @@ static inline void set_domain(unsigned val)
>  }
>  
>  static inline void
> -create_sections(uint32_t *ttb, unsigned long addr,
> -		int size_m, unsigned int flags)
> +__create_sections(uint32_t *ttb, unsigned long first,
> +		  unsigned long last, unsigned int flags)
>  {
> -	unsigned long ttb_start = addr >> 20;
> -	unsigned long ttb_end   = ttb_start + size_m;
> -	unsigned int i;
> +	unsigned long ttb_start = first >> 20;
> +	unsigned long ttb_end   = (last >> 20) + 1;
> +	unsigned int i, addr;
>  
> -	for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
> +	for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
>  		ttb[i] = addr | flags;
>  }
>  
> +#define create_sections(ttb, addr, size, flags)				\
> +	({								\
> +		typeof(addr) __addr = addr;				\
> +		typeof(size) __size = size;				\
> +		/* Check for overflow */				\
> +		BUG_ON(__addr > ULONG_MAX - __size + 1);		\
> +		__create_sections(ttb, __addr, __addr + (size)  - 1,	\
> +				  flags);				\
> +	})

Why do you preserve the original API of create_sections()? I would just
change it. We have only a few callers and they are easy to change.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

Re: [PATCH v2 09/28] ARM: mmu: Specify size in bytes in create_sections()

[Andrey Smirnov]

On Wed, May 16, 2018 at 11:55 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, May 16, 2018 at 01:00:17PM -0700, Andrey Smirnov wrote:
>> Seeing
>>
>>         create_sections(ttb, 0, PAGE_SIZE, ...);
>>
>> as the code the creates initial flat 4 GiB mapping is a bit less
>> intuitive then
>>
>>         create_sections(ttb, 0, SZ_4G, ...);
>>
>> so, for the sake of clarification, convert create_sections() to accept
>> size in bytes and do bytes -> MiB converstion as a part of the
>> function.
>>
>> NOTE: To keep all of the arguments of create_sections() 32-bit:
>>
>>     - Move all of the real code into a helper function accepting first
>>       and last addresses of the region (e.g passing 0 and U32_MAX
>>       means all 4GiB of address space)
>>
>>     - Convert create_section() into a macro that does necessary size
>>       -> last addres conversion under the hood to preserve original API
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  arch/arm/cpu/mmu-early.c |  4 ++--
>>  arch/arm/cpu/mmu.c       |  4 ++--
>>  arch/arm/cpu/mmu.h       | 22 ++++++++++++++++------
>>  3 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
>> index 70ece0d2f..136b33c3a 100644
>> --- a/arch/arm/cpu/mmu-early.c
>> +++ b/arch/arm/cpu/mmu-early.c
>> @@ -16,7 +16,7 @@ static void map_cachable(unsigned long start, unsigned long size)
>>       start = ALIGN_DOWN(start, SZ_1M);
>>       size  = ALIGN(size, SZ_1M);
>>
>> -     create_sections(ttb, start, size >> 20, PMD_SECT_AP_WRITE |
>> +     create_sections(ttb, start, size, PMD_SECT_AP_WRITE |
>>                       PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
>>  }
>>
>> @@ -30,7 +30,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
>>       set_ttbr(ttb);
>>       set_domain(DOMAIN_MANAGER);
>>
>> -     create_sections(ttb, 0, 4096, PMD_SECT_AP_WRITE |
>> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE |
>>                       PMD_SECT_AP_READ | PMD_TYPE_SECT);
>>
>>       map_cachable(membase, memsize);
>> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
>> index 0c367e47c..f02c99f65 100644
>> --- a/arch/arm/cpu/mmu.c
>> +++ b/arch/arm/cpu/mmu.c
>> @@ -460,7 +460,7 @@ static int mmu_init(void)
>>       set_domain(DOMAIN_MANAGER);
>>
>>       /* create a flat mapping using 1MiB sections */
>> -     create_sections(ttb, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
>> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
>>                       PMD_TYPE_SECT);
>>       __mmu_cache_flush();
>>
>> @@ -472,7 +472,7 @@ static int mmu_init(void)
>>        * below
>>        */
>>       for_each_memory_bank(bank) {
>> -             create_sections(ttb, bank->start, bank->size >> 20,
>> +             create_sections(ttb, bank->start, bank->size,
>>                               PMD_SECT_DEF_CACHED);
>>               __mmu_cache_flush();
>>       }
>> diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
>> index d71cd7e38..52689359a 100644
>> --- a/arch/arm/cpu/mmu.h
>> +++ b/arch/arm/cpu/mmu.h
>> @@ -27,16 +27,26 @@ static inline void set_domain(unsigned val)
>>  }
>>
>>  static inline void
>> -create_sections(uint32_t *ttb, unsigned long addr,
>> -             int size_m, unsigned int flags)
>> +__create_sections(uint32_t *ttb, unsigned long first,
>> +               unsigned long last, unsigned int flags)
>>  {
>> -     unsigned long ttb_start = addr >> 20;
>> -     unsigned long ttb_end   = ttb_start + size_m;
>> -     unsigned int i;
>> +     unsigned long ttb_start = first >> 20;
>> +     unsigned long ttb_end   = (last >> 20) + 1;
>> +     unsigned int i, addr;
>>
>> -     for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
>> +     for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
>>               ttb[i] = addr | flags;
>>  }
>>
>> +#define create_sections(ttb, addr, size, flags)                              \
>> +     ({                                                              \
>> +             typeof(addr) __addr = addr;                             \
>> +             typeof(size) __size = size;                             \
>> +             /* Check for overflow */                                \
>> +             BUG_ON(__addr > ULONG_MAX - __size + 1);                \
>> +             __create_sections(ttb, __addr, __addr + (size)  - 1,    \
>> +                               flags);                               \
>> +     })
>
> Why do you preserve the original API of create_sections()? I would just
> change it. We have only a few callers and they are easy to change.
>

Mostly because it keeps "addr + size - 1" arithmetic in one place
instead of spreading it to every caller. If you'd rather have that
instead of the macro above, I can change that in v3.

Thanks,
Andrey Smirnov

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

Re: [PATCH v2 09/28] ARM: mmu: Specify size in bytes in create_sections()

[Sascha Hauer]

On Thu, May 17, 2018 at 12:01:34AM -0700, Andrey Smirnov wrote:
> On Wed, May 16, 2018 at 11:55 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Wed, May 16, 2018 at 01:00:17PM -0700, Andrey Smirnov wrote:
> >> Seeing
> >>
> >>         create_sections(ttb, 0, PAGE_SIZE, ...);
> >>
> >> as the code the creates initial flat 4 GiB mapping is a bit less
> >> intuitive then
> >>
> >>         create_sections(ttb, 0, SZ_4G, ...);
> >>
> >> so, for the sake of clarification, convert create_sections() to accept
> >> size in bytes and do bytes -> MiB converstion as a part of the
> >> function.
> >>
> >> NOTE: To keep all of the arguments of create_sections() 32-bit:
> >>
> >>     - Move all of the real code into a helper function accepting first
> >>       and last addresses of the region (e.g passing 0 and U32_MAX
> >>       means all 4GiB of address space)
> >>
> >>     - Convert create_section() into a macro that does necessary size
> >>       -> last addres conversion under the hood to preserve original API
> >>
> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >> ---
> >>  arch/arm/cpu/mmu-early.c |  4 ++--
> >>  arch/arm/cpu/mmu.c       |  4 ++--
> >>  arch/arm/cpu/mmu.h       | 22 ++++++++++++++++------
> >>  3 files changed, 20 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
> >> index 70ece0d2f..136b33c3a 100644
> >> --- a/arch/arm/cpu/mmu-early.c
> >> +++ b/arch/arm/cpu/mmu-early.c
> >> @@ -16,7 +16,7 @@ static void map_cachable(unsigned long start, unsigned long size)
> >>       start = ALIGN_DOWN(start, SZ_1M);
> >>       size  = ALIGN(size, SZ_1M);
> >>
> >> -     create_sections(ttb, start, size >> 20, PMD_SECT_AP_WRITE |
> >> +     create_sections(ttb, start, size, PMD_SECT_AP_WRITE |
> >>                       PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
> >>  }
> >>
> >> @@ -30,7 +30,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
> >>       set_ttbr(ttb);
> >>       set_domain(DOMAIN_MANAGER);
> >>
> >> -     create_sections(ttb, 0, 4096, PMD_SECT_AP_WRITE |
> >> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE |
> >>                       PMD_SECT_AP_READ | PMD_TYPE_SECT);
> >>
> >>       map_cachable(membase, memsize);
> >> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> >> index 0c367e47c..f02c99f65 100644
> >> --- a/arch/arm/cpu/mmu.c
> >> +++ b/arch/arm/cpu/mmu.c
> >> @@ -460,7 +460,7 @@ static int mmu_init(void)
> >>       set_domain(DOMAIN_MANAGER);
> >>
> >>       /* create a flat mapping using 1MiB sections */
> >> -     create_sections(ttb, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
> >> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
> >>                       PMD_TYPE_SECT);
> >>       __mmu_cache_flush();
> >>
> >> @@ -472,7 +472,7 @@ static int mmu_init(void)
> >>        * below
> >>        */
> >>       for_each_memory_bank(bank) {
> >> -             create_sections(ttb, bank->start, bank->size >> 20,
> >> +             create_sections(ttb, bank->start, bank->size,
> >>                               PMD_SECT_DEF_CACHED);
> >>               __mmu_cache_flush();
> >>       }
> >> diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
> >> index d71cd7e38..52689359a 100644
> >> --- a/arch/arm/cpu/mmu.h
> >> +++ b/arch/arm/cpu/mmu.h
> >> @@ -27,16 +27,26 @@ static inline void set_domain(unsigned val)
> >>  }
> >>
> >>  static inline void
> >> -create_sections(uint32_t *ttb, unsigned long addr,
> >> -             int size_m, unsigned int flags)
> >> +__create_sections(uint32_t *ttb, unsigned long first,
> >> +               unsigned long last, unsigned int flags)
> >>  {
> >> -     unsigned long ttb_start = addr >> 20;
> >> -     unsigned long ttb_end   = ttb_start + size_m;
> >> -     unsigned int i;
> >> +     unsigned long ttb_start = first >> 20;
> >> +     unsigned long ttb_end   = (last >> 20) + 1;
> >> +     unsigned int i, addr;
> >>
> >> -     for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
> >> +     for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
> >>               ttb[i] = addr | flags;
> >>  }
> >>
> >> +#define create_sections(ttb, addr, size, flags)                              \
> >> +     ({                                                              \
> >> +             typeof(addr) __addr = addr;                             \
> >> +             typeof(size) __size = size;                             \
> >> +             /* Check for overflow */                                \
> >> +             BUG_ON(__addr > ULONG_MAX - __size + 1);                \
> >> +             __create_sections(ttb, __addr, __addr + (size)  - 1,    \
> >> +                               flags);                               \
> >> +     })
> >
> > Why do you preserve the original API of create_sections()? I would just
> > change it. We have only a few callers and they are easy to change.
> >
> 
> Mostly because it keeps "addr + size - 1" arithmetic in one place
> instead of spreading it to every caller. If you'd rather have that
> instead of the macro above, I can change that in v3.

I agree that the end address calculation often is a source for
off-by-one errors, still I would prefer not using macros to hide that.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

Re: [PATCH v2 09/28] ARM: mmu: Specify size in bytes in create_sections()

[Andrey Smirnov]

On Thu, May 17, 2018 at 12:08 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Thu, May 17, 2018 at 12:01:34AM -0700, Andrey Smirnov wrote:
>> On Wed, May 16, 2018 at 11:55 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Wed, May 16, 2018 at 01:00:17PM -0700, Andrey Smirnov wrote:
>> >> Seeing
>> >>
>> >>         create_sections(ttb, 0, PAGE_SIZE, ...);
>> >>
>> >> as the code the creates initial flat 4 GiB mapping is a bit less
>> >> intuitive then
>> >>
>> >>         create_sections(ttb, 0, SZ_4G, ...);
>> >>
>> >> so, for the sake of clarification, convert create_sections() to accept
>> >> size in bytes and do bytes -> MiB converstion as a part of the
>> >> function.
>> >>
>> >> NOTE: To keep all of the arguments of create_sections() 32-bit:
>> >>
>> >>     - Move all of the real code into a helper function accepting first
>> >>       and last addresses of the region (e.g passing 0 and U32_MAX
>> >>       means all 4GiB of address space)
>> >>
>> >>     - Convert create_section() into a macro that does necessary size
>> >>       -> last addres conversion under the hood to preserve original API
>> >>
>> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> >> ---
>> >>  arch/arm/cpu/mmu-early.c |  4 ++--
>> >>  arch/arm/cpu/mmu.c       |  4 ++--
>> >>  arch/arm/cpu/mmu.h       | 22 ++++++++++++++++------
>> >>  3 files changed, 20 insertions(+), 10 deletions(-)
>> >>
>> >> diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
>> >> index 70ece0d2f..136b33c3a 100644
>> >> --- a/arch/arm/cpu/mmu-early.c
>> >> +++ b/arch/arm/cpu/mmu-early.c
>> >> @@ -16,7 +16,7 @@ static void map_cachable(unsigned long start, unsigned long size)
>> >>       start = ALIGN_DOWN(start, SZ_1M);
>> >>       size  = ALIGN(size, SZ_1M);
>> >>
>> >> -     create_sections(ttb, start, size >> 20, PMD_SECT_AP_WRITE |
>> >> +     create_sections(ttb, start, size, PMD_SECT_AP_WRITE |
>> >>                       PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
>> >>  }
>> >>
>> >> @@ -30,7 +30,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
>> >>       set_ttbr(ttb);
>> >>       set_domain(DOMAIN_MANAGER);
>> >>
>> >> -     create_sections(ttb, 0, 4096, PMD_SECT_AP_WRITE |
>> >> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE |
>> >>                       PMD_SECT_AP_READ | PMD_TYPE_SECT);
>> >>
>> >>       map_cachable(membase, memsize);
>> >> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
>> >> index 0c367e47c..f02c99f65 100644
>> >> --- a/arch/arm/cpu/mmu.c
>> >> +++ b/arch/arm/cpu/mmu.c
>> >> @@ -460,7 +460,7 @@ static int mmu_init(void)
>> >>       set_domain(DOMAIN_MANAGER);
>> >>
>> >>       /* create a flat mapping using 1MiB sections */
>> >> -     create_sections(ttb, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
>> >> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
>> >>                       PMD_TYPE_SECT);
>> >>       __mmu_cache_flush();
>> >>
>> >> @@ -472,7 +472,7 @@ static int mmu_init(void)
>> >>        * below
>> >>        */
>> >>       for_each_memory_bank(bank) {
>> >> -             create_sections(ttb, bank->start, bank->size >> 20,
>> >> +             create_sections(ttb, bank->start, bank->size,
>> >>                               PMD_SECT_DEF_CACHED);
>> >>               __mmu_cache_flush();
>> >>       }
>> >> diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
>> >> index d71cd7e38..52689359a 100644
>> >> --- a/arch/arm/cpu/mmu.h
>> >> +++ b/arch/arm/cpu/mmu.h
>> >> @@ -27,16 +27,26 @@ static inline void set_domain(unsigned val)
>> >>  }
>> >>
>> >>  static inline void
>> >> -create_sections(uint32_t *ttb, unsigned long addr,
>> >> -             int size_m, unsigned int flags)
>> >> +__create_sections(uint32_t *ttb, unsigned long first,
>> >> +               unsigned long last, unsigned int flags)
>> >>  {
>> >> -     unsigned long ttb_start = addr >> 20;
>> >> -     unsigned long ttb_end   = ttb_start + size_m;
>> >> -     unsigned int i;
>> >> +     unsigned long ttb_start = first >> 20;
>> >> +     unsigned long ttb_end   = (last >> 20) + 1;
>> >> +     unsigned int i, addr;
>> >>
>> >> -     for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
>> >> +     for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
>> >>               ttb[i] = addr | flags;
>> >>  }
>> >>
>> >> +#define create_sections(ttb, addr, size, flags)                              \
>> >> +     ({                                                              \
>> >> +             typeof(addr) __addr = addr;                             \
>> >> +             typeof(size) __size = size;                             \
>> >> +             /* Check for overflow */                                \
>> >> +             BUG_ON(__addr > ULONG_MAX - __size + 1);                \
>> >> +             __create_sections(ttb, __addr, __addr + (size)  - 1,    \
>> >> +                               flags);                               \
>> >> +     })
>> >
>> > Why do you preserve the original API of create_sections()? I would just
>> > change it. We have only a few callers and they are easy to change.
>> >
>>
>> Mostly because it keeps "addr + size - 1" arithmetic in one place
>> instead of spreading it to every caller. If you'd rather have that
>> instead of the macro above, I can change that in v3.
>
> I agree that the end address calculation often is a source for
> off-by-one errors, still I would prefer not using macros to hide that.
>

I can't think of a way to keep arithmetic in one place and not to make
one of the arguments 64-bit at the same time without resorting to a
macro. AFAICT the options are either to drop the macro and change the
API or keep the API/arithmetic in one place and keep the macro.

I am happy with either, so let me know what you'd prefer.

Thanks,
Andrey Smirnov

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

Re: [PATCH v2 09/28] ARM: mmu: Specify size in bytes in create_sections()

[Sascha Hauer]

On Thu, May 17, 2018 at 12:35:10AM -0700, Andrey Smirnov wrote:
> On Thu, May 17, 2018 at 12:08 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Thu, May 17, 2018 at 12:01:34AM -0700, Andrey Smirnov wrote:
> >> On Wed, May 16, 2018 at 11:55 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >> > On Wed, May 16, 2018 at 01:00:17PM -0700, Andrey Smirnov wrote:
> >> >> Seeing
> >> >>
> >> >>         create_sections(ttb, 0, PAGE_SIZE, ...);
> >> >>
> >> >> as the code the creates initial flat 4 GiB mapping is a bit less
> >> >> intuitive then
> >> >>
> >> >>         create_sections(ttb, 0, SZ_4G, ...);
> >> >>
> >> >> so, for the sake of clarification, convert create_sections() to accept
> >> >> size in bytes and do bytes -> MiB converstion as a part of the
> >> >> function.
> >> >>
> >> >> NOTE: To keep all of the arguments of create_sections() 32-bit:
> >> >>
> >> >>     - Move all of the real code into a helper function accepting first
> >> >>       and last addresses of the region (e.g passing 0 and U32_MAX
> >> >>       means all 4GiB of address space)
> >> >>
> >> >>     - Convert create_section() into a macro that does necessary size
> >> >>       -> last addres conversion under the hood to preserve original API
> >> >>
> >> >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >> >> ---
> >> >>  arch/arm/cpu/mmu-early.c |  4 ++--
> >> >>  arch/arm/cpu/mmu.c       |  4 ++--
> >> >>  arch/arm/cpu/mmu.h       | 22 ++++++++++++++++------
> >> >>  3 files changed, 20 insertions(+), 10 deletions(-)
> >> >>
> >> >> diff --git a/arch/arm/cpu/mmu-early.c b/arch/arm/cpu/mmu-early.c
> >> >> index 70ece0d2f..136b33c3a 100644
> >> >> --- a/arch/arm/cpu/mmu-early.c
> >> >> +++ b/arch/arm/cpu/mmu-early.c
> >> >> @@ -16,7 +16,7 @@ static void map_cachable(unsigned long start, unsigned long size)
> >> >>       start = ALIGN_DOWN(start, SZ_1M);
> >> >>       size  = ALIGN(size, SZ_1M);
> >> >>
> >> >> -     create_sections(ttb, start, size >> 20, PMD_SECT_AP_WRITE |
> >> >> +     create_sections(ttb, start, size, PMD_SECT_AP_WRITE |
> >> >>                       PMD_SECT_AP_READ | PMD_TYPE_SECT | PMD_SECT_WB);
> >> >>  }
> >> >>
> >> >> @@ -30,7 +30,7 @@ void mmu_early_enable(unsigned long membase, unsigned long memsize,
> >> >>       set_ttbr(ttb);
> >> >>       set_domain(DOMAIN_MANAGER);
> >> >>
> >> >> -     create_sections(ttb, 0, 4096, PMD_SECT_AP_WRITE |
> >> >> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE |
> >> >>                       PMD_SECT_AP_READ | PMD_TYPE_SECT);
> >> >>
> >> >>       map_cachable(membase, memsize);
> >> >> diff --git a/arch/arm/cpu/mmu.c b/arch/arm/cpu/mmu.c
> >> >> index 0c367e47c..f02c99f65 100644
> >> >> --- a/arch/arm/cpu/mmu.c
> >> >> +++ b/arch/arm/cpu/mmu.c
> >> >> @@ -460,7 +460,7 @@ static int mmu_init(void)
> >> >>       set_domain(DOMAIN_MANAGER);
> >> >>
> >> >>       /* create a flat mapping using 1MiB sections */
> >> >> -     create_sections(ttb, 0, PAGE_SIZE, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
> >> >> +     create_sections(ttb, 0, SZ_4G, PMD_SECT_AP_WRITE | PMD_SECT_AP_READ |
> >> >>                       PMD_TYPE_SECT);
> >> >>       __mmu_cache_flush();
> >> >>
> >> >> @@ -472,7 +472,7 @@ static int mmu_init(void)
> >> >>        * below
> >> >>        */
> >> >>       for_each_memory_bank(bank) {
> >> >> -             create_sections(ttb, bank->start, bank->size >> 20,
> >> >> +             create_sections(ttb, bank->start, bank->size,
> >> >>                               PMD_SECT_DEF_CACHED);
> >> >>               __mmu_cache_flush();
> >> >>       }
> >> >> diff --git a/arch/arm/cpu/mmu.h b/arch/arm/cpu/mmu.h
> >> >> index d71cd7e38..52689359a 100644
> >> >> --- a/arch/arm/cpu/mmu.h
> >> >> +++ b/arch/arm/cpu/mmu.h
> >> >> @@ -27,16 +27,26 @@ static inline void set_domain(unsigned val)
> >> >>  }
> >> >>
> >> >>  static inline void
> >> >> -create_sections(uint32_t *ttb, unsigned long addr,
> >> >> -             int size_m, unsigned int flags)
> >> >> +__create_sections(uint32_t *ttb, unsigned long first,
> >> >> +               unsigned long last, unsigned int flags)
> >> >>  {
> >> >> -     unsigned long ttb_start = addr >> 20;
> >> >> -     unsigned long ttb_end   = ttb_start + size_m;
> >> >> -     unsigned int i;
> >> >> +     unsigned long ttb_start = first >> 20;
> >> >> +     unsigned long ttb_end   = (last >> 20) + 1;
> >> >> +     unsigned int i, addr;
> >> >>
> >> >> -     for (i = ttb_start; i < ttb_end; i++, addr += SZ_1M)
> >> >> +     for (i = ttb_start, addr = first; i < ttb_end; i++, addr += SZ_1M)
> >> >>               ttb[i] = addr | flags;
> >> >>  }
> >> >>
> >> >> +#define create_sections(ttb, addr, size, flags)                              \
> >> >> +     ({                                                              \
> >> >> +             typeof(addr) __addr = addr;                             \
> >> >> +             typeof(size) __size = size;                             \
> >> >> +             /* Check for overflow */                                \
> >> >> +             BUG_ON(__addr > ULONG_MAX - __size + 1);                \
> >> >> +             __create_sections(ttb, __addr, __addr + (size)  - 1,    \
> >> >> +                               flags);                               \
> >> >> +     })
> >> >
> >> > Why do you preserve the original API of create_sections()? I would just
> >> > change it. We have only a few callers and they are easy to change.
> >> >
> >>
> >> Mostly because it keeps "addr + size - 1" arithmetic in one place
> >> instead of spreading it to every caller. If you'd rather have that
> >> instead of the macro above, I can change that in v3.
> >
> > I agree that the end address calculation often is a source for
> > off-by-one errors, still I would prefer not using macros to hide that.
> >
> 
> I can't think of a way to keep arithmetic in one place and not to make
> one of the arguments 64-bit at the same time without resorting to a
> macro. AFAICT the options are either to drop the macro and change the
> API or keep the API/arithmetic in one place and keep the macro.
> 
> I am happy with either, so let me know what you'd prefer.

I meant that the prototype of create_sections should be changed to:

static void create_sections(uint32_t *ttb, unsigned long start,
			    unsigned long end, unsigned int flags);

No need for additional encapsulation, let's just change the callers.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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