2002-06-03 20:14:19

by Pavel Machek

[permalink] [raw]
Subject: Fix suspend-to-RAM in 2.5.20

Hi!

I created arch/i386/suspend.c not to clash with ACPI people so much in
future. (More stuff is going to move into it in the future, to clean
up functions that really do not belong to the headers.) Please apply,

Pavel

--- clean/arch/i386/kernel/Makefile Thu May 30 12:21:00 2002
+++ linux-swsusp/arch/i386/kernel/Makefile Mon Jun 3 19:06:05 2002
@@ -24,6 +24,8 @@
obj-$(CONFIG_SMP) += smp.o smpboot.o trampoline.o
obj-$(CONFIG_X86_LOCAL_APIC) += mpparse.o apic.o nmi.o
obj-$(CONFIG_X86_IO_APIC) += io_apic.o
+obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o
+obj-$(CONFIG_ACPI_SLEEP) += suspend.o
ifdef CONFIG_VISWS
obj-y += setup-visws.o
obj-$(CONFIG_X86_VISWS_APIC) += visws_apic.o
--- clean/arch/i386/kernel/suspend.c Mon Jun 3 11:48:17 2002
+++ linux-swsusp/arch/i386/kernel/suspend.c Mon Jun 3 19:01:07 2002
@@ -0,0 +1,45 @@
+/*
+ * Suspend support specific for i386.
+ *
+ * Distribute under GPLv2
+ *
+ * Copyright (c) 2002 Pavel Machek <[email protected]>
+ */
+
+#define ACPI_C
+#include <linux/config.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/poll.h>
+#include <linux/delay.h>
+#include <linux/sysrq.h>
+#include <linux/compatmac.h>
+#include <linux/proc_fs.h>
+#include <linux/irq.h>
+#include <linux/pm.h>
+#include <linux/device.h>
+#include <linux/suspend.h>
+#include <asm/uaccess.h>
+#include <asm/acpi.h>
+
+
+void do_suspend_lowlevel(int resume)
+{
+/*
+ * FIXME: This function should really be written in assembly. Actually
+ * requirement is that it does not touch stack, because %esp will be
+ * wrong during resume before restore_processor_context(). Check
+ * assembly if you modify this.
+ */
+ if (!resume) {
+ save_processor_context();
+ acpi_save_register_state((unsigned long)&&acpi_sleep_done);
+ acpi_enter_sleep_state(3);
+ return;
+ }
+acpi_sleep_done:
+ restore_processor_context();
+}
--- clean/drivers/acpi/system.c Mon Jun 3 11:43:30 2002
+++ linux-swsusp/drivers/acpi/system.c Mon Jun 3 18:57:34 2002
@@ -260,10 +260,8 @@
u32 state)
{
acpi_status status = AE_ERROR;
-#if 0
unsigned long flags = 0;

- /* this is very broken, so don't do anything until it's fixed */
save_flags(flags);

switch (state)
@@ -289,8 +287,6 @@
fix_processor_context();

restore_flags(flags);
-#endif
- printk("ACPI: ACPI-based suspend currently broken, aborting\n");

return status;
}
--- clean/include/asm-i386/suspend.h Thu May 30 12:21:13 2002
+++ linux-swsusp/include/asm-i386/suspend.h Mon Jun 3 19:15:52 2002
@@ -294,3 +294,27 @@
}
#endif

+#ifdef CONFIG_ACPI_SLEEP
+extern unsigned long saved_eip;
+extern unsigned long saved_esp;
+extern unsigned long saved_ebp;
+extern unsigned long saved_ebx;
+extern unsigned long saved_esi;
+extern unsigned long saved_edi;
+
+static inline void acpi_save_register_state(unsigned long return_point)
+{
+ saved_eip = return_point;
+ asm volatile ("movl %%esp,(%0)" : "=m" (saved_esp));
+ asm volatile ("movl %%ebp,(%0)" : "=m" (saved_ebp));
+ asm volatile ("movl %%ebx,(%0)" : "=m" (saved_ebx));
+ asm volatile ("movl %%edi,(%0)" : "=m" (saved_edi));
+ asm volatile ("movl %%esi,(%0)" : "=m" (saved_esi));
+}
+
+#define acpi_restore_register_state() do {} while (0)
+
+/* routines for saving/restoring kernel state */
+extern int acpi_save_state_mem(void);
+extern int acpi_save_state_disk(void);
+#endif

