arm64: update/clarify/relax Image and FDT placement rules

Currently, the FDT blob needs to be in the same 512 MB region as the kernel, so that it can be mapped into the kernel virtual memory space very early on using a minimal set of statically allocated translation tables.

Now that we have early fixmap support, we can relax this restriction, by moving the permanent FDT mapping to the fixmap region instead. This way, the FDT blob may be anywhere in memory.

This also moves the vetting of the FDT to mmu.c, since the early init code in head.S does not handle mapping of the FDT anymore. At the same time, fix up some comments in head.S that have gone stale.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 Documentation/arm64/booting.txt | 13 +++++----
 arch/arm64/include/asm/boot.h   | 14 +++++++++
 arch/arm64/include/asm/fixmap.h | 15 ++++++++++
 arch/arm64/include/asm/mmu.h    |  1 +
 arch/arm64/kernel/head.S        | 39 +------------------------
 arch/arm64/kernel/setup.c       | 32 +++++++-------------
 arch/arm64/mm/Makefile          |  2 ++
 arch/arm64/mm/init.c            |  1 -
 arch/arm64/mm/mmu.c             | 65 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 117 insertions(+), 65 deletions(-)
 create mode 100644 arch/arm64/include/asm/boot.h

Documentation/arm64/booting.txt +0 -0

index f3c05b5f9f08..53f18e13d51c 100644
@@ -45,11 +45,14 @@ sees fit.)
 
 Requirement: MANDATORY
 

arch/arm64/kernel/vmlinux.lds.S +0 -0

index ceec4def354b..338eaa7bcbfd 100644
@@ -68,6 +68,17 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 #define ALIGN_DEBUG_RO_MIN(min)		. = ALIGN(min);
 #endif
 

arch/arm64/kernel/vmlinux.lds.S +7 -1

