All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
@ 2010-05-28 20:32 Matthew Garrett
  2010-05-28 20:32 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Matthew Garrett
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Matthew Garrett @ 2010-05-28 20:32 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-pm, Matthew Garrett

Saving platform non-volatile state may be required for suspend to RAM as
well as hibernation. Move it to more generic code.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 arch/x86/kernel/e820.c       |    2 +-
 drivers/acpi/sleep.c         |   12 ++--
 include/linux/suspend.h      |   26 ++++----
 kernel/power/Kconfig         |    9 ++-
 kernel/power/Makefile        |    2 +-
 kernel/power/hibernate_nvs.c |  136 ------------------------------------------
 kernel/power/nvs.c           |  136 ++++++++++++++++++++++++++++++++++++++++++
 kernel/power/suspend.c       |    6 ++
 8 files changed, 168 insertions(+), 161 deletions(-)
 delete mode 100644 kernel/power/hibernate_nvs.c
 create mode 100644 kernel/power/nvs.c

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index 7bca3c6..0d6fc71 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -729,7 +729,7 @@ static int __init e820_mark_nvs_memory(void)
 		struct e820entry *ei = &e820.map[i];
 
 		if (ei->type == E820_NVS)
-			hibernate_nvs_register(ei->addr, ei->size);
+			suspend_nvs_register(ei->addr, ei->size);
 	}
 
 	return 0;
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index baa76bb..d3cbc55 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -546,7 +546,7 @@ static int acpi_hibernation_begin(void)
 {
 	int error;
 
-	error = s4_no_nvs ? 0 : hibernate_nvs_alloc();
+	error = s4_no_nvs ? 0 : suspend_nvs_alloc();
 	if (!error) {
 		acpi_target_sleep_state = ACPI_STATE_S4;
 		acpi_sleep_tts_switch(acpi_target_sleep_state);
@@ -560,7 +560,7 @@ static int acpi_hibernation_pre_snapshot(void)
 	int error = acpi_pm_prepare();
 
 	if (!error)
-		hibernate_nvs_save();
+		suspend_nvs_save();
 
 	return error;
 }
@@ -585,7 +585,7 @@ static int acpi_hibernation_enter(void)
 
 static void acpi_hibernation_finish(void)
 {
-	hibernate_nvs_free();
+	suspend_nvs_free();
 	acpi_pm_finish();
 }
 
@@ -605,7 +605,7 @@ static void acpi_hibernation_leave(void)
 		panic("ACPI S4 hardware signature mismatch");
 	}
 	/* Restore the NVS memory area */
-	hibernate_nvs_restore();
+	suspend_nvs_restore();
 }
 
 static int acpi_pm_pre_restore(void)
@@ -654,7 +654,7 @@ static int acpi_hibernation_begin_old(void)
 
 	if (!error) {
 		if (!s4_no_nvs)
-			error = hibernate_nvs_alloc();
+			error = suspend_nvs_alloc();
 		if (!error)
 			acpi_target_sleep_state = ACPI_STATE_S4;
 	}
@@ -666,7 +666,7 @@ static int acpi_hibernation_pre_snapshot_old(void)
 	int error = acpi_pm_disable_gpes();
 
 	if (!error)
-		hibernate_nvs_save();
+		suspend_nvs_save();
 
 	return error;
 }
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 5e781d8..bc7d6bb 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -256,22 +256,22 @@ static inline int hibernate(void) { return -ENOSYS; }
 static inline bool system_entering_hibernation(void) { return false; }
 #endif /* CONFIG_HIBERNATION */
 
-#ifdef CONFIG_HIBERNATION_NVS
-extern int hibernate_nvs_register(unsigned long start, unsigned long size);
-extern int hibernate_nvs_alloc(void);
-extern void hibernate_nvs_free(void);
-extern void hibernate_nvs_save(void);
-extern void hibernate_nvs_restore(void);
-#else /* CONFIG_HIBERNATION_NVS */
-static inline int hibernate_nvs_register(unsigned long a, unsigned long b)
+#ifdef CONFIG_SUSPEND_NVS
+extern int suspend_nvs_register(unsigned long start, unsigned long size);
+extern int suspend_nvs_alloc(void);
+extern void suspend_nvs_free(void);
+extern void suspend_nvs_save(void);
+extern void suspend_nvs_restore(void);
+#else /* CONFIG_SUSPEND_NVS */
+static inline int suspend_nvs_register(unsigned long a, unsigned long b)
 {
 	return 0;
 }
-static inline int hibernate_nvs_alloc(void) { return 0; }
-static inline void hibernate_nvs_free(void) {}
-static inline void hibernate_nvs_save(void) {}
-static inline void hibernate_nvs_restore(void) {}
-#endif /* CONFIG_HIBERNATION_NVS */
+static inline int suspend_nvs_alloc(void) { return 0; }
+static inline void suspend_nvs_free(void) {}
+static inline void suspend_nvs_save(void) {}
+static inline void suspend_nvs_restore(void) {}
+#endif /* CONFIG_SUSPEND_NVS */
 
 #ifdef CONFIG_PM_SLEEP
 void save_processor_state(void);
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 5c36ea9..ca6066a 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -99,9 +99,13 @@ config PM_SLEEP_ADVANCED_DEBUG
 	depends on PM_ADVANCED_DEBUG
 	default n
 
+config SUSPEND_NVS
+       bool
+
 config SUSPEND
 	bool "Suspend to RAM and standby"
 	depends on PM && ARCH_SUSPEND_POSSIBLE
+	select SUSPEND_NVS if HAS_IOMEM
 	default y
 	---help---
 	  Allow the system to enter sleep states in which main memory is
@@ -130,13 +134,10 @@ config SUSPEND_FREEZER
 
 	  Turning OFF this setting is NOT recommended! If in doubt, say Y.
 
-config HIBERNATION_NVS
-	bool
-
 config HIBERNATION
 	bool "Hibernation (aka 'suspend to disk')"
 	depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
-	select HIBERNATION_NVS if HAS_IOMEM
+	select SUSPEND_NVS if HAS_IOMEM
 	---help---
 	  Enable the suspend to disk (STD) functionality, which is usually
 	  called "hibernation" in user interfaces.  STD checkpoints the
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 524e058..f9063c6 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -10,6 +10,6 @@ obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PM_TEST_SUSPEND)	+= suspend_test.o
 obj-$(CONFIG_HIBERNATION)	+= hibernate.o snapshot.o swap.o user.o \
 				   block_io.o
-obj-$(CONFIG_HIBERNATION_NVS)	+= hibernate_nvs.o
+obj-$(CONFIG_SUSPEND_NVS)	+= nvs.o
 
 obj-$(CONFIG_MAGIC_SYSRQ)	+= poweroff.o
