Hibernation: Check if ACPI is enabled during restore in the right place

The following scenario leads to total confusion of the platform firmware on some boxes (eg. HPC nx6325): * Hibernate with ACPI enabled * Resume passing "acpi=off" to the boot kernel To prevent this from happening it's necessary to check if ACPI is enabled (and enable it if that's not the case) _right_ _after_ control has been transfered from the boot kernel to the image kernel, before device_power_up() is called (ie. with interrupts disabled). Enabling ACPI after calling device_power_up() turns out to be insufficient. For this reason, introduce new hibernation callback ->leave() that will be executed before device_power_up() by the restored image kernel. To make it work, it also is necessary to move swsusp_suspend() from swsusp.c to disk.c (it's name is changed to "create_image", which is more up to the point). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Hibernation: Check if ACPI is enabled during restore in the right place
The following scenario leads to total confusion of the platform firmware on some boxes (eg. HPC nx6325): * Hibernate with ACPI enabled * Resume passing "acpi=off" to the boot kernel To prevent this from happening it's necessary to check if ACPI is enabled (and enable it if that's not the case) _right_ _after_ control has been transfered from the boot kernel to the image kernel, before device_power_up() is called (ie. with interrupts disabled). Enabling ACPI after calling device_power_up() turns out to be insufficient. For this reason, introduce new hibernation callback ->leave() that will be executed before device_power_up() by the restored image kernel. To make it work, it also is necessary to move swsusp_suspend() from swsusp.c to disk.c (it's name is changed to "create_image", which is more up to the point). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Acked-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Rafael J. Wysocki · Linus Torvalds
1 parent efa4d2fb04
Showing 5 changed files with 74 additions and 35 deletions Side-by-side Diff
drivers/acpi/sleep/main.c
include/linux/suspend.h
kernel/power/disk.c
kernel/power/power.h
kernel/power/swsusp.c
@@ -257,6 +257,15 @@
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
  
+static void acpi_hibernation_leave(void)
+{
+	/*
+	 * If ACPI is not enabled by the BIOS and the boot kernel, we need to
+	 * enable it here.
+	 */
+	acpi_enable();
+}
+
 static void acpi_hibernation_finish(void)
 {
 	acpi_leave_sleep_state(ACPI_STATE_S4);
@@ -288,6 +297,7 @@
 	.finish = acpi_hibernation_finish,
 	.prepare = acpi_hibernation_prepare,
 	.enter = acpi_hibernation_enter,
+	.leave = acpi_hibernation_leave,
 	.pre_restore = acpi_hibernation_pre_restore,
 	.restore_cleanup = acpi_hibernation_restore_cleanup,
 };
@@ -156,6 +156,12 @@
  *	Called after the nonboot CPUs have been disabled and all of the low
  *	level devices have been shut down (runs with IRQs off).
  *
+ * @leave: Perform the first stage of the cleanup after the system sleep state
+ *	indicated by @set_target() has been left.
+ *	Called right after the control has been passed from the boot kernel to
+ *	the image kernel, before the nonboot CPUs are enabled and before devices
+ *	are resumed.  Executed with interrupts disabled.
+ *
  * @pre_restore: Prepare system for the restoration from a hibernation image.
  *	Called right after devices have been frozen and before the nonboot
  *	CPUs are disabled (runs with IRQs on).
@@ -170,6 +176,7 @@
 	void (*finish)(void);
 	int (*prepare)(void);
 	int (*enter)(void);
+	void (*leave)(void);
 	int (*pre_restore)(void);
 	void (*restore_cleanup)(void);
 };
@@ -93,6 +93,17 @@
 }
  
 /**
+ *	platform_leave - prepare the machine for switching to the normal mode
+ *	of operation using the platform driver (called with interrupts disabled)
+ */
+
+static void platform_leave(int platform_mode)
+{
+	if (platform_mode && hibernation_ops)
+		hibernation_ops->leave();
+}
+
+/**
  *	platform_finish - switch the machine to the normal mode of operation
  *	using the platform driver (must be called after platform_prepare())
  */
