[RFC PATCH 0/3] x86, boot: Low memory reservation fixes

[RFC PATCH 0/3] x86, boot: Low memory reservation fixes

[H. Peter Anvin]

From: "H. Peter Anvin" <hpa@linux.intel.com>

On all but the tiniest of modern systems, losing half a megabyte of
RAM is a small price to pay for system stability.  Thus, switch the
default for low memory reservation to 640K.  This has been possible,
but has not been the default, since v3.9, specifically checkin:

  95c9608478d6 x86, mm: Move reserving low memory later in initialization

The above checkin exposes a problem with our reservation ordering;
specifically setup_bios_corruption_check() is called too early, so fix
that.  This caused the BIOS corruption check to run on memory that
would have been reserved anyway.

Finally, change the low memory check scanner to scan 640K rather than
64K by default.  The low memory check scanner will be a noop in the
common case, as it now will only grab memory that is not otherwise
reserved; this makes it purely a diagnostic tool.

H. Peter Anvin (3):
      x86, boot: Move setup_bios_corruption_check() later
      x86, boot: Change the default for X86_RESERVE_LOW to 640K, make EXPERT
      x86, boot: Change the BIOS corruption checker to scan 640K

[RFC PATCH 1/3] x86, boot: Move setup_bios_corruption_check() later

[H. Peter Anvin]

From: "H. Peter Anvin" <hpa@linux.intel.com>

setup_bios_corruption_check() tries to claim any low memory regions
that we haven't already reserved.  Make sure we reserve any low memory
regions that we intend to either use or know/assume a priori is bad
*before* we initialize the checker.

Stable team note: adding this to the list to consider for stable
because it generates user-visible nuisance dmesg warnings in some
configurations.  However, it is not urgent and should be allowed to
percolate in Linus' tree for a bit first.

Reported-by: Olof Johansson <olof@lixom.net>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
Link: http://lkml.kernel.org/r/528168CB.7070602@linux.intel.com
Cc: <stable@vger.kernel.org> v3.9+ [not urgent]
---
 arch/x86/kernel/setup.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..8c01d6e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1080,10 +1080,6 @@ void __init setup_arch(char **cmdline_p)
 	/* preallocate 4k for mptable mpc */
 	early_reserve_e820_mpc_new();
 
-#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
-	setup_bios_corruption_check();
-#endif
-
 #ifdef CONFIG_X86_32
 	printk(KERN_DEBUG "initial memory mapped: [mem 0x00000000-%#010lx]\n",
 			(max_pfn_mapped<<PAGE_SHIFT) - 1);
@@ -1100,6 +1096,14 @@ void __init setup_arch(char **cmdline_p)
 
 	setup_real_mode();
 
+	/*
+	 * At this point we should have reserved any low memory regions
+	 * we need to reserve.
+	 */
+#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
+	setup_bios_corruption_check();
+#endif
+
 	memblock_set_current_limit(get_max_mapped());
 	dma_contiguous_reserve(0);
 
-- 
1.8.3.1

[RFC PATCH 2/3] x86, boot: Change the default for X86_RESERVE_LOW to 640K, make EXPERT

[H. Peter Anvin]

From: "H. Peter Anvin" <hpa@linux.intel.com>

We have been able to set X86_RESERVE_LOW to 640K since checkin

95c9608478d6 x86, mm: Move reserving low memory later in initialization

Actually do so by default, and make overriding the default an EXPERT
option.  It can still be overridden on the command line if someone is
so inclined.

Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
Cc: Olof Johansson <olof@lixom.net>
Link: http://lkml.kernel.org/r/528168CB.7070602@linux.intel.com
---
 arch/x86/Kconfig | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ee2fb9d..7631122 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1412,33 +1412,38 @@ config X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK
 	  on or off.
 
 config X86_RESERVE_LOW
-	int "Amount of low memory, in kilobytes, to reserve for the BIOS"
-	default 64
+	int "Amount of low memory, in kilobytes, to reserve for the BIOS" if EXPERT
+	default 640
 	range 4 640
 	---help---
 	  Specify the amount of low memory to reserve for the BIOS.
 
-	  The first page contains BIOS data structures that the kernel
-	  must not use, so that page must always be reserved.
+	  The first page (4K) contains BIOS data structures that the
+	  kernel must not use, so that page must always be reserved.
 
-	  By default we reserve the first 64K of physical RAM, as a
-	  number of BIOSes are known to corrupt that memory range
+	  By default we reserve all of the first 640K of physical RAM,
+	  as a number of BIOSes are known to corrupt that memory range
 	  during events such as suspend/resume or monitor cable
 	  insertion, so it must not be used by the kernel.
 
 	  You can set this to 4 if you are absolutely sure that you
 	  trust the BIOS to get all its memory reservations and usages