diff --git a/kernel/power/hibernate_nvs.c b/kernel/power/hibernate_nvs.c
deleted file mode 100644
index fdcad9e..0000000
--- a/kernel/power/hibernate_nvs.c
+++ /dev/null
@@ -1,136 +0,0 @@
-/*
- * linux/kernel/power/hibernate_nvs.c - Routines for handling NVS memory
- *
- * Copyright (C) 2008,2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
- *
- * This file is released under the GPLv2.
- */
-
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/list.h>
-#include <linux/mm.h>
-#include <linux/slab.h>
-#include <linux/suspend.h>
-
-/*
- * Platforms, like ACPI, may want us to save some memory used by them during
- * hibernation and to restore the contents of this memory during the subsequent
- * resume.  The code below implements a mechanism allowing us to do that.
- */
-
-struct nvs_page {
-	unsigned long phys_start;
-	unsigned int size;
-	void *kaddr;
-	void *data;
-	struct list_head node;
-};
-
-static LIST_HEAD(nvs_list);
-
-/**
- *	hibernate_nvs_register - register platform NVS memory region to save
- *	@start - physical address of the region
- *	@size - size of the region
- *
- *	The NVS region need not be page-aligned (both ends) and we arrange
- *	things so that the data from page-aligned addresses in this region will
- *	be copied into separate RAM pages.
- */
-int hibernate_nvs_register(unsigned long start, unsigned long size)
-{
-	struct nvs_page *entry, *next;
-
-	while (size > 0) {
-		unsigned int nr_bytes;
-
-		entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
-		if (!entry)
-			goto Error;
-
-		list_add_tail(&entry->node, &nvs_list);
-		entry->phys_start = start;
-		nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
-		entry->size = (size < nr_bytes) ? size : nr_bytes;
-
-		start += entry->size;
-		size -= entry->size;
-	}
-	return 0;
-
- Error:
-	list_for_each_entry_safe(entry, next, &nvs_list, node) {
-		list_del(&entry->node);
-		kfree(entry);
-	}
-	return -ENOMEM;
-}
-
-/**
- *	hibernate_nvs_free - free data pages allocated for saving NVS regions
- */
-void hibernate_nvs_free(void)
-{
-	struct nvs_page *entry;
-
-	list_for_each_entry(entry, &nvs_list, node)
-		if (entry->data) {
-			free_page((unsigned long)entry->data);
-			entry->data = NULL;
-			if (entry->kaddr) {
-				iounmap(entry->kaddr);
-				entry->kaddr = NULL;
-			}
-		}
-}
-
-/**
- *	hibernate_nvs_alloc - allocate memory necessary for saving NVS regions
- */
-int hibernate_nvs_alloc(void)
-{
-	struct nvs_page *entry;
-
-	list_for_each_entry(entry, &nvs_list, node) {
-		entry->data = (void *)__get_free_page(GFP_KERNEL);
-		if (!entry->data) {
-			hibernate_nvs_free();
-			return -ENOMEM;
-		}
-	}
-	return 0;
-}
-
-/**
- *	hibernate_nvs_save - save NVS memory regions
- */
-void hibernate_nvs_save(void)
-{
-	struct nvs_page *entry;
-
-	printk(KERN_INFO "PM: Saving platform NVS memory\n");
-
-	list_for_each_entry(entry, &nvs_list, node)
-		if (entry->data) {
-			entry->kaddr = ioremap(entry->phys_start, entry->size);
-			memcpy(entry->data, entry->kaddr, entry->size);
-		}
-}
-
-/**
- *	hibernate_nvs_restore - restore NVS memory regions
- *
- *	This function is going to be called with interrupts disabled, so it
- *	cannot iounmap the virtual addresses used to access the NVS region.
- */
-void hibernate_nvs_restore(void)
-{
-	struct nvs_page *entry;
-
-	printk(KERN_INFO "PM: Restoring platform NVS memory\n");
-
-	list_for_each_entry(entry, &nvs_list, node)
-		if (entry->data)
-			memcpy(entry->kaddr, entry->data, entry->size);
-}
diff --git a/kernel/power/nvs.c b/kernel/power/nvs.c
new file mode 100644
index 0000000..1836db6
--- /dev/null
+++ b/kernel/power/nvs.c
@@ -0,0 +1,136 @@
+/*
+ * linux/kernel/power/hibernate_nvs.c - Routines for handling NVS memory
+ *
+ * Copyright (C) 2008,2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+
+/*
+ * Platforms, like ACPI, may want us to save some memory used by them during
+ * suspend and to restore the contents of this memory during the subsequent
+ * resume.  The code below implements a mechanism allowing us to do that.
+ */
+
+struct nvs_page {
+	unsigned long phys_start;
+	unsigned int size;
+	void *kaddr;
+	void *data;
+	struct list_head node;
+};
+
+static LIST_HEAD(nvs_list);
+
+/**
+ *	suspend_nvs_register - register platform NVS memory region to save
+ *	@start - physical address of the region
+ *	@size - size of the region
+ *
+ *	The NVS region need not be page-aligned (both ends) and we arrange
+ *	things so that the data from page-aligned addresses in this region will
+ *	be copied into separate RAM pages.
+ */
+int suspend_nvs_register(unsigned long start, unsigned long size)
+{
+	struct nvs_page *entry, *next;
+
+	while (size > 0) {
+		unsigned int nr_bytes;
+
+		entry = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
+		if (!entry)
+			goto Error;
+
+		list_add_tail(&entry->node, &nvs_list);
+		entry->phys_start = start;
+		nr_bytes = PAGE_SIZE - (start & ~PAGE_MASK);
+		entry->size = (size < nr_bytes) ? size : nr_bytes;
+
+		start += entry->size;
+		size -= entry->size;
+	}
+	return 0;
+
+ Error:
+	list_for_each_entry_safe(entry, next, &nvs_list, node) {
+		list_del(&entry->node);
+		kfree(entry);
+	}
+	return -ENOMEM;
+}
+
+/**
+ *	suspend_nvs_free - free data pages allocated for saving NVS regions
+ */
+void suspend_nvs_free(void)
+{
+	struct nvs_page *entry;
+
+	list_for_each_entry(entry, &nvs_list, node)
+		if (entry->data) {
+			free_page((unsigned long)entry->data);
+			entry->data = NULL;
+			if (entry->kaddr) {
+				iounmap(entry->kaddr);
+				entry->kaddr = NULL;
+			}
+		}
+}
+
+/**
+ *	suspend_nvs_alloc - allocate memory necessary for saving NVS regions
+ */
+int suspend_nvs_alloc(void)
+{
+	struct nvs_page *entry;
+
+	list_for_each_entry(entry, &nvs_list, node) {
+		entry->data = (void *)__get_free_page(GFP_KERNEL);
+		if (!entry->data) {
+			suspend_nvs_free();
+			return -ENOMEM;
+		}
+	}
+	return 0;
+}
+
+/**
+ *	suspend_nvs_save - save NVS memory regions
+ */
+void suspend_nvs_save(void)
+{
+	struct nvs_page *entry;
+
+	printk(KERN_INFO "PM: Saving platform NVS memory\n");
+
+	list_for_each_entry(entry, &nvs_list, node)
+		if (entry->data) {
+			entry->kaddr = ioremap(entry->phys_start, entry->size);
+			memcpy(entry->data, entry->kaddr, entry->size);
+		}
+}
+
+/**
+ *	suspend_nvs_restore - restore NVS memory regions
+ *
+ *	This function is going to be called with interrupts disabled, so it
+ *	cannot iounmap the virtual addresses used to access the NVS region.
+ */
+void suspend_nvs_restore(void)
+{
+	struct nvs_page *entry;
+
+	printk(KERN_INFO "PM: Restoring platform NVS memory\n");
+
+	list_for_each_entry(entry, &nvs_list, node)
+		if (entry->data)
+			memcpy(entry->kaddr, entry->data, entry->size);
+}
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 56e7dbb..f37cb7d 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -16,6 +16,12 @@
 #include <linux/cpu.h>
 #include <linux/syscalls.h>
 #include <linux/gfp.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
 
 #include "power.h"
 
