arm64: relax Image placement rules

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

Hi Ard,

Apologies that it's taken me so long to get around to this...

Quoted 8 lines · Ard Biesheuvel

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

By adding new FIX_{PGD,PUD,PMD,PTE} indicees to the fixmap, we can allocate page tables from anywhere via memblock, and temporarily map them as we need to.

That would avoid the need for the bootstrap tables. In head.S we'd only need to create a temporary (coarse-grained, RWX) kernel mapping (with the fixmap bolted on). Later we would create a whole new set of tables with a fine-grained kernel mapping and a full linear mapping using the new fixmap entries to temporarily map tables, then switch over to those atomically.

Otherwise, one minor comment below.

arch/arm64/mm/mmu.c static void __init __map_memblock(phys_addr_t start, phys_addr_t end) · line 407

Quoted 3 lines

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

+static void __init bootstrap_early_mapping(unsigned long addr,
+                                        struct bootstrap_pgtables *reg,
+                                        bool pte_level)

The only caller in this patch passes true for pte_level.

Can we not introduce the argument when it is first needed? Or at least have something in the commit message as to why we'll need it later?

arch/arm64/mm/mmu.c static inline pte_t * fixmap_pte(unsigned long addr) · line 677

Quoted 5 lines

/*
* The boot-ioremap range spans multiple pmds, for which

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

-      * we are not preparted:
+      * we are not prepared:
       */

I cannot wait to see this typo go!

Otherwise, this looks fine to me.

Quoted 18 lines · Mark Rutland

Hi Ard,

Apologies that it's taken me so long to get around to this...

Quoted 8 lines · Ard Biesheuvel

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

Quoted 4 lines

By adding new FIX_{PGD,PUD,PMD,PTE} indicees to the fixmap, we can allocate page tables from anywhere via memblock, and temporarily map them as we need to.

Interesting. So how are you dealing with the va<->pa translations and vice versa that occur all over the place in create_mapping() et al ?

Quoted 7 lines

That would avoid the need for the bootstrap tables. In head.S we'd only need to create a temporary (coarse-grained, RWX) kernel mapping (with the fixmap bolted on). Later we would create a whole new set of tables with a fine-grained kernel mapping and a full linear mapping using the new fixmap entries to temporarily map tables, then switch over to those atomically.

If we change back to a full linear mapping, are we back to not putting the Image astride a 1GB/32MB/512MB boundary (depending on page size)?

Anyway, to illustrate where I am headed with this: in my next version of this series, I intend to move the kernel mapping to the start of the vmalloc area, which gets moved up 64 MB to make room for the module area (which also moves down). That way, we can still load modules as before, but no longer have a need for a dedicated carveout for the kernel below PAGE_OFFSET.

The next step is then to move the kernel Image up inside the vmalloc area based on some randomness we get from the bootloader, and relocate it in place (using the same approach as in the patches I sent out beginning of this year). I have implemented module PLTs so that the

Image and the modules no longer need to be within 128 MB of each other, which means that we can have full KASLR for modules and Image, and also place the kernel anywhere in physical memory.The module PLTs would be a runtime penalty only, i.e., a KASLR capable kernel running without KASLR would not incur the penalty of branching via PLTs. The only build time option is -mcmodel=large for modules so that data symbol references are absolute, but that is unlike to hurt performance.

Quoted 19 lines · Ard Biesheuvel

Quoted 18 lines · Mark Rutland

Hi Ard,

Apologies that it's taken me so long to get around to this...

Quoted 8 lines · Ard Biesheuvel

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

Quoted 7 lines

Quoted 4 lines

By adding new FIX_{PGD,PUD,PMD,PTE} indicees to the fixmap, we can allocate page tables from anywhere via memblock, and temporarily map them as we need to.

Interesting. So how are you dealing with the va<->pa translations and vice versa that occur all over the place in create_mapping() et al ?

By rewriting create_mapping() et al to not do that ;)

That's requiring a fair amount of massaging, but so far I've not hit anything that renders the approach impossible.

Quoted 8 lines

Quoted 7 lines

That would avoid the need for the bootstrap tables. In head.S we'd only need to create a temporary (coarse-grained, RWX) kernel mapping (with the fixmap bolted on). Later we would create a whole new set of tables with a fine-grained kernel mapping and a full linear mapping using the new fixmap entries to temporarily map tables, then switch over to those atomically.