--
(about SSSCA) "I don't say this lightly. However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa


2002-06-03 20:33:27

by Andrew Grover

[permalink] [raw]
Subject: RE: Fix suspend-to-RAM in 2.5.20

Pavel,

Please keep the ifdefs in place in acpi/system.c. It's marked broken for a
reason until I have a chance to look at it.

Also -- what's the point of moving obviously ACPI-specific code into
suspend.c, instead of leaving it in acpi.c?

Regards -- Andy

> From: Pavel Machek [mailto:[email protected]]
> I created arch/i386/suspend.c not to clash with ACPI people so much in
> future. (More stuff is going to move into it in the future, to clean
> up functions that really do not belong to the headers.) Please apply,
>
>
> Pavel
>
> --- clean/arch/i386/kernel/Makefile Thu May 30 12:21:00 2002
> +++ linux-swsusp/arch/i386/kernel/Makefile Mon Jun 3 19:06:05 2002
> @@ -24,6 +24,8 @@
> obj-$(CONFIG_SMP) += smp.o smpboot.o trampoline.o
> obj-$(CONFIG_X86_LOCAL_APIC) += mpparse.o apic.o nmi.o
> obj-$(CONFIG_X86_IO_APIC) += io_apic.o
> +obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o
> +obj-$(CONFIG_ACPI_SLEEP) += suspend.o
> ifdef CONFIG_VISWS
> obj-y += setup-visws.o
> obj-$(CONFIG_X86_VISWS_APIC) += visws_apic.o
> --- clean/arch/i386/kernel/suspend.c Mon Jun 3 11:48:17 2002
> +++ linux-swsusp/arch/i386/kernel/suspend.c Mon Jun 3 19:01:07 2002
> @@ -0,0 +1,45 @@
> +/*
> + * Suspend support specific for i386.
> + *
> + * Distribute under GPLv2
> + *
> + * Copyright (c) 2002 Pavel Machek <[email protected]>
> + */
> +
> +#define ACPI_C
> +#include <linux/config.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/spinlock.h>
> +#include <linux/poll.h>
> +#include <linux/delay.h>
> +#include <linux/sysrq.h>
> +#include <linux/compatmac.h>
> +#include <linux/proc_fs.h>
> +#include <linux/irq.h>
> +#include <linux/pm.h>
> +#include <linux/device.h>
> +#include <linux/suspend.h>
> +#include <asm/uaccess.h>
> +#include <asm/acpi.h>
> +
> +
> +void do_suspend_lowlevel(int resume)
> +{
> +/*
> + * FIXME: This function should really be written in
> assembly. Actually
> + * requirement is that it does not touch stack, because %esp will be
> + * wrong during resume before restore_processor_context(). Check
> + * assembly if you modify this.
> + */
> + if (!resume) {
> + save_processor_context();
> + acpi_save_register_state((unsigned
> long)&&acpi_sleep_done);
> + acpi_enter_sleep_state(3);
> + return;
> + }
> +acpi_sleep_done:
> + restore_processor_context();
> +}
> --- clean/drivers/acpi/system.c Mon Jun 3 11:43:30 2002
> +++ linux-swsusp/drivers/acpi/system.c Mon Jun 3 18:57:34 2002
> @@ -260,10 +260,8 @@
> u32 state)
> {
> acpi_status status = AE_ERROR;
> -#if 0
> unsigned long flags = 0;
>
> - /* this is very broken, so don't do anything until it's fixed */
> save_flags(flags);
>
> switch (state)
> @@ -289,8 +287,6 @@
> fix_processor_context();
>
> restore_flags(flags);
> -#endif
> - printk("ACPI: ACPI-based suspend currently broken, aborting\n");
>
> return status;
> }
> --- clean/include/asm-i386/suspend.h Thu May 30 12:21:13 2002
> +++ linux-swsusp/include/asm-i386/suspend.h Mon Jun 3 19:15:52 2002
> @@ -294,3 +294,27 @@
> }
> #endif
>
> +#ifdef CONFIG_ACPI_SLEEP
> +extern unsigned long saved_eip;
> +extern unsigned long saved_esp;
> +extern unsigned long saved_ebp;
> +extern unsigned long saved_ebx;
> +extern unsigned long saved_esi;
> +extern unsigned long saved_edi;
> +
> +static inline void acpi_save_register_state(unsigned long
> return_point)
> +{
> + saved_eip = return_point;
> + asm volatile ("movl %%esp,(%0)" : "=m" (saved_esp));
> + asm volatile ("movl %%ebp,(%0)" : "=m" (saved_ebp));
> + asm volatile ("movl %%ebx,(%0)" : "=m" (saved_ebx));
> + asm volatile ("movl %%edi,(%0)" : "=m" (saved_edi));
> + asm volatile ("movl %%esi,(%0)" : "=m" (saved_esi));
> +}
> +
> +#define acpi_restore_register_state() do {} while (0)
> +
> +/* routines for saving/restoring kernel state */
> +extern int acpi_save_state_mem(void);
> +extern int acpi_save_state_disk(void);
> +#endif
>
> --
> (about SSSCA) "I don't say this lightly. However, I really
> think that the U.S.
> no longer is classifiable as a democracy, but rather as a
> plutocracy." --hpa
> -
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2002-06-03 20:50:35

