On 23.02.16 14:17, Simon Glass wrote:
Hi Alex,
On 21 February 2016 at 18:57, Alexander Graf agraf@suse.de wrote:
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.
Signed-off-by: Alexander Graf agraf@suse.de
arch/arm/cpu/armv8/cache_v8.c | 346 +++++++++++++++++++++++++++++++------ 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, 332 insertions(+), 103 deletions(-)
Should the change to the thunderx file go in a separate patch?
We're changing semantics for some defines from "this define acts in L1/L2 page table entries" to "this define is for level/block type PTEs". So it's tied to this patch :).
diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c index 9229532..4369a83 100644 --- a/arch/arm/cpu/armv8/cache_v8.c +++ b/arch/arm/cpu/armv8/cache_v8.c @@ -2,6 +2,9 @@
- (C) Copyright 2013
- David Feng fenghua@phytium.com.cn
- (C) Copyright 2016
- Alexander Graf agraf@suse.de
*/
- SPDX-License-Identifier: GPL-2.0+
@@ -9,35 +12,40 @@ #include <asm/system.h> #include <asm/armv8/mmu.h>
-DECLARE_GLOBAL_DATA_PTR;
-#ifndef CONFIG_SYS_DCACHE_OFF +/* #define DEBUG_MMU */
-#ifdef CONFIG_SYS_FULL_VA -static void set_ptl1_entry(u64 index, u64 ptl2_entry) -{
u64 *pgd = (u64 *)gd->arch.tlb_addr;u64 value;+#ifdef DEBUG_MMU +#define DPRINTF(a, ...) printf("%s:%d: " a, __func__, __LINE__, __VA_ARGS__) +#else +#define DPRINTF(a, ...) do { } while(0) +#endif
Can you use the normal DEBUG and debug()?
Uh, I guess so, yeah.
value = ptl2_entry | PTL1_TYPE_TABLE;pgd[index] = value;-} +DECLARE_GLOBAL_DATA_PTR;
-static void set_ptl2_block(u64 ptl1, u64 bfn, u64 address, u64 memory_attrs) -{
u64 *pmd = (u64 *)ptl1;u64 value;+#ifndef CONFIG_SYS_DCACHE_OFF
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;
I am not ken on the idea of using a big #define table on these boards. Is there not a device-tree binding for this that we can use? It is just a data table, We are moving to Kconfig and eventually want to drop the config files.
I'll move this into board files which then can do whatever they like with it - take if from a #define in a header, populate it dynamically from device tree, do something halfway in between, whatever fits your needs best :).
Btw, if you want to use dt for it, you don't need to add any new bindings at all. Simply take all of your device regs/ranges and memory ranges and gobble them into the memory 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 +87,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 +98,196 @@ 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;DPRINTF("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;DPRINTF("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;+}
+/* Creates a new full table (512 entries) and sets *pte to refer to it */ +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",gd->arch.tlb_fillptr - gd->arch.tlb_addr,gd->arch.tlb_size);For each of these panic() calls can you please add a comment as to what the user should do? It needs to be very clear what action should be taken to resolve the problem.
Good idea. The only case where I can't think of a good text is at
if (pte_type(pte) != PTE_TYPE_TABLE) panic("PTE %p (%llx) for addr=%llx should be a table", pte, *pte, start);
because this really is more of an assert(). We should never reach it.
Thanks a bunch for the reviews,
Alex