Quoted 2 lines

If we change back to a full linear mapping, are we back to not putting the Image astride a 1GB/32MB/512MB boundary (depending on page size)?

I'm not exactly sure what you mean here.

The kernel mapping may inhibit using large section mappings, but this is necessary anyway due to permission changes at sub-section granularity (e.g. in fixup_init).

The idea is that when the kernel tables are set up, things are mapped at the largest possible granularity that permits later permission changes without breaking/making sections (such that we can avoid TLB conflicts).

So we'd map the kernel and memory in segments, where no two segments share a common last-level entry (i.e. they're all at least page-aligned, and don't share a section with another segment).

We'd have separate segments for:
* memory below TEXT_OFFSET
* text
* rodata
* init
* altinstr (I think this can be folded into rodata)
* bss / data, tables
* memory above _end

Later I think it should be relatively simple to move the memory segment mapping for split-VA.

Quoted 6 lines

Anyway, to illustrate where I am headed with this: in my next version of this series, I intend to move the kernel mapping to the start of the vmalloc area, which gets moved up 64 MB to make room for the module area (which also moves down). That way, we can still load modules as before, but no longer have a need for a dedicated carveout for the kernel below PAGE_OFFSET.

Ok.

Quoted 4 lines

The next step is then to move the kernel Image up inside the vmalloc area based on some randomness we get from the bootloader, and relocate it in place (using the same approach as in the patches I sent out beginning of this year). I have implemented module PLTs so that the

Quoted 8 lines

Image and the modules no longer need to be within 128 MB of each other, which means that we can have full KASLR for modules and Image, and also place the kernel anywhere in physical memory.The module PLTs would be a runtime penalty only, i.e., a KASLR capable kernel running without KASLR would not incur the penalty of branching via PLTs. The only build time option is -mcmodel=large for modules so that data symbol references are absolute, but that is unlike to hurt performance.

I'm certainly interested in seeing this!

Quoted 7 lines · Mark Rutland

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

By adding new FIX_{PGD,PUD,PMD,PTE} indicees to the fixmap, we can allocate page tables from anywhere via memblock, and temporarily map them as we need to.

Quoted 6 lines

That would avoid the need for the bootstrap tables. In head.S we'd only need to create a temporary (coarse-grained, RWX) kernel mapping (with the fixmap bolted on). Later we would create a whole new set of tables with a fine-grained kernel mapping and a full linear mapping using the new fixmap entries to temporarily map tables, then switch over to those atomically.

If we change back to a full linear mapping, are we back to not putting the Image astride a 1GB/32MB/512MB boundary (depending on page size)?

Anyway, to illustrate where I am headed with this: in my next version of this series, I intend to move the kernel mapping to the start of the vmalloc area, which gets moved up 64 MB to make room for the module area (which also moves down). That way, we can still load modules as before, but no longer have a need for a dedicated carveout for the kernel below PAGE_OFFSET.

The next step is then to move the kernel Image up inside the vmalloc area based on some randomness we get from the bootloader, and relocate it in place (using the same approach as in the patches I sent out beginning of this year). I have implemented module PLTs so that the

Image and the modules no longer need to be within 128 MB of each other, which means that we can have full KASLR for modules and Image, and also place the kernel anywhere in physical memory.The module PLTs would be a runtime penalty only, i.e., a KASLR capable kernel running without KASLR would not incur the penalty of branching via PLTs. The only build time option is -mcmodel=large for modules so that data symbol references are absolute, but that is unlike to hurt performance.

Quoted 8 lines · Ard Biesheuvel

Quoted 7 lines · Mark Rutland

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

By adding new FIX_{PGD,PUD,PMD,PTE} indicees to the fixmap, we can allocate page tables from anywhere via memblock, and temporarily map them as we need to.

[...]

Quoted 6 lines

Quoted 6 lines

That would avoid the need for the bootstrap tables. In head.S we'd only need to create a temporary (coarse-grained, RWX) kernel mapping (with the fixmap bolted on). Later we would create a whole new set of tables with a fine-grained kernel mapping and a full linear mapping using the new fixmap entries to temporarily map tables, then switch over to those atomically.