-	  right.  If you know your BIOS have problems beyond the
-	  default 64K area, you can set this to 640 to avoid using the
-	  entire low memory range.
-
-	  If you have doubts about the BIOS (e.g. suspend/resume does
-	  not work or there's kernel crashes after certain hardware
-	  hotplug events) then you might want to enable
-	  X86_CHECK_BIOS_CORRUPTION=y to allow the kernel to check
-	  typical corruption patterns.
-
-	  Leave this to the default value of 64 if you are unsure.
+	  right.  More BIOSes are known to have problems with the low
+	  64K than with the rest of the range, so if you wish to
+	  compromise between safety and memory consumption you can set
+	  this to 64.
+
+	  If you are planning to use low memory but have doubts about
+	  the BIOS (e.g. suspend/resume does not work or there's
+	  kernel crashes after certain hardware hotplug events) then
+	  you might want to enable X86_CHECK_BIOS_CORRUPTION=y to
+	  allow the kernel to check typical corruption patterns before
+	  disabling the checker in order to actually use the memory.
+
+	  This option can be overridden at runtime with the
+	  reservelow= kernel command line option.
+
+	  Leave this to the default value of 640 if you are unsure.
 
 config MATH_EMULATION
 	bool
-- 
1.8.3.1

[RFC PATCH 3/3] x86, boot: Change the BIOS corruption checker to scan 640K

[H. Peter Anvin]

From: "H. Peter Anvin" <hpa@linux.intel.com>

Change the BIOS corruption checker to scan 640K if enabled.  This is
the normal amount that we otherwise would reserve with the new default
settings; change the Kconfig help message to indicate that this is now
intended as a diagnostic tool when one is considering enabling any
chunk of low memory.

Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
Cc: Olof Johansson <olof@lixom.net>
Link: http://lkml.kernel.org/r/528168CB.7070602@linux.intel.com
---
 arch/x86/Kconfig        | 25 +++++++++++--------------
 arch/x86/kernel/check.c | 10 +++++-----
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7631122..554aedd 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1384,24 +1384,21 @@ config HIGHPTE
 config X86_CHECK_BIOS_CORRUPTION
 	bool "Check for low memory corruption"
 	---help---
-	  Periodically check for memory corruption in low memory, which
-	  is suspected to be caused by BIOS.  Even when enabled in the
-	  configuration, it is disabled at runtime.  Enable it by
-	  setting "memory_corruption_check=1" on the kernel command
-	  line.  By default it scans the low 64k of memory every 60
-	  seconds; see the memory_corruption_check_size and
+	  Periodically check for memory corruption in low memory,
+	  which is suspected to be caused by BIOS.  Even when enabled
+	  in the configuration, it is disabled at runtime.  Enable it
+	  by setting "memory_corruption_check=1" on the kernel command
+	  line.  By default it reserves and scans the low 640K of
+	  memory every 60 seconds; see the
+	  memory_corruption_check_size and
 	  memory_corruption_check_period parameters in
 	  Documentation/kernel-parameters.txt to adjust this.
 
-	  When enabled with the default parameters, this option has
-	  almost no overhead, as it reserves a relatively small amount
-	  of memory and scans it infrequently.  It both detects corruption
-	  and prevents it from affecting the running system.
-
-	  It is, however, intended as a diagnostic tool; if repeatable
+	  It is intended as a diagnostic tool; if repeatable
 	  BIOS-originated corruption always affects the same memory,
-	  you can use memmap= to prevent the kernel from using that
-	  memory.
+	  you should use the memmap= or reservelow= runtime options or
+	  the CONFIG_X86_RESERVE_LOW compile time option to prevent
+	  the kernel from using that memory.
 
 config X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK
 	bool "Set the default setting of memory_corruption_check"
diff --git a/arch/x86/kernel/check.c b/arch/x86/kernel/check.c
index e2dbcb7..c00182d 100644
--- a/arch/x86/kernel/check.c
+++ b/arch/x86/kernel/check.c
@@ -7,16 +7,16 @@
 #include <asm/proto.h>
 
 /*
- * Some BIOSes seem to corrupt the low 64k of memory during events
- * like suspend/resume and unplugging an HDMI cable.  Reserve all
- * remaining free memory in that area and fill it with a distinct
- * pattern.
+ * Some BIOSes and even some hardware devices seem to corrupt the low
+ * 640K of memory during events like suspend/resume and unplugging an
+ * HDMI cable.  Reserve all remaining free memory in that area and
+ * fill it with a distinct pattern.
  */
 #define MAX_SCAN_AREAS	8
 
 static int __read_mostly memory_corruption_check = -1;
 