+
 SECTIONS
 {
 	/*

@@ -160,7 +171,7 @@ SECTIONS
 
 	BSS_SECTION(0, 0, 0)
 
-	.pgdir (NOLOAD) : ALIGN(PAGE_SIZE) {
+	.pgdir (NOLOAD) : ALIGN(PGDIR_ALIGN) {
 		idmap_pg_dir = .;
 		. += IDMAP_DIR_SIZE;
 		swapper_pg_dir = .;

@@ -185,6 +196,11 @@ ASSERT(__idmap_text_end - (__idmap_text_start & ~(SZ_4K - 1)) <= SZ_4K,
 	"ID map text too big or misaligned")
 
 /*
+ * Check that the chosen PGDIR_ALIGN value if sufficient.
+ */
+ASSERT(SIZEOF(.pgdir) < ALIGNOF(.pgdir), ".pgdir size exceeds its alignment")
+
+/*
  * If padding is applied before .head.text, virt<->phys conversions will fail.
  */
 ASSERT(_text == (PAGE_OFFSET + TEXT_OFFSET), "HEAD is misaligned")

arch/arm64/mm/mmu.c +0 -0

index c27ab20a5ba9..93e5a2497f01 100644

arch/arm64/mm/mmu.c +39 -0

+	static struct bootstrap_pgtables linear_bs_pgtables __pgdir;
+	const phys_addr_t swapper_phys = __pa(swapper_pg_dir);
+	unsigned long swapper_virt = __phys_to_virt(swapper_phys) + va_offset;
+	struct memblock_region *reg;
+
+	bootstrap_early_mapping(swapper_virt, &linear_bs_pgtables,
+				IS_ENABLED(CONFIG_ARM64_64K_PAGES));
+
+	/* now find the memblock that covers swapper_pg_dir, and clip */
+	for_each_memblock(memory, reg) {
+		phys_addr_t start = reg->base;
+		phys_addr_t end = start + reg->size;
+		unsigned long vstart, vend;
+
+		if (start > swapper_phys || end <= swapper_phys)
+			continue;
+
+#ifdef CONFIG_ARM64_64K_PAGES
+		/* clip the region to PMD size */
+		vstart = max(swapper_virt & PMD_MASK,
+			     round_up(__phys_to_virt(start + va_offset),
+				      PAGE_SIZE));
+		vend = min(round_up(swapper_virt, PMD_SIZE),
+			   round_down(__phys_to_virt(end + va_offset),
+				      PAGE_SIZE));
+#else
+		/* clip the region to PUD size */
+		vstart = max(swapper_virt & PUD_MASK,
+			     round_up(__phys_to_virt(start + va_offset),
+				      PMD_SIZE));
+		vend = min(round_up(swapper_virt, PUD_SIZE),
+			   round_down(__phys_to_virt(end + va_offset),
+				      PMD_SIZE));
+#endif
+
+		create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
+			       PAGE_KERNEL_EXEC);
+
+		/*

Quoted 8 lines · Ard Biesheuvel

arch/arm64/kernel/vmlinux.lds.S +0 -0

index ceec4def354b..338eaa7bcbfd 100644
@@ -68,6 +68,17 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 #define ALIGN_DEBUG_RO_MIN(min)              . = ALIGN(min);
 #endif
 

arch/arm64/kernel/vmlinux.lds.S PECOFF_FILE_ALIGNMENT = 0x200; · line 80

Quoted 10 lines

arch/arm64/kernel/vmlinux.lds.S +10 -0

+/*
+ * The pgdir region needs to be mappable using a single PMD or PUD sized region,
+ * so it should not cross a 512 MB or 1 GB alignment boundary, respectively
+ * (depending on page size). So align to an upper bound of its size.
+ */
+#if CONFIG_ARM64_PGTABLE_LEVELS == 2
+#define PGDIR_ALIGN  (8 * PAGE_SIZE)
+#else
+#define PGDIR_ALIGN  (16 * PAGE_SIZE)
+#endif

Isn't 8 pages sufficient in both cases? Unless some other patch changes the idmap and swapper, I can count maximum 7 pages in total.

Quoted 33 lines

arch/arm64/kernel/vmlinux.lds.S +7 -1

+
 SECTIONS
 {
      /*

@@ -160,7 +171,7 @@ SECTIONS
 
      BSS_SECTION(0, 0, 0)
 
-     .pgdir (NOLOAD) : ALIGN(PAGE_SIZE) {
+     .pgdir (NOLOAD) : ALIGN(PGDIR_ALIGN) {
              idmap_pg_dir = .;
              . += IDMAP_DIR_SIZE;
              swapper_pg_dir = .;

@@ -185,6 +196,11 @@ ASSERT(__idmap_text_end - (__idmap_text_start & ~(SZ_4K - 1)) <= SZ_4K,
      "ID map text too big or misaligned")
 
 /*
+ * Check that the chosen PGDIR_ALIGN value if sufficient.
+ */
+ASSERT(SIZEOF(.pgdir) < ALIGNOF(.pgdir), ".pgdir size exceeds its alignment")
+
+/*
  * If padding is applied before .head.text, virt<->phys conversions will fail.
  */
 ASSERT(_text == (PAGE_OFFSET + TEXT_OFFSET), "HEAD is misaligned")

arch/arm64/mm/mmu.c +0 -0

index c27ab20a5ba9..93e5a2497f01 100644
@@ -380,26 +380,68 @@ static void __init bootstrap_early_mapping(unsigned long addr,
      }
 }
 

arch/arm64/mm/mmu.c static void __init bootstrap_early_mapping(unsigned long addr, · line 389

Quoted 7 lines

arch/arm64/mm/mmu.c +7 -0

+static void __init bootstrap_linear_mapping(unsigned long va_offset)
+{
+     /*
+      * Bootstrap the linear range that covers swapper_pg_dir so that the
+      * statically allocated page tables as well as newly allocated ones
+      * are accessible via the linear mapping.
+      */

Just move the comment outside the function.

Quoted 46 lines

arch/arm64/mm/mmu.c +46 -0

+     static struct bootstrap_pgtables linear_bs_pgtables __pgdir;
+     const phys_addr_t swapper_phys = __pa(swapper_pg_dir);
+     unsigned long swapper_virt = __phys_to_virt(swapper_phys) + va_offset;
+     struct memblock_region *reg;
+
+     bootstrap_early_mapping(swapper_virt, &linear_bs_pgtables,
+                             IS_ENABLED(CONFIG_ARM64_64K_PAGES));
+
+     /* now find the memblock that covers swapper_pg_dir, and clip */
+     for_each_memblock(memory, reg) {
+             phys_addr_t start = reg->base;
+             phys_addr_t end = start + reg->size;
+             unsigned long vstart, vend;
+
+             if (start > swapper_phys || end <= swapper_phys)
+                     continue;
+
+#ifdef CONFIG_ARM64_64K_PAGES
+             /* clip the region to PMD size */
+             vstart = max(swapper_virt & PMD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PAGE_SIZE));
+             vend = min(round_up(swapper_virt, PMD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PAGE_SIZE));
+#else
+             /* clip the region to PUD size */
+             vstart = max(swapper_virt & PUD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PMD_SIZE));
+             vend = min(round_up(swapper_virt, PUD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PMD_SIZE));
+#endif
+
+             create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
+                            PAGE_KERNEL_EXEC);
+
+             /*
+              * Temporarily limit the memblock range. We need to do this as
+              * create_mapping requires puds, pmds and ptes to be allocated
+              * from memory addressable from the early linear mapping.
+              */
+             memblock_set_current_limit(__pa(vend - va_offset));
+
+             return;

arch/arm64/mm/mmu.c static void __init bootstrap_early_mapping(unsigned long addr, · line 438

Quoted 3 lines

arch/arm64/mm/mmu.c +3 -0

+     }
+     BUG();
+}

