Skip to contentSkip to conversation

arm64: Unify MMU code

PATCH New Alexander Graf 4 participants 24 messages Feb 24, 2016 – Feb 29, 2016

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

cover arm64: Unify MMU code

Howdy,

Currently on arm64 there is a big pile of mess when it comes to MMU support and page tables. Each board does its own little thing and the generic code is pretty dumb and nobody actually uses it.

This patch set tries to clean that up. After this series is applied, all boards except for the FSL Layerscape ones are converted to the new generic page table logic and have icache+dcache enabled.

The new code always uses 4k page size. It dynamically allocates 1G or 2M pages for ranges that fit. When a dcache attribute request comes in that requires a smaller granularity than our previous allocation could fulfill, pages get automatically split.

I have tested and verified the code works on HiKey (bare metal), vexpress64 (Foundation Model) and zynqmp (QEMU). The TX1 target is untested, but given the simplicity of the maps I doubt it'll break. ThunderX in theory should also work, but I haven't tested it. I would be very happy if people with access to those system could give the patch set a try.

With this we're a big step closer to a good base line for EFI payload support, since we can now just require that all boards always have dcache enabled.

I would also be incredibly happy if some Freescale people could look at their MMU code and try to unify it into the now cleaned up generic code. I don't think we're far off here.

Alex

v1 -> v2:

- Fix comment for create_table()
- Rework page table size calculation
- Move mmu tables into board files
- New patch: thunderx: Move mmu table into board file

Alexander Graf (10):
thunderx: Calculate TCR dynamically
arm64: Make full va map code more dynamic
thunderx: Move mmu table into board file
zymqmp: Replace home grown mmu code with generic table approach
tegra: Replace home grown mmu code with generic table approach
vexpress64: Add MMU tables
dwmmc: Increase retry timeout
hikey: Add MMU tables
arm64: Remove non-full-va map code
arm64: Only allow dcache disabled in SPL builds

 arch/arm/cpu/armv8/cache_v8.c                  | 495 +++++++++++++++++++------
 arch/arm/cpu/armv8/fsl-layerscape/cpu.c        |  37 +-
 arch/arm/cpu/armv8/zynqmp/cpu.c                | 217 +++--------
 arch/arm/include/asm/arch-fsl-layerscape/cpu.h |  94 ++---
 arch/arm/include/asm/armv8/mmu.h               | 122 ++----
 arch/arm/include/asm/global_data.h             |   6 +-
 arch/arm/include/asm/system.h                  |   7 +-
 arch/arm/mach-tegra/arm64-mmu.c                | 132 +------
 board/armltd/vexpress64/vexpress64.c           |  21 ++
 board/cavium/thunderx/thunderx.c               |  24 ++
 board/hisilicon/hikey/hikey.c                  |  21 ++
 doc/README.arm64                               |  20 -
 drivers/mmc/dw_mmc.c                           |   2 +-
 include/configs/hikey.h                        |   4 +-
 include/configs/thunderx_88xx.h                |  30 --
 include/configs/vexpress_aemv8a.h              |   5 +-
 16 files changed, 627 insertions(+), 610 deletions(-)
Signature · 1 line
1.8.5.6
#1 lore raw

PATCH 1/10 thunderx: Calculate TCR dynamically

Based on the memory map we can determine a lot of hard coded fields of TCR, like the maximum VA and max PA we want to support. Calculate those dynamically to reduce the chance for pit falls.

---
 arch/arm/cpu/armv8/cache_v8.c    | 59 +++++++++++++++++++++++++++++++++++++++-
 arch/arm/include/asm/armv8/mmu.h |  6 +---
 include/configs/thunderx_88xx.h  |  3 --
 3 files changed, 59 insertions(+), 9 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +58 -1
index 71f0020..9229532 100644
@@ -38,6 +38,58 @@ static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP;
 #define PTL1_ENTRIES CONFIG_SYS_PTL1_ENTRIES
 #define PTL2_ENTRIES CONFIG_SYS_PTL2_ENTRIES
 
+static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
+{
+	u64 max_addr = 0;
+	u64 ips, va_bits;
+	u64 tcr;
+	int i;
+
+	/* Find the largest address we need to support */
+	for (i = 0; i < ARRAY_SIZE(mem_map); i++)
+		max_addr = max(max_addr, mem_map[i].base + mem_map[i].size);
+
+	/* Calculate the maximum physical (and thus virtual) address */
+	if (max_addr > (1ULL << 44)) {
+		ips = 5;
+		va_bits = 48;
+	} else  if (max_addr > (1ULL << 42)) {
+		ips = 4;
+		va_bits = 44;
+	} else  if (max_addr > (1ULL << 40)) {
+		ips = 3;
+		va_bits = 42;
+	} else  if (max_addr > (1ULL << 36)) {
+		ips = 2;
+		va_bits = 40;
+	} else  if (max_addr > (1ULL << 32)) {
+		ips = 1;
+		va_bits = 36;
+	} else {
+		ips = 0;
+		va_bits = 32;
+	}
+
+	if (el == 1) {
+		tcr = TCR_EL1_RSVD | (ips << 32);
+	} else if (el == 2) {
+		tcr = TCR_EL2_RSVD | (ips << 16);
+	} else {
+		tcr = TCR_EL3_RSVD | (ips << 16);
+	}
+
+	/* PTWs cacheable, inner/outer WBWA and inner shareable */
+	tcr |= TCR_TG0_64K | TCR_SHARED_INNER | TCR_ORGN_WBWA | TCR_IRGN_WBWA;
+	tcr |= TCR_T0SZ(VA_BITS);
+
+	if (pips)
+		*pips = ips;
+	if (pva_bits)
+		*pva_bits = va_bits;
+
+	return tcr;
+}
+
 static void setup_pgtables(void)
 {
 	int l1_e, l2_e;
@@ -110,6 +162,10 @@ __weak void mmu_setup(void)
 	/* Set up page tables only on BSP */
 	if (coreid == BSP_COREID)
 		setup_pgtables();
+
+	el = current_el();
+	set_ttbr_tcr_mair(el, gd->arch.tlb_addr, get_tcr(el, NULL, NULL),
+			  MEMORY_ATTRIBUTES);
 #else
 	/* Setup an identity-mapping for all spaces */
 	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
@@ -128,7 +184,6 @@ __weak void mmu_setup(void)
 		}
 	}
 
-#endif
 	/* load TTBR0 */
 	el = current_el();
 	if (el == 1) {
@@ -144,6 +199,8 @@ __weak void mmu_setup(void)
 				  TCR_EL3_RSVD | TCR_FLAGS | TCR_EL3_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	}
+#endif
+
 	/* enable the mmu */
 	set_sctlr(get_sctlr() | CR_M);
 }
arch/arm/include/asm/armv8/mmu.h +1 -5
index 897f010..39ff745 100644
@@ -159,11 +159,6 @@
 #define TCR_EL1_IPS_BITS	(UL(3) << 32)	/* 42 bits physical address */
 #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
-#else
-#define TCR_EL1_IPS_BITS	CONFIG_SYS_TCR_EL1_IPS_BITS
-#define TCR_EL2_IPS_BITS	CONFIG_SYS_TCR_EL2_IPS_BITS
-#define TCR_EL3_IPS_BITS	CONFIG_SYS_TCR_EL3_IPS_BITS
-#endif
 
 /* PTWs cacheable, inner/outer WBWA and inner shareable */
 #define TCR_FLAGS		(TCR_TG0_64K |		\
@@ -171,6 +166,7 @@
 				TCR_ORGN_WBWA |		\
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
+#endif
 
 #define TCR_EL1_RSVD		(1 << 31)
 #define TCR_EL2_RSVD		(1 << 31 | 1 << 23)
include/configs/thunderx_88xx.h +0 -3
index cece4dd..b9f93ad 100644
@@ -50,9 +50,6 @@
 #define CONFIG_SYS_PGTABLE_SIZE		\
 	((CONFIG_SYS_PTL1_ENTRIES + \
 	  CONFIG_SYS_MEM_MAP_SIZE * CONFIG_SYS_PTL2_ENTRIES) * 8)
-#define CONFIG_SYS_TCR_EL1_IPS_BITS	(5UL << 32)
-#define CONFIG_SYS_TCR_EL2_IPS_BITS	(5 << 16)
-#define CONFIG_SYS_TCR_EL3_IPS_BITS	(5 << 16)
 
 /* Link Definitions */
 #define CONFIG_SYS_TEXT_BASE		0x00500000
Signature · 1 line
1.8.5.6
#2 lore raw

PATCH 1/10 thunderx: Calculate TCR dynamically

Quoted 40 lines · Alexander Graf

Based on the memory map we can determine a lot of hard coded fields of TCR, like the maximum VA and max PA we want to support. Calculate those dynamically to reduce the chance for pit falls.

---
 arch/arm/cpu/armv8/cache_v8.c    | 59 +++++++++++++++++++++++++++++++++++++++-
 arch/arm/include/asm/armv8/mmu.h |  6 +---
 include/configs/thunderx_88xx.h  |  3 --
 3 files changed, 59 insertions(+), 9 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +21 -0
index 71f0020..9229532 100644
@@ -38,6 +38,58 @@ static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP;
 #define PTL1_ENTRIES CONFIG_SYS_PTL1_ENTRIES
 #define PTL2_ENTRIES CONFIG_SYS_PTL2_ENTRIES
 
+static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
+{
+	u64 max_addr = 0;
+	u64 ips, va_bits;
+	u64 tcr;
+	int i;
+
+	/* Find the largest address we need to support */
+	for (i = 0; i < ARRAY_SIZE(mem_map); i++)
+		max_addr = max(max_addr, mem_map[i].base + mem_map[i].size);
+
+	/* Calculate the maximum physical (and thus virtual) address */
+	if (max_addr > (1ULL << 44)) {
+		ips = 5;
+		va_bits = 48;
+	} else  if (max_addr > (1ULL << 42)) {
+		ips = 4;
+		va_bits = 44;
+	} else  if (max_addr > (1ULL << 40)) {
+		ips = 3;
+		va_bits = 42;

arch/arm/cpu/armv8/cache_v8.c static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP; · line 71

Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+	} else  if (max_addr > (1ULL << 36)) {
+		ips = 2;
+		va_bits = 40;
+	} else  if (max_addr > (1ULL << 32)) {
+		ips = 1;
+		va_bits = 36;
+	} else {
+		ips = 0;
+		va_bits = 32;
+	}

In Linux we program IPS to the maximum PARange from ID_AA64MMFR0.

If you did the same here you wouldn't have to iterate over all the memory map entries to determine the maximum PA you care about (though you may still need to do that for the VA size).

Quoted 69 lines
arch/arm/cpu/armv8/cache_v8.c +27 -1
+
+	if (el == 1) {
+		tcr = TCR_EL1_RSVD | (ips << 32);
+	} else if (el == 2) {
+		tcr = TCR_EL2_RSVD | (ips << 16);
+	} else {
+		tcr = TCR_EL3_RSVD | (ips << 16);
+	}
+
+	/* PTWs cacheable, inner/outer WBWA and inner shareable */
+	tcr |= TCR_TG0_64K | TCR_SHARED_INNER | TCR_ORGN_WBWA | TCR_IRGN_WBWA;
+	tcr |= TCR_T0SZ(VA_BITS);
+
+	if (pips)
+		*pips = ips;
+	if (pva_bits)
+		*pva_bits = va_bits;
+
+	return tcr;
+}
+
 static void setup_pgtables(void)
 {
 	int l1_e, l2_e;
@@ -110,6 +162,10 @@ __weak void mmu_setup(void)
 	/* Set up page tables only on BSP */
 	if (coreid == BSP_COREID)
 		setup_pgtables();
+
+	el = current_el();
+	set_ttbr_tcr_mair(el, gd->arch.tlb_addr, get_tcr(el, NULL, NULL),
+			  MEMORY_ATTRIBUTES);
 #else
 	/* Setup an identity-mapping for all spaces */
 	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
@@ -128,7 +184,6 @@ __weak void mmu_setup(void)
 		}
 	}
 
-#endif
 	/* load TTBR0 */
 	el = current_el();
 	if (el == 1) {
@@ -144,6 +199,8 @@ __weak void mmu_setup(void)
 				  TCR_EL3_RSVD | TCR_FLAGS | TCR_EL3_IPS_BITS,
 				  MEMORY_ATTRIBUTES);
 	}
+#endif
+
 	/* enable the mmu */
 	set_sctlr(get_sctlr() | CR_M);
 }
arch/arm/include/asm/armv8/mmu.h +0 -5
index 897f010..39ff745 100644
@@ -159,11 +159,6 @@
 #define TCR_EL1_IPS_BITS	(UL(3) << 32)	/* 42 bits physical address */
 #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */
 #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
-#else
-#define TCR_EL1_IPS_BITS	CONFIG_SYS_TCR_EL1_IPS_BITS
-#define TCR_EL2_IPS_BITS	CONFIG_SYS_TCR_EL2_IPS_BITS
-#define TCR_EL3_IPS_BITS	CONFIG_SYS_TCR_EL3_IPS_BITS
-#endif
 
 /* PTWs cacheable, inner/outer WBWA and inner shareable */
 #define TCR_FLAGS		(TCR_TG0_64K |		\

arch/arm/include/asm/armv8/mmu.h line 172

Quoted 7 lines
arch/arm/include/asm/armv8/mmu.h +1 -0
@@ -171,6 +166,7 @@
 				TCR_ORGN_WBWA |		\
 				TCR_IRGN_WBWA |		\
 				TCR_T0SZ(VA_BITS))
+#endif
 
 #define TCR_EL1_RSVD		(1 << 31)
 #define TCR_EL2_RSVD		(1 << 31 | 1 << 23)

I suspect you want bit 23 / EPD1 for EL1. Otherwise the core can make walks starting@whatever junk happens to be in TTBR1.

Thanks,
Mark.

Quoted 22 lines
include/configs/thunderx_88xx.h +0 -3
index cece4dd..b9f93ad 100644
@@ -50,9 +50,6 @@
 #define CONFIG_SYS_PGTABLE_SIZE		\
 	((CONFIG_SYS_PTL1_ENTRIES + \
 	  CONFIG_SYS_MEM_MAP_SIZE * CONFIG_SYS_PTL2_ENTRIES) * 8)
-#define CONFIG_SYS_TCR_EL1_IPS_BITS	(5UL << 32)
-#define CONFIG_SYS_TCR_EL2_IPS_BITS	(5 << 16)
-#define CONFIG_SYS_TCR_EL3_IPS_BITS	(5 << 16)
 
 /* Link Definitions */
 #define CONFIG_SYS_TEXT_BASE		0x00500000
Signature · 6 lines
1.8.5.6

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
#3 lore raw

PATCH 1/10 thunderx: Calculate TCR dynamically

Quoted 53 lines · Mark Rutland
Quoted 40 lines · Alexander Graf