-static unsigned __read_mostly corruption_check_size = 64*1024;
+static unsigned __read_mostly corruption_check_size = 640*1024;
 static unsigned __read_mostly corruption_check_period = 60; /* seconds */
 
 static struct scan_area {
-- 
1.8.3.1

Re: [RFC PATCH 3/3] x86, boot: Change the BIOS corruption checker to scan 640K

[Ingo Molnar]

* H. Peter Anvin <hpa@linux.intel.com> wrote:

> From: "H. Peter Anvin" <hpa@linux.intel.com>
> 
> Change the BIOS corruption checker to scan 640K if enabled.  This is
> the normal amount that we otherwise would reserve with the new default
> settings; change the Kconfig help message to indicate that this is now
> intended as a diagnostic tool when one is considering enabling any
> chunk of low memory.
> 
> Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
> Cc: Olof Johansson <olof@lixom.net>
> Link: http://lkml.kernel.org/r/528168CB.7070602@linux.intel.com
> ---
>  arch/x86/Kconfig        | 25 +++++++++++--------------
>  arch/x86/kernel/check.c | 10 +++++-----
>  2 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7631122..554aedd 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1384,24 +1384,21 @@ config HIGHPTE
>  config X86_CHECK_BIOS_CORRUPTION
>  	bool "Check for low memory corruption"
>  	---help---
> +	  Periodically check for memory corruption in low memory,
> +	  which is suspected to be caused by BIOS.  Even when enabled
> +	  in the configuration, it is disabled at runtime.  Enable it
> +	  by setting "memory_corruption_check=1" on the kernel command
> +	  line.  By default it reserves and scans the low 640K of
> +	  memory every 60 seconds; see the
> +	  memory_corruption_check_size and
>  	  memory_corruption_check_period parameters in
>  	  Documentation/kernel-parameters.txt to adjust this.

I agree with your patches so far, and I'd suggest we go even further: I'd 
say the config option is now a misnomer, it should probably be renamed to 
CONFIG_X86_FORCE_RESERVE_BIOS_LOW_1MB=y or so.

Btw., should we also force-reserve the remaining bits over 640K..1MB, if 
they are not marked as reserved in the memory maps, or do we already 
force-reserve them somewhere?

The CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK=y option and the 
memory_corruption_check=1 boot option then allow the activation of the low 
memory corrupion checker - which debug facility can be used on systems 
where someone wants to live dangerously and not reserve the low 1MB of RAM 
to the firmware.

Thanks,

	Ingo

Re: [RFC PATCH 3/3] x86, boot: Change the BIOS corruption checker to scan 640K

[H. Peter Anvin]

On 11/11/2013 08:07 PM, Ingo Molnar wrote:
> I agree with your patches so far, and I'd suggest we go even further: I'd 
> say the config option is now a misnomer, it should probably be renamed to 
> CONFIG_X86_FORCE_RESERVE_BIOS_LOW_1MB=y or so.

Why is that?  It doesn't seem to make much sense to me.  I think the
current option names seem to be just fine, but perhaps I'm missing
something.

> Btw., should we also force-reserve the remaining bits over 640K..1MB, if 
> they are not marked as reserved in the memory maps, or do we already 
> force-reserve them somewhere?

We do, in trim_bios_range().  We treat it as available for I/O
assignments, since that is necessary on some systems.

> The CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK=y option and the 
> memory_corruption_check=1 boot option then allow the activation of the low 
> memory corrupion checker - which debug facility can be used on systems 
> where someone wants to live dangerously and not reserve the low 1MB of RAM 
> to the firmware.

That is indeed what this patch does, I think...

	-hpa

Re: [RFC PATCH 3/3] x86, boot: Change the BIOS corruption checker to scan 640K

[Ingo Molnar]

* H. Peter Anvin <hpa@linux.intel.com> wrote:

> On 11/11/2013 08:07 PM, Ingo Molnar wrote:
>
> > I agree with your patches so far, and I'd suggest we go even further: 
> > I'd say the config option is now a misnomer, it should probably be 
> > renamed to CONFIG_X86_FORCE_RESERVE_BIOS_LOW_1MB=y or so.
> 
> Why is that?  It doesn't seem to make much sense to me.  I think the 
> current option names seem to be just fine, but perhaps I'm missing 
> something.

My argument is that CONFIG_X86_CHECK_BIOS_CORRUPTION=y doesn't actually 
_check_ any 'BIOS corruption'. Its largest operative effect to the vast 
majority of users is to reserve the whole 1MB lower range to the BIOS and 
it performs no checking whatsoever.

That used to be different btw., it used to be on by default, that's why I 
named the option in such a way original. That original role has been 
almost completely eliminated though.

The _real_ option that turns on memory corruption checking is 
CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK=y. (The 
memory_corruption_check=1 option is something very few users and 
essentially no distro will use - distros will use the .config option.)

> > Btw., should we also force-reserve the remaining bits over 640K..1MB, 
> > if they are not marked as reserved in the memory maps, or do we 
> > already force-reserve them somewhere?
> 
> We do, in trim_bios_range().  We treat it as available for I/O 
> assignments, since that is necessary on some systems.

Ok, good.

> > The CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK=y option and the 
> > memory_corruption_check=1 boot option then allow the activation of the 
> > low memory corrupion checker - which debug facility can be used on 
> > systems where someone wants to live dangerously and not reserve the 
> > low 1MB of RAM to the firmware.
> 
> That is indeed what this patch does, I think...

Yes, and that's my argument for simply renaming the option, no other 
change is needed IMO.

Thanks,

	Ingo