-- 
1.7.0.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-05-28 20:32 [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Matthew Garrett
@ 2010-05-28 20:32 ` Matthew Garrett
  2010-05-30 14:40   ` Maxim Levitsky
                     ` (2 more replies)
  2010-05-30 14:37 ` [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Maxim Levitsky
  2010-06-04 18:22 ` Len Brown
  2 siblings, 3 replies; 21+ messages in thread
From: Matthew Garrett @ 2010-05-28 20:32 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, linux-pm, Matthew Garrett

https://bugzilla.kernel.org/show_bug.cgi?id=13931 describes a bug where
a system fails to successfully resume after the second suspend. Maxim
Levitsky discovered that this could be rectified by forcibly saving
and restoring the ACPI non-volatile state. The spec indicates that this
is only required for S4, but testing the behaviour of Windows by adding
an ACPI NVS region to qemu's e820 map and registering a custom memory
read/write handler reveals that it's saved and restored even over suspend
to RAM. We should mimic that behaviour to avoid other broken platforms.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/sleep.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index d3cbc55..18aa074 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -128,6 +128,8 @@ static int __acpi_pm_prepare(void)
 {
 	int error = acpi_sleep_prepare(acpi_target_sleep_state);
 
+	suspend_nvs_save();
+
 	if (error)
 		acpi_target_sleep_state = ACPI_STATE_S0;
 	return error;
@@ -156,6 +158,8 @@ static void acpi_pm_finish(void)
 {
 	u32 acpi_state = acpi_target_sleep_state;
 
+	suspend_nvs_free();
+
 	if (acpi_state == ACPI_STATE_S0)
 		return;
 
@@ -205,6 +209,11 @@ static int acpi_suspend_begin(suspend_state_t pm_state)
 	u32 acpi_state = acpi_suspend_states[pm_state];
 	int error = 0;
 
+	error = suspend_nvs_alloc();
+
+	if (error)
+		return error;
+
 	if (sleep_states[acpi_state]) {
 		acpi_target_sleep_state = acpi_state;
 		acpi_sleep_tts_switch(acpi_target_sleep_state);
@@ -283,6 +292,8 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
 	if (acpi_state == ACPI_STATE_S3)
 		acpi_restore_state_mem();
 
+	suspend_nvs_restore();
+
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
 
@@ -583,12 +594,6 @@ static int acpi_hibernation_enter(void)
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
 
-static void acpi_hibernation_finish(void)
-{
-	suspend_nvs_free();
-	acpi_pm_finish();
-}
-
 static void acpi_hibernation_leave(void)
 {
 	/*
@@ -626,7 +631,7 @@ static struct platform_hibernation_ops acpi_hibernation_ops = {
 	.begin = acpi_hibernation_begin,
 	.end = acpi_pm_end,
 	.pre_snapshot = acpi_hibernation_pre_snapshot,
-	.finish = acpi_hibernation_finish,
+	.finish = acpi_pm_finish,
 	.prepare = acpi_pm_prepare,
 	.enter = acpi_hibernation_enter,
 	.leave = acpi_hibernation_leave,
@@ -679,7 +684,7 @@ static struct platform_hibernation_ops acpi_hibernation_ops_old = {
 	.begin = acpi_hibernation_begin_old,
 	.end = acpi_pm_end,
 	.pre_snapshot = acpi_hibernation_pre_snapshot_old,
-	.finish = acpi_hibernation_finish,
+	.finish = acpi_pm_finish,
 	.prepare = acpi_pm_disable_gpes,
 	.enter = acpi_hibernation_enter,
 	.leave = acpi_hibernation_leave,
-- 
1.7.0.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-05-28 20:32 [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Matthew Garrett
  2010-05-28 20:32 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Matthew Garrett
@ 2010-05-30 14:37 ` Maxim Levitsky
  2010-06-04 19:11   ` Len Brown
  2010-06-04 18:22 ` Len Brown
  2 siblings, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2010-05-30 14:37 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: lenb, linux-acpi, linux-pm, Rafael J. Wysocki, Alan Stern

On Fri, 2010-05-28 at 16:32 -0400, Matthew Garrett wrote: 
> Saving platform non-volatile state may be required for suspend to RAM as
> well as hibernation. Move it to more generic code.


Since the other patch 'ACPI / EC / PM: Fix race between EC transactions
and system suspend' is now merged, this should be rebased on top of it
(I did the opposite....)

.... 
> diff --git a/kernel/power/nvs.c b/kernel/power/nvs.c
> new file mode 100644
> index 0000000..1836db6
> --- /dev/null
> +++ b/kernel/power/nvs.c
> @@ -0,0 +1,136 @@
> +/*
> + * linux/kernel/power/hibernate_nvs.c - Routines for handling NVS memory
This should be updated (I did in the version I posted) 
> + *
> + * Copyright (C) 2008,2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/suspend.h>
> +
............


Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-05-28 20:32 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Matthew Garrett
@ 2010-05-30 14:40   ` Maxim Levitsky
  2010-05-30 14:51     ` Matthew Garrett
  2010-06-04 18:23   ` Len Brown
  2010-06-10  9:41   ` Maxim Levitsky
  2 siblings, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2010-05-30 14:40 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: lenb, linux-acpi, linux-pm

On Fri, 2010-05-28 at 16:32 -0400, Matthew Garrett wrote: 
> https://bugzilla.kernel.org/show_bug.cgi?id=13931 describes a bug where
> a system fails to successfully resume after the second suspend. Maxim
> Levitsky discovered that this could be rectified by forcibly saving
> and restoring the ACPI non-volatile state. The spec indicates that this
> is only required for S4, but testing the behaviour of Windows by adding
> an ACPI NVS region to qemu's e820 map and registering a custom memory
> read/write handler reveals that it's saved and restored even over suspend
> to RAM. We should mimic that behaviour to avoid other broken platforms.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> ---
>  drivers/acpi/sleep.c |   21 +++++++++++++--------
>  1 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index d3cbc55..18aa074 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -128,6 +128,8 @@ static int __acpi_pm_prepare(void)
>  {
>  	int error = acpi_sleep_prepare(acpi_target_sleep_state);


Just one note, I am sure somebody will ask for s3_no_nvs option, just
like s4_no_nvs.

Also this needs to be rebased, like the first patch.

Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-05-30 14:40   ` Maxim Levitsky
@ 2010-05-30 14:51     ` Matthew Garrett
  2010-06-02  3:32       ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 21+ messages in thread
From: Matthew Garrett @ 2010-05-30 14:51 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: lenb, linux-acpi, linux-pm

On Sun, May 30, 2010 at 05:40:04PM +0300, Maxim Levitsky wrote:

> Just one note, I am sure somebody will ask for s3_no_nvs option, just
> like s4_no_nvs.

I'd be surprised. Better to add the option when we think it's needed.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-05-30 14:51     ` Matthew Garrett
@ 2010-06-02  3:32       ` Henrique de Moraes Holschuh
  2010-06-02 10:15         ` Maxim Levitsky
  2010-06-02 11:47         ` Matthew Garrett
  0 siblings, 2 replies; 21+ messages in thread
From: Henrique de Moraes Holschuh @ 2010-06-02  3:32 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Maxim Levitsky, lenb, linux-acpi, linux-pm

On Sun, 30 May 2010, Matthew Garrett wrote:
> On Sun, May 30, 2010 at 05:40:04PM +0300, Maxim Levitsky wrote:
> > Just one note, I am sure somebody will ask for s3_no_nvs option, just
> > like s4_no_nvs.
> 
> I'd be surprised. Better to add the option when we think it's needed.

Do _all_ Windows versions restore NVS over S3 suspend/resume?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-06-02  3:32       ` Henrique de Moraes Holschuh
@ 2010-06-02 10:15         ` Maxim Levitsky
  2010-06-02 11:47         ` Matthew Garrett
  1 sibling, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-02 10:15 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Matthew Garrett, lenb, linux-acpi, linux-pm

On Wed, 2010-06-02 at 00:32 -0300, Henrique de Moraes Holschuh wrote: 
> On Sun, 30 May 2010, Matthew Garrett wrote:
> > On Sun, May 30, 2010 at 05:40:04PM +0300, Maxim Levitsky wrote:
> > > Just one note, I am sure somebody will ask for s3_no_nvs option, just
> > > like s4_no_nvs.
> > 
> > I'd be surprised. Better to add the option when we think it's needed.
> 
> Do _all_ Windows versions restore NVS over S3 suspend/resume?

This is very good question, I test it in future (if Matthew Garrett
doesn't beat me to it...)

I have a copy of XP,Vista,Win7, all obtained for free through
Microsoft's  "first dose is always free" university program.


Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-06-02  3:32       ` Henrique de Moraes Holschuh
  2010-06-02 10:15         ` Maxim Levitsky
@ 2010-06-02 11:47         ` Matthew Garrett
  2010-06-02 13:06           ` Maxim Levitsky
  1 sibling, 1 reply; 21+ messages in thread
From: Matthew Garrett @ 2010-06-02 11:47 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Maxim Levitsky, lenb, linux-acpi, linux-pm

On Wed, Jun 02, 2010 at 12:32:25AM -0300, Henrique de Moraes Holschuh wrote:
> On Sun, 30 May 2010, Matthew Garrett wrote:
> > On Sun, May 30, 2010 at 05:40:04PM +0300, Maxim Levitsky wrote:
> > > Just one note, I am sure somebody will ask for s3_no_nvs option, just
> > > like s4_no_nvs.
> > 
> > I'd be surprised. Better to add the option when we think it's needed.
> 
> Do _all_ Windows versions restore NVS over S3 suspend/resume?

XP does regardless of whether any OSI strings are presented, and we have 
systems that break unless it's done, so I think it's a safe bet.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-06-02 11:47         ` Matthew Garrett
@ 2010-06-02 13:06           ` Maxim Levitsky
  0 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-02 13:06 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Henrique de Moraes Holschuh, lenb, linux-acpi, linux-pm

On Wed, 2010-06-02 at 12:47 +0100, Matthew Garrett wrote: 
> On Wed, Jun 02, 2010 at 12:32:25AM -0300, Henrique de Moraes Holschuh wrote:
> > On Sun, 30 May 2010, Matthew Garrett wrote:
> > > On Sun, May 30, 2010 at 05:40:04PM +0300, Maxim Levitsky wrote:
> > > > Just one note, I am sure somebody will ask for s3_no_nvs option, just
> > > > like s4_no_nvs.
> > > 
> > > I'd be surprised. Better to add the option when we think it's needed.
> > 
> > Do _all_ Windows versions restore NVS over S3 suspend/resume?
> 
> XP does regardless of whether any OSI strings are presented, and we have 
> systems that break unless it's done, so I think it's a safe bet.
This laptop was shipped with Vista, and since suspend works I guess its
highly likely that vista does restore NVS this way too.

This only leaves W7. I doubt they would break older systems by fixing a
bug....

Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-05-28 20:32 [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Matthew Garrett
  2010-05-28 20:32 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Matthew Garrett
  2010-05-30 14:37 ` [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Maxim Levitsky
@ 2010-06-04 18:22 ` Len Brown
  2010-06-04 21:48   ` Rafael J. Wysocki
  2 siblings, 1 reply; 21+ messages in thread
From: Len Brown @ 2010-06-04 18:22 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-acpi, linux-pm, Rafael J. Wysocki

applied to acpi-test -- pending Rafael's ack

thanks,
Len Brown, Intel Open Source Technology Center


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-05-28 20:32 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Matthew Garrett
  2010-05-30 14:40   ` Maxim Levitsky
@ 2010-06-04 18:23   ` Len Brown
  2010-06-10  9:41   ` Maxim Levitsky
  2 siblings, 0 replies; 21+ messages in thread
From: Len Brown @ 2010-06-04 18:23 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-acpi, linux-pm

applied to acpi-test

thanks,
Len Brown, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-05-30 14:37 ` [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Maxim Levitsky
@ 2010-06-04 19:11   ` Len Brown
  2010-06-04 21:27     ` Maxim Levitsky
  2010-06-05 15:28     ` Maxim Levitsky
  0 siblings, 2 replies; 21+ messages in thread
From: Len Brown @ 2010-06-04 19:11 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Matthew Garrett, linux-acpi, linux-pm, Rafael J. Wysocki, Alan Stern


> Since the other patch 'ACPI / EC / PM: Fix race between EC transactions
> and system suspend' is now merged, this should be rebased on top of it
> (I did the opposite....)

Maxim,
Since your system needs both these fixes, and they are merged together
in the acpi git tree, it would be great if you could test that tree
and let me know if we got it right.

git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git test

thanks,
Len Brown, Intel Open Source Technology Center


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-06-04 19:11   ` Len Brown
@ 2010-06-04 21:27     ` Maxim Levitsky
  2010-06-04 21:30       ` Maxim Levitsky
  2010-06-05 15:28     ` Maxim Levitsky
  1 sibling, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-04 21:27 UTC (permalink / raw)
  To: Len Brown
  Cc: Matthew Garrett, linux-acpi, linux-pm, Rafael J. Wysocki, Alan Stern

On Fri, 2010-06-04 at 15:11 -0400, Len Brown wrote: 
> > Since the other patch 'ACPI / EC / PM: Fix race between EC transactions
> > and system suspend' is now merged, this should be rebased on top of it
> > (I did the opposite....)
> 
> Maxim,
> Since your system needs both these fixes, and they are merged together
> in the acpi git tree, it would be great if you could test that tree
> and let me know if we got it right.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git test

Done!
Works correctly.

Small unrelated note (this happened with my rebased version, yesterday,
while doing stress testing):
Can't think of why ioremap could fail... maybe memory allocation
failure, but seems strange.

<3>[ 1737.071949] ioremap error for 0x7fe99000-0x7fe9a000, requested 0x10, got 0x0
<1>[ 1737.071983] BUG: unable to handle kernel NULL pointer dereference at (null)
<1>[ 1737.071990] IP: [<ffffffff811d697b>] memcpy+0xb/0xb0
<4>[ 1737.072000] PGD 6e2be067 PUD 64a8d067 PMD 0 
<0>[ 1737.072007] Oops: 0000 [#1] PREEMPT SMP 
<0>[ 1737.072012] last sysfs file: /sys/power/state
<4>[ 1737.072017] CPU 0 
<4>[ 1737.072019] Modules linked in: nls_iso8859_1 nls_cp437 vfat fat mmc_block af_packet snd_hda_codec_realtek usb_storage usb_libusual cpufreq_powersave snd_hda_intel cpufreq_conservative snd_hda_codec uvcvideo cpufreq_userspace r852 acpi_cpufreq mperf sm_common snd_hwdep nand iwl3945 iTCO_wdt videodev nand_ids v4l2_compat_ioctl32 sdhci_pci iwlcore nand_ecc tg3 uhci_hcd lirc_ene0100 iTCO_vendor_support snd_pcm joydev sdhci mac80211 ehci_hcd mmc_core battery sg ac video lirc_dev coretemp libphy mtd snd_page_alloc usbcore evdev psmouse cfg80211 serio_raw nouveau ttm drm_kms_helper drm i2c_algo_bit
<4>[ 1737.072085] 
<4>[ 1737.072089] Pid: 10027, comm: pm-suspend Not tainted 2.6.35-rc1+ #51 Nettiling/Aspire 5720     
<4>[ 1737.072095] RIP: 0010:[<ffffffff811d697b>]  [<ffffffff811d697b>] memcpy+0xb/0xb0
<4>[ 1737.072102] RSP: 0018:ffff880043029d80  EFLAGS: 00010246
<4>[ 1737.072107] RAX: ffff8800517bc000 RBX: ffff88006ff8aa40 RCX: 0000000000000200
<4>[ 1737.072112] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8800517bc000
<4>[ 1737.072116] RBP: ffff880043029d98 R08: 0000000000000000 R09: 0000000000000000
<4>[ 1737.072121] R10: 0000000000000004 R11: 0000000000000001 R12: 0000000000000003
<4>[ 1737.072126] R13: 00000000003fffff R14: ffffffff813ba4f8 R15: 0000000000000003
<4>[ 1737.072132] FS:  00007f953038f700(0000) GS:ffff880002400000(0000) knlGS:0000000000000000
<4>[ 1737.072137] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
<4>[ 1737.072142] CR2: 0000000000000000 CR3: 000000006f35b000 CR4: 00000000000006f0
<4>[ 1737.072147] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
<4>[ 1737.072152] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
<4>[ 1737.072157] Process pm-suspend (pid: 10027, threadinfo ffff880043028000, task ffff88003e730000)
<0>[ 1737.072162] Stack:
<4>[ 1737.072165]  ffffffff8108225d 0000000000000000 0000000000000000 ffff880043029db8
<4>[ 1737.072171] <0> ffffffff81213432 0000000000000010 0000000000000000 ffff880043029dd8
<4>[ 1737.072179] <0> ffffffff812135e0 0000000000000003 0000000000000000 ffff880043029e08
<0>[ 1737.072187] Call Trace:
<4>[ 1737.072193]  [<ffffffff8108225d>] ? suspend_nvs_save+0x4d/0x70
<4>[ 1737.072199]  [<ffffffff81213432>] __acpi_pm_prepare+0x1b/0x2e
<4>[ 1737.072205]  [<ffffffff812135e0>] acpi_pm_prepare+0xe/0x1f
<4>[ 1737.072212]  [<ffffffff8107d618>] suspend_devices_and_enter+0x108/0x220
<4>[ 1737.072219]  [<ffffffff8107d85a>] enter_state+0x12a/0x150
<4>[ 1737.072225]  [<ffffffff8107ce61>] state_store+0x91/0x100
<4>[ 1737.072231]  [<ffffffff811cd9d7>] kobj_attr_store+0x17/0x20
<4>[ 1737.072238]  [<ffffffff81144e02>] sysfs_write_file+0xf2/0x170
<4>[ 1737.072245]  [<ffffffff810dce78>] vfs_write+0xb8/0x170
<4>[ 1737.072253]  [<ffffffff813a1c2a>] ? lockdep_sys_exit_thunk+0x35/0x67
<4>[ 1737.072259]  [<ffffffff810dd00c>] sys_write+0x4c/0x80
<4>[ 1737.072266]  [<ffffffff81002deb>] system_call_fastpath+0x16/0x1b
<0>[ 1737.072271] Code: 89 70 58 19 c0 41 c6 40 4c 04 83 e0 fc 83 c0 08 41 88 40 4d c9 c3 90 90 90 90 90 90 90 90 90 90 48 89 f8 89 d1 c1 e9 03 83 e2 07 <f3> 48 a5 89 d1 f3 a4 c3 8b 1e 4c 8b 46 08 4c 89 1f 4c 89 47 08 
<1>[ 1737.072321] RIP  [<ffffffff811d697b>] memcpy+0xb/0xb0
<4>[ 1737.072327]  RSP <ffff880043029d80>
<0>[ 1737.072330] CR2: 0000000000000000
<4>[ 1737.073047] ---[ end trace c49c9aa5a3d831e9 ]---
<0>[ 1737.073052] Kernel panic - not syncing: Fatal exception
<4>[ 1737.073057] Pid: 10027, comm: pm-suspend Tainted: G      D     2.6.35-rc1+ #51
<4>[ 1737.073062] Call Trace:
<4>[ 1737.073067]  [<ffffffff8139e3c7>] panic+0x90/0x10a
<4>[ 1737.073073]  [<ffffffff8100737c>] oops_end+0xcc/0xe0
<4>[ 1737.073079]  [<ffffffff81027c13>] no_context+0xf3/0x260
<4>[ 1737.073086]  [<ffffffff8106c25e>] ? put_lock_stats+0xe/0x30
<4>[ 1737.073092]  [<ffffffff81027e95>] __bad_area_nosemaphore+0x115/0x1d0
<4>[ 1737.073098]  [<ffffffff81027fa9>] bad_area+0x49/0x60
<4>[ 1737.073103]  [<ffffffff810281c4>] ? do_page_fault+0xe4/0x390
<4>[ 1737.073109]  [<ffffffff8102840f>] do_page_fault+0x32f/0x390
<4>[ 1737.073115]  [<ffffffff811dc60f>] ? debug_check_no_obj_freed+0x17f/0x200
<4>[ 1737.073122]  [<ffffffff81098130>] ? call_rcu+0x10/0x20
<4>[ 1737.073128]  [<ffffffff810d8d13>] ? put_object+0x33/0x50
<4>[ 1737.073134]  [<ffffffff8102b0fd>] ? free_memtype+0xdd/0x140
<4>[ 1737.073140]  [<ffffffff813a2fdf>] page_fault+0x1f/0x30
<4>[ 1737.073146]  [<ffffffff811d697b>] ? memcpy+0xb/0xb0
<4>[ 1737.073152]  [<ffffffff8108225d>] ? suspend_nvs_save+0x4d/0x70
<4>[ 1737.073157]  [<ffffffff81213432>] __acpi_pm_prepare+0x1b/0x2e
<4>[ 1737.073163]  [<ffffffff812135e0>] acpi_pm_prepare+0xe/0x1f
<4>[ 1737.073169]  [<ffffffff8107d618>] suspend_devices_and_enter+0x108/0x220
<4>[ 1737.073175]  [<ffffffff8107d85a>] enter_state+0x12a/0x150
<4>[ 1737.073181]  [<ffffffff8107ce61>] state_store+0x91/0x100
<4>[ 1737.073187]  [<ffffffff811cd9d7>] kobj_attr_store+0x17/0x20
<4>[ 1737.073192]  [<ffffffff81144e02>] sysfs_write_file+0xf2/0x170
<4>[ 1737.073199]  [<ffffffff810dce78>] vfs_write+0xb8/0x170
<4>[ 1737.073204]  [<ffffffff813a1c2a>] ? lockdep_sys_exit_thunk+0x35/0x67
<4>[ 1737.073211]  [<ffffffff810dd00c>] sys_write+0x4c/0x80
<4>[ 1737.073217]  [<ffffffff81002deb>] system_call_fastpath+0x16/0x1b
<3>[ 1737.073231] [drm:drm_fb_helper_panic] *ERROR* panic occurred, switching back to text console
<0>[ 1737.073296] Rebooting in 20 seconds..
maxim@maxim-laptop:~$ 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-06-04 21:27     ` Maxim Levitsky
@ 2010-06-04 21:30       ` Maxim Levitsky
  0 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-04 21:30 UTC (permalink / raw)
  To: Len Brown
  Cc: Matthew Garrett, linux-acpi, linux-pm, Rafael J. Wysocki, Alan Stern

On Sat, 2010-06-05 at 00:27 +0300, Maxim Levitsky wrote: 
> On Fri, 2010-06-04 at 15:11 -0400, Len Brown wrote: 
> > > Since the other patch 'ACPI / EC / PM: Fix race between EC transactions
> > > and system suspend' is now merged, this should be rebased on top of it
> > > (I did the opposite....)
> > 
> > Maxim,
> > Since your system needs both these fixes, and they are merged together
> > in the acpi git tree, it would be great if you could test that tree
> > and let me know if we got it right.
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git test
> 
> Done!
> Works correctly.
> 
> Small unrelated note (this happened with my rebased version, yesterday,
> while doing stress testing):
> Can't think of why ioremap could fail... maybe memory allocation
> failure, but seems strange.
Thinking again about this, it probably noise, because I have also have a
userspace script that touches nvs, and it runs to make ubuntu kernel
work (sometimes I boot it to test some backports). I usually disable
that script, but I forgot to do that then.

> 
> <3>[ 1737.071949] ioremap error for 0x7fe99000-0x7fe9a000, requested 0x10, got 0x0
> <1>[ 1737.071983] BUG: unable to handle kernel NULL pointer dereference at (null)
> <1>[ 1737.071990] IP: [<ffffffff811d697b>] memcpy+0xb/0xb0
> <4>[ 1737.072000] PGD 6e2be067 PUD 64a8d067 PMD 0 
> <0>[ 1737.072007] Oops: 0000 [#1] PREEMPT SMP 
> <0>[ 1737.072012] last sysfs file: /sys/power/state
> <4>[ 1737.072017] CPU 0 
> <4>[ 1737.072019] Modules linked in: nls_iso8859_1 nls_cp437 vfat fat mmc_block af_packet snd_hda_codec_realtek usb_storage usb_libusual cpufreq_powersave snd_hda_intel cpufreq_conservative snd_hda_codec uvcvideo cpufreq_userspace r852 acpi_cpufreq mperf sm_common snd_hwdep nand iwl3945 iTCO_wdt videodev nand_ids v4l2_compat_ioctl32 sdhci_pci iwlcore nand_ecc tg3 uhci_hcd lirc_ene0100 iTCO_vendor_support snd_pcm joydev sdhci mac80211 ehci_hcd mmc_core battery sg ac video lirc_dev coretemp libphy mtd snd_page_alloc usbcore evdev psmouse cfg80211 serio_raw nouveau ttm drm_kms_helper drm i2c_algo_bit
> <4>[ 1737.072085] 
> <4>[ 1737.072089] Pid: 10027, comm: pm-suspend Not tainted 2.6.35-rc1+ #51 Nettiling/Aspire 5720     
> <4>[ 1737.072095] RIP: 0010:[<ffffffff811d697b>]  [<ffffffff811d697b>] memcpy+0xb/0xb0
> <4>[ 1737.072102] RSP: 0018:ffff880043029d80  EFLAGS: 00010246
> <4>[ 1737.072107] RAX: ffff8800517bc000 RBX: ffff88006ff8aa40 RCX: 0000000000000200
> <4>[ 1737.072112] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8800517bc000
> <4>[ 1737.072116] RBP: ffff880043029d98 R08: 0000000000000000 R09: 0000000000000000
> <4>[ 1737.072121] R10: 0000000000000004 R11: 0000000000000001 R12: 0000000000000003
> <4>[ 1737.072126] R13: 00000000003fffff R14: ffffffff813ba4f8 R15: 0000000000000003
> <4>[ 1737.072132] FS:  00007f953038f700(0000) GS:ffff880002400000(0000) knlGS:0000000000000000
> <4>[ 1737.072137] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> <4>[ 1737.072142] CR2: 0000000000000000 CR3: 000000006f35b000 CR4: 00000000000006f0
> <4>[ 1737.072147] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> <4>[ 1737.072152] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> <4>[ 1737.072157] Process pm-suspend (pid: 10027, threadinfo ffff880043028000, task ffff88003e730000)
> <0>[ 1737.072162] Stack:
> <4>[ 1737.072165]  ffffffff8108225d 0000000000000000 0000000000000000 ffff880043029db8
> <4>[ 1737.072171] <0> ffffffff81213432 0000000000000010 0000000000000000 ffff880043029dd8
> <4>[ 1737.072179] <0> ffffffff812135e0 0000000000000003 0000000000000000 ffff880043029e08
> <0>[ 1737.072187] Call Trace:
> <4>[ 1737.072193]  [<ffffffff8108225d>] ? suspend_nvs_save+0x4d/0x70
> <4>[ 1737.072199]  [<ffffffff81213432>] __acpi_pm_prepare+0x1b/0x2e
> <4>[ 1737.072205]  [<ffffffff812135e0>] acpi_pm_prepare+0xe/0x1f
> <4>[ 1737.072212]  [<ffffffff8107d618>] suspend_devices_and_enter+0x108/0x220
> <4>[ 1737.072219]  [<ffffffff8107d85a>] enter_state+0x12a/0x150
> <4>[ 1737.072225]  [<ffffffff8107ce61>] state_store+0x91/0x100
> <4>[ 1737.072231]  [<ffffffff811cd9d7>] kobj_attr_store+0x17/0x20
> <4>[ 1737.072238]  [<ffffffff81144e02>] sysfs_write_file+0xf2/0x170
> <4>[ 1737.072245]  [<ffffffff810dce78>] vfs_write+0xb8/0x170
> <4>[ 1737.072253]  [<ffffffff813a1c2a>] ? lockdep_sys_exit_thunk+0x35/0x67
> <4>[ 1737.072259]  [<ffffffff810dd00c>] sys_write+0x4c/0x80
> <4>[ 1737.072266]  [<ffffffff81002deb>] system_call_fastpath+0x16/0x1b
> <0>[ 1737.072271] Code: 89 70 58 19 c0 41 c6 40 4c 04 83 e0 fc 83 c0 08 41 88 40 4d c9 c3 90 90 90 90 90 90 90 90 90 90 48 89 f8 89 d1 c1 e9 03 83 e2 07 <f3> 48 a5 89 d1 f3 a4 c3 8b 1e 4c 8b 46 08 4c 89 1f 4c 89 47 08 
> <1>[ 1737.072321] RIP  [<ffffffff811d697b>] memcpy+0xb/0xb0
> <4>[ 1737.072327]  RSP <ffff880043029d80>
> <0>[ 1737.072330] CR2: 0000000000000000
> <4>[ 1737.073047] ---[ end trace c49c9aa5a3d831e9 ]---
> <0>[ 1737.073052] Kernel panic - not syncing: Fatal exception
> <4>[ 1737.073057] Pid: 10027, comm: pm-suspend Tainted: G      D     2.6.35-rc1+ #51
> <4>[ 1737.073062] Call Trace:
> <4>[ 1737.073067]  [<ffffffff8139e3c7>] panic+0x90/0x10a
> <4>[ 1737.073073]  [<ffffffff8100737c>] oops_end+0xcc/0xe0
> <4>[ 1737.073079]  [<ffffffff81027c13>] no_context+0xf3/0x260
> <4>[ 1737.073086]  [<ffffffff8106c25e>] ? put_lock_stats+0xe/0x30
> <4>[ 1737.073092]  [<ffffffff81027e95>] __bad_area_nosemaphore+0x115/0x1d0
> <4>[ 1737.073098]  [<ffffffff81027fa9>] bad_area+0x49/0x60
> <4>[ 1737.073103]  [<ffffffff810281c4>] ? do_page_fault+0xe4/0x390
> <4>[ 1737.073109]  [<ffffffff8102840f>] do_page_fault+0x32f/0x390
> <4>[ 1737.073115]  [<ffffffff811dc60f>] ? debug_check_no_obj_freed+0x17f/0x200
> <4>[ 1737.073122]  [<ffffffff81098130>] ? call_rcu+0x10/0x20
> <4>[ 1737.073128]  [<ffffffff810d8d13>] ? put_object+0x33/0x50
> <4>[ 1737.073134]  [<ffffffff8102b0fd>] ? free_memtype+0xdd/0x140
> <4>[ 1737.073140]  [<ffffffff813a2fdf>] page_fault+0x1f/0x30
> <4>[ 1737.073146]  [<ffffffff811d697b>] ? memcpy+0xb/0xb0
> <4>[ 1737.073152]  [<ffffffff8108225d>] ? suspend_nvs_save+0x4d/0x70
> <4>[ 1737.073157]  [<ffffffff81213432>] __acpi_pm_prepare+0x1b/0x2e
> <4>[ 1737.073163]  [<ffffffff812135e0>] acpi_pm_prepare+0xe/0x1f
> <4>[ 1737.073169]  [<ffffffff8107d618>] suspend_devices_and_enter+0x108/0x220
> <4>[ 1737.073175]  [<ffffffff8107d85a>] enter_state+0x12a/0x150
> <4>[ 1737.073181]  [<ffffffff8107ce61>] state_store+0x91/0x100
> <4>[ 1737.073187]  [<ffffffff811cd9d7>] kobj_attr_store+0x17/0x20
> <4>[ 1737.073192]  [<ffffffff81144e02>] sysfs_write_file+0xf2/0x170
> <4>[ 1737.073199]  [<ffffffff810dce78>] vfs_write+0xb8/0x170
> <4>[ 1737.073204]  [<ffffffff813a1c2a>] ? lockdep_sys_exit_thunk+0x35/0x67
> <4>[ 1737.073211]  [<ffffffff810dd00c>] sys_write+0x4c/0x80
> <4>[ 1737.073217]  [<ffffffff81002deb>] system_call_fastpath+0x16/0x1b
> <3>[ 1737.073231] [drm:drm_fb_helper_panic] *ERROR* panic occurred, switching back to text console
> <0>[ 1737.073296] Rebooting in 20 seconds..
> maxim@maxim-laptop:~$ 
> 
> 



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-06-04 18:22 ` Len Brown
@ 2010-06-04 21:48   ` Rafael J. Wysocki
  0 siblings, 0 replies; 21+ messages in thread
From: Rafael J. Wysocki @ 2010-06-04 21:48 UTC (permalink / raw)
  To: Len Brown; +Cc: Matthew Garrett, linux-acpi, linux-pm

On Friday 04 June 2010, Len Brown wrote:
> applied to acpi-test -- pending Rafael's ack

You have it.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-06-04 19:11   ` Len Brown
  2010-06-04 21:27     ` Maxim Levitsky
@ 2010-06-05 15:28     ` Maxim Levitsky
  2010-06-10 15:11       ` Len Brown
  1 sibling, 1 reply; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-05 15:28 UTC (permalink / raw)
  To: Len Brown
  Cc: Matthew Garrett, linux-acpi, linux-pm, Rafael J. Wysocki, Alan Stern

On Fri, 2010-06-04 at 15:11 -0400, Len Brown wrote: 
> > Since the other patch 'ACPI / EC / PM: Fix race between EC transactions
> > and system suspend' is now merged, this should be rebased on top of it
> > (I did the opposite....)
> 
> Maxim,
> Since your system needs both these fixes, and they are merged together
> in the acpi git tree, it would be great if you could test that tree
> and let me know if we got it right.
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git test
> 
> thanks,
> Len Brown, Intel Open Source Technology Center
> 

Which kernel do you expect these patches to land on?

Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-05-28 20:32 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Matthew Garrett
  2010-05-30 14:40   ` Maxim Levitsky
  2010-06-04 18:23   ` Len Brown
@ 2010-06-10  9:41   ` Maxim Levitsky
  2 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-10  9:41 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: lenb, linux-acpi, linux-pm

On Fri, 2010-05-28 at 16:32 -0400, Matthew Garrett wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=13931 describes a bug where
> a system fails to successfully resume after the second suspend. Maxim
> Levitsky discovered that this could be rectified by forcibly saving
> and restoring the ACPI non-volatile state. The spec indicates that this
> is only required for S4, but testing the behaviour of Windows by adding
> an ACPI NVS region to qemu's e820 map and registering a custom memory
> read/write handler reveals that it's saved and restored even over suspend
> to RAM. We should mimic that behaviour to avoid other broken platforms.
Any chance to see this patch in 2.6.35?

Best regards,
	Maxim Levitsky


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-06-05 15:28     ` Maxim Levitsky
@ 2010-06-10 15:11       ` Len Brown
  2010-06-10 15:21         ` Maxim Levitsky
  0 siblings, 1 reply; 21+ messages in thread
From: Len Brown @ 2010-06-10 15:11 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: Matthew Garrett, linux-acpi, linux-pm, Rafael J. Wysocki, Alan Stern

On Sat, 5 Jun 2010, Maxim Levitsky wrote:

> On Fri, 2010-06-04 at 15:11 -0400, Len Brown wrote: 
> > > Since the other patch 'ACPI / EC / PM: Fix race between EC transactions
> > > and system suspend' is now merged, this should be rebased on top of it
> > > (I did the opposite....)
> > 
> > Maxim,
> > Since your system needs both these fixes, and they are merged together
> > in the acpi git tree, it would be great if you could test that tree
> > and let me know if we got it right.
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git test


> Which kernel do you expect these patches to land on?

the test tree above is in linux-next now.
there are some patches that will be 2.6.36 in there,
but I was considereing this one for 2.6.35-rc3 --
though your e-mail with a stack trace has caused me to pause --
did I mis-merge it?

thanks,
-Len Brown, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality
  2010-06-10 15:11       ` Len Brown
@ 2010-06-10 15:21         ` Maxim Levitsky
  0 siblings, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-10 15:21 UTC (permalink / raw)
  To: Len Brown
  Cc: Matthew Garrett, linux-acpi, linux-pm, Rafael J. Wysocki, Alan Stern

On Thu, 2010-06-10 at 11:11 -0400, Len Brown wrote: 
> On Sat, 5 Jun 2010, Maxim Levitsky wrote:
> 
> > On Fri, 2010-06-04 at 15:11 -0400, Len Brown wrote: 
> > > > Since the other patch 'ACPI / EC / PM: Fix race between EC transactions
> > > > and system suspend' is now merged, this should be rebased on top of it
> > > > (I did the opposite....)
> > > 
> > > Maxim,
> > > Since your system needs both these fixes, and they are merged together
> > > in the acpi git tree, it would be great if you could test that tree
> > > and let me know if we got it right.
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git test
> 
> 
> > Which kernel do you expect these patches to land on?
> 
> the test tree above is in linux-next now.
> there are some patches that will be 2.6.36 in there,
> but I was considereing this one for 2.6.35-rc3 --
> though your e-mail with a stack trace has caused me to pause --
> did I mis-merge it?

I can't reproduce that anymore.
I had a userspace script that would mmap /dev/mem and 'fix' the issue.
I used it to help some ubuntu users and to test some unrelated work on
ubuntu kernel.

Yet, the script should exit before write to /sys/power/state.

However when this happened I also closed the system's lid, which might
make the same script to run in parallel, and therefore an attempt to
ioremap the NVS area failed.
can't think for any other explanation why ioremap could fail.
Anyway, I think it would be safe to add a check to and abort suspend if
ioremap fails.


I don't use this script anymore.

Best regards,
Maxim Levitsky


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-06-01 18:19 [PATCH] Store BIOS NVS area over s2ram [V2] Maxim Levitsky
  2010-06-01 18:22 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Maxim Levitsky
@ 2010-06-01 18:22 ` Maxim Levitsky
  1 sibling, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-01 18:22 UTC (permalink / raw)
  To: linux-acpi
  Cc: linux-pm, Matthew Garrett, Rafael J. Wysocki, Alan Stern, Maxim Levitsky

https://bugzilla.kernel.org/show_bug.cgi?id=13931 describes a bug where
a system fails to successfully resume after the second suspend. Maxim
Levitsky discovered that this could be rectified by forcibly saving
and restoring the ACPI non-volatile state. The spec indicates that this
is only required for S4, but testing the behaviour of Windows by adding
an ACPI NVS region to qemu's e820 map and registering a custom memory
read/write handler reveals that it's saved and restored even over suspend
to RAM. We should mimic that behaviour to avoid other broken platforms.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/acpi/sleep.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index bdb306f..5d26d8e 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -114,6 +114,8 @@ static int __acpi_pm_prepare(void)
 {
 	int error = acpi_sleep_prepare(acpi_target_sleep_state);
 
+	suspend_nvs_save();
+
 	if (error)
 		acpi_target_sleep_state = ACPI_STATE_S0;
 	return error;
@@ -143,6 +145,8 @@ static void acpi_pm_finish(void)
 {
 	u32 acpi_state = acpi_target_sleep_state;
 
+	suspend_nvs_free();
+
 	if (acpi_state == ACPI_STATE_S0)
 		return;
 
@@ -192,6 +196,11 @@ static int acpi_suspend_begin(suspend_state_t pm_state)
 	u32 acpi_state = acpi_suspend_states[pm_state];
 	int error = 0;
 
+	error = suspend_nvs_alloc();
+
+	if (error)
+		return error;
+
 	if (sleep_states[acpi_state]) {
 		acpi_target_sleep_state = acpi_state;
 		acpi_sleep_tts_switch(acpi_target_sleep_state);
@@ -269,6 +278,8 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
 	if (acpi_state == ACPI_STATE_S3)
 		acpi_restore_state_mem();
 
+	suspend_nvs_restore();
+
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
 
@@ -443,7 +454,6 @@ static int acpi_hibernation_enter(void)
 
 static void acpi_hibernation_finish(void)
 {
-	suspend_nvs_free();
 	acpi_ec_unblock_transactions();
 	acpi_pm_finish();
 }
@@ -479,7 +489,7 @@ static struct platform_hibernation_ops acpi_hibernation_ops = {
 	.begin = acpi_hibernation_begin,
 	.end = acpi_pm_end,
 	.pre_snapshot = acpi_hibernation_pre_snapshot,
-	.finish = acpi_hibernation_finish,
+	.finish = acpi_pm_finish,
 	.prepare = acpi_pm_prepare,
 	.enter = acpi_hibernation_enter,
 	.leave = acpi_hibernation_leave,
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM
  2010-06-01 18:19 [PATCH] Store BIOS NVS area over s2ram [V2] Maxim Levitsky
@ 2010-06-01 18:22 ` Maxim Levitsky
  2010-06-01 18:22 ` Maxim Levitsky
  1 sibling, 0 replies; 21+ messages in thread
From: Maxim Levitsky @ 2010-06-01 18:22 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-pm, Matthew Garrett

https://bugzilla.kernel.org/show_bug.cgi?id=13931 describes a bug where
a system fails to successfully resume after the second suspend. Maxim
Levitsky discovered that this could be rectified by forcibly saving
and restoring the ACPI non-volatile state. The spec indicates that this
is only required for S4, but testing the behaviour of Windows by adding
an ACPI NVS region to qemu's e820 map and registering a custom memory
read/write handler reveals that it's saved and restored even over suspend
to RAM. We should mimic that behaviour to avoid other broken platforms.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/acpi/sleep.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index bdb306f..5d26d8e 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -114,6 +114,8 @@ static int __acpi_pm_prepare(void)
 {
 	int error = acpi_sleep_prepare(acpi_target_sleep_state);
 
+	suspend_nvs_save();
+
 	if (error)
 		acpi_target_sleep_state = ACPI_STATE_S0;
 	return error;
@@ -143,6 +145,8 @@ static void acpi_pm_finish(void)
 {
 	u32 acpi_state = acpi_target_sleep_state;
 
+	suspend_nvs_free();
+
 	if (acpi_state == ACPI_STATE_S0)
 		return;
 
@@ -192,6 +196,11 @@ static int acpi_suspend_begin(suspend_state_t pm_state)
 	u32 acpi_state = acpi_suspend_states[pm_state];
 	int error = 0;
 
+	error = suspend_nvs_alloc();
+
+	if (error)
+		return error;
+
 	if (sleep_states[acpi_state]) {
 		acpi_target_sleep_state = acpi_state;
 		acpi_sleep_tts_switch(acpi_target_sleep_state);
@@ -269,6 +278,8 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
 	if (acpi_state == ACPI_STATE_S3)
 		acpi_restore_state_mem();
 
+	suspend_nvs_restore();
+
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
 
@@ -443,7 +454,6 @@ static int acpi_hibernation_enter(void)
 
 static void acpi_hibernation_finish(void)
 {
-	suspend_nvs_free();
 	acpi_ec_unblock_transactions();
 	acpi_pm_finish();
 }
@@ -479,7 +489,7 @@ static struct platform_hibernation_ops acpi_hibernation_ops = {
 	.begin = acpi_hibernation_begin,
 	.end = acpi_pm_end,
 	.pre_snapshot = acpi_hibernation_pre_snapshot,
-	.finish = acpi_hibernation_finish,
+	.finish = acpi_pm_finish,
 	.prepare = acpi_pm_prepare,
 	.enter = acpi_hibernation_enter,
 	.leave = acpi_hibernation_leave,
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2010-06-10 15:21 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-28 20:32 [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Matthew Garrett
2010-05-28 20:32 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Matthew Garrett
2010-05-30 14:40   ` Maxim Levitsky
2010-05-30 14:51     ` Matthew Garrett
2010-06-02  3:32       ` Henrique de Moraes Holschuh
2010-06-02 10:15         ` Maxim Levitsky
2010-06-02 11:47         ` Matthew Garrett
2010-06-02 13:06           ` Maxim Levitsky
2010-06-04 18:23   ` Len Brown
2010-06-10  9:41   ` Maxim Levitsky
2010-05-30 14:37 ` [PATCH 1/2] suspend: Move NVS save/restore code to generic suspend functionality Maxim Levitsky
2010-06-04 19:11   ` Len Brown
2010-06-04 21:27     ` Maxim Levitsky
2010-06-04 21:30       ` Maxim Levitsky
2010-06-05 15:28     ` Maxim Levitsky
2010-06-10 15:11       ` Len Brown
2010-06-10 15:21         ` Maxim Levitsky
2010-06-04 18:22 ` Len Brown
2010-06-04 21:48   ` Rafael J. Wysocki
2010-06-01 18:19 [PATCH] Store BIOS NVS area over s2ram [V2] Maxim Levitsky
2010-06-01 18:22 ` [PATCH 2/2] ACPI: Store NVS state even when entering suspend to RAM Maxim Levitsky
2010-06-01 18:22 ` Maxim Levitsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.