Based on the memory map we can determine a lot of hard coded fields of TCR, like the maximum VA and max PA we want to support. Calculate those dynamically to reduce the chance for pit falls.

---
  arch/arm/cpu/armv8/cache_v8.c    | 59 +++++++++++++++++++++++++++++++++++++++-
  arch/arm/include/asm/armv8/mmu.h |  6 +---
  include/configs/thunderx_88xx.h  |  3 --
  3 files changed, 59 insertions(+), 9 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +21 -0
index 71f0020..9229532 100644
@@ -38,6 +38,58 @@ static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP;
  #define PTL1_ENTRIES CONFIG_SYS_PTL1_ENTRIES
  #define PTL2_ENTRIES CONFIG_SYS_PTL2_ENTRIES
 
+static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
+{
+	u64 max_addr = 0;
+	u64 ips, va_bits;
+	u64 tcr;
+	int i;
+
+	/* Find the largest address we need to support */
+	for (i = 0; i < ARRAY_SIZE(mem_map); i++)
+		max_addr = max(max_addr, mem_map[i].base + mem_map[i].size);
+
+	/* Calculate the maximum physical (and thus virtual) address */
+	if (max_addr > (1ULL << 44)) {
+		ips = 5;
+		va_bits = 48;
+	} else  if (max_addr > (1ULL << 42)) {
+		ips = 4;
+		va_bits = 44;
+	} else  if (max_addr > (1ULL << 40)) {
+		ips = 3;
+		va_bits = 42;

arch/arm/cpu/armv8/cache_v8.c static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP; · line 71

Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+	} else  if (max_addr > (1ULL << 36)) {
+		ips = 2;
+		va_bits = 40;
+	} else  if (max_addr > (1ULL << 32)) {
+		ips = 1;
+		va_bits = 36;
+	} else {
+		ips = 0;
+		va_bits = 32;
+	}

In Linux we program IPS to the maximum PARange from ID_AA64MMFR0.

Quoted 3 lines

If you did the same here you wouldn't have to iterate over all the memory map entries to determine the maximum PA you care about (though you may still need to do that for the VA size).

Since we'd want to find the largest number for VA to trim one level of page table if we can, I don't see how it would buy is much to take the maximum supported PARange of the core into account.

Quoted 75 lines
Quoted 75 lines
arch/arm/cpu/armv8/cache_v8.c +27 -1
+
+	if (el == 1) {
+		tcr = TCR_EL1_RSVD | (ips << 32);
+	} else if (el == 2) {
+		tcr = TCR_EL2_RSVD | (ips << 16);
+	} else {
+		tcr = TCR_EL3_RSVD | (ips << 16);
+	}
+
+	/* PTWs cacheable, inner/outer WBWA and inner shareable */
+	tcr |= TCR_TG0_64K | TCR_SHARED_INNER | TCR_ORGN_WBWA | TCR_IRGN_WBWA;
+	tcr |= TCR_T0SZ(VA_BITS);
+
+	if (pips)
+		*pips = ips;
+	if (pva_bits)
+		*pva_bits = va_bits;
+
+	return tcr;
+}
+
  static void setup_pgtables(void)
  {
  	int l1_e, l2_e;
@@ -110,6 +162,10 @@ __weak void mmu_setup(void)
  	/* Set up page tables only on BSP */
  	if (coreid == BSP_COREID)
  		setup_pgtables();
+
+	el = current_el();
+	set_ttbr_tcr_mair(el, gd->arch.tlb_addr, get_tcr(el, NULL, NULL),
+			  MEMORY_ATTRIBUTES);
  #else
  	/* Setup an identity-mapping for all spaces */
  	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
@@ -128,7 +184,6 @@ __weak void mmu_setup(void)
  		}
  	}
 
-#endif
  	/* load TTBR0 */
  	el = current_el();
  	if (el == 1) {
@@ -144,6 +199,8 @@ __weak void mmu_setup(void)
  				  TCR_EL3_RSVD | TCR_FLAGS | TCR_EL3_IPS_BITS,
  				  MEMORY_ATTRIBUTES);
  	}
+#endif
+
  	/* enable the mmu */
  	set_sctlr(get_sctlr() | CR_M);
  }
arch/arm/include/asm/armv8/mmu.h +1 -5
index 897f010..39ff745 100644
@@ -159,11 +159,6 @@
  #define TCR_EL1_IPS_BITS	(UL(3) << 32)	/* 42 bits physical address */
  #define TCR_EL2_IPS_BITS	(3 << 16)	/* 42 bits physical address */
  #define TCR_EL3_IPS_BITS	(3 << 16)	/* 42 bits physical address */
-#else
-#define TCR_EL1_IPS_BITS	CONFIG_SYS_TCR_EL1_IPS_BITS
-#define TCR_EL2_IPS_BITS	CONFIG_SYS_TCR_EL2_IPS_BITS
-#define TCR_EL3_IPS_BITS	CONFIG_SYS_TCR_EL3_IPS_BITS
-#endif
 
  /* PTWs cacheable, inner/outer WBWA and inner shareable */
  #define TCR_FLAGS		(TCR_TG0_64K |		\
@@ -171,6 +166,7 @@
  				TCR_ORGN_WBWA |		\
  				TCR_IRGN_WBWA |		\
  				TCR_T0SZ(VA_BITS))
+#endif
 
Quoted 4 lines

arch/arm/include/asm/armv8/mmu.h line 172

Quoted 2 lines
arch/arm/include/asm/armv8/mmu.h +0 -0
  #define TCR_EL1_RSVD		(1 << 31)
  #define TCR_EL2_RSVD		(1 << 31 | 1 << 23)

I suspect you want bit 23 / EPD1 for EL1. Otherwise the core can make walks starting at whatever junk happens to be in TTBR1.

Yes. Definitely. It's not reserved though, we have to add it in another place, but nice catch!

Alex

#4 lore raw

PATCH 1/10 thunderx: Calculate TCR dynamically

Quoted 24 lines · Alexander Graf
Quoted 23 lines · Mark Rutland
Quoted 10 lines · Alexander Graf
arch/arm/cpu/armv8/cache_v8.c +10 -0
+	/* Calculate the maximum physical (and thus virtual) address */
+	if (max_addr > (1ULL << 44)) {
+		ips = 5;
+		va_bits = 48;
+	} else  if (max_addr > (1ULL << 42)) {
+		ips = 4;
+		va_bits = 44;
+	} else  if (max_addr > (1ULL << 40)) {
+		ips = 3;
+		va_bits = 42;

arch/arm/cpu/armv8/cache_v8.c static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP; · line 71

Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+	} else  if (max_addr > (1ULL << 36)) {
+		ips = 2;
+		va_bits = 40;
+	} else  if (max_addr > (1ULL << 32)) {
+		ips = 1;
+		va_bits = 36;
+	} else {
+		ips = 0;
+		va_bits = 32;
+	}

In Linux we program IPS to the maximum PARange from ID_AA64MMFR0.

Quoted 7 lines
Quoted 3 lines

If you did the same here you wouldn't have to iterate over all the memory map entries to determine the maximum PA you care about (though you may still need to do that for the VA size).

Since we'd want to find the largest number for VA to trim one level of page table if we can, I don't see how it would buy is much to take the maximum supported PARange of the core into account.

It would simply be a saving of lines, as you'd program the same IPS value regardless of max_addr (and you have to expect that PARange is sufficient regardless).

Otherwise, yes, it doesn't buy you anything.

Thanks,
Mark.

#5 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

In reply to Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

With all this in place, there is very little reason to create your own page tables in board specific files.

---

v1 -> v2:

- Fix comment for create_table()
- Rework page table size calculation
- s/DPRINTF/debug/g
- Improve panic messages

---
 arch/arm/cpu/armv8/cache_v8.c      | 403 ++++++++++++++++++++++++++++++++-----
 arch/arm/include/asm/armv8/mmu.h   |  68 +++----
 arch/arm/include/asm/global_data.h |   4 +-
 arch/arm/include/asm/system.h      |   3 +-
 include/configs/thunderx_88xx.h    |  14 +-
 5 files changed, 390 insertions(+), 102 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +349 -54
index 9229532..9cf46b1 100644
@@ -2,6 +2,9 @@
  * (C) Copyright 2013
  * David Feng <[email protected]>
  *
+ * (C) Copyright 2016
+ * Alexander Graf <[email protected]>
+ *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
@@ -13,31 +16,28 @@ DECLARE_GLOBAL_DATA_PTR;
 
 #ifndef CONFIG_SYS_DCACHE_OFF
 
-#ifdef CONFIG_SYS_FULL_VA
-static void set_ptl1_entry(u64 index, u64 ptl2_entry)
-{
-	u64 *pgd = (u64 *)gd->arch.tlb_addr;
-	u64 value;
-
-	value = ptl2_entry | PTL1_TYPE_TABLE;
-	pgd[index] = value;
-}
-
-static void set_ptl2_block(u64 ptl1, u64 bfn, u64 address, u64 memory_attrs)
-{
-	u64 *pmd = (u64 *)ptl1;
-	u64 value;
-
-	value = address | PTL2_TYPE_BLOCK | PTL2_BLOCK_AF;
-	value |= memory_attrs;
-	pmd[bfn] = value;
-}
+/*
+ *  With 4k page granule, a virtual address is split into 4 lookup parts
+ *  spanning 9 bits each:
+ *
+ *    _______________________________________________
+ *   |       |       |       |       |       |       |
+ *   |   0   |  Lv0  |  Lv1  |  Lv2  |  Lv3  |  off  |
+ *   |_______|_______|_______|_______|_______|_______|
+ *     63-48   47-39   38-30   29-21   20-12   11-00
+ *
+ *             mask        page size
+ *
+ *    Lv0: FF8000000000       --
+ *    Lv1:   7FC0000000       1G
+ *    Lv2:     3FE00000       2M
+ *    Lv3:       1FF000       4K
+ *    off:          FFF
+ */
 
+#ifdef CONFIG_SYS_FULL_VA
 static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP;
 
-#define PTL1_ENTRIES CONFIG_SYS_PTL1_ENTRIES
-#define PTL2_ENTRIES CONFIG_SYS_PTL2_ENTRIES
-
 static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
 {
 	u64 max_addr = 0;
@@ -79,8 +79,8 @@ static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
 	}
 
 	/* PTWs cacheable, inner/outer WBWA and inner shareable */
-	tcr |= TCR_TG0_64K | TCR_SHARED_INNER | TCR_ORGN_WBWA | TCR_IRGN_WBWA;
-	tcr |= TCR_T0SZ(VA_BITS);
+	tcr |= TCR_TG0_4K | TCR_SHARED_INNER | TCR_ORGN_WBWA | TCR_IRGN_WBWA;
+	tcr |= TCR_T0SZ(va_bits);
 
 	if (pips)
 		*pips = ips;
@@ -90,39 +90,263 @@ static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
 	return tcr;
 }
 