If we separate the kernel image mapping from the linear one, I think things would be slightly simpler to avoid TLB conflicts (but I haven't looked at Mark's patches yet).

Quoted 9 lines

If we change back to a full linear mapping, are we back to not putting the Image astride a 1GB/32MB/512MB boundary (depending on page size)?

Anyway, to illustrate where I am headed with this: in my next version of this series, I intend to move the kernel mapping to the start of the vmalloc area, which gets moved up 64 MB to make room for the module area (which also moves down). That way, we can still load modules as before, but no longer have a need for a dedicated carveout for the kernel below PAGE_OFFSET.

This makes sense, I guess it can be easily added to the existing series just by changing the KIMAGE_OFFSET macro.

Quoted 4 lines

The next step is then to move the kernel Image up inside the vmalloc area based on some randomness we get from the bootloader, and relocate it in place (using the same approach as in the patches I sent out beginning of this year). I have implemented module PLTs so that the

Quoted 8 lines

Image and the modules no longer need to be within 128 MB of each other, which means that we can have full KASLR for modules and Image, and also place the kernel anywhere in physical memory.The module PLTs would be a runtime penalty only, i.e., a KASLR capable kernel running without KASLR would not incur the penalty of branching via PLTs. The only build time option is -mcmodel=large for modules so that data symbol references are absolute, but that is unlike to hurt performance.

I guess full KASLR would be conditional on a config option.

Apologies that it's taken me so long to get around to this...

Quoted 4 lines · Ard Biesheuvel

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

Quoted 4 lines

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

Quoted 7 lines · Mark Rutland

Apologies that it's taken me so long to get around to this...

Quoted 4 lines · Ard Biesheuvel

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

Quoted 8 lines

Quoted 4 lines

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

Given that the Christmas break is around the corner and your TLB series is probably going to take some time to get right, I suggest we persevere with Ard's current patch series for 4.5 and merge the TLB conflict solution for 4.6. I don't want us to end up in a situation where this is needlessly blocked on something that isn't quite ready.

Any objections? If not, Ard -- can you post a new version of this, please?

Quoted 17 lines · Will Deacon

Quoted 7 lines · Mark Rutland

Apologies that it's taken me so long to get around to this...

Quoted 4 lines · Ard Biesheuvel

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

Quoted 8 lines

Quoted 4 lines

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

Quoted 8 lines

Given that the Christmas break is around the corner and your TLB series is probably going to take some time to get right, I suggest we persevere with Ard's current patch series for 4.5 and merge the TLB conflict solution for 4.6. I don't want us to end up in a situation where this is needlessly blocked on something that isn't quite ready.

Any objections? If not, Ard -- can you post a new version of this, please?

Happy to post a new version, with the following remarks
- my current private tree has evolved in the mean time, and I am now putting the kernel image at the base of the vmalloc region (and the module region right before)
- I think Mark's changes would allow me to deobfuscate the VA bias that redirects __va() translations into the kernel VA space rather than the linear mapping

Quoted 27 lines · Ard Biesheuvel

Quoted 17 lines · Will Deacon

Quoted 7 lines · Mark Rutland

Apologies that it's taken me so long to get around to this...

Quoted 4 lines · Ard Biesheuvel

This splits off and generalises the population of the statically allocated fixmap page tables so that we may reuse it later for the linear mapping once we move the kernel text mapping out of it.

Quoted 8 lines

Quoted 4 lines

This also involves taking into account that table entries at any of the levels we are populating may have been populated already, since the fixmap mapping might not be disjoint up to the pgd level anymore from other early mappings.

As a heads-up, for avoiding TLB conflicts, I'm currently working on alternative way of creating the kernel page tables which will definitely conflict here, and may or may not supercede this approach.

Quoted 8 lines

Given that the Christmas break is around the corner and your TLB series is probably going to take some time to get right, I suggest we persevere with Ard's current patch series for 4.5 and merge the TLB conflict solution for 4.6. I don't want us to end up in a situation where this is needlessly blocked on something that isn't quite ready.

Any objections? If not, Ard -- can you post a new version of this, please?

Quoted 7 lines

Happy to post a new version, with the following remarks
- my current private tree has evolved in the mean time, and I am now putting the kernel image at the base of the vmalloc region (and the module region right before)
- I think Mark's changes would allow me to deobfuscate the VA bias that redirects __va() translations into the kernel VA space rather than the linear mapping

