[PATCH v3 0/2] x86/setup: consolidate early memory reservations

[PATCH v3 0/2] x86/setup: consolidate early memory reservations

[Mike Rapoport]

From: Mike Rapoport <rppt@linux.ibm.com>

Hi,

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

https://lore.kernel.org/lkml/6ba6bde3-1520-5cd0-f987-32d543f0b79f@redhat.com

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/20210128105711.10428-1-rppt@kernel.org
* 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/20210115083255.12744-1-rppt@kernel.org

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(-)

-- 
2.28.0

[PATCH v3 1/2] x86/setup: consolidate early memory reservations

[Mike Rapoport]

From: Mike Rapoport <rppt@linux.ibm.com>

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/20201217201214.3414100-2-guro@fb.com

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Acked-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/setup.c | 92 ++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 48 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d883176ef2ce..3e3c6036b023 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -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
+	 * 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();
+
+	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();
-- 
2.28.0

Re: [PATCH v3 1/2] x86/setup: consolidate early memory reservations

[Baoquan He]

On 03/02/21 at 12:04pm, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
> 
> 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/20201217201214.3414100-2-guro@fb.com
> 
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> Acked-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/kernel/setup.c | 92 ++++++++++++++++++++---------------------
>  1 file changed, 44 insertions(+), 48 deletions(-)
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index d883176ef2ce..3e3c6036b023 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -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
> +	 * 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 <bhe@redhat.com>

Thanks
Baoquan

> +
> +	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();
> -- 
> 2.28.0
> 

Re: [PATCH v3 1/2] x86/setup: consolidate early memory reservations

[Mike Rapoport]

On Tue, Mar 02, 2021 at 09:04:09PM +0800, Baoquan He wrote:
> On 03/02/21 at 12:04pm, Mike Rapoport wrote:
> > From: Mike Rapoport <rppt@linux.ibm.com>
> > 
> > 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/20201217201214.3414100-2-guro@fb.com
> > 
> > Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> > Acked-by: Borislav Petkov <bp@suse.de>
> > ---
> >  arch/x86/kernel/setup.c | 92 ++++++++++++++++++++---------------------
> >  1 file changed, 44 insertions(+), 48 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index d883176ef2ce..3e3c6036b023 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -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
> > +	 * 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.

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 :)
 
> FWIW,
> 
> Reviewed-by: Baoquan He <bhe@redhat.com>

Thanks!
 
> Thanks
> Baoquan
> 
> > +
> > +	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();
> > -- 
> > 2.28.0
> > 
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v3 1/2] x86/setup: consolidate early memory reservations

[Baoquan He]

On 03/02/21 at 05:17pm, Mike Rapoport wrote:
> On Tue, Mar 02, 2021 at 09:04:09PM +0800, Baoquan He wrote:
...
> > > +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
> > > +	 * 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.
> 
> 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 :)

I see, thanks for explanation.

[PATCH v3 2/2] x86/setup: merge several reservations of start of the memory

[Mike Rapoport]

From: Mike Rapoport <rppt@linux.ibm.com>

Currently the first several pages are reserved both to avoid leaking their
contents on systems with L1TF and to avoid corrupting BIOS memory.

Merge the two memory reservations.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kernel/setup.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3e3c6036b023..776fc9b3fafe 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -713,11 +713,6 @@ static int __init parse_reservelow(char *p)
 
 early_param("reservelow", parse_reservelow);
 