I'll probably revisit this function after I see the whole series. But in the meantime, if the kernel is not loaded in the first memblock (in address order), isn't there a risk that we allocate memory from the first memblock which is not mapped yet?

Quoted 9 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

arch/arm64/kernel/vmlinux.lds.S +0 -0

index ceec4def354b..338eaa7bcbfd 100644
@@ -68,6 +68,17 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 #define ALIGN_DEBUG_RO_MIN(min)              . = ALIGN(min);
 #endif
 

Quoted 13 lines

arch/arm64/kernel/vmlinux.lds.S PECOFF_FILE_ALIGNMENT = 0x200; · line 80

Quoted 10 lines

arch/arm64/kernel/vmlinux.lds.S +10 -0

+/*
+ * The pgdir region needs to be mappable using a single PMD or PUD sized region,
+ * so it should not cross a 512 MB or 1 GB alignment boundary, respectively
+ * (depending on page size). So align to an upper bound of its size.
+ */
+#if CONFIG_ARM64_PGTABLE_LEVELS == 2
+#define PGDIR_ALIGN  (8 * PAGE_SIZE)
+#else
+#define PGDIR_ALIGN  (16 * PAGE_SIZE)
+#endif

Isn't 8 pages sufficient in both cases? Unless some other patch changes the idmap and swapper, I can count maximum 7 pages in total.

The preceding patch moves the fixmap page tables to this region as well. But the logic is still incorrect -> we only need 16x for 4 levels (7 + 3 == 10), the remaining ones are all <= 8

Quoted 46 lines

Quoted 46 lines

arch/arm64/kernel/vmlinux.lds.S +46 -0

+     static struct bootstrap_pgtables linear_bs_pgtables __pgdir;
+     const phys_addr_t swapper_phys = __pa(swapper_pg_dir);
+     unsigned long swapper_virt = __phys_to_virt(swapper_phys) + va_offset;
+     struct memblock_region *reg;
+
+     bootstrap_early_mapping(swapper_virt, &linear_bs_pgtables,
+                             IS_ENABLED(CONFIG_ARM64_64K_PAGES));
+
+     /* now find the memblock that covers swapper_pg_dir, and clip */
+     for_each_memblock(memory, reg) {
+             phys_addr_t start = reg->base;
+             phys_addr_t end = start + reg->size;
+             unsigned long vstart, vend;
+
+             if (start > swapper_phys || end <= swapper_phys)
+                     continue;
+
+#ifdef CONFIG_ARM64_64K_PAGES
+             /* clip the region to PMD size */
+             vstart = max(swapper_virt & PMD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PAGE_SIZE));
+             vend = min(round_up(swapper_virt, PMD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PAGE_SIZE));
+#else
+             /* clip the region to PUD size */
+             vstart = max(swapper_virt & PUD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PMD_SIZE));
+             vend = min(round_up(swapper_virt, PUD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PMD_SIZE));
+#endif
+
+             create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
+                            PAGE_KERNEL_EXEC);
+
+             /*
+              * Temporarily limit the memblock range. We need to do this as
+              * create_mapping requires puds, pmds and ptes to be allocated
+              * from memory addressable from the early linear mapping.
+              */
+             memblock_set_current_limit(__pa(vend - va_offset));
+
+             return;

Quoted 8 lines

arch/arm64/kernel/vmlinux.lds.S

Quoted 3 lines

arch/arm64/kernel/vmlinux.lds.S +3 -0

+     }
+     BUG();
+}

I'll probably revisit this function after I see the whole series. But in the meantime, if the kernel is not loaded in the first memblock (in address order), isn't there a risk that we allocate memory from the first memblock which is not mapped yet?

memblock allocates top down, so it should only allocate from this region, unless the remaining room is completely reserved.

I think that is a theoretical problem which exists currently as well, i.e., the boot protocol does not mandate that the 512MB/1GB region containing the kernel contains unreserved room.

Quoted 24 lines · Ard Biesheuvel

Quoted 22 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

arch/arm64/kernel/vmlinux.lds.S +0 -0

index ceec4def354b..338eaa7bcbfd 100644
@@ -68,6 +68,17 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 #define ALIGN_DEBUG_RO_MIN(min)              . = ALIGN(min);
 #endif
 

arch/arm64/kernel/vmlinux.lds.S PECOFF_FILE_ALIGNMENT = 0x200; · line 80

Quoted 10 lines