I'll leave that up to you. I'm just trying to avoid you growing a dependency on something that's unlikely to make it for 4.5. If Mark separates out the parts you need, perhaps that offers us some middle ground.

arch/arm64/kernel/head.S +0 -0

index 23cfc08fc8ba..d3e4b5d6a8d2 100644

Quoted 4 lines · Ard Biesheuvel

arch/arm64/kernel/head.S +0 -0

index 23cfc08fc8ba..d3e4b5d6a8d2 100644

[...]

arch/arm64/kernel/head.S section_table: · line 219

Quoted 12 lines

arch/arm64/kernel/head.S +7 -1

@@ -210,7 +210,15 @@ section_table:
 ENTRY(stext)
      bl      preserve_boot_args
      bl      el2_setup                       // Drop to EL1, w20=cpu_boot_mode
-     adrp    x24, __PHYS_OFFSET
+
+     /*
+      * 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
+      * the linear mapping. We will undo this in map_mem().
+      */

Minor typo in comment: I guess you meant "__pa() translations will not produce usable...".

Quoted 8 lines

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

index 5a22a119a74c..f6272f450688 100644
@@ -63,7 +63,8 @@ static struct addr_marker address_markers[] = {
      { PCI_IO_END,           "PCI I/O end" },
      { MODULES_VADDR,        "Modules start" },
      { MODULES_END,          "Modules end" },

arch/arm64/mm/dump.c static struct addr_marker address_markers[] = { · line 69

Quoted 5 lines

arch/arm64/mm/dump.c +2 -1

-     { PAGE_OFFSET,          "Kernel Mapping" },
+     { KIMAGE_VADDR,         "Kernel Mapping" },
+     { PAGE_OFFSET,          "Linear Mapping" },
      { -1,                   NULL },
 };

Apart from this, please change the pr_notice() in mem_init() to show the linear mapping at the end (keep them in ascending order).

Quoted 6 lines · Catalin Marinas

Quoted 4 lines · Ard Biesheuvel

arch/arm64/kernel/head.S +0 -0

index 23cfc08fc8ba..d3e4b5d6a8d2 100644

[...]

Quoted 16 lines

arch/arm64/kernel/head.S section_table: · line 219

Quoted 12 lines

arch/arm64/kernel/head.S +7 -1

@@ -210,7 +210,15 @@ section_table:
 ENTRY(stext)
      bl      preserve_boot_args
      bl      el2_setup                       // Drop to EL1, w20=cpu_boot_mode
-     adrp    x24, __PHYS_OFFSET
+
+     /*
+      * 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
+      * the linear mapping. We will undo this in map_mem().
+      */

Minor typo in comment: I guess you meant "__pa() translations will not produce usable...".

No, not quite. __va() translations will normally produce addresses in the linear mapping, which will not be set up when we first start using it in create_mapping(). So until that time, we have to redirect __va() translations into the kernel mapping, where swapper_pg_dir is shadowed. I am hoping that Mark's planned changes to create_mapping() will make this unnecessary, but I haven't seen any of his code yet.

As far as __pa() is concerned, that translation is actually tweaked so it will always produce usable addresses, regardless of whether the bias is still set or not. The reason is that va-to-pa translations are always unambiguous.

Quoted 8 lines

Quoted 8 lines

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

index 5a22a119a74c..f6272f450688 100644
@@ -63,7 +63,8 @@ static struct addr_marker address_markers[] = {
      { PCI_IO_END,           "PCI I/O end" },
      { MODULES_VADDR,        "Modules start" },
      { MODULES_END,          "Modules end" },

Quoted 9 lines

arch/arm64/mm/dump.c static struct addr_marker address_markers[] = { · line 69

Quoted 5 lines

arch/arm64/mm/dump.c +2 -1

-     { PAGE_OFFSET,          "Kernel Mapping" },
+     { KIMAGE_VADDR,         "Kernel Mapping" },
+     { PAGE_OFFSET,          "Linear Mapping" },
      { -1,                   NULL },
 };

Apart from this, please change the pr_notice() in mem_init() to show the linear mapping at the end (keep them in ascending order).

OK

Quoted 23 lines · Ard Biesheuvel

Quoted 21 lines · Catalin Marinas

Quoted 4 lines · Ard Biesheuvel

arch/arm64/kernel/head.S +0 -0

index 23cfc08fc8ba..d3e4b5d6a8d2 100644

[...]

arch/arm64/kernel/head.S section_table: · line 219

Quoted 12 lines

arch/arm64/kernel/head.S +7 -1

@@ -210,7 +210,15 @@ section_table:
 ENTRY(stext)
      bl      preserve_boot_args
      bl      el2_setup                       // Drop to EL1, w20=cpu_boot_mode
-     adrp    x24, __PHYS_OFFSET
+
+     /*
+      * 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
+      * the linear mapping. We will undo this in map_mem().
+      */

Minor typo in comment: I guess you meant "__pa() translations will not produce usable...".

Quoted 5 lines

No, not quite. __va() translations will normally produce addresses in the linear mapping, which will not be set up when we first start using it in create_mapping(). So until that time, we have to redirect __va() translations into the kernel mapping, where swapper_pg_dir is shadowed.

Quoted 2 lines

I am hoping that Mark's planned changes to create_mapping() will make this unnecessary, but I haven't seen any of his code yet.

Quoted 4 lines

As far as __pa() is concerned, that translation is actually tweaked so it will always produce usable addresses, regardless of whether the bias is still set or not. The reason is that va-to-pa translations are always unambiguous.

... so of course, the comment is still wrong, -> s/virtual/physical/ addresses

arch/arm64/mm/mmu.c +1 -38

index c7ba171951c8..526eeb7e1e97 100644
@@ -357,47 +357,10 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
 				phys, virt, size, prot, late_alloc);
 }
 
