Re: [patch] hibernation: utilize ACPI hardware signature

On Fri, 2008-01-04 at 01:04 +0800, Rafael J. Wysocki wrote:

On Thursday, 3 of January 2008, Shaohua Li wrote:

quoted

On Wed, 2008-01-02 at 22:05 +0800, Rafael J. Wysocki wrote:

quoted

On Wednesday, 2 of January 2008, Shaohua Li wrote:

quoted

ACPI defines a hardware signature. BIOS calculates the signature
according to hardware configure, if hardware changes, the

signature

quoted

quoted

will

quoted

change, in this case, S4 resume should fail.

The idea is fine, but I'd prefer to do that in a more

straightforward

quoted

quoted

way.
Namely, we can just:
* write the signature into a variable in, for example,
  acpi_hibernation_prepare() (then, the "old" signature value will

be

quoted

quoted

  automatically saved in the image)
* compare it with a the "new" value read from the BIOS in
  acpi_hibernation_leave() and panic if there's a mismatch
* add a configuration option to disable this behavior (just in

case)

quoted

quoted

This way we can avoid modifying the entire generic interface to

add

quoted

quoted

the feature
specific to ACPI.

it would be better we do the check in boot kernel.

Franky, I think we should also check in the image kernel, in case the
boot
one doesn't support ACPI as I said.

Ok, makes sense. I changed to check the signature in .higberation_leave

Signed-off-by: Shaohua Li <redacted>
---
 drivers/acpi/sleep/main.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Index: linux/drivers/acpi/sleep/main.c
===================================================================

--- linux.orig/drivers/acpi/sleep/main.c	2008-01-03 13:37:08.000000000 +0800
+++ linux/drivers/acpi/sleep/main.c	2008-01-04 13:36:10.000000000 +0800

@@ -256,6 +256,17 @@ static int acpi_hibernation_enter(void)
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
 
+static unsigned long s4_hardware_signature;
+static struct acpi_table_facs *facs;
+static int nosigcheck;
+
+static int __init acpi_s4_nosigcheck(char *str)
+{
+	nosigcheck = 1;
+	return 1;
+}
+__setup("acpi_s4_nosigcheck", acpi_s4_nosigcheck);
+
 static void acpi_hibernation_leave(void)
 {
 	/*

@@ -263,6 +274,11 @@ static void acpi_hibernation_leave(void)
 	 * enable it here.
 	 */
 	acpi_enable();
+	if (facs && s4_hardware_signature != facs->hardware_signature) {
+		printk(KERN_EMERG"PM: Hardware changed in the S4 circle, can't resume\n");
+		if (!nosigcheck)
+			panic("S4 resume error");
+	}
 }
 
 static void acpi_hibernation_finish(void)

@@ -449,6 +465,10 @@ int __init acpi_sleep_init(void)
 		sleep_states[ACPI_STATE_S4] = 1;
 		printk(" S4");
 	}
+	acpi_get_table_by_index(ACPI_TABLE_INDEX_FACS,
+		(struct acpi_table_header **)&facs);
+	if (facs)
+		s4_hardware_signature = facs->hardware_signature;
 #endif
 	status = acpi_get_sleep_type_data(ACPI_STATE_S5, &type_a, &type_b);
 	if (ACPI_SUCCESS(status)) {