x86/setup: consolidate early memory reservations

From: Mike Rapoport <[email protected]>

The early reservations of memory areas used by the firmware, bootloader, kernel text and data are spread over setup_arch(). Moreover, some of them happen *after* memblock allocations, e.g trim_platform_memory_ranges() and trim_low_memory_range() are called after reserve_real_mode() that allocates memory.

There was no corruption of these memory regions because memblock always allocates memory either from the end of memory (in top-down mode) or above the kernel image (in bottom-up mode). However, the bottom up mode is going to be updated to span the entire memory [1] to avoid limitations caused by KASLR.

Consolidate early memory reservations in a dedicated function to improve robustness against future changes. Having the early reservations in one place also makes it clearer what memory must be reserved before we allow memblock allocations.

[1] https://lore.kernel.org/lkml/[email protected]

Signed-off-by Mike Rapoport <[email protected]>

Acked-by Borislav Petkov <[email protected]>

---
 arch/x86/kernel/setup.c | 92 ++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 48 deletions(-)

arch/x86/kernel/setup.c +14 -13

index d883176ef2ce..3e3c6036b023 100644
@@ -645,18 +645,6 @@ static void __init trim_snb_memory(void)
 	}
 }
 
-/*
- * Here we put platform-specific memory range workarounds, i.e.
- * memory known to be corrupt or otherwise in need to be reserved on
- * specific platforms.
- *
- * If this gets used more widely it could use a real dispatch mechanism.
- */
-static void __init trim_platform_memory_ranges(void)
-{
-	trim_snb_memory();
-}
-
 static void __init trim_bios_range(void)
 {
 	/*

@@ -729,7 +717,38 @@ static void __init trim_low_memory_range(void)
 {
 	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
 }
-	
+
+static void __init early_reserve_memory(void)
+{
+	/*
+	 * Reserve the memory occupied by the kernel between _text and
+	 * __end_of_kernel_reserve symbols. Any kernel sections after the
+	 * __end_of_kernel_reserve symbol must be explicitly reserved with a
+	 * separate memblock_reserve() or they will be discarded.
+	 */
+	memblock_reserve(__pa_symbol(_text),
+			 (unsigned long)__end_of_kernel_reserve - (unsigned long)_text);
+
+	/*
+	 * Make sure page 0 is always reserved because on systems with

arch/x86/kernel/setup.c +20 -35

+
+	reserve_ibft_region();
+	reserve_bios_regions();
+
+	trim_snb_memory();
+	trim_low_memory_range();
+}
+
 /*
  * Dump out kernel offset information on panic.
  */

@@ -764,29 +783,6 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
 
 void __init setup_arch(char **cmdline_p)
 {
-	/*
-	 * Reserve the memory occupied by the kernel between _text and
-	 * __end_of_kernel_reserve symbols. Any kernel sections after the
-	 * __end_of_kernel_reserve symbol must be explicitly reserved with a
-	 * separate memblock_reserve() or they will be discarded.
-	 */
-	memblock_reserve(__pa_symbol(_text),
-			 (unsigned long)__end_of_kernel_reserve - (unsigned long)_text);
-
-	/*
-	 * Make sure page 0 is always reserved because on systems with
-	 * L1TF its contents can be leaked to user processes.
-	 */
-	memblock_reserve(0, PAGE_SIZE);
-
-	early_reserve_initrd();
-
-	/*
-	 * At this point everything still needed from the boot loader
-	 * or BIOS or kernel text should be early reserved or marked not
-	 * RAM in e820. All other memory is free game.
-	 */
-
 #ifdef CONFIG_X86_32
 	memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
 

@@ -910,8 +906,18 @@ void __init setup_arch(char **cmdline_p)
 
 	parse_early_param();
 
-	if (efi_enabled(EFI_BOOT))
-		efi_memblock_x86_reserve_range();
+	/*
+	 * Do some memory reservations *before* memory is added to
+	 * memblock, so memblock allocations won't overwrite it.
+	 * Do it after early param, so we could get (unlikely) panic from
+	 * serial.
+	 *
+	 * After this point everything still needed from the boot loader or
+	 * firmware or kernel text should be early reserved or marked not
+	 * RAM in e820. All other memory is free game.
+	 */
+	early_reserve_memory();
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 	/*
 	 * Memory used by the kernel cannot be hot-removed because Linux

@@ -938,9 +944,6 @@ void __init setup_arch(char **cmdline_p)
 
 	x86_report_nx();
 
-	/* after early param, so could get panic from serial */
-	memblock_x86_reserve_range_setup_data();
-
 	if (acpi_mps_check()) {
 #ifdef CONFIG_X86_LOCAL_APIC
 		disable_apic = 1;

@@ -1032,8 +1035,6 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	find_smp_config();
 
-	reserve_ibft_region();
-
 	early_alloc_pgt_buf();
 
 	/*

@@ -1054,8 +1055,6 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	sev_setup_arch();
 
-	reserve_bios_regions();
-
 	efi_fake_memmap();
 	efi_find_mirror();
 	efi_esrt_init();

@@ -1081,9 +1080,6 @@ void __init setup_arch(char **cmdline_p)
 
 	reserve_real_mode();
 
-	trim_platform_memory_ranges();
-	trim_low_memory_range();
-
 	init_mem_mapping();
 
 	idt_setup_early_pf();

Signature · 1 line

2.28.0

Quoted 69 lines · Mike Rapoport

From: Mike Rapoport <[email protected]>

The early reservations of memory areas used by the firmware, bootloader, kernel text and data are spread over setup_arch(). Moreover, some of them happen *after* memblock allocations, e.g trim_platform_memory_ranges() and trim_low_memory_range() are called after reserve_real_mode() that allocates memory.

There was no corruption of these memory regions because memblock always allocates memory either from the end of memory (in top-down mode) or above the kernel image (in bottom-up mode). However, the bottom up mode is going to be updated to span the entire memory [1] to avoid limitations caused by KASLR.

Consolidate early memory reservations in a dedicated function to improve robustness against future changes. Having the early reservations in one place also makes it clearer what memory must be reserved before we allow memblock allocations.

[1] https://lore.kernel.org/lkml/[email protected]

Signed-off-by Mike Rapoport <[email protected]>

Acked-by Borislav Petkov <[email protected]>

---
 arch/x86/kernel/setup.c | 92 ++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 48 deletions(-)

arch/x86/kernel/setup.c +14 -13

index d883176ef2ce..3e3c6036b023 100644
@@ -645,18 +645,6 @@ static void __init trim_snb_memory(void)
 	}
 }
 
-/*
- * Here we put platform-specific memory range workarounds, i.e.
- * memory known to be corrupt or otherwise in need to be reserved on
- * specific platforms.
- *
- * If this gets used more widely it could use a real dispatch mechanism.
- */
-static void __init trim_platform_memory_ranges(void)
-{
-	trim_snb_memory();
-}
-
 static void __init trim_bios_range(void)
 {
 	/*

@@ -729,7 +717,38 @@ static void __init trim_low_memory_range(void)
 {
 	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
 }
-	
+
+static void __init early_reserve_memory(void)
+{
+	/*
+	 * Reserve the memory occupied by the kernel between _text and
+	 * __end_of_kernel_reserve symbols. Any kernel sections after the
+	 * __end_of_kernel_reserve symbol must be explicitly reserved with a
+	 * separate memblock_reserve() or they will be discarded.
+	 */
+	memblock_reserve(__pa_symbol(_text),
+			 (unsigned long)__end_of_kernel_reserve - (unsigned long)_text);
+
+	/*
+	 * Make sure page 0 is always reserved because on systems with

arch/x86/kernel/setup.c static void __init trim_low_memory_range(void) · line 743

Quoted 10 lines

arch/x86/kernel/setup.c +10 -0

+	 * L1TF its contents can be leaked to user processes.
+	 */
+	memblock_reserve(0, PAGE_SIZE);
+
+	early_reserve_initrd();
+
+	if (efi_enabled(EFI_BOOT))
+		efi_memblock_x86_reserve_range();
+
+	memblock_x86_reserve_range_setup_data();

This patch looks good to me, thanks for the effort.

While at it, wondering if we can rename the above function to memblock_reserve_setup_data() just as its e820 counterpart e820__reserve_setup_data(), adding 'x86' to a function under arch/x86 seems redundant.

FWIW,

Reviewed-by Baoquan He <[email protected]>

Thanks
Baoquan

Quoted 103 lines

arch/x86/kernel/setup.c +20 -35

+
+	reserve_ibft_region();
+	reserve_bios_regions();
+
+	trim_snb_memory();
+	trim_low_memory_range();
+}
+
 /*
  * Dump out kernel offset information on panic.
  */

@@ -764,29 +783,6 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
 
 void __init setup_arch(char **cmdline_p)
 {
-	/*
-	 * Reserve the memory occupied by the kernel between _text and
-	 * __end_of_kernel_reserve symbols. Any kernel sections after the
-	 * __end_of_kernel_reserve symbol must be explicitly reserved with a
-	 * separate memblock_reserve() or they will be discarded.
-	 */
-	memblock_reserve(__pa_symbol(_text),
-			 (unsigned long)__end_of_kernel_reserve - (unsigned long)_text);
-
-	/*
-	 * Make sure page 0 is always reserved because on systems with
-	 * L1TF its contents can be leaked to user processes.
-	 */
-	memblock_reserve(0, PAGE_SIZE);
-
-	early_reserve_initrd();
-
-	/*
-	 * At this point everything still needed from the boot loader
-	 * or BIOS or kernel text should be early reserved or marked not
-	 * RAM in e820. All other memory is free game.
-	 */
-
 #ifdef CONFIG_X86_32
 	memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
 

@@ -910,8 +906,18 @@ void __init setup_arch(char **cmdline_p)
 
 	parse_early_param();
 
-	if (efi_enabled(EFI_BOOT))
-		efi_memblock_x86_reserve_range();
+	/*
+	 * Do some memory reservations *before* memory is added to
+	 * memblock, so memblock allocations won't overwrite it.
+	 * Do it after early param, so we could get (unlikely) panic from
+	 * serial.
+	 *
+	 * After this point everything still needed from the boot loader or
+	 * firmware or kernel text should be early reserved or marked not
+	 * RAM in e820. All other memory is free game.
+	 */
+	early_reserve_memory();
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 	/*
 	 * Memory used by the kernel cannot be hot-removed because Linux

@@ -938,9 +944,6 @@ void __init setup_arch(char **cmdline_p)
 
 	x86_report_nx();
 
-	/* after early param, so could get panic from serial */
-	memblock_x86_reserve_range_setup_data();
-
 	if (acpi_mps_check()) {
 #ifdef CONFIG_X86_LOCAL_APIC
 		disable_apic = 1;

@@ -1032,8 +1035,6 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	find_smp_config();
 
-	reserve_ibft_region();
-
 	early_alloc_pgt_buf();
 
 	/*

@@ -1054,8 +1055,6 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	sev_setup_arch();
 
-	reserve_bios_regions();
-
 	efi_fake_memmap();
 	efi_find_mirror();
 	efi_esrt_init();

@@ -1081,9 +1080,6 @@ void __init setup_arch(char **cmdline_p)
 
 	reserve_real_mode();
 
-	trim_platform_memory_ranges();
-	trim_low_memory_range();
-
 	init_mem_mapping();
 
 	idt_setup_early_pf();

Signature · 1 line

2.28.0

On Tue, Mar 02, 2021 at 09:04:09PM +0800, Baoquan He wrote:

Quoted 26 lines

Quoted 13 lines

arch/x86/kernel/setup.c +13 -0

+static void __init early_reserve_memory(void)
+{
+	/*
+	 * Reserve the memory occupied by the kernel between _text and
+	 * __end_of_kernel_reserve symbols. Any kernel sections after the
+	 * __end_of_kernel_reserve symbol must be explicitly reserved with a
+	 * separate memblock_reserve() or they will be discarded.
+	 */
+	memblock_reserve(__pa_symbol(_text),
+			 (unsigned long)__end_of_kernel_reserve - (unsigned long)_text);
+
+	/*
+	 * Make sure page 0 is always reserved because on systems with

arch/x86/kernel/setup.c static void __init trim_low_memory_range(void) · line 743

Quoted 10 lines

arch/x86/kernel/setup.c +10 -0

+	 * L1TF its contents can be leaked to user processes.
+	 */
+	memblock_reserve(0, PAGE_SIZE);
+
+	early_reserve_initrd();
+
+	if (efi_enabled(EFI_BOOT))
+		efi_memblock_x86_reserve_range();
+
+	memblock_x86_reserve_range_setup_data();

This patch looks good to me, thanks for the effort.

Quoted 4 lines

While at it, wondering if we can rename the above function to memblock_reserve_setup_data() just as its e820 counterpart e820__reserve_setup_data(), adding 'x86' to a function under arch/x86 seems redundant.

I'd rather keep these names for now. First, it's easier to dig to them in the git history and second, I'm planning more changes in this area and these names are as good as FIXME: to remind what still needs to be checked :)

From: Mike Rapoport <[email protected]>

Hi,

David noticed that we do some of memblock_reserve() calls after allocations are possible:

https://lore.kernel.org/lkml/[email protected]

The below patches consolidate early memory reservations done during setup_arch() so that memory used by firmware, bootloader, kernel text/data and the memory that should be excluded from the available memory for whatever other reason is reserved before memblock allocations are possible.

The patches are rebased on v5.12-rc1 and I think x86 tree is the best way to merge them.

v3:
* rebase on v5.12-rc1

v2: https://lore.kernel.org/lkml/[email protected]
* get rid of trim_platform_memory_ranges() and call trim_snb_memory() directly, per Boris comments
* massage changelog and comments to use passive voice, per Boris
* add Acked-by and Reviewed-by, thanks Boris and David

v1: https://lore.kernel.org/lkml/[email protected]

Mike Rapoport (2):
x86/setup: consolidate early memory reservations x86/setup: merge several reservations of start of the memory

 arch/x86/kernel/setup.c | 95 ++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 49 deletions(-)

Signature · 1 line

2.28.0

Any comments on these?

Any comments on these?

Quoted 38 lines · Mike Rapoport

From: Mike Rapoport <[email protected]>

Hi,

David noticed that we do some of memblock_reserve() calls after allocations are possible:

https://lore.kernel.org/lkml/[email protected]

The below patches consolidate early memory reservations done during setup_arch() so that memory used by firmware, bootloader, kernel text/data and the memory that should be excluded from the available memory for whatever other reason is reserved before memblock allocations are possible.

The patches are rebased on v5.12-rc1 and I think x86 tree is the best way to merge them.

v3:
* rebase on v5.12-rc1

v2: https://lore.kernel.org/lkml/[email protected]
* get rid of trim_platform_memory_ranges() and call trim_snb_memory() directly, per Boris comments
* massage changelog and comments to use passive voice, per Boris
* add Acked-by and Reviewed-by, thanks Boris and David

v1: https://lore.kernel.org/lkml/[email protected]

Mike Rapoport (2):
x86/setup: consolidate early memory reservations x86/setup: merge several reservations of start of the memory

 arch/x86/kernel/setup.c | 95 ++++++++++++++++++++---------------------
 1 file changed, 46 insertions(+), 49 deletions(-)

Signature · 1 line

2.28.0

Signature · 2 lines

Sincerely yours,
Mike.