-static void setup_pgtables(void)
+#define MAX_PTE_ENTRIES 512
+
+static int pte_type(u64 *pte)
+{
+	return *pte & PTE_TYPE_MASK;
+}
+
+/* Returns the LSB number for a PTE on level <level> */
+static int level2shift(int level)
 {
-	int l1_e, l2_e;
-	unsigned long pmd = 0;
-	unsigned long address;
-
-	/* Setup the PMD pointers */
-	for (l1_e = 0; l1_e < CONFIG_SYS_MEM_MAP_SIZE; l1_e++) {
-		gd->arch.pmd_addr[l1_e] = gd->arch.tlb_addr +
-						PTL1_ENTRIES * sizeof(u64);
-		gd->arch.pmd_addr[l1_e] += PTL2_ENTRIES * sizeof(u64) * l1_e;
-		gd->arch.pmd_addr[l1_e] = ALIGN(gd->arch.pmd_addr[l1_e],
-						0x10000UL);
+	/* Page is 12 bits wide, every level translates 9 bits */
+	return (12 + 9 * (3 - level));
+}
+
+static u64 *find_pte(u64 addr, int level)
+{
+	int start_level = 0;
+	u64 *pte;
+	u64 idx;
+	u64 va_bits;
+	int i;
+
+	debug("addr=%llx level=%d\n", addr, level);
+
+	get_tcr(0, NULL, &va_bits);
+	if (va_bits < 39)
+		start_level = 1;
+
+	if (level < start_level)
+		return NULL;
+
+	/* Walk through all page table levels to find our PTE */
+	pte = (u64*)gd->arch.tlb_addr;
+	for (i = start_level; i < 4; i++) {
+		idx = (addr >> level2shift(i)) & 0x1FF;
+		pte += idx;
+		debug("idx=%llx PTE %p at level %d: %llx\n", idx, pte, i, *pte);
+
+		/* Found it */
+		if (i == level)
+			return pte;
+		/* PTE is no table (either invalid or block), can't traverse */
+		if (pte_type(pte) != PTE_TYPE_TABLE)
+			return NULL;
+		/* Off to the next level */
+		pte = (u64*)(*pte & 0x0000fffffffff000ULL);
 	}
 
-	/* Setup the page tables */
-	for (l1_e = 0; l1_e < PTL1_ENTRIES; l1_e++) {
-		if (mem_map[pmd].base ==
-			(uintptr_t)l1_e << PTL2_BITS) {
-			set_ptl1_entry(l1_e, gd->arch.pmd_addr[pmd]);
-
-			for (l2_e = 0; l2_e < PTL2_ENTRIES; l2_e++) {
-				address = mem_map[pmd].base
-					+ (uintptr_t)l2_e * BLOCK_SIZE;
-				set_ptl2_block(gd->arch.pmd_addr[pmd], l2_e,
-					       address, mem_map[pmd].attrs);
+	/* Should never reach here */
+	return NULL;
+}
+
+/* Returns and creates a new full table (512 entries) */
+static u64 *create_table(void)
+{
+	u64 *new_table = (u64*)gd->arch.tlb_fillptr;
+	u64 pt_len = MAX_PTE_ENTRIES * sizeof(u64);
+
+	/* Allocate MAX_PTE_ENTRIES pte entries */
+	gd->arch.tlb_fillptr += pt_len;
+
+	if (gd->arch.tlb_fillptr - gd->arch.tlb_addr > gd->arch.tlb_size)
+		panic("Insufficient RAM for page table: 0x%lx > 0x%lx. "
+		      "Please increase the size in get_page_table_size()",
+			gd->arch.tlb_fillptr - gd->arch.tlb_addr,
+			gd->arch.tlb_size);
+
+	/* Mark all entries as invalid */
+	memset(new_table, 0, pt_len);
+
+	return new_table;
+}
+
+static void set_pte_table(u64 *pte, u64 *table)
+{
+	/* Point *pte to the new table */
+	debug("Setting %p to addr=%p\n", pte, table);
+	*pte = PTE_TYPE_TABLE | (ulong)table;
+}
+
+/* Add one mm_region map entry to the page tables */
+static void add_map(struct mm_region *map)
+{
+	u64 *pte;
+	u64 addr = map->base;
+	u64 size = map->size;
+	u64 attrs = map->attrs | PTE_TYPE_BLOCK | PTE_BLOCK_AF;
+	u64 blocksize;
+	int level;
+	u64 *new_table;
+
+	while (size) {
+		pte = find_pte(addr, 0);
+		if (pte && (pte_type(pte) == PTE_TYPE_FAULT)) {
+			debug("Creating table for addr 0x%llx\n", addr);
+			new_table = create_table();
+			set_pte_table(pte, new_table);
+		}
+
+		for (level = 1; level < 4; level++) {
+			pte = find_pte(addr, level);
+			blocksize = 1ULL << level2shift(level);
+			debug("Checking if pte fits for addr=%llx size=%llx "
+			      "blocksize=%llx\n", addr, size, blocksize);
+			if (size >= blocksize && !(addr & (blocksize - 1))) {
+				/* Page fits, create block PTE */
+				debug("Setting PTE %p to block addr=%llx\n",
+				      pte, addr);
+				*pte = addr | attrs;
+				addr += blocksize;
+				size -= blocksize;
+				break;
+			} else if ((pte_type(pte) == PTE_TYPE_FAULT)) {
+				/* Page doesn't fit, create subpages */
+				debug("Creating subtable for addr 0x%llx "
+				      "blksize=%llx\n", addr, blocksize);
+				new_table = create_table();
+				set_pte_table(pte, new_table);
 			}
+		}
+	}
+}
 
-			pmd++;
-		} else {
-			set_ptl1_entry(l1_e, 0);
+/* Splits a block PTE into table with subpages spanning the old block */
+static void split_block(u64 *pte, int level)
+{
+	u64 old_pte = *pte;
+	u64 *new_table;
+	u64 i = 0;
+	/* level describes the parent level, we need the child ones */
+	int levelshift = level2shift(level + 1);
+
+	if (pte_type(pte) != PTE_TYPE_BLOCK)
+		panic("PTE %p (%llx) is not a block. Some driver code wants to "
+		      "modify dcache settings for an range not covered in "
+		      "mem_map.", pte, old_pte);
+
+	new_table = create_table();
+	debug("Splitting pte %p (%llx) into %p\n", pte, old_pte, new_table);
+
+	for (i = 0; i < MAX_PTE_ENTRIES; i++) {
+		new_table[i] = old_pte | (i << levelshift);
+		debug("Setting new_table[%lld] = %llx\n", i, new_table[i]);
+	}
+
+	/* Set the new table into effect */
+	set_pte_table(pte, new_table);
+}
+
+/*
+ * This is a recursively called function to count the number of
+ * page tables we need to cover a particular PTE range. If you
+ * call this with level = -1 you basically get the full 48 bit
+ * coverage.
+ */
+static int count_required_pts(u64 addr, int level, u64 maxaddr)
+{
+	int levelshift = level2shift(level);
+	u64 levelsize = 1ULL << levelshift;
+	u64 levelmask = levelsize - 1;
+	u64 levelend = addr + levelsize;
+	int r = 0;
+	int i;
+	bool is_level = false;
+
+	for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+		struct mm_region *map = &mem_map[i];
+		u64 start = map->base;
+		u64 end = start + map->size;
+
+		/* Check if the PTE would overlap with the map */
+		if (max(addr, start) <= min(levelend, end)) {
+			start = max(addr, start);
+			end = min(levelend, end);
+
+			/* We need a sub-pt for this level */
+			if ((start & levelmask) || (end & levelmask)) {
+				is_level = true;
+				break;
+			}
+
+			/* Lv0 can not do block PTEs, so do levels here too */
+			if (level <= 0) {
+				is_level = true;
+				break;
+			}
+		}
+	}
+
+	/*
+	 * Block PTEs@this level are already covered by the parent page
+	 * table, so we only need to count sub page tables.
+	 */
+	if (is_level) {
+		int sublevel = level + 1;
+		u64 sublevelsize = 1ULL << level2shift(sublevel);
+
+		/* Account for the new sub page table ... */
+		r = 1;
+
+		/* ... and for all child page tables that one might have */
+		for (i = 0; i < MAX_PTE_ENTRIES; i++) {
+			r += count_required_pts(addr, sublevel, maxaddr);
+			addr += sublevelsize;
+
+			if (addr >= maxaddr) {
+				/*
+				 * We reached the end of address space, no need
+				 * to look any further.
+				 */
+				break;
+			}
 		}
 	}
+
+	return r;
+}
+
+/* Returns the estimated required size of all page tables */
+u64 get_page_table_size(void)
+{
+	u64 one_pt = MAX_PTE_ENTRIES * sizeof(u64);
+	u64 size = 0;
+	u64 va_bits;
+	int start_level = 0;
+
+	get_tcr(0, NULL, &va_bits);
+	if (va_bits < 39)
+		start_level = 1;
+
+	/* Account for all page tables we would need to cover our memory map */
+	size = one_pt * count_required_pts(0, start_level - 1, 1ULL << va_bits);
+
+	/*
+	 * We may need to split page tables later on if dcache settings change,
+	 * so reserve up to 4 (random pick) page tables for that.
+	 */
+	size += one_pt * 4;
+
+	return size;
+}
+
+static void setup_pgtables(void)
+{
+	int i;
+
+	/*
+	 * Allocate the first level we're on with invalidate entries.
+	 * If the starting level is 0 (va_bits >= 39), then this is our
+	 * Lv0 page table, otherwise it's the entry Lv1 page table.
+	 */
+	gd->arch.tlb_fillptr = gd->arch.tlb_addr;
+	create_table();
+
+	/* Now add all MMU table entries one after another to the table */
+	for (i = 0; i < ARRAY_SIZE(mem_map); i++)
+		add_map(&mem_map[i]);
 }
 
 #else
@@ -157,10 +381,8 @@ __weak void mmu_setup(void)
 	int el;
 
 #ifdef CONFIG_SYS_FULL_VA
-	unsigned long coreid = read_mpidr() & CONFIG_COREID_MASK;
-
-	/* Set up page tables only on BSP */
-	if (coreid == BSP_COREID)
+	/* Set up page tables only once */
+	if (!gd->arch.tlb_fillptr)
 		setup_pgtables();
 
 	el = current_el();
@@ -311,6 +533,79 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 	flush_dcache_range(start, end);
 	asm volatile("dsb sy");
 }
+#else
+static bool is_aligned(u64 addr, u64 size, u64 align)
+{
+	return !(addr & (align - 1)) && !(size & (align - 1));
+}
+
+static u64 set_one_region(u64 start, u64 size, u64 attrs, int level)
+{
+	int levelshift = level2shift(level);
+	u64 levelsize = 1ULL << levelshift;
+	u64 *pte = find_pte(start, level);
+
+	/* Can we can just modify the current level block PTE? */
+	if (is_aligned(start, size, levelsize)) {
+		*pte &= ~PMD_ATTRINDX_MASK;
+		*pte |= attrs;
+		debug("Set attrs=%llx pte=%p level=%d\n", attrs, pte, level);
+
+		return levelsize;
+	}
+
+	/* Unaligned or doesn't fit, maybe split block into table */
+	debug("addr=%llx level=%d pte=%p (%llx)\n", start, level, pte, *pte);
+
+	/* Maybe we need to split the block into a table */
+	if (pte_type(pte) == PTE_TYPE_BLOCK)
+		split_block(pte, level);
+
+	/* And then double-check it became a table or already is one */
+	if (pte_type(pte) != PTE_TYPE_TABLE)
+		panic("PTE %p (%llx) for addr=%llx should be a table",
+		      pte, *pte, start);
+
+	/* Roll on to the next page table level */
+	return 0;
+}
+
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option)
+{
+	u64 attrs = PMD_ATTRINDX(option);
+	u64 real_start = start;
+	u64 real_size = size;
+
+	debug("start=%lx size=%lx\n", (ulong)start, (ulong)size);
+
+	/*
+	 * Loop through the address range until we find a page granule that fits
+	 * our alignment constraints, then set it to the new cache attributes
+	 */
+	while (size > 0) {
+		int level;
+		u64 r;
+
+		for (level = 1; level < 4; level++) {
+			r = set_one_region(start, size, attrs, level);
+			if (r) {
+				/* PTE successfully replaced */
+				size -= r;
+				start += r;
+				break;
+			}
+		}
+
+	}
+
+	asm volatile("dsb sy");
+	__asm_invalidate_tlb_all();
+	asm volatile("dsb sy");
+	asm volatile("isb");
+	flush_dcache_range(real_start, real_start + real_size);
+	asm volatile("dsb sy");
+}
 #endif
 
 #else	/* CONFIG_SYS_DCACHE_OFF */
arch/arm/include/asm/armv8/mmu.h +31 -37

95 lines not on this page · raw message

arch/arm/include/asm/global_data.h +2 -2
index dcfa098..3dec1db 100644
@@ -38,10 +38,10 @@ struct arch_global_data {
 	unsigned long long timer_reset_value;
 #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
 	unsigned long tlb_addr;
+	unsigned long tlb_size;
 #if defined(CONFIG_SYS_FULL_VA)
-	unsigned long pmd_addr[CONFIG_SYS_PTL1_ENTRIES];
+	unsigned long tlb_fillptr;
 #endif
-	unsigned long tlb_size;
 #endif
 
 #ifdef CONFIG_OMAP_COMMON
arch/arm/include/asm/system.h +2 -1
index 026e7ef..ffd6fe5 100644
@@ -20,7 +20,8 @@
 #ifndef CONFIG_SYS_FULL_VA
 #define PGTABLE_SIZE	(0x10000)
 #else
-#define PGTABLE_SIZE	CONFIG_SYS_PGTABLE_SIZE
+u64 get_page_table_size(void);
+#define PGTABLE_SIZE	get_page_table_size()
 #endif
 
 /* 2MB granularity */
include/configs/thunderx_88xx.h +6 -8
index b9f93ad..20b25f7 100644
@@ -22,21 +22,19 @@
 
 #define MEM_BASE			0x00500000
 
-#define CONFIG_COREID_MASK             0xffffff
-
 #define CONFIG_SYS_FULL_VA
 
 #define CONFIG_SYS_LOWMEM_BASE		MEM_BASE
 
 #define CONFIG_SYS_MEM_MAP		{{0x000000000000UL, 0x40000000000UL, \
-					  PTL2_MEMTYPE(MT_NORMAL) |	     \
-					  PTL2_BLOCK_NON_SHARE},	     \
+					  PTE_BLOCK_MEMTYPE(MT_NORMAL) |     \
+					  PTE_BLOCK_NON_SHARE},	     \
 					 {0x800000000000UL, 0x40000000000UL, \
-					  PTL2_MEMTYPE(MT_DEVICE_NGNRNE) |   \
-					  PTL2_BLOCK_NON_SHARE},	     \
+					  PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | \
+					  PTE_BLOCK_NON_SHARE},	     \
 					 {0x840000000000UL, 0x40000000000UL, \
-					  PTL2_MEMTYPE(MT_DEVICE_NGNRNE) |   \
-					  PTL2_BLOCK_NON_SHARE},	     \
+					  PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | \
+					  PTE_BLOCK_NON_SHARE},	     \
 					}
 
 #define CONFIG_SYS_MEM_MAP_SIZE		3
Signature · 1 line
1.8.5.6
#6 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

Hi,

This is a good cleanup!

Quoted 11 lines · Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

This latter part scares me a bit. It's _very_ difficult to get that right, and bugs in incorrect code are latent with asynchronous triggers. We had to rework the Linux page table creation to avoid issues with splitting/creating sections [1,2] early in boot.

For Linux we can't split arbitrary sections dynamically (in the kernel mapping) because that risks unmapping code/data in active use as part of the required Break-Before-Make sequence.

I suspect the same applies to U-Boot, and that splitting section dynamically is unsafe once the MMU is on and the mappings are active.

In the ARM ARM (ARM DDI 0487A.i) see D4.7.1 General TLB maintenance requirements, which has a sub-section "Using break-before-make when updating translation table entries".

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/401434.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/386209.html

arch/arm/cpu/armv8/cache_v8.c static u64 get_tcr(int el, u64 *pips, u64 *pva_bits) · line 171

Quoted 6 lines
arch/arm/cpu/armv8/cache_v8.c +6 -0
+static void set_pte_table(u64 *pte, u64 *table)
+{
+	/* Point *pte to the new table */
+	debug("Setting %p to addr=%p\n", pte, table);
+	*pte = PTE_TYPE_TABLE | (ulong)table;
+}

When the MMU is on, you will need barriers between creating/updating tables and plumbing them into the next level. Otherwise the stores may not be observed in-order by the TLB, and stale (garbage) might be allocated into the TLB.

Either that needs to live at the end of the table creation function, or the beginning of this one.

[...]

