[PATCH] i386: Do not restore reserved memory after hibernation

[PATCH] i386: Do not restore reserved memory after hibernation

[Rafael J. Wysocki]

Hi,

This patch is designed to fix http://bugzilla.kernel.org/show_bug.cgi?id=7995

Unfortunately, the reporter is unresponsive, so it hasn't been explicitly
confirmed to fix the issue, but it's based on some previous patches that
worked and definitely does what it's meant to do. :-)

Greetings,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>

On some systems the ACPI NVS area is located in the first 1 MB of RAM and
it is overwritten by the i386 code during the restore after hibernation.
This confuses the ACPI platform firmware that doesn't update the AC adapter
status appropriately as a result
(http://bugzilla.kernel.org/show_bug.cgi?id=7995).

The solution is to register the reserved memory in the first 1 MB as 'nosave',
so that swsusp doesn't touch it during the restore.  Also, this has been done
on x86_64 for a long time now, so the patch makes the i386 restore behaviour
equivalent to the one of x86_64.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/i386/kernel/e820.c  |   30 ++++++++++++++++++++++++++++++
 arch/i386/kernel/setup.c |    1 +
 include/asm-i386/e820.h  |    1 +
 3 files changed, 32 insertions(+)

Index: linux-2.6.22-rc3/arch/i386/kernel/e820.c
===================================================================
--- linux-2.6.22-rc3.orig/arch/i386/kernel/e820.c
+++ linux-2.6.22-rc3/arch/i386/kernel/e820.c
@@ -10,6 +10,7 @@
 #include <linux/efi.h>
 #include <linux/pfn.h>
 #include <linux/uaccess.h>
+#include <linux/suspend.h>
 
 #include <asm/pgtable.h>
 #include <asm/page.h>
@@ -320,6 +321,35 @@ static int __init request_standard_resou
 
 subsys_initcall(request_standard_resources);
 
+/**
+ * e820_mark_nosave_regions - Find the ranges of physical addresses that do not
+ * correspond to e820 RAM areas and mark the corresponding pages as nosave for
+ * hibernation.
+ *
+ * This function requires the e820 map to be sorted and without any
+ * overlapping entries and assumes the first e820 area to be RAM.
+ */
+void __init e820_mark_nosave_regions(void)
+{
+	int i;
+	unsigned long pfn;
+
+	pfn = PFN_DOWN(e820.map[0].addr + e820.map[0].size);
+	for (i = 1; i < e820.nr_map; i++) {
+		struct e820entry *ei = &e820.map[i];
+
+		if (pfn < PFN_UP(ei->addr))
+			register_nosave_region(pfn, PFN_UP(ei->addr));
+
+		pfn = PFN_DOWN(ei->addr + ei->size);
+		if (ei->type != E820_RAM)
+			register_nosave_region(PFN_UP(ei->addr), pfn);
+
+		if (pfn >= max_low_pfn)
+			break;
+	}
+}
+
 void __init add_memory_region(unsigned long long start,
 			      unsigned long long size, int type)
 {
Index: linux-2.6.22-rc3/arch/i386/kernel/setup.c
===================================================================
--- linux-2.6.22-rc3.orig/arch/i386/kernel/setup.c
+++ linux-2.6.22-rc3/arch/i386/kernel/setup.c
@@ -648,6 +648,7 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 	e820_register_memory();
+	e820_mark_nosave_regions();
 
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
Index: linux-2.6.22-rc3/include/asm-i386/e820.h
===================================================================
--- linux-2.6.22-rc3.orig/include/asm-i386/e820.h
+++ linux-2.6.22-rc3/include/asm-i386/e820.h
@@ -44,6 +44,7 @@ extern void register_bootmem_low_pages(u
 extern void e820_register_memory(void);
 extern void limit_regions(unsigned long long size);
 extern void print_memory_map(char *who);
+extern void e820_mark_nosave_regions(void);
 
 #endif/*!__ASSEMBLY__*/
 

Re: [PATCH] i386: Do not restore reserved memory after hibernation

[Pavel Machek]

Hi!

> This patch is designed to fix http://bugzilla.kernel.org/show_bug.cgi?id=7995
> 
> Unfortunately, the reporter is unresponsive, so it hasn't been explicitly
> confirmed to fix the issue, but it's based on some previous patches that
> worked and definitely does what it's meant to do. :-)

Thanks!

> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack, but it needs to be in -mm for a while.

/me feels tricky area.
						Pavel

>  arch/i386/kernel/e820.c  |   30 ++++++++++++++++++++++++++++++
>  arch/i386/kernel/setup.c |    1 +
>  include/asm-i386/e820.h  |    1 +
>  3 files changed, 32 insertions(+)
> 
> Index: linux-2.6.22-rc3/arch/i386/kernel/e820.c
> ===================================================================
> --- linux-2.6.22-rc3.orig/arch/i386/kernel/e820.c
> +++ linux-2.6.22-rc3/arch/i386/kernel/e820.c
> @@ -10,6 +10,7 @@
>  #include <linux/efi.h>
>  #include <linux/pfn.h>
>  #include <linux/uaccess.h>
> +#include <linux/suspend.h>
>  
>  #include <asm/pgtable.h>
>  #include <asm/page.h>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] i386: Do not restore reserved memory after hibernation

[Andrey Borzenkov]

[-- Attachment #1.1: Type: text/plain, Size: 4219 bytes --]

On Friday 01 June 2007, Rafael J. Wysocki wrote:
> Hi,
>
> This patch is designed to fix
> http://bugzilla.kernel.org/show_bug.cgi?id=7995
>
> Unfortunately, the reporter is unresponsive, 

well, we have our life too ...

I confirm that this patch works. Unfortunately 2.6.22-rc here tends to hang on 
resume from STD often enough to be annoying so I tend to avoid it for daily 
use. It just sits there with cursor in upper left corner, no output on 
screen, nothing. I believe to have seen discussion about USB deadlock during 
resume but cannot find it anymore. The problem is sporadic enough to render 
bisect useless.

-andrey

> so it hasn't been explicitly 
> confirmed to fix the issue, but it's based on some previous patches that
> worked and definitely does what it's meant to do. :-)
>
> Greetings,
> Rafael
>
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> On some systems the ACPI NVS area is located in the first 1 MB of RAM and
> it is overwritten by the i386 code during the restore after hibernation.
> This confuses the ACPI platform firmware that doesn't update the AC adapter
> status appropriately as a result
> (http://bugzilla.kernel.org/show_bug.cgi?id=7995).
>
> The solution is to register the reserved memory in the first 1 MB as
> 'nosave', so that swsusp doesn't touch it during the restore.  Also, this
> has been done on x86_64 for a long time now, so the patch makes the i386
> restore behaviour equivalent to the one of x86_64.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/i386/kernel/e820.c  |   30 ++++++++++++++++++++++++++++++
>  arch/i386/kernel/setup.c |    1 +
>  include/asm-i386/e820.h  |    1 +
>  3 files changed, 32 insertions(+)
>
> Index: linux-2.6.22-rc3/arch/i386/kernel/e820.c
> ===================================================================
> --- linux-2.6.22-rc3.orig/arch/i386/kernel/e820.c
> +++ linux-2.6.22-rc3/arch/i386/kernel/e820.c
> @@ -10,6 +10,7 @@
>  #include <linux/efi.h>
>  #include <linux/pfn.h>
>  #include <linux/uaccess.h>
> +#include <linux/suspend.h>
>
>  #include <asm/pgtable.h>
>  #include <asm/page.h>
> @@ -320,6 +321,35 @@ static int __init request_standard_resou
>
>  subsys_initcall(request_standard_resources);
>
> +/**
> + * e820_mark_nosave_regions - Find the ranges of physical addresses that
> do not + * correspond to e820 RAM areas and mark the corresponding pages as
> nosave for + * hibernation.
> + *
> + * This function requires the e820 map to be sorted and without any
> + * overlapping entries and assumes the first e820 area to be RAM.
> + */
> +void __init e820_mark_nosave_regions(void)
> +{
> +	int i;
> +	unsigned long pfn;
> +
> +	pfn = PFN_DOWN(e820.map[0].addr + e820.map[0].size);
> +	for (i = 1; i < e820.nr_map; i++) {
> +		struct e820entry *ei = &e820.map[i];
> +
> +		if (pfn < PFN_UP(ei->addr))
> +			register_nosave_region(pfn, PFN_UP(ei->addr));
> +
> +		pfn = PFN_DOWN(ei->addr + ei->size);
> +		if (ei->type != E820_RAM)
> +			register_nosave_region(PFN_UP(ei->addr), pfn);
> +
> +		if (pfn >= max_low_pfn)
> +			break;
> +	}
> +}
> +
>  void __init add_memory_region(unsigned long long start,
>  			      unsigned long long size, int type)
>  {
> Index: linux-2.6.22-rc3/arch/i386/kernel/setup.c
> ===================================================================
> --- linux-2.6.22-rc3.orig/arch/i386/kernel/setup.c
> +++ linux-2.6.22-rc3/arch/i386/kernel/setup.c
> @@ -648,6 +648,7 @@ void __init setup_arch(char **cmdline_p)
>  #endif
>
>  	e820_register_memory();
> +	e820_mark_nosave_regions();
>
>  #ifdef CONFIG_VT
>  #if defined(CONFIG_VGA_CONSOLE)
> Index: linux-2.6.22-rc3/include/asm-i386/e820.h
> ===================================================================
> --- linux-2.6.22-rc3.orig/include/asm-i386/e820.h
> +++ linux-2.6.22-rc3/include/asm-i386/e820.h
> @@ -44,6 +44,7 @@ extern void register_bootmem_low_pages(u
>  extern void e820_register_memory(void);
>  extern void limit_regions(unsigned long long size);
>  extern void print_memory_map(char *who);
> +extern void e820_mark_nosave_regions(void);
>
>  #endif/*!__ASSEMBLY__*/



[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] i386: Do not restore reserved memory after hibernation

[Nigel Cunningham]

[-- Attachment #1.1: Type: text/plain, Size: 991 bytes --]

Hi.

On Sat, 2007-06-02 at 08:50 +0400, Andrey Borzenkov wrote:
> On Friday 01 June 2007, Rafael J. Wysocki wrote:
> > Hi,
> >
> > This patch is designed to fix
> > http://bugzilla.kernel.org/show_bug.cgi?id=7995
> >
> > Unfortunately, the reporter is unresponsive, 
> 
> well, we have our life too ...
> 
> I confirm that this patch works. Unfortunately 2.6.22-rc here tends to hang on 
> resume from STD often enough to be annoying so I tend to avoid it for daily 
> use. It just sits there with cursor in upper left corner, no output on 
> screen, nothing. I believe to have seen discussion about USB deadlock during 
> resume but cannot find it anymore. The problem is sporadic enough to render 
> bisect useless.
> 
> -andrey

I believe the thread you're thinking of had the subject line:

regression 2.6.22-rc2 suspend to ram broken ( usb_hcd_pci_suspend
+0x0/0x170() returns -16 )

The associated patch was merged prior to 2.6.22-rc3.

Regards,

Nigel

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] i386: Do not restore reserved memory after hibernation

[Rafael J. Wysocki]

On Saturday, 2 June 2007 06:50, Andrey Borzenkov wrote:
> On Friday 01 June 2007, Rafael J. Wysocki wrote:
> > Hi,
> >
> > This patch is designed to fix
> > http://bugzilla.kernel.org/show_bug.cgi?id=7995
> >
> > Unfortunately, the reporter is unresponsive, 
> 
> well, we have our life too ...
> 
> I confirm that this patch works.

OK, thanks for the confirmation.

> Unfortunately 2.6.22-rc here tends to hang on resume from STD often enough
> use. It just sits there with cursor in upper left corner, no output on 
> screen, nothing. I believe to have seen discussion about USB deadlock during 
> resume but cannot find it anymore. The problem is sporadic enough to render 
> bisect useless.

I think we had a bug report related to it yesterday, from a system where it
happened 100% of the time.  Hopefully, we'll be able to nail it down.

As usual, it's not reproducible on any of my test boxes ...

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth