[RFC] [PATCH 1/3] ACPI: save/restore ACPI NVS memory during hibernation

[RFC] [PATCH 1/3] ACPI: save/restore ACPI NVS memory during hibernation

[Zhang Rui]

According to the ACPI spec, ACPI NVS memory region is required to be
saved/restored by OS during hibernation.

Section 15.3.2 ACPI Spec 3.0b,
"OSPM will call the _PTS control method some time before entering a sleeping state,
to allow the platform’s AML code to update this memory image before entering the
sleeping state. After the system awakes from an S4 state, OSPM will restore this memory
area and call the _WAK control method to enable the BIOS to reclaim its memory image."

Add the mechanism to save/restore ACPI NVS memory during hibernation.

Note: now Linux save ACPI NVS memory in acpi_hibernation_pre_snapshot, and restore it in
	acpi_hibernation_leave. Both of these functions will be invoked only once during
	the hibernate and resume.
Note: in Section 14.3 ACPI spec 3.0b, I only get
	"EfiACPIMemoryNVS: The OS and loader must preserve this memory range in
	the working and ACPI S1–S3 states."
	whether we should save/restore this piece of memory is not cleared.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/sleep/main.c |  122 +++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/acpi.h      |    3 +
 2 files changed, 124 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep/main.c	2008-06-20 09:27:00.000000000 +0800
+++ linux-2.6/drivers/acpi/sleep/main.c	2008-06-24 14:51:20.000000000 +0800
@@ -15,6 +15,7 @@
 #include <linux/dmi.h>
 #include <linux/device.h>
 #include <linux/suspend.h>
+#include <linux/highmem.h>
 
 #include <asm/io.h>
 
@@ -255,6 +256,109 @@
 #endif /* CONFIG_SUSPEND */
 
 #ifdef CONFIG_HIBERNATION
+
+/* ACPI NVS memory need to be saved/stored during hibernation */
+struct nvs_page {
+	unsigned long pfn;
+	void *data;
+	struct nvs_page *next;
+};
+static struct nvs_page *nvs_pages;
+
+int acpi_mark_nvs_region(unsigned long start, unsigned long end)
+{
+	struct nvs_page *tmp;
+
+	while (start <= end) {
+		tmp = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
+		if (!tmp)
+			return -ENOMEM;
+		tmp->pfn = start;
+		tmp->next = nvs_pages;
+		start++;
+		nvs_pages = tmp;
+	}
+	return 0;
+}
+
+static int acpi_hibernation_allocate_nvs_pages(void)
+{
+	struct nvs_page *tmp = nvs_pages;
+
+	while (tmp) {
+		tmp->data = (void *)__get_free_page(GFP_KERNEL);
+		if (!tmp->data)
+			return -ENOMEM;
+		tmp = tmp->next;
+	}
+	return 0;
+}
+
+static int acpi_hibernation_free_nvs_pages(void)
+{
+	struct nvs_page *tmp = nvs_pages;
+
+	while (tmp) {
+		if (tmp->data) {
+			free_page((long)tmp->data);
+			tmp->data = NULL;
+		}
+		tmp = tmp->next;
+	}
+	return 0;
+}
+
+static int
+acpi_hibernation_notifier_cb(struct notifier_block *nb,
+			unsigned long val, void *data)
+{
+	int result = 0;
+
+	switch (val) {
+	case PM_HIBERNATION_PREPARE:
+		result = acpi_hibernation_allocate_nvs_pages();
+		break;
+	case PM_POST_RESTORE: /* Failed */
+	case PM_POST_HIBERNATION:
+		acpi_hibernation_free_nvs_pages();
+	}
+	return result;
+}
+
+struct notifier_block acpi_hibernation_nb = {
+	.notifier_call = acpi_hibernation_notifier_cb,
+};
+
+static void acpi_save_nvs_memory(void)
+{
+	void *kaddr;
+	struct nvs_page *tmp = nvs_pages;
+
+	pr_debug("ACPI: Saving ACPI NVS memory\n");
+	while (tmp) {
+		/* nvs page might not have a 'struct page' */
+		kaddr = kmap_atomic_pfn(tmp->pfn, KM_USER0);
+		copy_page(tmp->data, kaddr);
+		kunmap_atomic(kaddr, KM_USER0);
+		tmp = tmp->next;
+	}
+	return;
+}
+
+static void acpi_restore_nvs_memory(void)
+{
+	void *kaddr;
+	struct nvs_page *tmp = nvs_pages;
+
+	pr_debug("ACPI: Restoring ACPI NVS memory\n");
+	while (tmp) {
+		kaddr = kmap_atomic_pfn(tmp->pfn, KM_USER0);
+		copy_page(kaddr, tmp->data);
+		kunmap_atomic(kaddr, KM_USER0);
+		tmp = tmp->next;
+	}
+}
+
 static int acpi_hibernation_begin(void)
 {
 	acpi_target_sleep_state = ACPI_STATE_S4;
@@ -274,6 +378,20 @@
 	return ACPI_SUCCESS(acpi_hw_disable_all_gpes()) ? 0 : -EFAULT;
 }
 