arch/arm64/kernel/vmlinux.lds.S +10 -0

+/*
+ * The pgdir region needs to be mappable using a single PMD or PUD sized region,
+ * so it should not cross a 512 MB or 1 GB alignment boundary, respectively
+ * (depending on page size). So align to an upper bound of its size.
+ */
+#if CONFIG_ARM64_PGTABLE_LEVELS == 2
+#define PGDIR_ALIGN  (8 * PAGE_SIZE)
+#else
+#define PGDIR_ALIGN  (16 * PAGE_SIZE)
+#endif

Isn't 8 pages sufficient in both cases? Unless some other patch changes the idmap and swapper, I can count maximum 7 pages in total.

Quoted 3 lines

The preceding patch moves the fixmap page tables to this region as well. But the logic is still incorrect -> we only need 16x for 4 levels (7 + 3 == 10), the remaining ones are all <= 8

You should improve the comment here to include the maths, "upper bound of its size" is not very clear ;).

Quoted 55 lines

Quoted 54 lines

Quoted 39 lines

arch/arm64/kernel/vmlinux.lds.S +39 -0

+     static struct bootstrap_pgtables linear_bs_pgtables __pgdir;
+     const phys_addr_t swapper_phys = __pa(swapper_pg_dir);
+     unsigned long swapper_virt = __phys_to_virt(swapper_phys) + va_offset;
+     struct memblock_region *reg;
+
+     bootstrap_early_mapping(swapper_virt, &linear_bs_pgtables,
+                             IS_ENABLED(CONFIG_ARM64_64K_PAGES));
+
+     /* now find the memblock that covers swapper_pg_dir, and clip */
+     for_each_memblock(memory, reg) {
+             phys_addr_t start = reg->base;
+             phys_addr_t end = start + reg->size;
+             unsigned long vstart, vend;
+
+             if (start > swapper_phys || end <= swapper_phys)
+                     continue;
+
+#ifdef CONFIG_ARM64_64K_PAGES
+             /* clip the region to PMD size */
+             vstart = max(swapper_virt & PMD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PAGE_SIZE));
+             vend = min(round_up(swapper_virt, PMD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PAGE_SIZE));
+#else
+             /* clip the region to PUD size */
+             vstart = max(swapper_virt & PUD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PMD_SIZE));
+             vend = min(round_up(swapper_virt, PUD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PMD_SIZE));
+#endif
+
+             create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
+                            PAGE_KERNEL_EXEC);
+
+             /*

arch/arm64/kernel/vmlinux.lds.S PECOFF_FILE_ALIGNMENT = 0x200;

Quoted 10 lines

arch/arm64/kernel/vmlinux.lds.S +10 -0

+              * Temporarily limit the memblock range. We need to do this as
+              * create_mapping requires puds, pmds and ptes to be allocated
+              * from memory addressable from the early linear mapping.
+              */
+             memblock_set_current_limit(__pa(vend - va_offset));
+
+             return;
+     }
+     BUG();
+}

I'll probably revisit this function after I see the whole series. But in the meantime, if the kernel is not loaded in the first memblock (in address order), isn't there a risk that we allocate memory from the first memblock which is not mapped yet?

Quoted 2 lines

memblock allocates top down, so it should only allocate from this region, unless the remaining room is completely reserved.

I don't like to rely on this, it's not guaranteed behaviour.

Quoted 3 lines

I think that is a theoretical problem which exists currently as well, i.e., the boot protocol does not mandate that the 512MB/1GB region containing the kernel contains unreserved room.

That's more of a documentation problem, we can make the requirements clearer. Debugging is probably easier as well, it fails to allocate memory. But for the other case, not placing the kernel in the first memblock has high chances of allocating unmapped memory.

Can we not have another set of level 2,3(,4) page tables pre-allocated in swapper for the first block (start of RAM)? It gets hairy, in total we would need:

1) idmap
2) swapper
2.a) kernel image outside the linear mapping
2.b) fixmap
2.c) start-of-ram
2.d) swapper mapping in the linear mapping

Can we avoid accessing 2.d (swapper in linear mapping) until we finished mapping 2.c? Once we mapped the start of RAM and set the memblock limit, we can allocate pages to start mapping the rest.

Quoted 25 lines · Catalin Marinas

Quoted 24 lines · Ard Biesheuvel

Quoted 22 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

arch/arm64/kernel/vmlinux.lds.S +0 -0

index ceec4def354b..338eaa7bcbfd 100644
@@ -68,6 +68,17 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 #define ALIGN_DEBUG_RO_MIN(min)              . = ALIGN(min);
 #endif
 

arch/arm64/kernel/vmlinux.lds.S PECOFF_FILE_ALIGNMENT = 0x200; · line 80