@@ -129,6 +140,51 @@
 }
  
 /**
+ *	create_image - freeze devices that need to be frozen with interrupts
+ *	off, create the hibernation image and thaw those devices.  Control
+ *	reappears in this routine after a restore.
+ */
+
+int create_image(int platform_mode)
+{
+	int error;
+
+	error = arch_prepare_suspend();
+	if (error)
+		return error;
+
+	local_irq_disable();
+	/* At this point, device_suspend() has been called, but *not*
+	 * device_power_down(). We *must* call device_power_down() now.
+	 * Otherwise, drivers for some devices (e.g. interrupt controllers)
+	 * become desynchronized with the actual state of the hardware
+	 * at resume time, and evil weirdness ensues.
+	 */
+	error = device_power_down(PMSG_FREEZE);
+	if (error) {
+		printk(KERN_ERR "Some devices failed to power down, "
+			KERN_ERR "aborting suspend\n");
+		goto Enable_irqs;
+	}
+
+	save_processor_state();
+	error = swsusp_arch_suspend();
+	if (error)
+		printk(KERN_ERR "Error %d while creating the image\n", error);
+	/* Restore control flow magically appears here */
+	restore_processor_state();
+	if (!in_suspend)
+		platform_leave(platform_mode);
+	/* NOTE:  device_power_up() is just a resume() for devices
+	 * that suspended with irqs off ... no overall powerup.
+	 */
+	device_power_up();
+ Enable_irqs:
+	local_irq_enable();
+	return error;
+}
+
+/**
  *	hibernation_snapshot - quiesce devices and create the hibernation
  *	snapshot image.
  *	@platform_mode - if set, use the platform driver, if available, to
@@ -163,7 +219,7 @@
 	if (!error) {
 		if (hibernation_mode != HIBERNATION_TEST) {
 			in_suspend = 1;
-			error = swsusp_suspend();
+			error = create_image(platform_mode);
 			/* Control returns here after successful restore */
 		} else {
 			printk("swsusp debug: Waiting for 5 seconds.\n");
@@ -183,7 +183,6 @@
 extern int swsusp_check(void);
 extern int swsusp_shrink_memory(void);
 extern void swsusp_free(void);
-extern int swsusp_suspend(void);
 extern int swsusp_resume(void);
 extern int swsusp_read(unsigned int *flags_p);
 extern int swsusp_write(unsigned int flags);
@@ -270,39 +270,6 @@
 	return 0;
 }
  
-int swsusp_suspend(void)
-{
-	int error;
-
-	if ((error = arch_prepare_suspend()))
-		return error;
-
-	local_irq_disable();
-	/* At this point, device_suspend() has been called, but *not*
-	 * device_power_down(). We *must* device_power_down() now.
-	 * Otherwise, drivers for some devices (e.g. interrupt controllers)
-	 * become desynchronized with the actual state of the hardware
-	 * at resume time, and evil weirdness ensues.
-	 */
-	if ((error = device_power_down(PMSG_FREEZE))) {
-		printk(KERN_ERR "Some devices failed to power down, aborting suspend\n");
-		goto Enable_irqs;
-	}
-
-	save_processor_state();
-	if ((error = swsusp_arch_suspend()))
-		printk(KERN_ERR "Error %d suspending\n", error);
-	/* Restore control flow magically appears here */
-	restore_processor_state();
-	/* NOTE:  device_power_up() is just a resume() for devices
-	 * that suspended with irqs off ... no overall powerup.
-	 */
-	device_power_up();
- Enable_irqs:
-	local_irq_enable();
-	return error;
-}
-
 int swsusp_resume(void)
 {
 	int error;
...	...	@@ -257,6 +257,15 @@
257	257	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
258	258	}
259	259
	260	+static void acpi_hibernation_leave(void)
	261	+{
	262	+ /*
	263	+ * If ACPI is not enabled by the BIOS and the boot kernel, we need to
	264	+ * enable it here.
	265	+ */
	266	+ acpi_enable();
	267	+}
	268	+
260	269	static void acpi_hibernation_finish(void)
261	270	{
262	271	acpi_leave_sleep_state(ACPI_STATE_S4);
...	...	@@ -288,6 +297,7 @@
288	297	.finish = acpi_hibernation_finish,
289	298	.prepare = acpi_hibernation_prepare,
290	299	.enter = acpi_hibernation_enter,
	300	+ .leave = acpi_hibernation_leave,
291	301	.pre_restore = acpi_hibernation_pre_restore,
292	302	.restore_cleanup = acpi_hibernation_restore_cleanup,
293	303	};
...	...	@@ -156,6 +156,12 @@
156	156	* Called after the nonboot CPUs have been disabled and all of the low
157	157	* level devices have been shut down (runs with IRQs off).
158	158	*
	159	+ * @leave: Perform the first stage of the cleanup after the system sleep state
	160	+ * indicated by @set_target() has been left.
	161	+ * Called right after the control has been passed from the boot kernel to
	162	+ * the image kernel, before the nonboot CPUs are enabled and before devices
	163	+ * are resumed. Executed with interrupts disabled.
	164	+ *
159	165	* @pre_restore: Prepare system for the restoration from a hibernation image.
160	166	* Called right after devices have been frozen and before the nonboot
161	167	* CPUs are disabled (runs with IRQs on).
...	...	@@ -170,6 +176,7 @@
170	176	void (*finish)(void);
171	177	int (*prepare)(void);
172	178	int (*enter)(void);
	179	+ void (*leave)(void);