+static int acpi_hibernation_pre_snapshot(void)
+{
+	int error = acpi_sleep_prepare(ACPI_STATE_S4);
+
+	if (error) {
+		acpi_target_sleep_state = ACPI_STATE_S0;
+		return error;
+	}
+
+	acpi_save_nvs_memory();
+
+	return ACPI_SUCCESS(acpi_hw_disable_all_gpes()) ? 0 : -EFAULT;
+}
+
 static int acpi_hibernation_enter(void)
 {
 	acpi_status status = AE_OK;
@@ -301,6 +419,7 @@
 	acpi_enable();
 	/* Reprogram control registers and execute _BFS */
 	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
+	acpi_restore_nvs_memory();
 }
 
 static void acpi_hibernation_finish(void)
@@ -340,7 +459,7 @@
 static struct platform_hibernation_ops acpi_hibernation_ops = {
 	.begin = acpi_hibernation_begin,
 	.end = acpi_hibernation_end,
-	.pre_snapshot = acpi_hibernation_prepare,
+	.pre_snapshot = acpi_hibernation_pre_snapshot,
 	.finish = acpi_hibernation_finish,
 	.prepare = acpi_hibernation_prepare,
 	.enter = acpi_hibernation_enter,
@@ -498,6 +617,7 @@
 #ifdef CONFIG_HIBERNATION
 	status = acpi_get_sleep_type_data(ACPI_STATE_S4, &type_a, &type_b);
 	if (ACPI_SUCCESS(status)) {
+		register_pm_notifier(&acpi_hibernation_nb);
 		hibernation_set_ops(&acpi_hibernation_ops);
 		sleep_states[ACPI_STATE_S4] = 1;
 		printk(" S4");
Index: linux-2.6/include/linux/acpi.h
===================================================================
--- linux-2.6.orig/include/linux/acpi.h	2008-06-20 09:27:00.000000000 +0800
+++ linux-2.6/include/linux/acpi.h	2008-06-24 11:11:03.000000000 +0800
@@ -106,6 +106,9 @@
 void acpi_numa_arch_fixup(void);
 #endif
 
+#ifdef CONFIG_HIBERNATION
+int acpi_mark_nvs_region(unsigned long, unsigned long);
+#endif
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */
 int acpi_map_lsapic(acpi_handle handle, int *pcpu);


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [PATCH 1/3] ACPI: save/restore ACPI NVS memory during hibernation

[Rafael J. Wysocki]

On Tuesday, 24 of June 2008, Zhang Rui wrote:
> According to the ACPI spec, ACPI NVS memory region is required to be
> saved/restored by OS during hibernation.
> 
> Section 15.3.2 ACPI Spec 3.0b,
> "OSPM will call the _PTS control method some time before entering a sleeping state,
> to allow the platform’s AML code to update this memory image before entering the
> sleeping state. After the system awakes from an S4 state, OSPM will restore this memory
> area and call the _WAK control method to enable the BIOS to reclaim its memory image."
> 
> Add the mechanism to save/restore ACPI NVS memory during hibernation.
> 
> Note: now Linux save ACPI NVS memory in acpi_hibernation_pre_snapshot, and restore it in
> 	acpi_hibernation_leave. Both of these functions will be invoked only once during
> 	the hibernate and resume.
> Note: in Section 14.3 ACPI spec 3.0b, I only get
> 	"EfiACPIMemoryNVS: The OS and loader must preserve this memory range in
> 	the working and ACPI S1–S3 states."
> 	whether we should save/restore this piece of memory is not cleared.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>

Well, I know what the specification says, but we tried that in the past and it
caused regressions.

> ---
>  drivers/acpi/sleep/main.c |  122 +++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/acpi.h      |    3 +
>  2 files changed, 124 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/drivers/acpi/sleep/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/sleep/main.c	2008-06-20 09:27:00.000000000 +0800
> +++ linux-2.6/drivers/acpi/sleep/main.c	2008-06-24 14:51:20.000000000 +0800
> @@ -15,6 +15,7 @@
>  #include <linux/dmi.h>
>  #include <linux/device.h>
>  #include <linux/suspend.h>
> +#include <linux/highmem.h>
>  
>  #include <asm/io.h>
>  
> @@ -255,6 +256,109 @@
>  #endif /* CONFIG_SUSPEND */
>  
>  #ifdef CONFIG_HIBERNATION
> +
> +/* ACPI NVS memory need to be saved/stored during hibernation */
> +struct nvs_page {
> +	unsigned long pfn;
> +	void *data;
> +	struct nvs_page *next;
> +};
> +static struct nvs_page *nvs_pages;
> +
> +int acpi_mark_nvs_region(unsigned long start, unsigned long end)
> +{
> +	struct nvs_page *tmp;
> +
> +	while (start <= end) {
> +		tmp = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
> +		if (!tmp)
> +			return -ENOMEM;

We should free the already allocated structures in this case IMO.

> +		tmp->pfn = start;
> +		tmp->next = nvs_pages;
> +		start++;
> +		nvs_pages = tmp;
> +	}
> +	return 0;
> +}
> +
> +static int acpi_hibernation_allocate_nvs_pages(void)
> +{
> +	struct nvs_page *tmp = nvs_pages;
> +
> +	while (tmp) {
> +		tmp->data = (void *)__get_free_page(GFP_KERNEL);
> +		if (!tmp->data)
> +			return -ENOMEM;

Perhaps call acpi_hibernation_free_nvs_pages() here too.

> +		tmp = tmp->next;
> +	}
> +	return 0;
> +}
> +
> +static int acpi_hibernation_free_nvs_pages(void)
> +{
> +	struct nvs_page *tmp = nvs_pages;
> +
> +	while (tmp) {
> +		if (tmp->data) {
> +			free_page((long)tmp->data);
> +			tmp->data = NULL;
> +		}
> +		tmp = tmp->next;
> +	}

Make nvs_pages = NULL explicitly, please.

> +	return 0;
> +}
> +
> +static int
> +acpi_hibernation_notifier_cb(struct notifier_block *nb,
> +			unsigned long val, void *data)
> +{
> +	int result = 0;
> +
> +	switch (val) {
> +	case PM_HIBERNATION_PREPARE:
> +		result = acpi_hibernation_allocate_nvs_pages();
> +		break;
> +	case PM_POST_RESTORE: /* Failed */
> +	case PM_POST_HIBERNATION:
> +		acpi_hibernation_free_nvs_pages();
> +	}
> +	return result;
> +}
> +
> +struct notifier_block acpi_hibernation_nb = {
> +	.notifier_call = acpi_hibernation_notifier_cb,
> +};

This is over the top, IMO.  Please put the appropriate call into
acpi_hibernation_begin() and create acpi_hibernation_end() with the call to
acpi_hibernation_free_nvs_pages() (it must do what acpi_pm_end() does apart
from this).

> +
> +static void acpi_save_nvs_memory(void)
> +{
> +	void *kaddr;
> +	struct nvs_page *tmp = nvs_pages;
> +
> +	pr_debug("ACPI: Saving ACPI NVS memory\n");
> +	while (tmp) {
> +		/* nvs page might not have a 'struct page' */
> +		kaddr = kmap_atomic_pfn(tmp->pfn, KM_USER0);
> +		copy_page(tmp->data, kaddr);
> +		kunmap_atomic(kaddr, KM_USER0);
> +		tmp = tmp->next;
> +	}
> +	return;
> +}
> +
> +static void acpi_restore_nvs_memory(void)
> +{
> +	void *kaddr;
> +	struct nvs_page *tmp = nvs_pages;
> +
> +	pr_debug("ACPI: Restoring ACPI NVS memory\n");
> +	while (tmp) {
> +		kaddr = kmap_atomic_pfn(tmp->pfn, KM_USER0);
> +		copy_page(kaddr, tmp->data);
> +		kunmap_atomic(kaddr, KM_USER0);
> +		tmp = tmp->next;
> +	}
> +}
> +
>  static int acpi_hibernation_begin(void)
>  {
>  	acpi_target_sleep_state = ACPI_STATE_S4;
> @@ -274,6 +378,20 @@
>  	return ACPI_SUCCESS(acpi_hw_disable_all_gpes()) ? 0 : -EFAULT;
>  }
>  
> +static int acpi_hibernation_pre_snapshot(void)
> +{
> +	int error = acpi_sleep_prepare(ACPI_STATE_S4);
> +
> +	if (error) {
> +		acpi_target_sleep_state = ACPI_STATE_S0;
> +		return error;
> +	}
> +
> +	acpi_save_nvs_memory();
> +
> +	return ACPI_SUCCESS(acpi_hw_disable_all_gpes()) ? 0 : -EFAULT;
> +}
> +
>  static int acpi_hibernation_enter(void)
>  {
>  	acpi_status status = AE_OK;
> @@ -301,6 +419,7 @@
>  	acpi_enable();
>  	/* Reprogram control registers and execute _BFS */
>  	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
> +	acpi_restore_nvs_memory();
>  }
>  
>  static void acpi_hibernation_finish(void)
> @@ -340,7 +459,7 @@
>  static struct platform_hibernation_ops acpi_hibernation_ops = {
>  	.begin = acpi_hibernation_begin,
>  	.end = acpi_hibernation_end,
> -	.pre_snapshot = acpi_hibernation_prepare,
> +	.pre_snapshot = acpi_hibernation_pre_snapshot,
>  	.finish = acpi_hibernation_finish,
>  	.prepare = acpi_hibernation_prepare,
>  	.enter = acpi_hibernation_enter,
> @@ -498,6 +617,7 @@
>  #ifdef CONFIG_HIBERNATION
>  	status = acpi_get_sleep_type_data(ACPI_STATE_S4, &type_a, &type_b);
>  	if (ACPI_SUCCESS(status)) {
> +		register_pm_notifier(&acpi_hibernation_nb);
>  		hibernation_set_ops(&acpi_hibernation_ops);
>  		sleep_states[ACPI_STATE_S4] = 1;
>  		printk(" S4");
> Index: linux-2.6/include/linux/acpi.h
> ===================================================================
> --- linux-2.6.orig/include/linux/acpi.h	2008-06-20 09:27:00.000000000 +0800
> +++ linux-2.6/include/linux/acpi.h	2008-06-24 11:11:03.000000000 +0800
> @@ -106,6 +106,9 @@
>  void acpi_numa_arch_fixup(void);
>  #endif
>  
> +#ifdef CONFIG_HIBERNATION
> +int acpi_mark_nvs_region(unsigned long, unsigned long);
> +#endif
>  #ifdef CONFIG_ACPI_HOTPLUG_CPU
>  /* Arch dependent functions for cpu hotplug support */
>  int acpi_map_lsapic(acpi_handle handle, int *pcpu);

The rest looks good to me.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [PATCH 1/3] ACPI: save/restore ACPI NVS memory during hibernation

[Rafael J. Wysocki]

On Tuesday, 24 of June 2008, Rafael J. Wysocki wrote:
> On Tuesday, 24 of June 2008, Zhang Rui wrote:
> > According to the ACPI spec, ACPI NVS memory region is required to be
> > saved/restored by OS during hibernation.
> > 
> > Section 15.3.2 ACPI Spec 3.0b,
> > "OSPM will call the _PTS control method some time before entering a sleeping state,
> > to allow the platform’s AML code to update this memory image before entering the
> > sleeping state. After the system awakes from an S4 state, OSPM will restore this memory
> > area and call the _WAK control method to enable the BIOS to reclaim its memory image."
> > 
> > Add the mechanism to save/restore ACPI NVS memory during hibernation.
> > 
> > Note: now Linux save ACPI NVS memory in acpi_hibernation_pre_snapshot, and restore it in
> > 	acpi_hibernation_leave. Both of these functions will be invoked only once during
> > 	the hibernate and resume.
> > Note: in Section 14.3 ACPI spec 3.0b, I only get
> > 	"EfiACPIMemoryNVS: The OS and loader must preserve this memory range in
> > 	the working and ACPI S1–S3 states."
> > 	whether we should save/restore this piece of memory is not cleared.
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> 
> Well, I know what the specification says, but we tried that in the past and it
> caused regressions.
> 
> > ---
> >  drivers/acpi/sleep/main.c |  122 +++++++++++++++++++++++++++++++++++++++++++++-
> >  include/linux/acpi.h      |    3 +
> >  2 files changed, 124 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6/drivers/acpi/sleep/main.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/sleep/main.c	2008-06-20 09:27:00.000000000 +0800
> > +++ linux-2.6/drivers/acpi/sleep/main.c	2008-06-24 14:51:20.000000000 +0800
> > @@ -15,6 +15,7 @@
> >  #include <linux/dmi.h>
> >  #include <linux/device.h>
> >  #include <linux/suspend.h>
> > +#include <linux/highmem.h>
> >  
> >  #include <asm/io.h>
> >  
> > @@ -255,6 +256,109 @@
> >  #endif /* CONFIG_SUSPEND */
> >  
> >  #ifdef CONFIG_HIBERNATION
> > +
> > +/* ACPI NVS memory need to be saved/stored during hibernation */
> > +struct nvs_page {
> > +	unsigned long pfn;
> > +	void *data;
> > +	struct nvs_page *next;
> > +};

One more thing: I'd slightly prefer to use a list.h list instead of the open
coded one here.  It won't cost us much, so IMO it's worth doing for consistency
with the rest of the kernel.

> > +static struct nvs_page *nvs_pages;
> > +
> > +int acpi_mark_nvs_region(unsigned long start, unsigned long end)
> > +{
> > +	struct nvs_page *tmp;
> > +
> > +	while (start <= end) {
> > +		tmp = kzalloc(sizeof(struct nvs_page), GFP_KERNEL);
> > +		if (!tmp)
> > +			return -ENOMEM;
> 
> We should free the already allocated structures in this case IMO.
> 
> > +		tmp->pfn = start;
> > +		tmp->next = nvs_pages;
> > +		start++;
> > +		nvs_pages = tmp;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int acpi_hibernation_allocate_nvs_pages(void)
> > +{
> > +	struct nvs_page *tmp = nvs_pages;
> > +
> > +	while (tmp) {
> > +		tmp->data = (void *)__get_free_page(GFP_KERNEL);
> > +		if (!tmp->data)
> > +			return -ENOMEM;
> 
> Perhaps call acpi_hibernation_free_nvs_pages() here too.
> 
> > +		tmp = tmp->next;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int acpi_hibernation_free_nvs_pages(void)
> > +{
> > +	struct nvs_page *tmp = nvs_pages;
> > +
> > +	while (tmp) {
> > +		if (tmp->data) {
> > +			free_page((long)tmp->data);
> > +			tmp->data = NULL;
> > +		}
> > +		tmp = tmp->next;
> > +	}
> 
> Make nvs_pages = NULL explicitly, please.

Ah, sorry, scratch that.  I misread the code.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [PATCH 1/3] ACPI: save/restore ACPI NVS memory during hibernation

[Zhang Rui]

On Tue, 2008-06-24 at 18:18 +0800, Rafael J. Wysocki wrote:
> On Tuesday, 24 of June 2008, Zhang Rui wrote:
> > According to the ACPI spec, ACPI NVS memory region is required to be
> > saved/restored by OS during hibernation.
> >
> > Section 15.3.2 ACPI Spec 3.0b,
> > "OSPM will call the _PTS control method some time before entering a
> sleeping state,
> > to allow the platform’s AML code to update this memory image before
> entering the
> > sleeping state. After the system awakes from an S4 state, OSPM will
> restore this memory
> > area and call the _WAK control method to enable the BIOS to reclaim
> its memory image."
> >
> > Add the mechanism to save/restore ACPI NVS memory during
> hibernation.
> >
> > Note: now Linux save ACPI NVS memory in
> acpi_hibernation_pre_snapshot, and restore it in
> >       acpi_hibernation_leave. Both of these functions will be
> invoked only once during
> >       the hibernate and resume.
> > Note: in Section 14.3 ACPI spec 3.0b, I only get
> >       "EfiACPIMemoryNVS: The OS and loader must preserve this memory
> range in
> >       the working and ACPI S1–S3 states."
> >       whether we should save/restore this piece of memory is not
> cleared.
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> 
> Well, I know what the specification says, but we tried that in the
> past and it
> caused regressions.
Yes, Shaohua generated the prototype patches a couple of years ago.
But this is slightly different from the original one, e.g. not touching
ACPI DATA memory, saving ACPI NVS memory after _PTS and restore it
before _WAK strictly, etc.
We generate this patch set and hope it be helpful for some hibernate
specific issues.

> > +
> > +static int
> > +acpi_hibernation_notifier_cb(struct notifier_block *nb,
> > +                     unsigned long val, void *data)
> > +{
> > +     int result = 0;
> > +
> > +     switch (val) {
> > +     case PM_HIBERNATION_PREPARE:
> > +             result = acpi_hibernation_allocate_nvs_pages();
> > +             break;
> > +     case PM_POST_RESTORE: /* Failed */
> > +     case PM_POST_HIBERNATION:
> > +             acpi_hibernation_free_nvs_pages();
> > +     }
> > +     return result;
> > +}
> > +
> > +struct notifier_block acpi_hibernation_nb = {
> > +     .notifier_call = acpi_hibernation_notifier_cb,
> > +};
> 
> This is over the top, IMO.  Please put the appropriate call into
> acpi_hibernation_begin() and create acpi_hibernation_end() with the
> call to
> acpi_hibernation_free_nvs_pages() (it must do what acpi_pm_end() does
> apart
> from this).
Pages for ACPI NVS must be allocated before swsusp_shrink_memory.
And none platform hibernation ops can be used in this case.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [PATCH 1/3] ACPI: save/restore ACPI NVS memory during hibernation

[Rafael J. Wysocki]

On Wednesday, 25 of June 2008, Zhang Rui wrote:
> 
> On Tue, 2008-06-24 at 18:18 +0800, Rafael J. Wysocki wrote:
> > On Tuesday, 24 of June 2008, Zhang Rui wrote:
> > > According to the ACPI spec, ACPI NVS memory region is required to be
> > > saved/restored by OS during hibernation.
> > >
> > > Section 15.3.2 ACPI Spec 3.0b,
> > > "OSPM will call the _PTS control method some time before entering a
> > sleeping state,
> > > to allow the platform’s AML code to update this memory image before
> > entering the
> > > sleeping state. After the system awakes from an S4 state, OSPM will
> > restore this memory
> > > area and call the _WAK control method to enable the BIOS to reclaim
> > its memory image."
> > >
> > > Add the mechanism to save/restore ACPI NVS memory during
> > hibernation.
> > >
> > > Note: now Linux save ACPI NVS memory in
> > acpi_hibernation_pre_snapshot, and restore it in
> > >       acpi_hibernation_leave. Both of these functions will be
> > invoked only once during
> > >       the hibernate and resume.
> > > Note: in Section 14.3 ACPI spec 3.0b, I only get
> > >       "EfiACPIMemoryNVS: The OS and loader must preserve this memory
> > range in
> > >       the working and ACPI S1–S3 states."
> > >       whether we should save/restore this piece of memory is not
> > cleared.
> > >
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > 
> > Well, I know what the specification says, but we tried that in the
> > past and it
> > caused regressions.
> Yes, Shaohua generated the prototype patches a couple of years ago.
> But this is slightly different from the original one, e.g. not touching
> ACPI DATA memory, saving ACPI NVS memory after _PTS and restore it
> before _WAK strictly, etc.
> We generate this patch set and hope it be helpful for some hibernate
> specific issues.
> 
> > > +
> > > +static int
> > > +acpi_hibernation_notifier_cb(struct notifier_block *nb,
> > > +                     unsigned long val, void *data)
> > > +{
> > > +     int result = 0;
> > > +
> > > +     switch (val) {
> > > +     case PM_HIBERNATION_PREPARE:
> > > +             result = acpi_hibernation_allocate_nvs_pages();
> > > +             break;
> > > +     case PM_POST_RESTORE: /* Failed */
> > > +     case PM_POST_HIBERNATION:
> > > +             acpi_hibernation_free_nvs_pages();
> > > +     }
> > > +     return result;
> > > +}
> > > +
> > > +struct notifier_block acpi_hibernation_nb = {
> > > +     .notifier_call = acpi_hibernation_notifier_cb,
> > > +};
> > 
> > This is over the top, IMO.  Please put the appropriate call into
> > acpi_hibernation_begin() and create acpi_hibernation_end() with the
> > call to
> > acpi_hibernation_free_nvs_pages() (it must do what acpi_pm_end() does
> > apart
> > from this).
> Pages for ACPI NVS must be allocated before swsusp_shrink_memory.

Saying "must" in this context is way too strong IMO. :-)

First, you can allocate memory after swsusp_shrink_memory(), although you
can't allocate too much memory, where how much is "too much" really depends on
the requested size of the image.  It certanly cannot be more than the I/O and
spare pages reserve which is 5 MB at present.  I don't think there's so much
NVS memory to save in any case.

Still, if you really want to avoid the memory problem, it's better to change
the ordering between swsusp_shrink_memory() and platform_begin() in
kernel/power/disk.c:hibernation_snapshot() than to add a new notifier.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] [PATCH 1/3] ACPI: save/restore ACPI NVS memory during hibernation

[Len Brown]

note that linux-pm should be cc'd on suspend patches.

thanks,
-len