Quoted 10 lines

arch/arm64/kernel/vmlinux.lds.S +10 -0

+/*
+ * The pgdir region needs to be mappable using a single PMD or PUD sized region,
+ * so it should not cross a 512 MB or 1 GB alignment boundary, respectively
+ * (depending on page size). So align to an upper bound of its size.
+ */
+#if CONFIG_ARM64_PGTABLE_LEVELS == 2
+#define PGDIR_ALIGN  (8 * PAGE_SIZE)
+#else
+#define PGDIR_ALIGN  (16 * PAGE_SIZE)
+#endif

Isn't 8 pages sufficient in both cases? Unless some other patch changes the idmap and swapper, I can count maximum 7 pages in total.

Quoted 6 lines

Quoted 3 lines

The preceding patch moves the fixmap page tables to this region as well. But the logic is still incorrect -> we only need 16x for 4 levels (7 + 3 == 10), the remaining ones are all <= 8

You should improve the comment here to include the maths, "upper bound of its size" is not very clear ;).

Yes, you are right, it should read 'power-of-2 upper bound'

Quoted 58 lines

Quoted 57 lines

Quoted 46 lines

Quoted 46 lines

arch/arm64/kernel/vmlinux.lds.S +46 -0

+     static struct bootstrap_pgtables linear_bs_pgtables __pgdir;
+     const phys_addr_t swapper_phys = __pa(swapper_pg_dir);
+     unsigned long swapper_virt = __phys_to_virt(swapper_phys) + va_offset;
+     struct memblock_region *reg;
+
+     bootstrap_early_mapping(swapper_virt, &linear_bs_pgtables,
+                             IS_ENABLED(CONFIG_ARM64_64K_PAGES));
+
+     /* now find the memblock that covers swapper_pg_dir, and clip */
+     for_each_memblock(memory, reg) {
+             phys_addr_t start = reg->base;
+             phys_addr_t end = start + reg->size;
+             unsigned long vstart, vend;
+
+             if (start > swapper_phys || end <= swapper_phys)
+                     continue;
+
+#ifdef CONFIG_ARM64_64K_PAGES
+             /* clip the region to PMD size */
+             vstart = max(swapper_virt & PMD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PAGE_SIZE));
+             vend = min(round_up(swapper_virt, PMD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PAGE_SIZE));
+#else
+             /* clip the region to PUD size */
+             vstart = max(swapper_virt & PUD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PMD_SIZE));
+             vend = min(round_up(swapper_virt, PUD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PMD_SIZE));
+#endif
+
+             create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
+                            PAGE_KERNEL_EXEC);
+
+             /*
+              * Temporarily limit the memblock range. We need to do this as
+              * create_mapping requires puds, pmds and ptes to be allocated
+              * from memory addressable from the early linear mapping.
+              */
+             memblock_set_current_limit(__pa(vend - va_offset));
+
+             return;

Quoted 8 lines

arch/arm64/kernel/vmlinux.lds.S

Quoted 3 lines

arch/arm64/kernel/vmlinux.lds.S +3 -0

+     }
+     BUG();
+}

I'll probably revisit this function after I see the whole series. But in the meantime, if the kernel is not loaded in the first memblock (in address order), isn't there a risk that we allocate memory from the first memblock which is not mapped yet?

memblock allocates top down, so it should only allocate from this region, unless the remaining room is completely reserved.

Quoted 1 line

I don't like to rely on this, it's not guaranteed behaviour.

Actually, it is. Allocation is always top-down unless you call memblock_set_bottom_up(), which is a NOP unless CONFIG_MOVABLE_NODE is selected.
That is why the memblock limit only limits at the top afaict

Quoted 30 lines · Ard Biesheuvel

Quoted 29 lines · Catalin Marinas

Quoted 27 lines · Ard Biesheuvel

Quoted 9 lines · Catalin Marinas

Quoted 8 lines · Ard Biesheuvel

arch/arm64/kernel/vmlinux.lds.S +0 -0

index ceec4def354b..338eaa7bcbfd 100644
@@ -68,6 +68,17 @@ PECOFF_FILE_ALIGNMENT = 0x200;
 #define ALIGN_DEBUG_RO_MIN(min)              . = ALIGN(min);
 #endif
 

Quoted 13 lines

arch/arm64/kernel/vmlinux.lds.S PECOFF_FILE_ALIGNMENT = 0x200; · line 80

Quoted 10 lines

arch/arm64/kernel/vmlinux.lds.S +10 -0