-#ifdef CONFIG_DEBUG_RODATA
 static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
 {
-	/*
-	 * Set up the executable regions using the existing section mappings
-	 * for now. This will get more fine grained later once all memory
-	 * is mapped
-	 */
-	unsigned long kernel_x_start = round_down(__pa(_stext), SWAPPER_BLOCK_SIZE);
-	unsigned long kernel_x_end = round_up(__pa(__init_end), SWAPPER_BLOCK_SIZE);
-
-	if (end < kernel_x_start) {
-		create_mapping(start, __phys_to_virt(start),
-			end - start, PAGE_KERNEL);
-	} else if (start >= kernel_x_end) {
-		create_mapping(start, __phys_to_virt(start),
-			end - start, PAGE_KERNEL);
-	} else {
-		if (start < kernel_x_start)
-			create_mapping(start, __phys_to_virt(start),
-				kernel_x_start - start,
-				PAGE_KERNEL);
-		create_mapping(kernel_x_start,
-				__phys_to_virt(kernel_x_start),
-				kernel_x_end - kernel_x_start,
-				PAGE_KERNEL_EXEC);
-		if (kernel_x_end < end)
-			create_mapping(kernel_x_end,
-				__phys_to_virt(kernel_x_end),
-				end - kernel_x_end,
-				PAGE_KERNEL);
-	}
-
-}
-#else
-static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
-{
-	create_mapping(start, __phys_to_virt(start), end - start,
-			PAGE_KERNEL_EXEC);
+	create_mapping(start, __phys_to_virt(start), end - start, PAGE_KERNEL);
 }
-#endif
 
 struct bootstrap_pgtables {
 	pte_t	pte[PTRS_PER_PTE];

Quoted 60 lines · Ard Biesheuvel

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

index c7ba171951c8..526eeb7e1e97 100644
@@ -357,47 +357,10 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
                              phys, virt, size, prot, late_alloc);
 }
 