173	180	int (*pre_restore)(void);
174	181	void (*restore_cleanup)(void);
175	182	};
...	...	@@ -93,6 +93,17 @@
93	93	}
94	94
95	95	/**
	96	+ * platform_leave - prepare the machine for switching to the normal mode
	97	+ * of operation using the platform driver (called with interrupts disabled)
	98	+ */
	99	+
	100	+static void platform_leave(int platform_mode)
	101	+{
	102	+ if (platform_mode && hibernation_ops)
	103	+ hibernation_ops->leave();
	104	+}
	105	+
	106	+/**
96	107	* platform_finish - switch the machine to the normal mode of operation
97	108	* using the platform driver (must be called after platform_prepare())
98	109	*/
...	...	@@ -129,6 +140,51 @@
129	140	}
130	141
131	142	/**
	143	+ * create_image - freeze devices that need to be frozen with interrupts
	144	+ * off, create the hibernation image and thaw those devices. Control
	145	+ * reappears in this routine after a restore.
	146	+ */
	147	+
	148	+int create_image(int platform_mode)
	149	+{
	150	+ int error;
	151	+
	152	+ error = arch_prepare_suspend();
	153	+ if (error)
	154	+ return error;
	155	+
	156	+ local_irq_disable();
	157	+ /* At this point, device_suspend() has been called, but not
	158	+ * device_power_down(). We must call device_power_down() now.
	159	+ * Otherwise, drivers for some devices (e.g. interrupt controllers)
	160	+ * become desynchronized with the actual state of the hardware
	161	+ * at resume time, and evil weirdness ensues.
	162	+ */
	163	+ error = device_power_down(PMSG_FREEZE);
	164	+ if (error) {
	165	+ printk(KERN_ERR "Some devices failed to power down, "
	166	+ KERN_ERR "aborting suspend\n");
	167	+ goto Enable_irqs;
	168	+ }
	169	+
	170	+ save_processor_state();
	171	+ error = swsusp_arch_suspend();
	172	+ if (error)
	173	+ printk(KERN_ERR "Error %d while creating the image\n", error);
	174	+ /* Restore control flow magically appears here */
	175	+ restore_processor_state();
	176	+ if (!in_suspend)
	177	+ platform_leave(platform_mode);
	178	+ /* NOTE: device_power_up() is just a resume() for devices
	179	+ * that suspended with irqs off ... no overall powerup.
	180	+ */
	181	+ device_power_up();
	182	+ Enable_irqs:
	183	+ local_irq_enable();
	184	+ return error;
	185	+}
	186	+
	187	+/**
132	188	* hibernation_snapshot - quiesce devices and create the hibernation
133	189	* snapshot image.
134	190	* @platform_mode - if set, use the platform driver, if available, to
...	...	@@ -163,7 +219,7 @@
163	219	if (!error) {
164	220	if (hibernation_mode != HIBERNATION_TEST) {
165	221	in_suspend = 1;
166		- error = swsusp_suspend();
	222	+ error = create_image(platform_mode);
167	223	/* Control returns here after successful restore */
168	224	} else {
169	225	printk("swsusp debug: Waiting for 5 seconds.\n");
...	...	@@ -183,7 +183,6 @@
183	183	extern int swsusp_check(void);
184	184	extern int swsusp_shrink_memory(void);
185	185	extern void swsusp_free(void);
186		-extern int swsusp_suspend(void);
187	186	extern int swsusp_resume(void);
188	187	extern int swsusp_read(unsigned int *flags_p);
189	188	extern int swsusp_write(unsigned int flags);
...	...	@@ -270,39 +270,6 @@
270	270	return 0;
271	271	}
272	272
273		-int swsusp_suspend(void)
274		-{
275		- int error;
276		-
277		- if ((error = arch_prepare_suspend()))
278		- return error;
279		-
280		- local_irq_disable();
281		- /* At this point, device_suspend() has been called, but not
282		- * device_power_down(). We must device_power_down() now.
283		- * Otherwise, drivers for some devices (e.g. interrupt controllers)
284		- * become desynchronized with the actual state of the hardware
285		- * at resume time, and evil weirdness ensues.
286		- */
287		- if ((error = device_power_down(PMSG_FREEZE))) {
288		- printk(KERN_ERR "Some devices failed to power down, aborting suspend\n");
289		- goto Enable_irqs;
290		- }
291		-
292		- save_processor_state();
293		- if ((error = swsusp_arch_suspend()))
294		- printk(KERN_ERR "Error %d suspending\n", error);
295		- /* Restore control flow magically appears here */
296		- restore_processor_state();
297		- /* NOTE: device_power_up() is just a resume() for devices
298		- * that suspended with irqs off ... no overall powerup.
299		- */
300		- device_power_up();
301		- Enable_irqs:
302		- local_irq_enable();
303		- return error;
304		-}
305		-
306	273	int swsusp_resume(void)
307	274	{
308	275	int error;