On 02/21/2016 06:57 PM, Alexander Graf 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.
diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
+/* Creates a new full table (512 entries) and sets *pte to refer to it */
+static u64 *create_table(void)
I think that comment is stale (there's no *pte; I assume it should say
"returns").
+/* Returns the estimated required size of all page tables */
+u64 get_page_table_size(void)
+{
+ int i;
+ u64 one_pt = MAX_PTE_ENTRIES * sizeof(u64);
+ u64 size = 0;
+
+ /* root page table */
+ size += one_pt;
Isn't the root page table level 0? So, that accounts for the page table
that's indexed by VA bits 47:39.
+ for (i = 0; i < ARRAY_SIZE(mem_map); i++) {
+ struct mm_region *map = &mem_map[i];
+
+ /* Account for Lv0 page tables */
+ size += one_pt * ((map->size >> 39) + 1);
So here, isn't the code accounting for level 1 tables, not level 0
tables? That is, the tables indexed by VA bits 38:30.
(As an aside for myself when I come back and read this later, the shift
is 39, since we need to allocate as many tables as there are values for
bits 39 and above).
+ /* 1GB aligned pages fit already, so count the others */
+ if (map->size & 0x3fffffffULL)
+ size += one_pt;
+ if (map->base & 0x3fffffffULL)
+ size += one_pt;
Here, I believe we're accounting for any required level 2 tables (which
are indexed by VA bits 29:21).
We seem to be missing code inside the loop that accounts for any
required level 3 tables (indexed by VA bits 20:12).
+ }
+
+ /* Assume we may have to split up to 4 more page tables off */
+ size += one_pt * 4;
Is this meant to be accounting for level 3 tables? If so, I'm not sure
where the value "4" comes from. I would have expected "2" instead (for
misaligned start/end) *and* for this calculation to be inside the loop
rather than outside. Doesn't putting the calculation outside the loop
make assumptions about how many mem_map[] entries are mis-aligned
relative to 2MB sections?
diff --git a/arch/arm/include/asm/armv8/mmu.h b/arch/arm/include/asm/armv8/mmu.h
#define VA_BITS (42) /* 42 bits virtual address */
#else
#define VA_BITS CONFIG_SYS_VA_BITS
-#define PTL2_BITS CONFIG_SYS_PTL2_BITS
+#define PTE_BLOCK_BITS CONFIG_SYS_PTL2_BITS
#endif
When I "wrote" the Tegra ARMv8 MMU code (i.e. cut/pasted it from other
ARMv8 MMU code), I recall finding some inconsistencies between the value
of VA_BITS and *SECTION_SHIFT between different header files. I think
that was e.g.:
arch/arm/include/asm/armv8/mmu.h:26:#define VA_BITS (42)
/* 42 bits virtual address */
arch/arm/mach-tegra/arm64-mmu.c:25:#define TEGRA_VA_BITS 40
arch/arm/cpu/armv8/zynqmp/cpu.c:59:#define ZYNQMO_VA_BITS 40
asm/armv8/mmu.h's value for SECTION_SHIFT and asm/system.h's value for
MMU_SECTION_SHIFT.
(Also see the related parts of the description of git commit
376cb1a45315 "ARM: tegra: add custom MMU setup on ARMv8")
Does this patch entirely address those discrepancies?
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