-#ifdef CONFIG_DEBUG_RODATA
 static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
 {
-     /*
-      * Set up the executable regions using the existing section mappings
-      * for now. This will get more fine grained later once all memory
-      * is mapped
-      */
-     unsigned long kernel_x_start = round_down(__pa(_stext), SWAPPER_BLOCK_SIZE);
-     unsigned long kernel_x_end = round_up(__pa(__init_end), SWAPPER_BLOCK_SIZE);
-
-     if (end < kernel_x_start) {
-             create_mapping(start, __phys_to_virt(start),
-                     end - start, PAGE_KERNEL);
-     } else if (start >= kernel_x_end) {
-             create_mapping(start, __phys_to_virt(start),
-                     end - start, PAGE_KERNEL);
-     } else {
-             if (start < kernel_x_start)
-                     create_mapping(start, __phys_to_virt(start),
-                             kernel_x_start - start,
-                             PAGE_KERNEL);
-             create_mapping(kernel_x_start,
-                             __phys_to_virt(kernel_x_start),
-                             kernel_x_end - kernel_x_start,
-                             PAGE_KERNEL_EXEC);
-             if (kernel_x_end < end)
-                     create_mapping(kernel_x_end,
-                             __phys_to_virt(kernel_x_end),
-                             end - kernel_x_end,
-                             PAGE_KERNEL);
-     }
-
-}
-#else
-static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
-{
-     create_mapping(start, __phys_to_virt(start), end - start,
-                     PAGE_KERNEL_EXEC);
+     create_mapping(start, __phys_to_virt(start), end - start, PAGE_KERNEL);
 }
-#endif
 
 struct bootstrap_pgtables {
      pte_t   pte[PTRS_PER_PTE];

@@ -471,7 +434,7 @@ static unsigned long __init bootstrap_region(struct bootstrap_pgtables *reg,
                                    SWAPPER_BLOCK_SIZE));
 
              create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
-                            PAGE_KERNEL_EXEC);
+                            PAGE_KERNEL);
 

arch/arm64/mm/mmu.c static unsigned long __init bootstrap_region(struct bootstrap_pgtables *reg, · line 440

Quoted 2 lines

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

              return vend;
      }

These make sense. However, shall we go a step further and unmap the kernel image completely from the linear mapping, maybe based on CONFIG_DEBUG_RODATA? The mark_rodata_ro() function changes the text to read-only but you can still get writable access to it via __va(__pa(_stext)).

Quoted 61 lines · Catalin Marinas

Quoted 60 lines · Ard Biesheuvel

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

index c7ba171951c8..526eeb7e1e97 100644
@@ -357,47 +357,10 @@ static void create_mapping_late(phys_addr_t phys, unsigned long virt,
                              phys, virt, size, prot, late_alloc);
 }
 
-#ifdef CONFIG_DEBUG_RODATA
 static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
 {
-     /*
-      * Set up the executable regions using the existing section mappings
-      * for now. This will get more fine grained later once all memory
-      * is mapped
-      */
-     unsigned long kernel_x_start = round_down(__pa(_stext), SWAPPER_BLOCK_SIZE);
-     unsigned long kernel_x_end = round_up(__pa(__init_end), SWAPPER_BLOCK_SIZE);
-
-     if (end < kernel_x_start) {
-             create_mapping(start, __phys_to_virt(start),
-                     end - start, PAGE_KERNEL);
-     } else if (start >= kernel_x_end) {
-             create_mapping(start, __phys_to_virt(start),
-                     end - start, PAGE_KERNEL);
-     } else {
-             if (start < kernel_x_start)
-                     create_mapping(start, __phys_to_virt(start),
-                             kernel_x_start - start,
-                             PAGE_KERNEL);
-             create_mapping(kernel_x_start,
-                             __phys_to_virt(kernel_x_start),
-                             kernel_x_end - kernel_x_start,
-                             PAGE_KERNEL_EXEC);
-             if (kernel_x_end < end)
-                     create_mapping(kernel_x_end,
-                             __phys_to_virt(kernel_x_end),
-                             end - kernel_x_end,
-                             PAGE_KERNEL);
-     }
-
-}
-#else
-static void __init __map_memblock(phys_addr_t start, phys_addr_t end)
-{
-     create_mapping(start, __phys_to_virt(start), end - start,
-                     PAGE_KERNEL_EXEC);
+     create_mapping(start, __phys_to_virt(start), end - start, PAGE_KERNEL);
 }
-#endif
 
 struct bootstrap_pgtables {
      pte_t   pte[PTRS_PER_PTE];

@@ -471,7 +434,7 @@ static unsigned long __init bootstrap_region(struct bootstrap_pgtables *reg,
                                    SWAPPER_BLOCK_SIZE));
 
              create_mapping(__pa(vstart - va_offset), vstart, vend - vstart,
-                            PAGE_KERNEL_EXEC);
+                            PAGE_KERNEL);
 