+/*
+ * The pgdir region needs to be mappable using a single PMD or PUD sized region,
+ * so it should not cross a 512 MB or 1 GB alignment boundary, respectively
+ * (depending on page size). So align to an upper bound of its size.
+ */
+#if CONFIG_ARM64_PGTABLE_LEVELS == 2
+#define PGDIR_ALIGN  (8 * PAGE_SIZE)
+#else
+#define PGDIR_ALIGN  (16 * PAGE_SIZE)
+#endif

Isn't 8 pages sufficient in both cases? Unless some other patch changes the idmap and swapper, I can count maximum 7 pages in total.

The preceding patch moves the fixmap page tables to this region as well. But the logic is still incorrect -> we only need 16x for 4 levels (7 + 3 == 10), the remaining ones are all <= 8

Quoted 4 lines

Quoted 2 lines

You should improve the comment here to include the maths, "upper bound of its size" is not very clear ;).

Yes, you are right, it should read 'power-of-2 upper bound'

And the number of pages required for the initial page tables (I figured out it's a power of two already ;)).

Quoted 58 lines

Quoted 58 lines

Quoted 57 lines

Quoted 46 lines

Quoted 46 lines

arch/arm64/kernel/vmlinux.lds.S +46 -0

+     static struct bootstrap_pgtables linear_bs_pgtables __pgdir;
+     const phys_addr_t swapper_phys = __pa(swapper_pg_dir);
+     unsigned long swapper_virt = __phys_to_virt(swapper_phys) + va_offset;
+     struct memblock_region *reg;
+
+     bootstrap_early_mapping(swapper_virt, &linear_bs_pgtables,
+                             IS_ENABLED(CONFIG_ARM64_64K_PAGES));
+
+     /* now find the memblock that covers swapper_pg_dir, and clip */
+     for_each_memblock(memory, reg) {
+             phys_addr_t start = reg->base;
+             phys_addr_t end = start + reg->size;
+             unsigned long vstart, vend;
+
+             if (start > swapper_phys || end <= swapper_phys)
+                     continue;
+
+#ifdef CONFIG_ARM64_64K_PAGES
+             /* clip the region to PMD size */
+             vstart = max(swapper_virt & PMD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PAGE_SIZE));
+             vend = min(round_up(swapper_virt, PMD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PAGE_SIZE));
+#else
+             /* clip the region to PUD size */
+             vstart = max(swapper_virt & PUD_MASK,
+                          round_up(__phys_to_virt(start + va_offset),
+                                   PMD_SIZE));
+             vend = min(round_up(swapper_virt, PUD_SIZE),
+                        round_down(__phys_to_virt(end + va_offset),
+                                   PMD_SIZE));
+#endif
+
+             create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
+                            PAGE_KERNEL_EXEC);
+
+             /*
+              * Temporarily limit the memblock range. We need to do this as
+              * create_mapping requires puds, pmds and ptes to be allocated
+              * from memory addressable from the early linear mapping.
+              */
+             memblock_set_current_limit(__pa(vend - va_offset));
+
+             return;

Quoted 8 lines

arch/arm64/kernel/vmlinux.lds.S

Quoted 3 lines

arch/arm64/kernel/vmlinux.lds.S +3 -0

+     }
+     BUG();
+}

I'll probably revisit this function after I see the whole series. But in the meantime, if the kernel is not loaded in the first memblock (in address order), isn't there a risk that we allocate memory from the first memblock which is not mapped yet?

memblock allocates top down, so it should only allocate from this region, unless the remaining room is completely reserved.

Quoted 6 lines

Quoted 1 line

I don't like to rely on this, it's not guaranteed behaviour.

Actually, it is. Allocation is always top-down unless you call memblock_set_bottom_up(), which is a NOP unless CONFIG_MOVABLE_NODE is selected.
That is why the memblock limit only limits at the top afaict

It currently works like this but I'm not sure it is guaranteed to always behave this way (e.g. someone "improves" the memblock allocator in the future). And you never know, we may need memory hotplug on arm64 at some point in the future (together with CONFIG_MOVABLE_NODE).

arch/arm64/include/asm/memory.h +9 -2

index f800d45ea226..801331793bd3 100644
@@ -24,6 +24,7 @@
 #include <linux/compiler.h>
 #include <linux/const.h>
 #include <linux/types.h>
+#include <asm/boot.h>
 #include <asm/sizes.h>
 
 /*

@@ -39,7 +40,12 @@
 #define PCI_IO_SIZE		SZ_16M
 
 /*
- * PAGE_OFFSET - the virtual address of the start of the kernel image (top
+ * Offset below PAGE_OFFSET where to map the kernel Image.
+ */
+#define KIMAGE_OFFSET		MAX_KIMG_SIZE
+
+/*
+ * PAGE_OFFSET - the virtual address of the base of the linear mapping (top
  *		 (VA_BITS - 1))
  * VA_BITS - the maximum number of bits for virtual addresses.
  * TASK_SIZE - the maximum size of a user space task.

@@ -49,7 +55,8 @@
  */
 #define VA_BITS			(CONFIG_ARM64_VA_BITS)
 #define PAGE_OFFSET		(UL(0xffffffffffffffff) << (VA_BITS - 1))