by Pavel Machek

[permalink] [raw]
Subject: Re: Fix suspend-to-RAM in 2.5.20

Hi!

> Please keep the ifdefs in place in acpi/system.c. It's marked broken for a
> reason until I have a chance to look at it.

I tried to fix it. I believe I'll have it working within 30 minutes
with followup patch. Well, so it was less than 30 ;-).

> Also -- what's the point of moving obviously ACPI-specific code into
> suspend.c, instead of leaving it in acpi.c?

The code is also suspend-specific, and suspend-to-disk will have very
similar function in very near future.

Here's followup patch that makes it work. Notice freeze_processes() --
if you don't do that you risk data corruption. Please apply,
Pavel

--- linux-swsusp.linus/arch/i386/kernel/suspend.c Mon Jun 3 19:01:07 2002
+++ linux-swsusp/arch/i386/kernel/suspend.c Mon Jun 3 22:35:15 2002
@@ -41,5 +41,6 @@
return;
}
acpi_sleep_done:
+ acpi_restore_register_state();
restore_processor_context();
}
--- linux-swsusp.linus/drivers/acpi/system.c Mon Jun 3 18:57:34 2002
+++ linux-swsusp/drivers/acpi/system.c Mon Jun 3 22:42:25 2002
@@ -267,25 +267,15 @@
switch (state)
{
case ACPI_STATE_S1:
- /* do nothing */
+ barrier();
+ status = acpi_enter_sleep_state(state);
break;

case ACPI_STATE_S2:
case ACPI_STATE_S3:
- save_processor_context();
- /* TODO: this is horribly broken, fix it */
- /* TODO: inline this function in acpi_suspend,or something. */
+ do_suspend_lowlevel(0);
break;
}
-
- barrier();
- status = acpi_enter_sleep_state(state);
-
-acpi_sleep_done:
-
- restore_processor_context();
- fix_processor_context();
-
restore_flags(flags);

return status;
@@ -307,6 +297,8 @@
if (state < ACPI_STATE_S1 || state > ACPI_STATE_S5)
return AE_ERROR;

+ freeze_processes(); /* device_suspend needs processes to be stopped */
+
/* do we have a wakeup address for S2 and S3? */
if (state == ACPI_STATE_S2 || state == ACPI_STATE_S3) {
if (!acpi_wakeup_address)
@@ -339,6 +331,7 @@

/* reset firmware waking vector */
acpi_set_firmware_waking_vector((ACPI_PHYSICAL_ADDRESS) 0);
+ thaw_processes();

return status;
}


--
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.