Quoted 8 lines

arch/arm64/mm/mmu.c static unsigned long __init bootstrap_region(struct bootstrap_pgtables *reg, · line 440

Quoted 2 lines

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

              return vend;
      }

These make sense. However, shall we go a step further and unmap the kernel image completely from the linear mapping, maybe based on CONFIG_DEBUG_RODATA? The mark_rodata_ro() function changes the text to read-only but you can still get writable access to it via __va(__pa(_stext)).

If we can tolerate the fragmentation, then yes, let's unmap it completely. As long as we don't unmap the.pgdir section, since that will be referenced via the linear mapping

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

index 3148691bc80a..d6a237bda1f9 100644
@@ -120,13 +120,10 @@ extern phys_addr_t		memstart_addr;
 extern u64 kernel_va_offset;
 
 /*

arch/arm64/mm/init.c +22 -0

index b3b0175d7135..29a7dc5327b6 100644
@@ -158,9 +159,55 @@ static int __init early_mem(char *p)
 }
 early_param("mem", early_mem);
 
+static void __init enforce_memory_limit(void)
+{
+	const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+	u64 to_remove = memblock_phys_mem_size() - memory_limit;
+	phys_addr_t max_addr = 0;
+	struct memblock_region *r;
+
+	if (memory_limit == (phys_addr_t)ULLONG_MAX)
+		return;
+
+	/*
+	 * The kernel may be high up in physical memory, so try to apply the
+	 * limit below the kernel first, and only let the generic handling
+	 * take over if it turns out we haven't clipped enough memory yet.
+	 */
+	for_each_memblock(memory, r) {
+		if (r->base + r->size > kbase) {
+			u64 rem = min(to_remove, kbase - r->base);
+
+			max_addr = r->base + rem;
+			to_remove -= rem;
+			break;

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

index 526eeb7e1e97..1b9d7e48ba1e 100644
@@ -481,11 +482,33 @@ static void __init bootstrap_linear_mapping(unsigned long va_offset)
 static void __init map_mem(void)
 {
 	struct memblock_region *reg;
+	u64 new_memstart_addr;

Quoted 7 lines · Ard Biesheuvel

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

index 3148691bc80a..d6a237bda1f9 100644
@@ -120,13 +120,10 @@ extern phys_addr_t              memstart_addr;
 extern u64 kernel_va_offset;
 

arch/arm64/include/asm/memory.h extern phys_addr_t memstart_addr; · line 126

Quoted 11 lines

arch/arm64/include/asm/memory.h +3 -6

 /*
- * The maximum physical address that the linear direct mapping
- * of system RAM can cover. (PAGE_OFFSET can be interpreted as
- * a 2's complement signed quantity and negated to derive the
- * maximum size of the linear mapping.)
+ * Allow all memory at the discovery stage. We will clip it later.
  */
-#define MAX_MEMBLOCK_ADDR    ({ memstart_addr - PAGE_OFFSET - 1; })
-#define MIN_MEMBLOCK_ADDR    __pa(KIMAGE_VADDR)
+#define MIN_MEMBLOCK_ADDR    0
+#define MAX_MEMBLOCK_ADDR    U64_MAX

Just in case we get some random memblock information, shall we cap the maximum to PHYS_MASK?

Quoted 36 lines

arch/arm64/mm/init.c +28 -0

index b3b0175d7135..29a7dc5327b6 100644
@@ -158,9 +159,55 @@ static int __init early_mem(char *p)
 }
 early_param("mem", early_mem);
 
+static void __init enforce_memory_limit(void)
+{
+     const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+     u64 to_remove = memblock_phys_mem_size() - memory_limit;
+     phys_addr_t max_addr = 0;
+     struct memblock_region *r;
+
+     if (memory_limit == (phys_addr_t)ULLONG_MAX)
+             return;
+
+     /*
+      * The kernel may be high up in physical memory, so try to apply the
+      * limit below the kernel first, and only let the generic handling
+      * take over if it turns out we haven't clipped enough memory yet.
+      */
+     for_each_memblock(memory, r) {
+             if (r->base + r->size > kbase) {
+                     u64 rem = min(to_remove, kbase - r->base);
+
+                     max_addr = r->base + rem;
+                     to_remove -= rem;
+                     break;
+             }
+             if (to_remove <= r->size) {
+                     max_addr = r->base + to_remove;
+                     to_remove = 0;
+                     break;
+             }

arch/arm64/mm/init.c static int __init early_mem(char *p) · line 193

Quoted 4 lines

arch/arm64/mm/init.c +4 -0

+             to_remove -= r->size;
+     }
+
+     memblock_remove(0, max_addr);

I don't fully get the reason for this function. Do you want to keep the kernel around in memblock? How do we guarantee that the call below wouldn't remove it anyway?

arch/arm64/mm/init.c static int __init early_mem(char *p) · line 196

Quoted 3 lines

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

+
+     if (to_remove)
+             memblock_enforce_memory_limit(memory_limit);

Shouldn't this be memblock_enforce_memory_limit(to_remove)?

Quoted 12 lines

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

index 526eeb7e1e97..1b9d7e48ba1e 100644
@@ -481,11 +482,33 @@ static void __init bootstrap_linear_mapping(unsigned long va_offset)
 static void __init map_mem(void)
 {
      struct memblock_region *reg;
+     u64 new_memstart_addr;
+     u64 new_va_offset;
 

arch/arm64/mm/mmu.c static void __init bootstrap_linear_mapping(unsigned long va_offset) · line 494

Quoted 8 lines

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

-     bootstrap_linear_mapping(KIMAGE_OFFSET);
+     /*
+      * Select a suitable value for the base of physical memory.
+      * This should be equal to or below the lowest usable physical
+      * memory address, and aligned to PUD/PMD size so that we can map
+      * it efficiently.
+      */
+     new_memstart_addr = round_down(memblock_start_of_DRAM(), SZ_1G);

With this trick, we can no longer assume we have a mapping at PAGE_OFFSET. I don't think we break any expectations but we probably don't free the unused memmap at the beginning. We can probably set prev_end to this rounded down address in free_unused_memmap().

Quoted 37 lines · Catalin Marinas

Quoted 36 lines · Ard Biesheuvel

arch/arm64/mm/init.c +28 -0

index b3b0175d7135..29a7dc5327b6 100644
@@ -158,9 +159,55 @@ static int __init early_mem(char *p)
 }
 early_param("mem", early_mem);
 
+static void __init enforce_memory_limit(void)
+{
+     const phys_addr_t kbase = round_down(__pa(_text), MIN_KIMG_ALIGN);
+     u64 to_remove = memblock_phys_mem_size() - memory_limit;
+     phys_addr_t max_addr = 0;
+     struct memblock_region *r;
+
+     if (memory_limit == (phys_addr_t)ULLONG_MAX)
+             return;
+
+     /*
+      * The kernel may be high up in physical memory, so try to apply the
+      * limit below the kernel first, and only let the generic handling
+      * take over if it turns out we haven't clipped enough memory yet.
+      */
+     for_each_memblock(memory, r) {
+             if (r->base + r->size > kbase) {
+                     u64 rem = min(to_remove, kbase - r->base);
+
+                     max_addr = r->base + rem;
+                     to_remove -= rem;
+                     break;
+             }
+             if (to_remove <= r->size) {
+                     max_addr = r->base + to_remove;
+                     to_remove = 0;
+                     break;
+             }

Quoted 8 lines

arch/arm64/mm/init.c static int __init early_mem(char *p) · line 193

Quoted 4 lines

arch/arm64/mm/init.c +4 -0

+             to_remove -= r->size;
+     }
+
+     memblock_remove(0, max_addr);

I don't fully get the reason for this function. Do you want to keep the kernel around in memblock? How do we guarantee that the call below wouldn't remove it anyway?

The problem is that the ordinary memblock_enforce_memory_limit() removes memory from the top, which means it will happily remove the memory that covers your kernel image if it happens to be loaded high up in physical memory.

Quoted 5 lines

arch/arm64/mm/init.c static int __init early_mem(char *p) · line 196

Quoted 3 lines

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

+
+     if (to_remove)
+             memblock_enforce_memory_limit(memory_limit);

Shouldn't this be memblock_enforce_memory_limit(to_remove)?

No, it takes the memory limit as input. 'to_remove + memory_limit' will be exactly the remaining memory at this point.