-#define MODULES_END		(PAGE_OFFSET)
+#define KIMAGE_VADDR		(PAGE_OFFSET - KIMAGE_OFFSET)
+#define MODULES_END		KIMAGE_VADDR
 #define MODULES_VADDR		(MODULES_END - SZ_64M)
 #define PCI_IO_END		(MODULES_VADDR - SZ_2M)
 #define PCI_IO_START		(PCI_IO_END - PCI_IO_SIZE)

arch/arm64/include/asm/memory.h +1 -1

+
 #define __phys_to_virt(x)	((unsigned long)((x) - PHYS_OFFSET + PAGE_OFFSET))
 
 /*

@@ -111,7 +122,16 @@
 
 extern phys_addr_t		memstart_addr;
 /* PHYS_OFFSET - the physical address of the start of memory. */
-#define PHYS_OFFSET		({ memstart_addr; })

Quoted 40 lines · Ard Biesheuvel

arch/arm64/include/asm/memory.h +9 -2

index f800d45ea226..801331793bd3 100644
@@ -24,6 +24,7 @@
 #include <linux/compiler.h>
 #include <linux/const.h>
 #include <linux/types.h>
+#include <asm/boot.h>
 #include <asm/sizes.h>
 
 /*

@@ -39,7 +40,12 @@
 #define PCI_IO_SIZE          SZ_16M
 
 /*
- * PAGE_OFFSET - the virtual address of the start of the kernel image (top
+ * Offset below PAGE_OFFSET where to map the kernel Image.
+ */
+#define KIMAGE_OFFSET                MAX_KIMG_SIZE
+
+/*
+ * PAGE_OFFSET - the virtual address of the base of the linear mapping (top
  *            (VA_BITS - 1))
  * VA_BITS - the maximum number of bits for virtual addresses.
  * TASK_SIZE - the maximum size of a user space task.

@@ -49,7 +55,8 @@
  */
 #define VA_BITS                      (CONFIG_ARM64_VA_BITS)
 #define PAGE_OFFSET          (UL(0xffffffffffffffff) << (VA_BITS - 1))
-#define MODULES_END          (PAGE_OFFSET)
+#define KIMAGE_VADDR         (PAGE_OFFSET - KIMAGE_OFFSET)
+#define MODULES_END          KIMAGE_VADDR
 #define MODULES_VADDR                (MODULES_END - SZ_64M)
 #define PCI_IO_END           (MODULES_VADDR - SZ_2M)
 #define PCI_IO_START         (PCI_IO_END - PCI_IO_SIZE)

@@ -77,7 +84,11 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */

arch/arm64/include/asm/memory.h line 90

Quoted 5 lines

arch/arm64/include/asm/memory.h +4 -1

-#define __virt_to_phys(x)    (((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET))
+#define __virt_to_phys(x) ({                                         \
+     long __x = (long)(x) - PAGE_OFFSET;                             \
+     __x >= 0 ? (phys_addr_t)(__x + PHYS_OFFSET) :                   \
+                (phys_addr_t)(__x + PHYS_OFFSET + kernel_va_offset); })

Just wondering, when do we need a __pa on kernel addresses? But it looks to me like second case is always (__x + PHYS_OFFSET + KIMAGE_OFFSET).

Before map_mem(), we have phys_offset_bias set but kernel_va_offset 0. After map_mem(), we reset the former and set the latter. Maybe we can get rid of kernel_va_offset entirely (see more below about phys_offset_bias).

Quoted 15 lines

arch/arm64/include/asm/memory.h +7 -1

+
 #define __phys_to_virt(x)    ((unsigned long)((x) - PHYS_OFFSET + PAGE_OFFSET))
 
 /*

@@ -111,7 +122,16 @@
 
 extern phys_addr_t           memstart_addr;
 /* PHYS_OFFSET - the physical address of the start of memory. */
-#define PHYS_OFFSET          ({ memstart_addr; })
+#define PHYS_OFFSET          ({ memstart_addr + phys_offset_bias; })
+
+/*
+ * Before the linear mapping has been set up, __va() translations will
+ * not produce usable virtual addresses unless we tweak PHYS_OFFSET to
+ * compensate for the offset between the kernel mapping and the base of

arch/arm64/include/asm/memory.h line 134

Quoted 4 lines

arch/arm64/include/asm/memory.h +4 -0

+ * the linear mapping. We will undo this in map_mem().
+ */
+extern u64 phys_offset_bias;
+extern u64 kernel_va_offset;