-static void __init trim_low_memory_range(void)
-{
-	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
-}
-
 static void __init early_reserve_memory(void)
 {
 	/*
@@ -730,10 +725,17 @@ static void __init early_reserve_memory(void)
 			 (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.
+	 * The first 4Kb of memory is a BIOS owned area, but generally it is
+	 * not listed as such in the E820 table.
+	 *
+	 * Reserve the first memory page and typically some additional
+	 * memory (64KiB by default) since some BIOSes are known to corrupt
+	 * low memory. See the Kconfig help text for X86_RESERVE_LOW.
+	 *
+	 * In addition, 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);
+	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
 
 	early_reserve_initrd();
 
@@ -746,7 +748,6 @@ static void __init early_reserve_memory(void)
 	reserve_bios_regions();
 
 	trim_snb_memory();
-	trim_low_memory_range();
 }
 
 /*
-- 
2.28.0

[tip: x86/boot] x86/setup: Merge several reservations of start of memory

[tip-bot2 for Mike Rapoport]

The following commit has been merged into the x86/boot branch of tip:

Commit-ID:     4c674481dcf9974834b96622fa4b079c176f36f9
Gitweb:        https://git.kernel.org/tip/4c674481dcf9974834b96622fa4b079c176f36f9
Author:        Mike Rapoport <rppt@linux.ibm.com>
AuthorDate:    Tue, 02 Mar 2021 12:04:06 +02:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 23 Mar 2021 17:17:36 +01:00

x86/setup: Merge several reservations of start of memory

Currently, the first several pages are reserved both to avoid leaking
their contents on systems with L1TF and to avoid corrupting BIOS memory.

Merge the two memory reservations.

Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20210302100406.22059-3-rppt@kernel.org
---
 arch/x86/kernel/setup.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3e3c603..776fc9b 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -713,11 +713,6 @@ static int __init parse_reservelow(char *p)
 
 early_param("reservelow", parse_reservelow);
 
-static void __init trim_low_memory_range(void)
-{
-	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
-}
-
 static void __init early_reserve_memory(void)
 {
 	/*
@@ -730,10 +725,17 @@ static void __init early_reserve_memory(void)
 			 (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.
+	 * The first 4Kb of memory is a BIOS owned area, but generally it is
+	 * not listed as such in the E820 table.
+	 *
+	 * Reserve the first memory page and typically some additional
+	 * memory (64KiB by default) since some BIOSes are known to corrupt
+	 * low memory. See the Kconfig help text for X86_RESERVE_LOW.
+	 *
+	 * In addition, 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);
+	memblock_reserve(0, ALIGN(reserve_low, PAGE_SIZE));
 
 	early_reserve_initrd();
 
@@ -746,7 +748,6 @@ static void __init early_reserve_memory(void)
 	reserve_bios_regions();
 
 	trim_snb_memory();
-	trim_low_memory_range();
 }
 
 /*

Re: [PATCH v3 0/2] x86/setup: consolidate early memory reservations

[Mike Rapoport]

Any comments on these?

On Tue, Mar 02, 2021 at 12:04:04PM +0200, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
> 
> Hi,
> 
> David noticed that we do some of memblock_reserve() calls after allocations
> are possible:
> 
> https://lore.kernel.org/lkml/6ba6bde3-1520-5cd0-f987-32d543f0b79f@redhat.com
> 
> 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/20210128105711.10428-1-rppt@kernel.org
> * 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/20210115083255.12744-1-rppt@kernel.org
> 
> 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(-)
> 
> -- 
> 2.28.0
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v3 0/2] x86/setup: consolidate early memory reservations

[David Hildenbrand]

On 10.03.21 18:52, Mike Rapoport wrote:
> Any comments on these?
> 

Take my

Acked-by: David Hildenbrand <david@redhat.com>

it looks sane to me ... but not sure what it's worth in that x86-64 code :)

-- 
Thanks,

David / dhildenb

Re: [PATCH v3 0/2] x86/setup: consolidate early memory reservations

[Mike Rapoport]

Another gentle ping.

On Wed, Mar 10, 2021 at 07:52:13PM +0200, Mike Rapoport wrote:
> Any comments on these?
> 
> On Tue, Mar 02, 2021 at 12:04:04PM +0200, Mike Rapoport wrote:
> > From: Mike Rapoport <rppt@linux.ibm.com>
> > 
> > Hi,
> > 
> > David noticed that we do some of memblock_reserve() calls after allocations
> > are possible:
> > 
> > https://lore.kernel.org/lkml/6ba6bde3-1520-5cd0-f987-32d543f0b79f@redhat.com
> > 
> > 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/20210128105711.10428-1-rppt@kernel.org
> > * 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/20210115083255.12744-1-rppt@kernel.org
> > 
> > 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(-)
> > 
> > -- 
> > 2.28.0
> > 
> 
> -- 
> Sincerely yours,
> Mike.

-- 
Sincerely yours,
Mike.