Quoted 11 lines
arch/arm/cpu/armv8/cache_v8.c +11 -0
+/* Splits a block PTE into table with subpages spanning the old block */
+static void split_block(u64 *pte, int level)
+{
+	u64 old_pte = *pte;
+	u64 *new_table;
+	u64 i = 0;
+	/* level describes the parent level, we need the child ones */
+	int levelshift = level2shift(level + 1);
+
+	if (pte_type(pte) != PTE_TYPE_BLOCK)
+		panic("PTE %p (%llx) is not a block. Some driver code wants to "

arch/arm/cpu/armv8/cache_v8.c static u64 get_tcr(int el, u64 *pips, u64 *pva_bits) · line 236

Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+		      "modify dcache settings for an range not covered in "
+		      "mem_map.", pte, old_pte);
+
+	new_table = create_table();
+	debug("Splitting pte %p (%llx) into %p\n", pte, old_pte, new_table);
+
+	for (i = 0; i < MAX_PTE_ENTRIES; i++) {
+		new_table[i] = old_pte | (i << levelshift);
+		debug("Setting new_table[%lld] = %llx\n", i, new_table[i]);
+	}

You need a barrier here to ensure ordering of the above modifications with the below plumbing into the next level table.

arch/arm/cpu/armv8/cache_v8.c static u64 get_tcr(int el, u64 *pips, u64 *pva_bits) · line 239

Quoted 3 lines
arch/arm/cpu/armv8/cache_v8.c +3 -0
+
+	/* Set the new table into effect */
+	set_pte_table(pte, new_table);

Splitting blocks in this manner requires Break-Before-Make. You must go via an invalid entry.

[...]

Quoted 4 lines
arch/arm/cpu/armv8/cache_v8.c +4 -0
+static u64 set_one_region(u64 start, u64 size, u64 attrs, int level)
+{
+	int levelshift = level2shift(level);
+	u64 levelsize = 1ULL << levelshift;

arch/arm/cpu/armv8/cache_v8.c void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, · line 555

Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+	u64 *pte = find_pte(start, level);
+
+	/* Can we can just modify the current level block PTE? */
+	if (is_aligned(start, size, levelsize)) {
+		*pte &= ~PMD_ATTRINDX_MASK;
+		*pte |= attrs;
+		debug("Set attrs=%llx pte=%p level=%d\n", attrs, pte, level);
+
+		return levelsize;
+	}

When the MMU is on, I believe you need Break-Before-Make here due to the change of memory attributes.

Quoted 39 lines
arch/arm/cpu/armv8/cache_v8.c +39 -0
+
+	/* Unaligned or doesn't fit, maybe split block into table */
+	debug("addr=%llx level=%d pte=%p (%llx)\n", start, level, pte, *pte);
+
+	/* Maybe we need to split the block into a table */
+	if (pte_type(pte) == PTE_TYPE_BLOCK)
+		split_block(pte, level);
+
+	/* And then double-check it became a table or already is one */
+	if (pte_type(pte) != PTE_TYPE_TABLE)
+		panic("PTE %p (%llx) for addr=%llx should be a table",
+		      pte, *pte, start);
+
+	/* Roll on to the next page table level */
+	return 0;
+}
+
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option)
+{
+	u64 attrs = PMD_ATTRINDX(option);
+	u64 real_start = start;
+	u64 real_size = size;
+
+	debug("start=%lx size=%lx\n", (ulong)start, (ulong)size);
+
+	/*
+	 * Loop through the address range until we find a page granule that fits
+	 * our alignment constraints, then set it to the new cache attributes
+	 */
+	while (size > 0) {
+		int level;
+		u64 r;
+
+		for (level = 1; level < 4; level++) {
+			r = set_one_region(start, size, attrs, level);
+			if (r) {
+				/* PTE successfully replaced */
+				size -= r;

arch/arm/cpu/armv8/cache_v8.c void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, · line 604

Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+				start += r;
+				break;
+			}
+		}
+
+	}
+
+	asm volatile("dsb sy");
+	__asm_invalidate_tlb_all();
+	asm volatile("dsb sy");

This all needs to happen above with the Break-Before-Make sequence. It is too late to have this here due to asynchronous TLB behaviour.

arch/arm/cpu/armv8/cache_v8.c void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, · line 605

Quoted 1 line
arch/arm/cpu/armv8/cache_v8.c +1 -0
+	asm volatile("isb");

I'm not sure this ISB is necessary.

arch/arm/cpu/armv8/cache_v8.c void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, · line 607

Quoted 2 lines
arch/arm/cpu/armv8/cache_v8.c +2 -0
+	flush_dcache_range(real_start, real_start + real_size);
+	asm volatile("dsb sy");

Does flush_dcache_range not have this barrier implicitly?

Thanks,
Mark.

#7 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

In reply to Alexander Graf
Quoted 17 lines · Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

With all this in place, there is very little reason to create your own page tables in board specific files.

arch/arm/cpu/armv8/cache_v8.c static u64 get_tcr(int el, u64 *pips, u64 *pva_bits) · line 248

Quoted 7 lines
arch/arm/cpu/armv8/cache_v8.c +7 -0
+/*
+ * This is a recursively called function to count the number of
+ * page tables we need to cover a particular PTE range. If you
+ * call this with level = -1 you basically get the full 48 bit
+ * coverage.
+ */
+static int count_required_pts(u64 addr, int level, u64 maxaddr)

I think this looks correct now. Nits below if a respin is needed for other reasons.

arch/arm/cpu/armv8/cache_v8.c static u64 get_tcr(int el, u64 *pips, u64 *pva_bits) · line 256

Quoted 8 lines
arch/arm/cpu/armv8/cache_v8.c +8 -0
+{
+	int levelshift = level2shift(level);
+	u64 levelsize = 1ULL << levelshift;
+	u64 levelmask = levelsize - 1;
+	u64 levelend = addr + levelsize;
+	int r = 0;
+	int i;
+	bool is_level = false;

I might suggest renaming that is_level_needed. It's not obvious to me exactly what the name "is_level" is intended to represent; the name seems to represent whether something (unspecified) is a level or not.

Quoted 25 lines
arch/arm/cpu/armv8/cache_v8.c +25 -0
+
+	for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+		struct mm_region *map = &mem_map[i];
+		u64 start = map->base;
+		u64 end = start + map->size;
+
+		/* Check if the PTE would overlap with the map */
+		if (max(addr, start) <= min(levelend, end)) {
+			start = max(addr, start);
+			end = min(levelend, end);
+
+			/* We need a sub-pt for this level */
+			if ((start & levelmask) || (end & levelmask)) {
+				is_level = true;
+				break;
+			}
+
+			/* Lv0 can not do block PTEs, so do levels here too */
+			if (level <= 0) {
+				is_level = true;
+				break;
+			}
+		}
+	}
+

arch/arm/cpu/armv8/cache_v8.c static u64 get_tcr(int el, u64 *pips, u64 *pva_bits) · line 291

Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+	/*
+	 * Block PTEs at this level are already covered by the parent page
+	 * table, so we only need to count sub page tables.
+	 */
+	if (is_level) {
+		int sublevel = level + 1;
+		u64 sublevelsize = 1ULL << level2shift(sublevel);
+
+		/* Account for the new sub page table ... */
+		r = 1;

"Account for the page table at /this/ level"? This may represent the top-level (0/1) page table, not just sub page tables. The sub-level accounting is via the recursive call to count_required_pts() I believe.

Quoted 5 lines
arch/arm/cpu/armv8/cache_v8.c +5 -0
+
+		/* ... and for all child page tables that one might have */
+		for (i = 0; i < MAX_PTE_ENTRIES; i++) {
+			r += count_required_pts(addr, sublevel, maxaddr);
+			addr += sublevelsize;
#8 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

Quoted 38 lines · Stephen Warren
Quoted 17 lines · Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

With all this in place, there is very little reason to create your own page tables in board specific files.

Review

Quoted 7 lines
(quoted diff) +7 -0
+/*
+ * This is a recursively called function to count the number of
+ * page tables we need to cover a particular PTE range. If you
+ * call this with level = -1 you basically get the full 48 bit
+ * coverage.
+ */
+static int count_required_pts(u64 addr, int level, u64 maxaddr)

I think this looks correct now. Nits below if a respin is needed for other reasons.

Review

Quoted 8 lines
(quoted diff) +8 -0
+{
+    int levelshift = level2shift(level);
+    u64 levelsize = 1ULL << levelshift;
+    u64 levelmask = levelsize - 1;
+    u64 levelend = addr + levelsize;
+    int r = 0;
+    int i;
+    bool is_level = false;
Quoted 3 lines

I might suggest renaming that is_level_needed. It's not obvious to me exactly what the name "is_level" is intended to represent; the name seems to represent whether something (unspecified) is a level or not.

We're basically asking the function whether a PTE for address <addr> at level <level> would be an inval/block/level PTE. is_level marks it as a level pte.

I could maybe rename this into pte_type and create an enum that is either PTE_INVAL, PTE_BLOCK or PTE_LEVEL.

Quoted 32 lines
Quoted 32 lines
(quoted diff) +31 -0
+
+    for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+        struct mm_region *map = &mem_map[i];
+        u64 start = map->base;
+        u64 end = start + map->size;
+
+        /* Check if the PTE would overlap with the map */
+        if (max(addr, start) <= min(levelend, end)) {
+            start = max(addr, start);
+            end = min(levelend, end);
+
+            /* We need a sub-pt for this level */
+            if ((start & levelmask) || (end & levelmask)) {
+                is_level = true;
+                break;
+            }
+
+            /* Lv0 can not do block PTEs, so do levels here too */
+            if (level <= 0) {
+                is_level = true;
+                break;
+            }
+        }
+    }
+
+    /*
+     * Block PTEs at this level are already covered by the parent page
+     * table, so we only need to count sub page tables.
+     */
+    if (is_level) {
+        int sublevel = level + 1;
Quoted 8 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+        u64 sublevelsize = 1ULL << level2shift(sublevel);
+
+        /* Account for the new sub page table ... */
+        r = 1;

"Account for the page table at /this/ level"? This may represent the top-level (0/1) page table, not just sub page tables. The sub-level accounting is via the recursive call to count_required_pts() I believe.

I think the easiest way to visualize what's going on is if we start with level -1.

We basically ask the function at level -1 whether a PTE at level -1 (48 bits) would fit into a block PTE at level -1 (so the PTE spans all 48 bits) or whether we need to create a new level entry plus page table for it.

Obviously for all entries this resolves into a "yes, create a level PTE entry and a new page table to point to". So at level=-1 we account for the root page table which really is a "sub page table" here.

Then we recursively go through the address space that we cover, checking whether a page fits into inval/block PTEs or we need to create page tables for it as well.

So at level=0, we really are checking for PTEs that you'd have at level=0 (38 bits). If the <addr, level> combination at level=0 results in "level", we need to create a level=1 page table and walk through that one again.

I agree that the logic seems backwards, but it's the only thing that I could come up with that has low memory footprint (required to run from SRAM).

Alex

Does the diff below make it more obvious? It's slightly more runtime overhead because we set the PTE_BLOCK case as well, but maintainability of the code wins vs speed imho :).

arch/arm/cpu/armv8/cache_v8.c +6 -0
index 7236455..e8f2632 100644
@@ -236,6 +236,12 @@ static void split_block(u64 *pte, int level)
 	set_pte_table(pte, new_table);
 }
 
+enum pte_type {
+	PTE_INVAL,
+	PTE_BLOCK,
+	PTE_LEVEL,
+};
+
 /*
  * This is a recursively called function to count the number of
  * page tables we need to cover a particular PTE range. If you
@@ -250,7 +256,7 @@ static int count_required_pts(u64 addr, int level,

u64 maxaddr)
u64 levelend = addr + levelsize;
int r = 0;
int i;
- bool is_level = false;
+ enum pte_type pte_type = PTE_INVAL;

for (i = 0; mem_map[i].size || mem_map[i].attrs; i++) {
struct mm_region *map = &mem_map[i];
@@ -264,15 +270,18 @@ static int count_required_pts(u64 addr, int level, u64 maxaddr)

/* We need a sub-pt for this level */ if ((start & levelmask) || (end & levelmask)) {
- is_level = true;
+ pte_type = PTE_LEVEL;
break;
}

/* Lv0 can not do block PTEs, so do levels here too */ if (level <= 0) {
- is_level = true;
+ pte_type = PTE_LEVEL;
break;
}
+
+ /* PTE is active, but fits into a block */
+ pte_type = PTE_BLOCK;
}
}

@@ -280,7 +289,7 @@ static int count_required_pts(u64 addr, int level, u64 maxaddr)
* Block PTEs@this level are already covered by the parent page
* table, so we only need to count sub page tables. */
- if (is_level) {
+ if (pte_type == PTE_LEVEL) {
int sublevel = level + 1;
u64 sublevelsize = 1ULL << level2shift(sublevel);

#9 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

Quoted 93 lines · Alexander Graf
Quoted 38 lines · Stephen Warren
Quoted 17 lines · Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

With all this in place, there is very little reason to create your own page tables in board specific files.

Review

Quoted 7 lines
(quoted diff) +7 -0
+/*
+ * This is a recursively called function to count the number of
+ * page tables we need to cover a particular PTE range. If you
+ * call this with level = -1 you basically get the full 48 bit
+ * coverage.
+ */
+static int count_required_pts(u64 addr, int level, u64 maxaddr)

I think this looks correct now. Nits below if a respin is needed for other reasons.

Review

Quoted 8 lines
(quoted diff) +8 -0
+{
+    int levelshift = level2shift(level);
+    u64 levelsize = 1ULL << levelshift;
+    u64 levelmask = levelsize - 1;
+    u64 levelend = addr + levelsize;
+    int r = 0;
+    int i;
+    bool is_level = false;
Quoted 3 lines

I might suggest renaming that is_level_needed. It's not obvious to me exactly what the name "is_level" is intended to represent; the name seems to represent whether something (unspecified) is a level or not.

We're basically asking the function whether a PTE for address <addr> at level <level> would be an inval/block/level PTE. is_level marks it as a level pte.

I could maybe rename this into pte_type and create an enum that is either PTE_INVAL, PTE_BLOCK or PTE_LEVEL.

Quoted 32 lines
Quoted 32 lines
(quoted diff) +31 -0
+
+    for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+        struct mm_region *map = &mem_map[i];
+        u64 start = map->base;
+        u64 end = start + map->size;
+
+        /* Check if the PTE would overlap with the map */
+        if (max(addr, start) <= min(levelend, end)) {
+            start = max(addr, start);
+            end = min(levelend, end);
+
+            /* We need a sub-pt for this level */
+            if ((start & levelmask) || (end & levelmask)) {
+                is_level = true;
+                break;
+            }
+
+            /* Lv0 can not do block PTEs, so do levels here too */
+            if (level <= 0) {
+                is_level = true;
+                break;
+            }
+        }
+    }
+
+    /*
+     * Block PTEs at this level are already covered by the parent page
+     * table, so we only need to count sub page tables.
+     */
+    if (is_level) {
+        int sublevel = level + 1;
Quoted 8 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+        u64 sublevelsize = 1ULL << level2shift(sublevel);
+
+        /* Account for the new sub page table ... */
+        r = 1;

"Account for the page table at /this/ level"? This may represent the top-level (0/1) page table, not just sub page tables. The sub-level accounting is via the recursive call to count_required_pts() I believe.

Quoted 6 lines

I think the easiest way to visualize what's going on is if we start with level -1.

We basically ask the function at level -1 whether a PTE at level -1 (48 bits) would fit into a block PTE at level -1 (so the PTE spans all 48 bits) or whether we need to create a new level entry plus page table for it.

Hmm, I had overlooked that the call to count_required_pts() passed "start_level - 1" not "start_level". Now that I see that, your explanation makes more sense to me.

It'd be nice if some/all of this explanation were comments in the code to aid readers.

That said, I would have expected a slightly more direct calculation; why not:

int start_level = 0; // or 1 if small VA space total_pts = count_required_pts(start_level); total_pts += 4; // "random" number to account for later splits

int count_required_pts(int level, ...) {
int npts = 1; // the table at this level
for each pte slot at this level:
if a mem_map entry starts/stops within the pte slot:
npts += count_required_pts(level + 1, ...);
else:
// nothing; pte is either a block or inval at this level
return npts;
}

Still, the current code appears to work find, and can be ammended later if we want.

Quoted 16 lines

Obviously for all entries this resolves into a "yes, create a level PTE entry and a new page table to point to". So at level=-1 we account for the root page table which really is a "sub page table" here.

Then we recursively go through the address space that we cover, checking whether a page fits into inval/block PTEs or we need to create page tables for it as well.

So at level=0, we really are checking for PTEs that you'd have at level=0 (38 bits). If the <addr, level> combination at level=0 results in "level", we need to create a level=1 page table and walk through that one again.

I agree that the logic seems backwards, but it's the only thing that I could come up with that has low memory footprint (required to run from SRAM).

#10 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

Quoted 106 lines · Stephen Warren
Quoted 93 lines · Alexander Graf
Quoted 38 lines · Stephen Warren
Quoted 17 lines · Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

With all this in place, there is very little reason to create your own page tables in board specific files.

Review

Quoted 7 lines
(quoted diff) +7 -0
+/*
+ * This is a recursively called function to count the number of
+ * page tables we need to cover a particular PTE range. If you
+ * call this with level = -1 you basically get the full 48 bit
+ * coverage.
+ */
+static int count_required_pts(u64 addr, int level, u64 maxaddr)

I think this looks correct now. Nits below if a respin is needed for other reasons.

Review

Quoted 8 lines
(quoted diff) +8 -0
+{
+    int levelshift = level2shift(level);
+    u64 levelsize = 1ULL << levelshift;
+    u64 levelmask = levelsize - 1;
+    u64 levelend = addr + levelsize;
+    int r = 0;
+    int i;
+    bool is_level = false;
Quoted 3 lines

I might suggest renaming that is_level_needed. It's not obvious to me exactly what the name "is_level" is intended to represent; the name seems to represent whether something (unspecified) is a level or not.

We're basically asking the function whether a PTE for address <addr> at level <level> would be an inval/block/level PTE. is_level marks it as a level pte.

I could maybe rename this into pte_type and create an enum that is either PTE_INVAL, PTE_BLOCK or PTE_LEVEL.

Quoted 32 lines
Quoted 32 lines
(quoted diff) +31 -0
+
+    for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+        struct mm_region *map = &mem_map[i];
+        u64 start = map->base;
+        u64 end = start + map->size;
+
+        /* Check if the PTE would overlap with the map */
+        if (max(addr, start) <= min(levelend, end)) {
+            start = max(addr, start);
+            end = min(levelend, end);
+
+            /* We need a sub-pt for this level */
+            if ((start & levelmask) || (end & levelmask)) {
+                is_level = true;
+                break;
+            }
+
+            /* Lv0 can not do block PTEs, so do levels here too */
+            if (level <= 0) {
+                is_level = true;
+                break;
+            }
+        }
+    }
+
+    /*
+     * Block PTEs at this level are already covered by the parent page
+     * table, so we only need to count sub page tables.
+     */
+    if (is_level) {
+        int sublevel = level + 1;
Quoted 8 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+        u64 sublevelsize = 1ULL << level2shift(sublevel);
+
+        /* Account for the new sub page table ... */
+        r = 1;

"Account for the page table at /this/ level"? This may represent the top-level (0/1) page table, not just sub page tables. The sub-level accounting is via the recursive call to count_required_pts() I believe.

Quoted 7 lines

I think the easiest way to visualize what's going on is if we start with level -1.

We basically ask the function at level -1 whether a PTE at level -1 (48 bits) would fit into a block PTE at level -1 (so the PTE spans all 48 bits) or whether we need to create a new level entry plus page table for it.

Hmm, I had overlooked that the call to count_required_pts() passed "start_level - 1" not "start_level". Now that I see that, your explanation makes more sense to me.

Quoted 8 lines

It'd be nice if some/all of this explanation were comments in the code to aid readers.

That said, I would have expected a slightly more direct calculation; why not:

int start_level = 0; // or 1 if small VA space
total_pts = count_required_pts(start_level);

We need to account for the count twice, to have an (immutable) copy of our page tables around when we split them.

Quoted 3 lines

total_pts += 4; // "random" number to account for later splits

int count_required_pts(int level, ...) {

... includes the address, as I have it today, I guess?

Quoted 4 lines

int npts = 1; // the table at this level
for each pte slot at this level:
if a mem_map entry starts/stops within the pte slot:
npts += count_required_pts(level + 1, ...);

This means we would count some levels multiple times. Imagine two entries from

	1G - 1.25G
	1.25G - 1.5G

With your pseudo-code, we would count for both cases while checking for 1G page table entries. Hence we need to jump out of the loop here and do the counting outside.

At which point this is basically my code, right? :)

Alex

Quoted 7 lines
        else:
            // nothing; pte is either a block or inval at this level
    return npts;
}

Still, the current code appears to work find, and can be ammended later if we want.

#11 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

Quoted 112 lines · Alexander Graf
Quoted 109 lines · Stephen Warren
Quoted 93 lines · Alexander Graf
Quoted 38 lines · Stephen Warren
Quoted 17 lines · Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

With all this in place, there is very little reason to create your own page tables in board specific files.

Review

Quoted 7 lines
(quoted diff) +7 -0
+/*
+ * This is a recursively called function to count the number of
+ * page tables we need to cover a particular PTE range. If you
+ * call this with level = -1 you basically get the full 48 bit
+ * coverage.
+ */
+static int count_required_pts(u64 addr, int level, u64 maxaddr)

I think this looks correct now. Nits below if a respin is needed for other reasons.

Review

Quoted 8 lines
(quoted diff) +8 -0
+{
+    int levelshift = level2shift(level);
+    u64 levelsize = 1ULL << levelshift;
+    u64 levelmask = levelsize - 1;
+    u64 levelend = addr + levelsize;
+    int r = 0;
+    int i;
+    bool is_level = false;
Quoted 3 lines

I might suggest renaming that is_level_needed. It's not obvious to me exactly what the name "is_level" is intended to represent; the name seems to represent whether something (unspecified) is a level or not.

We're basically asking the function whether a PTE for address <addr> at level <level> would be an inval/block/level PTE. is_level marks it as a level pte.

I could maybe rename this into pte_type and create an enum that is either PTE_INVAL, PTE_BLOCK or PTE_LEVEL.

Quoted 32 lines
Quoted 32 lines
(quoted diff) +31 -0
+
+    for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+        struct mm_region *map = &mem_map[i];
+        u64 start = map->base;
+        u64 end = start + map->size;
+
+        /* Check if the PTE would overlap with the map */
+        if (max(addr, start) <= min(levelend, end)) {
+            start = max(addr, start);
+            end = min(levelend, end);
+
+            /* We need a sub-pt for this level */
+            if ((start & levelmask) || (end & levelmask)) {
+                is_level = true;
+                break;
+            }
+
+            /* Lv0 can not do block PTEs, so do levels here too */
+            if (level <= 0) {
+                is_level = true;
+                break;
+            }
+        }
+    }
+
+    /*
+     * Block PTEs at this level are already covered by the parent page
+     * table, so we only need to count sub page tables.
+     */
+    if (is_level) {
+        int sublevel = level + 1;
Quoted 8 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+        u64 sublevelsize = 1ULL << level2shift(sublevel);
+
+        /* Account for the new sub page table ... */
+        r = 1;

"Account for the page table at /this/ level"? This may represent the top-level (0/1) page table, not just sub page tables. The sub-level accounting is via the recursive call to count_required_pts() I believe.

Quoted 7 lines

I think the easiest way to visualize what's going on is if we start with level -1.

We basically ask the function at level -1 whether a PTE at level -1 (48 bits) would fit into a block PTE at level -1 (so the PTE spans all 48 bits) or whether we need to create a new level entry plus page table for it.

Hmm, I had overlooked that the call to count_required_pts() passed "start_level - 1" not "start_level". Now that I see that, your explanation makes more sense to me.

It'd be nice if some/all of this explanation were comments in the code to aid readers.

Quoted 8 lines
Quoted 5 lines

That said, I would have expected a slightly more direct calculation; why not:

int start_level = 0; // or 1 if small VA space
total_pts = count_required_pts(start_level);

We need to account for the count twice, to have an (immutable) copy of our page tables around when we split them.

I think that's just a hard-coded "* 2" there, unless I'm missing your point?

Quoted 14 lines
Quoted 3 lines

total_pts += 4; // "random" number to account for later splits

int count_required_pts(int level, ...) {

... includes the address, as I have it today, I guess?

Quoted 4 lines

int npts = 1; // the table at this level
for each pte slot at this level:
if a mem_map entry starts/stops within the pte slot:
npts += count_required_pts(level + 1, ...);

This means we would count some levels multiple times. Imagine two entries from

Quoted 6 lines
	1G - 1.25G
	1.25G - 1.5G

With your pseudo-code, we would count for both cases while checking for 1G page table entries. Hence we need to jump out of the loop here and do the counting outside.

I don't think so; "if a mem_map entry starts/stops within the pte slot" is a single decision for that PT entry at the current level, not a loop that recurses once per memory block. Hence, you'd only ever recurse once, so there's no double-counting.

Quoted 14 lines

At which point this is basically my code, right? :)

Alex

Quoted 7 lines
         else:
             // nothing; pte is either a block or inval at this level
     return npts;
}

Still, the current code appears to work find, and can be ammended later if we want.

#12 lore raw

PATCH 2/10 arm64: Make full va map code more dynamic

Quoted 119 lines · Stephen Warren
Quoted 118 lines · Alexander Graf
Quoted 115 lines · Stephen Warren
Quoted 95 lines · Alexander Graf
Quoted 38 lines · Stephen Warren
Quoted 17 lines · Alexander Graf

The idea to generate our pages tables from an array of memory ranges is very sound. However, instead of hard coding the code to create up to 2 levels of 64k granule page tables, we really should just create normal 4k page tables that allow us to set caching attributes on 2M or 4k level later on.

So this patch moves the full_va mapping code to 4k page size and makes it fully flexible to dynamically create as many levels as necessary for a map (including dynamic 1G/2M pages). It also adds support to dynamically split a large map into smaller ones when some code wants to set dcache attributes.

With all this in place, there is very little reason to create your own page tables in board specific files.

Review

Quoted 7 lines
(quoted diff) +7 -0
+/*
+ * This is a recursively called function to count the number of
+ * page tables we need to cover a particular PTE range. If you
+ * call this with level = -1 you basically get the full 48 bit
+ * coverage.
+ */
+static int count_required_pts(u64 addr, int level, u64 maxaddr)

I think this looks correct now. Nits below if a respin is needed for other reasons.

Review

Quoted 8 lines
(quoted diff) +8 -0
+{
+    int levelshift = level2shift(level);
+    u64 levelsize = 1ULL << levelshift;
+    u64 levelmask = levelsize - 1;
+    u64 levelend = addr + levelsize;
+    int r = 0;
+    int i;
+    bool is_level = false;
Quoted 3 lines

I might suggest renaming that is_level_needed. It's not obvious to me exactly what the name "is_level" is intended to represent; the name seems to represent whether something (unspecified) is a level or not.

We're basically asking the function whether a PTE for address <addr> at level <level> would be an inval/block/level PTE. is_level marks it as a level pte.

I could maybe rename this into pte_type and create an enum that is either PTE_INVAL, PTE_BLOCK or PTE_LEVEL.

Quoted 34 lines
Quoted 34 lines
(quoted diff) +27 -0
+
+    for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+        struct mm_region *map = &mem_map[i];
+        u64 start = map->base;
+        u64 end = start + map->size;
+
+        /* Check if the PTE would overlap with the map */
+        if (max(addr, start) <= min(levelend, end)) {
+            start = max(addr, start);
+            end = min(levelend, end);
+
+            /* We need a sub-pt for this level */
+            if ((start & levelmask) || (end & levelmask)) {
+                is_level = true;
+                break;
+            }
+
+            /* Lv0 can not do block PTEs, so do levels here too */
+            if (level <= 0) {
+                is_level = true;
+                break;
+            }
+        }
+    }
+
+    /*
+     * Block PTEs at this level are already covered by the parent

page

(quoted diff) +5 -0
+     * table, so we only need to count sub page tables.
+     */
+    if (is_level) {
+        int sublevel = level + 1;
+        u64 sublevelsize = 1ULL << level2shift(sublevel);
Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+
+        /* Account for the new sub page table ... */
+        r = 1;

"Account for the page table at /this/ level"? This may represent the top-level (0/1) page table, not just sub page tables. The sub-level accounting is via the recursive call to count_required_pts() I believe.

Quoted 8 lines

I think the easiest way to visualize what's going on is if we start with
level -1.

We basically ask the function at level -1 whether a PTE at level -1 (48 bits) would fit into a block PTE at level -1 (so the PTE spans all 48 bits) or whether we need to create a new level entry plus page table for it.

Hmm, I had overlooked that the call to count_required_pts() passed "start_level - 1" not "start_level". Now that I see that, your explanation makes more sense to me.

It'd be nice if some/all of this explanation were comments in the code to aid readers.

That said, I would have expected a slightly more direct calculation; why not:

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

int start_level = 0; // or 1 if small VA space
total_pts = count_required_pts(start_level);

We need to account for the count twice, to have an (immutable) copy of our page tables around when we split them.

I think that's just a hard-coded "* 2" there, unless I'm missing your point?

Yes :).

Quoted 18 lines
Quoted 18 lines
Quoted 4 lines

total_pts += 4; // "random" number to account for later splits

int count_required_pts(int level, ...) {

... includes the address, as I have it today, I guess?

Quoted 4 lines

int npts = 1; // the table at this level
for each pte slot at this level:
if a mem_map entry starts/stops within the pte slot:
npts += count_required_pts(level + 1, ...);

This means we would count some levels multiple times. Imagine two entries from

    1G - 1.25G
    1.25G - 1.5G
Quoted 8 lines
Quoted 3 lines

With your pseudo-code, we would count for both cases while checking for 1G page table entries. Hence we need to jump out of the loop here and do the counting outside.

I don't think so; "if a mem_map entry starts/stops within the pte slot" is a single decision for that PT entry at the current level, not a loop that recurses once per memory block. Hence, you'd only ever recurse once, so there's no double-counting.

Ah, "if a" means another loop over all mem_maps, eventually saying "yes, I am overlapping".

Either way, I am not fully convinced the code would end up much more readable than it is today, but I'll be happy to get persuaded otherwise :).

And yes, that's definitely something we can still work on down the road in future follow-up patches.

Alex

#13 lore raw

PATCH 3/10 thunderx: Move mmu table into board file

In reply to Alexander Graf

The MMU range table can vary depending on things we may only find out at runtime. While the very simple ThunderX variant does not change, other boards will, so move the definition from a static entry in a header file to the board file.

---
 arch/arm/cpu/armv8/cache_v8.c    |  8 +++-----
 arch/arm/include/asm/armv8/mmu.h |  2 ++
 board/cavium/thunderx/thunderx.c | 24 ++++++++++++++++++++++++
 include/configs/thunderx_88xx.h  | 11 -----------
 4 files changed, 29 insertions(+), 16 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +3 -5
index 9cf46b1..601199b 100644
@@ -36,8 +36,6 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 
 #ifdef CONFIG_SYS_FULL_VA
-static struct mm_region mem_map[] = CONFIG_SYS_MEM_MAP;
-
 static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
 {
 	u64 max_addr = 0;
@@ -46,7 +44,7 @@ static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
 	int i;
 
 	/* Find the largest address we need to support */
-	for (i = 0; i < ARRAY_SIZE(mem_map); i++)
+	for (i = 0; mem_map[i].size || mem_map[i].attrs; i++)
 		max_addr = max(max_addr, mem_map[i].base + mem_map[i].size);
 
 	/* Calculate the maximum physical (and thus virtual) address */
@@ -255,7 +253,7 @@ static int count_required_pts(u64 addr, int level, u64 maxaddr)
 	int i;
 	bool is_level = false;
 
-	for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+	for (i = 0; mem_map[i].size || mem_map[i].attrs; i++) {
 		struct mm_region *map = &mem_map[i];
 		u64 start = map->base;
 		u64 end = start + map->size;
@@ -345,7 +343,7 @@ static void setup_pgtables(void)
 	create_table();
 
 	/* Now add all MMU table entries one after another to the table */
-	for (i = 0; i < ARRAY_SIZE(mem_map); i++)
+	for (i = 0; mem_map[i].size || mem_map[i].attrs; i++)
 		add_map(&mem_map[i]);
 }
 
arch/arm/include/asm/armv8/mmu.h +2 -0
index 1711433..ff43d6c 100644
@@ -202,6 +202,8 @@ struct mm_region {
 	u64 size;
 	u64 attrs;
 };
+
+extern struct mm_region *mem_map;
 #endif
 
 #endif /* _ASM_ARMV8_MMU_H_ */
board/cavium/thunderx/thunderx.c +24 -0
index b926767..9131a38 100644
@@ -10,6 +10,7 @@
 #include <linux/compiler.h>
 
 #include <cavium/atf.h>
+#include <asm/armv8/mmu.h>
 
 #if !CONFIG_IS_ENABLED(OF_CONTROL)
 #include <dm/platdata.h>
@@ -42,6 +43,29 @@ U_BOOT_DEVICE(thunderx_serial1) = {
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static struct mm_region thunderx_mem_map[] = {
+	{
+		.base = 0x000000000000UL,
+		.size = 0x40000000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_NON_SHARE,
+	}, {
+		.base = 0x800000000000UL,
+		.size = 0x40000000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE,
+	}, {
+		.base = 0x840000000000UL,
+		.size = 0x40000000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE,
+	}, {
+		/* List terminator */
+		0,
+	}
+};
+
+struct mm_region *mem_map = thunderx_mem_map;
+
 int board_init(void)
 {
 	return 0;
include/configs/thunderx_88xx.h +0 -11
index 20b25f7..64e4616 100644
@@ -26,17 +26,6 @@
 
 #define CONFIG_SYS_LOWMEM_BASE		MEM_BASE
 
-#define CONFIG_SYS_MEM_MAP		{{0x000000000000UL, 0x40000000000UL, \
-					  PTE_BLOCK_MEMTYPE(MT_NORMAL) |     \
-					  PTE_BLOCK_NON_SHARE},	     \
-					 {0x800000000000UL, 0x40000000000UL, \
-					  PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | \
-					  PTE_BLOCK_NON_SHARE},	     \
-					 {0x840000000000UL, 0x40000000000UL, \
-					  PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) | \
-					  PTE_BLOCK_NON_SHARE},	     \
-					}
-
 #define CONFIG_SYS_MEM_MAP_SIZE		3
 
 #define CONFIG_SYS_VA_BITS		48
Signature · 1 line
1.8.5.6
#14 lore raw

PATCH 4/10 zymqmp: Replace home grown mmu code with generic table approach

In reply to Alexander Graf

Now that we have nice table driven page table creating code that gives us everything we need, move to that.

---

v1 -> v2:

- Move mmu tables into board file

---
 arch/arm/cpu/armv8/zynqmp/cpu.c | 217 +++++++++-------------------------------
 include/configs/xilinx_zynqmp.h |   2 +
 2 files changed, 50 insertions(+), 169 deletions(-)
arch/arm/cpu/armv8/zynqmp/cpu.c +48 -169
index c71f291..5dd3cd8 100644
@@ -8,6 +8,7 @@
 #include <common.h>
 #include <asm/arch/hardware.h>
 #include <asm/arch/sys_proto.h>
+#include <asm/armv8/mmu.h>
 #include <asm/io.h>
 
 #define ZYNQ_SILICON_VER_MASK	0xF000
@@ -15,6 +16,53 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static struct mm_region zynqmp_mem_map[] = {
+	{
+		.base = 0x0UL,
+		.size = 0x80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_INNER_SHARE
+	}, {
+		.base = 0x80000000UL,
+		.size = 0x70000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		.base = 0xf8000000UL,
+		.size = 0x07e00000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		.base = 0xffe00000UL,
+		.size = 0x00200000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_INNER_SHARE
+	}, {
+		.base = 0x400000000UL,
+		.size = 0x200000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		.base = 0x600000000UL,
+		.size = 0x800000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_INNER_SHARE
+	}, {
+		.base = 0xe00000000UL,
+		.size = 0xf200000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		/* List terminator */
+		0,
+	}
+};
+struct mm_region *mem_map = zynqmp_mem_map;
+
 static unsigned int zynqmp_get_silicon_version_secure(void)
 {
 	u32 ver;
@@ -44,172 +92,3 @@ unsigned int zynqmp_get_silicon_version(void)
 
 	return ZYNQMP_CSU_VERSION_SILICON;
 }
-
-#ifndef CONFIG_SYS_DCACHE_OFF
-#include <asm/armv8/mmu.h>
-
-#define SECTION_SHIFT_L1	30UL
-#define SECTION_SHIFT_L2	21UL
-#define BLOCK_SIZE_L0		0x8000000000UL
-#define BLOCK_SIZE_L1		(1 << SECTION_SHIFT_L1)
-#define BLOCK_SIZE_L2		(1 << SECTION_SHIFT_L2)
-
-#define TCR_TG1_4K		(1 << 31)
-#define TCR_EPD1_DISABLE	(1 << 23)
-#define ZYNQMO_VA_BITS		40
-#define ZYNQMP_TCR		TCR_TG1_4K | \
-				TCR_EPD1_DISABLE | \
-				TCR_SHARED_OUTER | \
-				TCR_SHARED_INNER | \
-				TCR_IRGN_WBWA | \
-				TCR_ORGN_WBWA | \
-				TCR_T0SZ(ZYNQMO_VA_BITS)
-
-#define MEMORY_ATTR	PMD_SECT_AF | PMD_SECT_INNER_SHARE |	\
-			PMD_ATTRINDX(MT_NORMAL) |	\
-			PMD_TYPE_SECT
-#define DEVICE_ATTR	PMD_SECT_AF | PMD_SECT_PXN |	\
-			PMD_SECT_UXN | PMD_ATTRINDX(MT_DEVICE_NGNRNE) |	\
-			PMD_TYPE_SECT
-
-/* 4K size is required to place 512 entries in each level */
-#define TLB_TABLE_SIZE	0x1000
-
-struct attr_tbl {
-	u32 num;
-	u64 attr;
-};
-
-static struct attr_tbl attr_tbll1t0[4] = { {16, 0x0},
-					   {8, DEVICE_ATTR},
-					   {32, MEMORY_ATTR},
-					   {456, DEVICE_ATTR}
-					 };
-static struct attr_tbl attr_tbll2t3[4] = { {0x180, DEVICE_ATTR},
-					   {0x40, 0x0},
-					   {0x3F, DEVICE_ATTR},
-					   {0x1, MEMORY_ATTR}
-					 };
-
-/*
- * This mmu table looks as below
- * Level 0 table contains two entries to 512GB sizes. One is Level1 Table 0
- * and other Level1 Table1.
- * Level1 Table0 contains entries for each 1GB from 0 to 511GB.
- * Level1 Table1 contains entries for each 1GB from 512GB to 1TB.
- * Level2 Table0, Level2 Table1, Level2 Table2 and Level2 Table3 contains
- * entries for each 2MB starting from 0GB, 1GB, 2GB and 3GB respectively.
- */
-static void zynqmp_mmu_setup(void)
-{
-	int el;
-	u32 index_attr;
-	u64 i, section_l1t0, section_l1t1;
-	u64 section_l2t0, section_l2t1, section_l2t2, section_l2t3;
-	u64 *level0_table = (u64 *)gd->arch.tlb_addr;
-	u64 *level1_table_0 = (u64 *)(gd->arch.tlb_addr + TLB_TABLE_SIZE);
-	u64 *level1_table_1 = (u64 *)(gd->arch.tlb_addr + (2 * TLB_TABLE_SIZE));
-	u64 *level2_table_0 = (u64 *)(gd->arch.tlb_addr + (3 * TLB_TABLE_SIZE));
-	u64 *level2_table_1 = (u64 *)(gd->arch.tlb_addr + (4 * TLB_TABLE_SIZE));
-	u64 *level2_table_2 = (u64 *)(gd->arch.tlb_addr + (5 * TLB_TABLE_SIZE));
-	u64 *level2_table_3 = (u64 *)(gd->arch.tlb_addr + (6 * TLB_TABLE_SIZE));
-
-	level0_table[0] =
-		(u64)level1_table_0 | PMD_TYPE_TABLE;
-	level0_table[1] =
-		(u64)level1_table_1 | PMD_TYPE_TABLE;
-
-	/*
-	 * set level 1 table 0, covering 0 to 512GB
-	 * set level 1 table 1, covering 512GB to 1TB
-	 */
-	section_l1t0 = 0;
-	section_l1t1 = BLOCK_SIZE_L0;
-
-	index_attr = 0;
-	for (i = 0; i < 512; i++) {
-		level1_table_0[i] = section_l1t0;
-		level1_table_0[i] |= attr_tbll1t0[index_attr].attr;
-		attr_tbll1t0[index_attr].num--;
-		if (attr_tbll1t0[index_attr].num == 0)
-			index_attr++;
-		level1_table_1[i] = section_l1t1;
-		level1_table_1[i] |= DEVICE_ATTR;
-		section_l1t0 += BLOCK_SIZE_L1;
-		section_l1t1 += BLOCK_SIZE_L1;
-	}
-
-	level1_table_0[0] =
-		(u64)level2_table_0 | PMD_TYPE_TABLE;
-	level1_table_0[1] =
-		(u64)level2_table_1 | PMD_TYPE_TABLE;
-	level1_table_0[2] =
-		(u64)level2_table_2 | PMD_TYPE_TABLE;
-	level1_table_0[3] =
-		(u64)level2_table_3 | PMD_TYPE_TABLE;
-
-	section_l2t0 = 0;
-	section_l2t1 = section_l2t0 + BLOCK_SIZE_L1; /* 1GB */
-	section_l2t2 = section_l2t1 + BLOCK_SIZE_L1; /* 2GB */
-	section_l2t3 = section_l2t2 + BLOCK_SIZE_L1; /* 3GB */
-
-	index_attr = 0;
-
-	for (i = 0; i < 512; i++) {
-		level2_table_0[i] = section_l2t0 | MEMORY_ATTR;
-		level2_table_1[i] = section_l2t1 | MEMORY_ATTR;
-		level2_table_2[i] = section_l2t2 | DEVICE_ATTR;
-		level2_table_3[i] = section_l2t3 |
-				    attr_tbll2t3[index_attr].attr;
-		attr_tbll2t3[index_attr].num--;
-		if (attr_tbll2t3[index_attr].num == 0)
-			index_attr++;
-		section_l2t0 += BLOCK_SIZE_L2;
-		section_l2t1 += BLOCK_SIZE_L2;
-		section_l2t2 += BLOCK_SIZE_L2;
-		section_l2t3 += BLOCK_SIZE_L2;
-	}
-
-	/* flush new MMU table */
-	flush_dcache_range(gd->arch.tlb_addr,
-			   gd->arch.tlb_addr + gd->arch.tlb_size);
-
-	/* point TTBR to the new table */
-	el = current_el();
-	set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-			  ZYNQMP_TCR, MEMORY_ATTRIBUTES);
-
-	set_sctlr(get_sctlr() | CR_M);
-}
-
-int arch_cpu_init(void)
-{
-	icache_enable();
-	__asm_invalidate_dcache_all();
-	__asm_invalidate_tlb_all();
-	return 0;
-}
-
-/*
- * This function is called from lib/board.c.
- * It recreates MMU table in main memory. MMU and d-cache are enabled earlier.
- * There is no need to disable d-cache for this operation.
- */
-void enable_caches(void)
-{
-	/* The data cache is not active unless the mmu is enabled */
-	if (!(get_sctlr() & CR_M)) {
-		invalidate_dcache_all();
-		__asm_invalidate_tlb_all();
-		zynqmp_mmu_setup();
-	}
-	puts("Enabling Caches...\n");
-
-	set_sctlr(get_sctlr() | CR_C);
-}
-
-u64 *arch_get_page_table(void)
-{
-	return (u64 *)(gd->arch.tlb_addr + 0x3000);
-}
-#endif
include/configs/xilinx_zynqmp.h +2 -0
index da868b8..08f430c 100644
@@ -29,6 +29,8 @@
 #define CONFIG_SYS_MEMTEST_START	CONFIG_SYS_SDRAM_BASE
 #define CONFIG_SYS_MEMTEST_END		CONFIG_SYS_SDRAM_SIZE
 
+#define CONFIG_SYS_FULL_VA
+
 /* Have release address at the end of 256MB for now */
 #define CPU_RELEASE_ADDR	0xFFFFFF0
 
Signature · 1 line
1.8.5.6
#15 lore raw

PATCH 5/10 tegra: Replace home grown mmu code with generic table approach

In reply to Alexander Graf

Now that we have nice table driven page table creating code that gives us everything we need, move to that.

---

v1 -> v2:

- Move mmu tables into .c file

---
 arch/arm/mach-tegra/arm64-mmu.c   | 132 +++++---------------------------------
 include/configs/tegra210-common.h |   2 +
 2 files changed, 19 insertions(+), 115 deletions(-)
arch/arm/mach-tegra/arm64-mmu.c +17 -115
index c227652..501c4f0 100644
@@ -12,120 +12,22 @@
 #include <asm/system.h>
 #include <asm/armv8/mmu.h>
 
-DECLARE_GLOBAL_DATA_PTR;
-
-#define SECTION_SHIFT_L1	30UL
-#define SECTION_SHIFT_L2	21UL
-#define BLOCK_SIZE_L0		0x8000000000UL
-#define BLOCK_SIZE_L1		(1 << SECTION_SHIFT_L1)
-#define BLOCK_SIZE_L2		(1 << SECTION_SHIFT_L2)
-
-#define TCR_TG1_4K		(1 << 31)
-#define TCR_EPD1_DISABLE	(1 << 23)
-#define TEGRA_VA_BITS		40
-#define TEGRA_TCR		TCR_TG1_4K | \
-				TCR_EPD1_DISABLE | \
-				TCR_SHARED_OUTER | \
-				TCR_SHARED_INNER | \
-				TCR_IRGN_WBWA | \
-				TCR_ORGN_WBWA | \
-				TCR_T0SZ(TEGRA_VA_BITS)
-
-#define MEMORY_ATTR	PMD_SECT_AF | PMD_SECT_INNER_SHARE |	\
-			PMD_ATTRINDX(MT_NORMAL) |	\
-			PMD_TYPE_SECT
-#define DEVICE_ATTR	PMD_SECT_AF | PMD_SECT_PXN |	\
-			PMD_SECT_UXN | PMD_ATTRINDX(MT_DEVICE_NGNRNE) |	\
-			PMD_TYPE_SECT
-
-/* 4K size is required to place 512 entries in each level */
-#define TLB_TABLE_SIZE	0x1000
-
-/*
- * This mmu table looks as below
- * Level 0 table contains two entries to 512GB sizes. One is Level1 Table 0
- * and other Level1 Table1.
- * Level1 Table0 contains entries for each 1GB from 0 to 511GB.
- * Level1 Table1 contains entries for each 1GB from 512GB to 1TB.
- * Level2 Table0, Level2 Table1, Level2 Table2 and Level2 Table3 contains
- * entries for each 2MB starting from 0GB, 1GB, 2GB and 3GB respectively.
- */
-void mmu_setup(void)
-{
-	int el;
-	u64 i, section_l1t0, section_l1t1;
-	u64 section_l2t0, section_l2t1, section_l2t2, section_l2t3;
-	u64 *level0_table = (u64 *)gd->arch.tlb_addr;
-	u64 *level1_table_0 = (u64 *)(gd->arch.tlb_addr + TLB_TABLE_SIZE);
-	u64 *level1_table_1 = (u64 *)(gd->arch.tlb_addr + (2 * TLB_TABLE_SIZE));
-	u64 *level2_table_0 = (u64 *)(gd->arch.tlb_addr + (3 * TLB_TABLE_SIZE));
-	u64 *level2_table_1 = (u64 *)(gd->arch.tlb_addr + (4 * TLB_TABLE_SIZE));
-	u64 *level2_table_2 = (u64 *)(gd->arch.tlb_addr + (5 * TLB_TABLE_SIZE));
-	u64 *level2_table_3 = (u64 *)(gd->arch.tlb_addr + (6 * TLB_TABLE_SIZE));
-
-	/* Invalidate all table entries */
-	memset(level0_table, 0, PGTABLE_SIZE);
-
-	level0_table[0] =
-		(u64)level1_table_0 | PMD_TYPE_TABLE;
-	level0_table[1] =
-		(u64)level1_table_1 | PMD_TYPE_TABLE;
-
-	/*
-	 * set level 1 table 0, covering 0 to 512GB
-	 * set level 1 table 1, covering 512GB to 1TB
-	 */
-	section_l1t0 = 0;
-	section_l1t1 = BLOCK_SIZE_L0;
-
-	for (i = 0; i < 512; i++) {
-		level1_table_0[i] = section_l1t0;
-		if (i >= 4)
-			level1_table_0[i] |= MEMORY_ATTR;
-		level1_table_1[i] = section_l1t1;
-		level1_table_1[i] |= MEMORY_ATTR;
-		section_l1t0 += BLOCK_SIZE_L1;
-		section_l1t1 += BLOCK_SIZE_L1;
+static struct mm_region tegra_mem_map[] = {
+	{
+		.base = 0x0UL,
+		.size = 0x80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		.base = 0x80000000UL,
+		.size = 0xff80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_INNER_SHARE
+	}, {
+		/* List terminator */
+		0,
 	}
+};
 
-	level1_table_0[0] =
-		(u64)level2_table_0 | PMD_TYPE_TABLE;
-	level1_table_0[1] =
-		(u64)level2_table_1 | PMD_TYPE_TABLE;
-	level1_table_0[2] =
-		(u64)level2_table_2 | PMD_TYPE_TABLE;
-	level1_table_0[3] =
-		(u64)level2_table_3 | PMD_TYPE_TABLE;
-
-	section_l2t0 = 0;
-	section_l2t1 = section_l2t0 + BLOCK_SIZE_L1; /* 1GB */
-	section_l2t2 = section_l2t1 + BLOCK_SIZE_L1; /* 2GB */
-	section_l2t3 = section_l2t2 + BLOCK_SIZE_L1; /* 3GB */
-
-	for (i = 0; i < 512; i++) {
-		level2_table_0[i] = section_l2t0 | DEVICE_ATTR;
-		level2_table_1[i] = section_l2t1 | DEVICE_ATTR;
-		level2_table_2[i] = section_l2t2 | MEMORY_ATTR;
-		level2_table_3[i] = section_l2t3 | MEMORY_ATTR;
-		section_l2t0 += BLOCK_SIZE_L2;
-		section_l2t1 += BLOCK_SIZE_L2;
-		section_l2t2 += BLOCK_SIZE_L2;
-		section_l2t3 += BLOCK_SIZE_L2;
-	}
-
-	/* flush new MMU table */
-	flush_dcache_range(gd->arch.tlb_addr,
-			   gd->arch.tlb_addr + gd->arch.tlb_size);
-
-	/* point TTBR to the new table */
-	el = current_el();
-	set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-			  TEGRA_TCR, MEMORY_ATTRIBUTES);
-
-	set_sctlr(get_sctlr() | CR_M);
-}
-
-u64 *arch_get_page_table(void)
-{
-	return (u64 *)(gd->arch.tlb_addr + (3 * TLB_TABLE_SIZE));
-}
+struct mm_region *mem_map = tegra_mem_map;
include/configs/tegra210-common.h +2 -0
index 8f35a7b..2a6e317 100644
@@ -13,6 +13,8 @@
 /* Cortex-A57 uses a cache line size of 64 bytes */
 #define CONFIG_SYS_CACHELINE_SIZE	64
 
+#define CONFIG_SYS_FULL_VA
+
 /*
  * NS16550 Configuration
  */
Signature · 1 line
1.8.5.6
#17 lore raw

PATCH 6/10 vexpress64: Add MMU tables

In reply to Alexander Graf

There's no good excuse for running with caches disabled on AArch64, so let's just move the vexpress64 target to enable the MMU and run with caches on.

---

v1 -> v2:

- Move tables to .c file

---
 board/armltd/vexpress64/vexpress64.c | 21 +++++++++++++++++++++
 include/configs/vexpress_aemv8a.h    |  6 +++---
 2 files changed, 24 insertions(+), 3 deletions(-)
board/armltd/vexpress64/vexpress64.c +21 -0
index 6efc8c1..973b579 100644
@@ -14,6 +14,7 @@
 #include <dm/platdata.h>
 #include <dm/platform_data/serial_pl01x.h>
 #include "pcie.h"
+#include <asm/armv8/mmu.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -28,6 +29,26 @@ U_BOOT_DEVICE(vexpress_serials) = {
 	.platdata = &serial_platdata,
 };
 
+static struct mm_region vexpress64_mem_map[] = {
+	{
+		.base = 0x0UL,
+		.size = 0x80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		.base = 0x80000000UL,
+		.size = 0xff80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_INNER_SHARE
+	}, {
+		/* List terminator */
+		0,
+	}
+};
+
+struct mm_region *mem_map = vexpress64_mem_map;
+
 /* This function gets replaced by platforms supporting PCIe.
  * The replacement function, eg. on Juno, initialises the PCIe bus.
  */
include/configs/vexpress_aemv8a.h +3 -3
index 133041b..ddb9848 100644
@@ -19,9 +19,9 @@
 
 #define CONFIG_SUPPORT_RAW_INITRD
 
-/* Cache Definitions */
-#define CONFIG_SYS_DCACHE_OFF
-#define CONFIG_SYS_ICACHE_OFF
+/* MMU Definitions */
+#define CONFIG_SYS_CACHELINE_SIZE	64
+#define CONFIG_SYS_FULL_VA
 
 #define CONFIG_IDENT_STRING		" vexpress_aemv8a"
 #define CONFIG_BOOTP_VCI_STRING		"U-Boot.armv8.vexpress_aemv8a"
Signature · 1 line
1.8.5.6
#18 lore raw

PATCH 7/10 dwmmc: Increase retry timeout

In reply to Alexander Graf

When enable dcache on HiKey, we're running into MMC command timeouts because our retry loop is now faster than the eMMC (or an external SD card) can answer.

Increase the retry count to the same as the timeout value for status reports.

The real fix is obviously to not base this whole thing on a cycle counter but on real wall time, but that would be slightly more intrusive.

---
 drivers/mmc/dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/dw_mmc.c +1 -1
index 909e3ca..7329f40 100644
@@ -189,7 +189,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
 				 data ? DIV_ROUND_UP(data->blocks, 8) : 0);
 	int ret = 0, flags = 0, i;
 	unsigned int timeout = 100000;
-	u32 retry = 10000;
+	u32 retry = 100000;
 	u32 mask, ctrl;
 	ulong start = get_timer(0);
 	struct bounce_buffer bbstate;
Signature · 1 line
1.8.5.6
#19 lore raw

PATCH 8/10 hikey: Add MMU tables

In reply to Alexander Graf

The hikey runs with dcache disabled today. There really should be no reason not to use caches on AArch64, so let's add MMU definitions and enable the dcache.

---

v1 -> v2:

- Move tables to .c file

---
 board/hisilicon/hikey/hikey.c | 21 +++++++++++++++++++++
 include/configs/hikey.h       |  5 +++--
 2 files changed, 24 insertions(+), 2 deletions(-)
board/hisilicon/hikey/hikey.c +21 -0
index c4ae40b..1edc807 100644
@@ -19,6 +19,7 @@
 #include <asm/arch/periph.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/hi6220.h>
+#include <asm/armv8/mmu.h>
 
 /*TODO drop this table in favour of device tree */
 static const struct hikey_gpio_platdata hi6220_gpio[] = {
@@ -87,6 +88,26 @@ U_BOOT_DEVICE(hikey_seriala) = {
 	.platdata = &serial_platdata,
 };
 
+static struct mm_region hikey_mem_map[] = {
+	{
+		.base = 0x0UL,
+		.size = 0x80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_INNER_SHARE
+	}, {
+		.base = 0x80000000UL,
+		.size = 0x80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		/* List terminator */
+		0,
+	}
+};
+
+struct mm_region *mem_map = hikey_mem_map;
+
 #ifdef CONFIG_BOARD_EARLY_INIT_F
 int board_uart_init(void)
 {
include/configs/hikey.h +3 -2
index 796861e..d33dcef 100644
@@ -21,8 +21,9 @@
 
 #define CONFIG_SUPPORT_RAW_INITRD
 
-/* Cache Definitions */
-#define CONFIG_SYS_DCACHE_OFF
+/* MMU Definitions */
+#define CONFIG_SYS_CACHELINE_SIZE	64
+#define CONFIG_SYS_FULL_VA
 
 #define CONFIG_IDENT_STRING		"hikey"
 
Signature · 1 line
1.8.5.6
#20 lore raw

PATCH 9/10 arm64: Remove non-full-va map code

In reply to Alexander Graf

By now the code to only have a single page table level with 64k page size and 42 bit address space is no longer used by any board in tree, so we can safely remove it.

To clean up code, move the layerscape mmu code to the new defines, removing redundant field definitions.

---

v1 -> v2:

- Add layerscape empty mmu table in .c file

---
 arch/arm/cpu/armv8/cache_v8.c                  | 90 ------------------------
 arch/arm/cpu/armv8/fsl-layerscape/cpu.c        | 37 ++++++++--
 arch/arm/include/asm/arch-fsl-layerscape/cpu.h | 94 +++++++++++++-------------
 arch/arm/include/asm/armv8/mmu.h               | 66 +-----------------
 arch/arm/include/asm/global_data.h             |  2 +-
 arch/arm/include/asm/system.h                  |  4 --
 doc/README.arm64                               | 20 ------
 include/configs/hikey.h                        |  1 -
 include/configs/tegra210-common.h              |  2 -
 include/configs/thunderx_88xx.h                | 14 ----
 include/configs/vexpress_aemv8a.h              |  1 -
 include/configs/xilinx_zynqmp.h                |  2 -
 12 files changed, 81 insertions(+), 252 deletions(-)
arch/arm/cpu/armv8/cache_v8.c +0 -90
index 601199b..727f21b 100644
@@ -35,7 +35,6 @@ DECLARE_GLOBAL_DATA_PTR;
  *    off:          FFF
  */
 
-#ifdef CONFIG_SYS_FULL_VA
 static u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
 {
 	u64 max_addr = 0;
@@ -347,38 +346,11 @@ static void setup_pgtables(void)
 		add_map(&mem_map[i]);
 }
 
-#else
-
-inline void set_pgtable_section(u64 *page_table, u64 index, u64 section,
-			 u64 memory_type, u64 attribute)
-{
-	u64 value;
-
-	value = section | PMD_TYPE_SECT | PMD_SECT_AF;
-	value |= PMD_ATTRINDX(memory_type);
-	value |= attribute;
-	page_table[index] = value;
-}
-
-inline void set_pgtable_table(u64 *page_table, u64 index, u64 *table_addr)
-{
-	u64 value;
-
-	value = (u64)table_addr | PMD_TYPE_TABLE;
-	page_table[index] = value;
-}
-#endif
-
 /* to activate the MMU we need to set up virtual memory */
 __weak void mmu_setup(void)
 {
-#ifndef CONFIG_SYS_FULL_VA
-	bd_t *bd = gd->bd;
-	u64 *page_table = (u64 *)gd->arch.tlb_addr, i, j;
-#endif
 	int el;
 
-#ifdef CONFIG_SYS_FULL_VA
 	/* Set up page tables only once */
 	if (!gd->arch.tlb_fillptr)
 		setup_pgtables();
@@ -386,40 +358,6 @@ __weak void mmu_setup(void)
 	el = current_el();
 	set_ttbr_tcr_mair(el, gd->arch.tlb_addr, get_tcr(el, NULL, NULL),
 			  MEMORY_ATTRIBUTES);
-#else
-	/* Setup an identity-mapping for all spaces */
-	for (i = 0; i < (PGTABLE_SIZE >> 3); i++) {
-		set_pgtable_section(page_table, i, i << SECTION_SHIFT,
-				    MT_DEVICE_NGNRNE, PMD_SECT_NON_SHARE);
-	}
-
-	/* Setup an identity-mapping for all RAM space */
-	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
-		ulong start = bd->bi_dram[i].start;
-		ulong end = bd->bi_dram[i].start + bd->bi_dram[i].size;
-		for (j = start >> SECTION_SHIFT;
-		     j < end >> SECTION_SHIFT; j++) {
-			set_pgtable_section(page_table, j, j << SECTION_SHIFT,
-					    MT_NORMAL, PMD_SECT_NON_SHARE);
-		}
-	}
-
-	/* load TTBR0 */
-	el = current_el();
-	if (el == 1) {
-		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_EL1_RSVD | TCR_FLAGS | TCR_EL1_IPS_BITS,
-				  MEMORY_ATTRIBUTES);
-	} else if (el == 2) {
-		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_EL2_RSVD | TCR_FLAGS | TCR_EL2_IPS_BITS,
-				  MEMORY_ATTRIBUTES);
-	} else {
-		set_ttbr_tcr_mair(el, gd->arch.tlb_addr,
-				  TCR_EL3_RSVD | TCR_FLAGS | TCR_EL3_IPS_BITS,
-				  MEMORY_ATTRIBUTES);
-	}
-#endif
 
 	/* enable the mmu */
 	set_sctlr(get_sctlr() | CR_M);
@@ -505,33 +443,6 @@ u64 *__weak arch_get_page_table(void) {
 	return NULL;
 }
 
-#ifndef CONFIG_SYS_FULL_VA
-void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
-				     enum dcache_option option)
-{
-	u64 *page_table = arch_get_page_table();
-	u64 upto, end;
-
-	if (page_table == NULL)
-		return;
-
-	end = ALIGN(start + size, (1 << MMU_SECTION_SHIFT)) >>
-	      MMU_SECTION_SHIFT;
-	start = start >> MMU_SECTION_SHIFT;
-	for (upto = start; upto < end; upto++) {
-		page_table[upto] &= ~PMD_ATTRINDX_MASK;
-		page_table[upto] |= PMD_ATTRINDX(option);
-	}
-	asm volatile("dsb sy");
-	__asm_invalidate_tlb_all();
-	asm volatile("dsb sy");
-	asm volatile("isb");
-	start = start << MMU_SECTION_SHIFT;
-	end = end << MMU_SECTION_SHIFT;
-	flush_dcache_range(start, end);
-	asm volatile("dsb sy");
-}
-#else
 static bool is_aligned(u64 addr, u64 size, u64 align)
 {
 	return !(addr & (align - 1)) && !(size & (align - 1));
@@ -604,7 +515,6 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 	flush_dcache_range(real_start, real_start + real_size);
 	asm volatile("dsb sy");
 }
-#endif
 
 #else	/* CONFIG_SYS_DCACHE_OFF */
 
arch/arm/cpu/armv8/fsl-layerscape/cpu.c +32 -5
index 6ea28ed..7404bd9 100644
@@ -26,6 +26,14 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static struct mm_region layerscape_mem_map[] = {
+	{
+		/* List terminator */
+		0,
+	}
+};
+struct mm_region *mem_map = layerscape_mem_map;
+
 void cpu_name(char *name)
 {
 	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
@@ -48,6 +56,25 @@ void cpu_name(char *name)
 }
 
 #ifndef CONFIG_SYS_DCACHE_OFF
+static void set_pgtable_section(u64 *page_table, u64 index, u64 section,
+			u64 memory_type, u64 attribute)
+{
+       u64 value;
+
+       value = section | PTE_TYPE_BLOCK | PTE_BLOCK_AF;
+       value |= PMD_ATTRINDX(memory_type);
+       value |= attribute;
+       page_table[index] = value;
+}
+
+static void set_pgtable_table(u64 *page_table, u64 index, u64 *table_addr)
+{
+       u64 value;
+
+       value = (u64)table_addr | PTE_TYPE_TABLE;
+       page_table[index] = value;
+}
+
 /*
  * Set the block entries according to the information of the table.
  */
@@ -114,10 +141,10 @@ static int find_table(const struct sys_mmu_table *list,
 
 		temp_base -= block_size;
 
-		if ((level_table[index - 1] & PMD_TYPE_MASK) ==
-		    PMD_TYPE_TABLE) {
+		if ((level_table[index - 1] & PTE_TYPE_MASK) ==
+		    PTE_TYPE_TABLE) {
 			level_table = (u64 *)(level_table[index - 1] &
-				      ~PMD_TYPE_MASK);
+				      ~PTE_TYPE_MASK);
 			level++;
 			continue;
 		} else {
@@ -220,7 +247,7 @@ static inline int final_secure_ddr(u64 *level0_table,
 	struct table_info table = {};
 	struct sys_mmu_table ddr_entry = {
 		0, 0, BLOCK_SIZE_L1, MT_NORMAL,
-		PMD_SECT_OUTER_SHARE | PMD_SECT_NS
+		PTE_BLOCK_OUTER_SHARE | PTE_BLOCK_NS
 	};
 	u64 index;
 
@@ -243,7 +270,7 @@ static inline int final_secure_ddr(u64 *level0_table,
 	ddr_entry.virt_addr = phys_addr;
 	ddr_entry.phys_addr = phys_addr;
 	ddr_entry.size = CONFIG_SYS_MEM_RESERVE_SECURE;
-	ddr_entry.attribute = PMD_SECT_OUTER_SHARE;
+	ddr_entry.attribute = PTE_BLOCK_OUTER_SHARE;
 	ret = find_table(&ddr_entry, &table, level0_table);
 	if (ret) {
 		printf("MMU error: could not find secure ddr table\n");
arch/arm/include/asm/arch-fsl-layerscape/cpu.h +47 -47

191 lines not on this page · raw message

arch/arm/include/asm/armv8/mmu.h +1 -65

113 lines not on this page · raw message

arch/arm/include/asm/global_data.h +1 -1
index 3dec1db..e982d9e 100644
@@ -39,7 +39,7 @@ struct arch_global_data {
 #if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
 	unsigned long tlb_addr;
 	unsigned long tlb_size;
-#if defined(CONFIG_SYS_FULL_VA)
+#if defined(CONFIG_ARM64)
 	unsigned long tlb_fillptr;
 #endif
 #endif
arch/arm/include/asm/system.h +0 -4
index ffd6fe5..5663d22 100644
@@ -17,12 +17,8 @@
 #define CR_WXN		(1 << 19)	/* Write Permision Imply XN	*/
 #define CR_EE		(1 << 25)	/* Exception (Big) Endian	*/
 
-#ifndef CONFIG_SYS_FULL_VA
-#define PGTABLE_SIZE	(0x10000)
-#else
 u64 get_page_table_size(void);
 #define PGTABLE_SIZE	get_page_table_size()
-#endif
 
 /* 2MB granularity */
 #define MMU_SECTION_SHIFT	21
doc/README.arm64 +0 -20
index de669cb..f658fa2 100644
@@ -36,26 +36,6 @@ Notes
 6. CONFIG_ARM64 instead of CONFIG_ARMV8 is used to distinguish aarch64 and
    aarch32 specific codes.
 
-7. CONFIG_SYS_FULL_VA is used to enable 2-level page tables. For cores
-   supporting 64k pages it allows usage of full 48+ virtual/physical addresses
-
-   Enabling this option requires the following ones to be defined:
-       - CONFIG_SYS_MEM_MAP - an array of 'struct mm_region' describing the
-         system memory map (start, length, attributes)
-       - CONFIG_SYS_MEM_MAP_SIZE - number of entries in CONFIG_SYS_MEM_MAP
-       - CONFIG_SYS_PTL1_ENTRIES - number of 1st level page table entries
-       - CONFIG_SYS_PTL2_ENTRIES - number of 1nd level page table entries
-         for the largest CONFIG_SYS_MEM_MAP entry
-       - CONFIG_COREID_MASK - the mask value used to get the core from the
-         MPIDR_EL1 register
-       - CONFIG_SYS_PTL2_BITS - number of bits addressed by the 2nd level
-         page tables
-       - CONFIG_SYS_BLOCK_SHIFT - number of bits addressed by a single block
-         entry from L2 page tables
-       - CONFIG_SYS_PGTABLE_SIZE - total size of the page table
-       - CONFIG_SYS_TCR_EL{1,2,3}_IPS_BITS - the IPS field of the TCR_EL{1,2,3}
-
-
 
 
 Contributor
include/configs/hikey.h +0 -1
index d33dcef..2d9ace9 100644
@@ -23,7 +23,6 @@
 
 /* MMU Definitions */
 #define CONFIG_SYS_CACHELINE_SIZE	64
-#define CONFIG_SYS_FULL_VA
 
 #define CONFIG_IDENT_STRING		"hikey"
 
include/configs/tegra210-common.h +0 -2
index 2a6e317..8f35a7b 100644
@@ -13,8 +13,6 @@
 /* Cortex-A57 uses a cache line size of 64 bytes */
 #define CONFIG_SYS_CACHELINE_SIZE	64
 
-#define CONFIG_SYS_FULL_VA
-
 /*
  * NS16550 Configuration
  */
include/configs/thunderx_88xx.h +0 -14
index 64e4616..736d0a5 100644
@@ -22,22 +22,8 @@
 
 #define MEM_BASE			0x00500000
 
-#define CONFIG_SYS_FULL_VA
-
 #define CONFIG_SYS_LOWMEM_BASE		MEM_BASE
 
-#define CONFIG_SYS_MEM_MAP_SIZE		3
-
-#define CONFIG_SYS_VA_BITS		48
-#define CONFIG_SYS_PTL2_BITS		42
-#define CONFIG_SYS_BLOCK_SHIFT		29
-#define CONFIG_SYS_PTL1_ENTRIES		64
-#define CONFIG_SYS_PTL2_ENTRIES		8192
-
-#define CONFIG_SYS_PGTABLE_SIZE		\
-	((CONFIG_SYS_PTL1_ENTRIES + \
-	  CONFIG_SYS_MEM_MAP_SIZE * CONFIG_SYS_PTL2_ENTRIES) * 8)
-
 /* Link Definitions */
 #define CONFIG_SYS_TEXT_BASE		0x00500000
 #define CONFIG_SYS_INIT_SP_ADDR		(CONFIG_SYS_SDRAM_BASE + 0x7fff0)
include/configs/vexpress_aemv8a.h +0 -1
index ddb9848..2949170 100644
@@ -21,7 +21,6 @@
 
 /* MMU Definitions */
 #define CONFIG_SYS_CACHELINE_SIZE	64
-#define CONFIG_SYS_FULL_VA
 
 #define CONFIG_IDENT_STRING		" vexpress_aemv8a"
 #define CONFIG_BOOTP_VCI_STRING		"U-Boot.armv8.vexpress_aemv8a"
include/configs/xilinx_zynqmp.h +0 -2
index 08f430c..da868b8 100644
@@ -29,8 +29,6 @@
 #define CONFIG_SYS_MEMTEST_START	CONFIG_SYS_SDRAM_BASE
 #define CONFIG_SYS_MEMTEST_END		CONFIG_SYS_SDRAM_SIZE
 
-#define CONFIG_SYS_FULL_VA
-
 /* Have release address@the end of 256MB for now */
 #define CPU_RELEASE_ADDR	0xFFFFFF0
 
Signature · 1 line
1.8.5.6
#21 lore raw

PATCH 10/10 arm64: Only allow dcache disabled in SPL builds

In reply to Alexander Graf

Now that we have an easy way to describe memory regions and enable the MMU, there really shouldn't be anything holding people back from running with caches enabled on AArch64. To make sure people catch early if they're missing on the caching fun, give them a compile error.

---
 arch/arm/cpu/armv8/cache_v8.c | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm/cpu/armv8/cache_v8.c +9 -0
index 727f21b..92940de 100644
@@ -518,6 +518,15 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 
 #else	/* CONFIG_SYS_DCACHE_OFF */
 
+/*
+ * For SPL builds, we may want to not have dcache enabled. Any real U-Boot
+ * running however really wants to have dcache and the MMU active. Check that
+ * everything is sane and give the developer a hint if it isn't.
+ */
+#ifndef CONFIG_SPL_BUILD
+#error Please describe your MMU layout in CONFIG_SYS_MEM_MAP and enable dcache.
+#endif
+
 void invalidate_dcache_all(void)
 {
 }
Signature · 1 line
1.8.5.6
#22 lore raw

cover arm64: Unify MMU code

In reply to Alexander Graf
Quoted 31 lines · Alexander Graf

Howdy,

Currently on arm64 there is a big pile of mess when it comes to MMU support and page tables. Each board does its own little thing and the generic code is pretty dumb and nobody actually uses it.

This patch set tries to clean that up. After this series is applied, all boards except for the FSL Layerscape ones are converted to the new generic page table logic and have icache+dcache enabled.

The new code always uses 4k page size. It dynamically allocates 1G or 2M pages for ranges that fit. When a dcache attribute request comes in that requires a smaller granularity than our previous allocation could fulfill, pages get automatically split.

I have tested and verified the code works on HiKey (bare metal), vexpress64 (Foundation Model) and zynqmp (QEMU). The TX1 target is untested, but given the simplicity of the maps I doubt it'll break. ThunderX in theory should also work, but I haven't tested it. I would be very happy if people with access to those system could give the patch set a try.

With this we're a big step closer to a good base line for EFI payload support, since we can now just require that all boards always have dcache enabled.

I would also be incredibly happy if some Freescale people could look at their MMU code and try to unify it into the now cleaned up generic code. I don't think we're far off here.

Quoted 8 lines

Alex

v1 -> v2:

- Fix comment for create_table()
- Rework page table size calculation
- Move mmu tables into board files
- New patch: thunderx: Move mmu table into board file

You miss v2 in subject.

Thanks,
Michal

#23 lore raw

cover arm64: Unify MMU code

Quoted 38 lines

Am 24.02.2016 um 13:19 schrieb Michal Simek <[email protected]>:

Quoted 34 lines · Alexander Graf

Howdy,

Currently on arm64 there is a big pile of mess when it comes to MMU support and page tables. Each board does its own little thing and the generic code is pretty dumb and nobody actually uses it.

This patch set tries to clean that up. After this series is applied, all boards except for the FSL Layerscape ones are converted to the new generic page table logic and have icache+dcache enabled.

The new code always uses 4k page size. It dynamically allocates 1G or 2M pages for ranges that fit. When a dcache attribute request comes in that requires a smaller granularity than our previous allocation could fulfill, pages get automatically split.

I have tested and verified the code works on HiKey (bare metal), vexpress64 (Foundation Model) and zynqmp (QEMU). The TX1 target is untested, but given the simplicity of the maps I doubt it'll break. ThunderX in theory should also work, but I haven't tested it. I would be very happy if people with access to those system could give the patch set a try.

With this we're a big step closer to a good base line for EFI payload support, since we can now just require that all boards always have dcache enabled.

I would also be incredibly happy if some Freescale people could look at their MMU code and try to unify it into the now cleaned up generic code. I don't think we're far off here.

Alex

Quoted 8 lines
Quoted 6 lines

v1 -> v2:

- Fix comment for create_table()
- Rework page table size calculation
- Move mmu tables into board files
- New patch: thunderx: Move mmu table into board file

You miss v2 in subject.

Yeah, I realized that 2 seconds after I sent the mails out, but figured I'd not spam everyone just to admit I did something stupid ;).

The rest still stands, testing and review are both greatly appreciated.

Alex