Can we not add the bias to memstart_addr during boot and reset it later in map_mem()? Otherwise the run-time kernel ends up having to do a dummy addition any time it needs PHYS_OFFSET.

Quoted 41 lines · Catalin Marinas

Quoted 40 lines · Ard Biesheuvel

arch/arm64/include/asm/memory.h +9 -2

index f800d45ea226..801331793bd3 100644
@@ -24,6 +24,7 @@
 #include <linux/compiler.h>
 #include <linux/const.h>
 #include <linux/types.h>
+#include <asm/boot.h>
 #include <asm/sizes.h>
 
 /*

@@ -39,7 +40,12 @@
 #define PCI_IO_SIZE          SZ_16M
 
 /*
- * PAGE_OFFSET - the virtual address of the start of the kernel image (top
+ * Offset below PAGE_OFFSET where to map the kernel Image.
+ */
+#define KIMAGE_OFFSET                MAX_KIMG_SIZE
+
+/*
+ * PAGE_OFFSET - the virtual address of the base of the linear mapping (top
  *            (VA_BITS - 1))
  * VA_BITS - the maximum number of bits for virtual addresses.
  * TASK_SIZE - the maximum size of a user space task.

@@ -49,7 +55,8 @@
  */
 #define VA_BITS                      (CONFIG_ARM64_VA_BITS)
 #define PAGE_OFFSET          (UL(0xffffffffffffffff) << (VA_BITS - 1))
-#define MODULES_END          (PAGE_OFFSET)
+#define KIMAGE_VADDR         (PAGE_OFFSET - KIMAGE_OFFSET)
+#define MODULES_END          KIMAGE_VADDR
 #define MODULES_VADDR                (MODULES_END - SZ_64M)
 #define PCI_IO_END           (MODULES_VADDR - SZ_2M)
 #define PCI_IO_START         (PCI_IO_END - PCI_IO_SIZE)

@@ -77,7 +84,11 @@
  * private definitions which should NOT be used outside memory.h
  * files.  Use virt_to_phys/phys_to_virt/__pa/__va instead.
  */

Quoted 8 lines

arch/arm64/include/asm/memory.h line 90

Quoted 5 lines

arch/arm64/include/asm/memory.h +4 -1

-#define __virt_to_phys(x)    (((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET))
+#define __virt_to_phys(x) ({                                         \
+     long __x = (long)(x) - PAGE_OFFSET;                             \
+     __x >= 0 ? (phys_addr_t)(__x + PHYS_OFFSET) :                   \
+                (phys_addr_t)(__x + PHYS_OFFSET + kernel_va_offset); })

Just wondering, when do we need a __pa on kernel addresses? But it looks to me like second case is always (__x + PHYS_OFFSET + KIMAGE_OFFSET).

For now, yes. But when the kernel Image moves up in physical memory, and/or the kernel virtual image moves down in virtual memory (for kaslr) this offset could increase.

Quoted 22 lines

Before map_mem(), we have phys_offset_bias set but kernel_va_offset 0. After map_mem(), we reset the former and set the latter. Maybe we can get rid of kernel_va_offset entirely (see more below about phys_offset_bias).

Quoted 15 lines

arch/arm64/include/asm/memory.h +7 -1

+
 #define __phys_to_virt(x)    ((unsigned long)((x) - PHYS_OFFSET + PAGE_OFFSET))
 
 /*

@@ -111,7 +122,16 @@
 
 extern phys_addr_t           memstart_addr;
 /* PHYS_OFFSET - the physical address of the start of memory. */
-#define PHYS_OFFSET          ({ memstart_addr; })
+#define PHYS_OFFSET          ({ memstart_addr + phys_offset_bias; })
+
+/*
+ * Before the linear mapping has been set up, __va() translations will
+ * not produce usable virtual addresses unless we tweak PHYS_OFFSET to
+ * compensate for the offset between the kernel mapping and the base of

Quoted 9 lines

arch/arm64/include/asm/memory.h line 134

Quoted 4 lines

arch/arm64/include/asm/memory.h +4 -0

+ * the linear mapping. We will undo this in map_mem().
+ */
+extern u64 phys_offset_bias;
+extern u64 kernel_va_offset;

Can we not add the bias to memstart_addr during boot and reset it later in map_mem()? Otherwise the run-time kernel ends up having to do a dummy addition any time it needs PHYS_OFFSET.

Yes, that is how I started out. At some point during development, that became a bit cumbersome, because for instance, when you remove the memory that is inaccessible, you want memstart_addr to contain a meaningful value and not have to undo the bias. But looking at this version of the series, I think there are no references left to memstart_addr.