Jan Beulich
2008-Apr-04 11:44 UTC
[Xen-devel] [PATCH] linux/x86: fix powering off certain machines
Dell''s Precision490, for example, fails to properly power off without
going through the full sequence of operations in the hypervisor.
Also fix a compiler warning and the improper use of a hypervisor
return value as ACPI status.
As usual, written and tested on 2.6.25-rc8 and 2.6.16.60 and made apply
to the 2.6.18 tree without further testing.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Index: sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c
==================================================================---
sle10sp2-2008-03-31.orig/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04
11:12:58.000000000 +0200
+++ sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04
11:14:46.000000000 +0200
@@ -110,25 +110,4 @@ static int __init acpisleep_dmi_init(voi
}
core_initcall(acpisleep_dmi_init);
-
-#else /* CONFIG_ACPI_PV_SLEEP */
-#include <asm/hypervisor.h>
-#include <xen/interface/platform.h>
-int acpi_notify_hypervisor_state(u8 sleep_state,
- u32 pm1a_cnt, u32 pm1b_cnt)
-{
- struct xen_platform_op op = {
- .cmd = XENPF_enter_acpi_sleep,
- .interface_version = XENPF_INTERFACE_VERSION,
- .u = {
- .enter_acpi_sleep = {
- .pm1a_cnt_val = (u16)pm1a_cnt,
- .pm1b_cnt_val = (u16)pm1b_cnt,
- .sleep_state = sleep_state,
- },
- },
- };
-
- return HYPERVISOR_platform_op(&op);
-}
#endif /* CONFIG_ACPI_PV_SLEEP */
Index: sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c
==================================================================---
sle10sp2-2008-03-31.orig/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04
11:12:58.000000000 +0200
+++ sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04
11:15:23.000000000 +0200
@@ -137,29 +137,6 @@ static int __init acpi_sleep_setup(char
}
__setup("acpi_sleep=", acpi_sleep_setup);
-
-#else /* CONFIG_ACPI_PV_SLEEP */
-#include <asm/hypervisor.h>
-#include <xen/interface/platform.h>
-int acpi_notify_hypervisor_state(u8 sleep_state,
- u32 pm1a_cnt, u32 pm1b_cnt)
-{
- struct xen_platform_op op = {
- .cmd = XENPF_enter_acpi_sleep,
- .interface_version = XENPF_INTERFACE_VERSION,
- .u = {
- .enter_acpi_sleep = {
- .pm1a_cnt_val = (u16)pm1a_cnt,
- .pm1b_cnt_val = (u16)pm1b_cnt,
- .sleep_state = sleep_state,
- },
- },
- };
-
- return HYPERVISOR_platform_op(&op);
-}
-#endif /* CONFIG_ACPI_PV_SLEEP */
-
#endif /*CONFIG_ACPI_SLEEP */
void acpi_pci_link_exit(void)
Index: sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c
==================================================================---
sle10sp2-2008-03-31.orig/drivers/acpi/hardware/hwsleep.c 2008-04-04
11:12:58.000000000 +0200
+++ sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c 2008-04-04
11:16:18.000000000 +0200
@@ -227,7 +227,11 @@ acpi_status asmlinkage acpi_enter_sleep_
u32 PM1Bcontrol;
struct acpi_bit_register_info *sleep_type_reg_info;
struct acpi_bit_register_info *sleep_enable_reg_info;
+#ifndef CONFIG_XEN
u32 in_value;
+#else
+ int err;
+#endif
acpi_status status;
ACPI_FUNCTION_TRACE(acpi_enter_sleep_state);
@@ -327,7 +329,7 @@ acpi_status asmlinkage acpi_enter_sleep_
ACPI_FLUSH_CPU_CACHE();
-#ifndef CONFIG_ACPI_PV_SLEEP
+#ifndef CONFIG_XEN
status = acpi_hw_register_write(ACPI_MTX_DO_NOT_LOCK,
ACPI_REGISTER_PM1A_CONTROL,
PM1Acontrol);
@@ -377,17 +379,18 @@ acpi_status asmlinkage acpi_enter_sleep_
/* Spin until we wake */
} while (!in_value);
-
- return_ACPI_STATUS(AE_OK);
#else
/* PV ACPI just need check hypercall return value */
- status = acpi_notify_hypervisor_state(sleep_state,
+ err = acpi_notify_hypervisor_state(sleep_state,
PM1Acontrol, PM1Bcontrol);
- if (ACPI_FAILURE(status))
- return_ACPI_STATUS(status);
- else
- return_ACPI_STATUS(AE_OK);
+ if (err) {
+ ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+ "Hypervisor failure [%d]\n", err));
+ return_ACPI_STATUS(AE_ERROR);
+ }
#endif
+
+ return_ACPI_STATUS(AE_OK);
}
ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state)
Index: sle10sp2-2008-03-31/include/asm-i386/acpi.h
==================================================================---
sle10sp2-2008-03-31.orig/include/asm-i386/acpi.h 2007-12-10 11:30:46.000000000
+0100
+++ sle10sp2-2008-03-31/include/asm-i386/acpi.h 2008-04-04 11:21:10.000000000
+0200
@@ -31,6 +31,9 @@
#include <acpi/pdc_intel.h>
#include <asm/system.h> /* defines cmpxchg */
+#ifdef CONFIG_XEN
+#include <xen/interface/platform.h>
+#endif
#define COMPILER_DEPENDENT_INT64 long long
#define COMPILER_DEPENDENT_UINT64 unsigned long long
@@ -154,6 +157,27 @@ static inline void acpi_disable_pci(void
}
extern int acpi_irq_balance_set(char *str);
+#ifdef CONFIG_XEN
+static inline int acpi_notify_hypervisor_state(u8 sleep_state,
+ u32 pm1a_cnt_val,
+ u32 pm1b_cnt_val)
+{
+ struct xen_platform_op op = {
+ .cmd = XENPF_enter_acpi_sleep,
+ .interface_version = XENPF_INTERFACE_VERSION,
+ .u = {
+ .enter_acpi_sleep = {
+ .pm1a_cnt_val = pm1a_cnt_val,
+ .pm1b_cnt_val = pm1b_cnt_val,
+ .sleep_state = sleep_state,
+ },
+ },
+ };
+
+ return HYPERVISOR_platform_op(&op);
+}
+#endif /* CONFIG_XEN */
+
#else /* !CONFIG_ACPI */
#define acpi_lapic 0
@@ -175,10 +199,6 @@ extern unsigned long acpi_wakeup_address
/* early initialization routine */
extern void acpi_reserve_bootmem(void);
-#ifdef CONFIG_ACPI_PV_SLEEP
-extern int acpi_notify_hypervisor_state(u8 sleep_state,
- u32 pm1a_cnt, u32 pm1b_cnt);
-#endif /* CONFIG_ACPI_PV_SLEEP */
#endif /*CONFIG_ACPI_SLEEP*/
extern u8 x86_acpiid_to_apicid[];
Index: sle10sp2-2008-03-31/include/asm-x86_64/acpi.h
==================================================================---
sle10sp2-2008-03-31.orig/include/asm-x86_64/acpi.h 2008-01-31 11:23:28.000000000
+0100
+++ sle10sp2-2008-03-31/include/asm-x86_64/acpi.h 2008-04-04 11:20:51.000000000
+0200
@@ -28,6 +28,9 @@
#ifdef __KERNEL__
+#ifdef CONFIG_XEN
+#include <xen/interface/platform.h>
+#endif
#include <acpi/pdc_intel.h>
#define COMPILER_DEPENDENT_INT64 long long
@@ -129,6 +132,27 @@ static inline void acpi_disable_pci(void
}
extern int acpi_irq_balance_set(char *str);
+#ifdef CONFIG_XEN
+static inline int acpi_notify_hypervisor_state(u8 sleep_state,
+ u32 pm1a_cnt_val,
+ u32 pm1b_cnt_val)
+{
+ struct xen_platform_op op = {
+ .cmd = XENPF_enter_acpi_sleep,
+ .interface_version = XENPF_INTERFACE_VERSION,
+ .u = {
+ .enter_acpi_sleep = {
+ .pm1a_cnt_val = pm1a_cnt_val,
+ .pm1b_cnt_val = pm1b_cnt_val,
+ .sleep_state = sleep_state,
+ },
+ },
+ };
+
+ return HYPERVISOR_platform_op(&op);
+}
+#endif /* CONFIG_XEN */
+
#else /* !CONFIG_ACPI */
#define acpi_lapic 0
@@ -152,11 +176,6 @@ extern unsigned long acpi_wakeup_address
/* early initialization routine */
extern void acpi_reserve_bootmem(void);
-
-#ifdef CONFIG_ACPI_PV_SLEEP
-extern int acpi_notify_hypervisor_state(u8 sleep_state,
- u32 pm1a_cnt, u32 pm1b_cnt);
-#endif /* CONFIG_ACPI_PV_SLEEP */
#endif /*CONFIG_ACPI_SLEEP*/
#define boot_cpu_physical_apicid boot_cpu_id
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2008-Apr-04 11:55 UTC
Re: [Xen-devel] [PATCH] linux/x86: fix powering off certain machines
The patch seems to mostly move code around. What does it actually fix, and how? -- Keir On 4/4/08 12:44, "Jan Beulich" <jbeulich@novell.com> wrote:> Dell''s Precision490, for example, fails to properly power off without > going through the full sequence of operations in the hypervisor. > > Also fix a compiler warning and the improper use of a hypervisor > return value as ACPI status. > > As usual, written and tested on 2.6.25-rc8 and 2.6.16.60 and made apply > to the 2.6.18 tree without further testing. > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > > Index: sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c > ==================================================================> --- sle10sp2-2008-03-31.orig/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04 > 11:12:58.000000000 +0200 > +++ sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04 > 11:14:46.000000000 +0200 > @@ -110,25 +110,4 @@ static int __init acpisleep_dmi_init(voi > } > > core_initcall(acpisleep_dmi_init); > - > -#else /* CONFIG_ACPI_PV_SLEEP */ > -#include <asm/hypervisor.h> > -#include <xen/interface/platform.h> > -int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt) > -{ > - struct xen_platform_op op = { > - .cmd = XENPF_enter_acpi_sleep, > - .interface_version = XENPF_INTERFACE_VERSION, > - .u = { > - .enter_acpi_sleep = { > - .pm1a_cnt_val = (u16)pm1a_cnt, > - .pm1b_cnt_val = (u16)pm1b_cnt, > - .sleep_state = sleep_state, > - }, > - }, > - }; > - > - return HYPERVISOR_platform_op(&op); > -} > #endif /* CONFIG_ACPI_PV_SLEEP */ > Index: sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c > ==================================================================> --- sle10sp2-2008-03-31.orig/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04 > 11:12:58.000000000 +0200 > +++ sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04 > 11:15:23.000000000 +0200 > @@ -137,29 +137,6 @@ static int __init acpi_sleep_setup(char > } > > __setup("acpi_sleep=", acpi_sleep_setup); > - > -#else /* CONFIG_ACPI_PV_SLEEP */ > -#include <asm/hypervisor.h> > -#include <xen/interface/platform.h> > -int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt) > -{ > - struct xen_platform_op op = { > - .cmd = XENPF_enter_acpi_sleep, > - .interface_version = XENPF_INTERFACE_VERSION, > - .u = { > - .enter_acpi_sleep = { > - .pm1a_cnt_val = (u16)pm1a_cnt, > - .pm1b_cnt_val = (u16)pm1b_cnt, > - .sleep_state = sleep_state, > - }, > - }, > - }; > - > - return HYPERVISOR_platform_op(&op); > -} > -#endif /* CONFIG_ACPI_PV_SLEEP */ > - > #endif /*CONFIG_ACPI_SLEEP */ > > void acpi_pci_link_exit(void) > Index: sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c > ==================================================================> --- sle10sp2-2008-03-31.orig/drivers/acpi/hardware/hwsleep.c 2008-04-04 > 11:12:58.000000000 +0200 > +++ sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c 2008-04-04 > 11:16:18.000000000 +0200 > @@ -227,7 +227,11 @@ acpi_status asmlinkage acpi_enter_sleep_ > u32 PM1Bcontrol; > struct acpi_bit_register_info *sleep_type_reg_info; > struct acpi_bit_register_info *sleep_enable_reg_info; > +#ifndef CONFIG_XEN > u32 in_value; > +#else > + int err; > +#endif > acpi_status status; > > ACPI_FUNCTION_TRACE(acpi_enter_sleep_state); > @@ -327,7 +329,7 @@ acpi_status asmlinkage acpi_enter_sleep_ > > ACPI_FLUSH_CPU_CACHE(); > > -#ifndef CONFIG_ACPI_PV_SLEEP > +#ifndef CONFIG_XEN > status = acpi_hw_register_write(ACPI_MTX_DO_NOT_LOCK, > ACPI_REGISTER_PM1A_CONTROL, > PM1Acontrol); > @@ -377,17 +379,18 @@ acpi_status asmlinkage acpi_enter_sleep_ > /* Spin until we wake */ > > } while (!in_value); > - > - return_ACPI_STATUS(AE_OK); > #else > /* PV ACPI just need check hypercall return value */ > - status = acpi_notify_hypervisor_state(sleep_state, > + err = acpi_notify_hypervisor_state(sleep_state, > PM1Acontrol, PM1Bcontrol); > - if (ACPI_FAILURE(status)) > - return_ACPI_STATUS(status); > - else > - return_ACPI_STATUS(AE_OK); > + if (err) { > + ACPI_DEBUG_PRINT((ACPI_DB_ERROR, > + "Hypervisor failure [%d]\n", err)); > + return_ACPI_STATUS(AE_ERROR); > + } > #endif > + > + return_ACPI_STATUS(AE_OK); > } > > ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state) > Index: sle10sp2-2008-03-31/include/asm-i386/acpi.h > ==================================================================> --- sle10sp2-2008-03-31.orig/include/asm-i386/acpi.h 2007-12-10 > 11:30:46.000000000 +0100 > +++ sle10sp2-2008-03-31/include/asm-i386/acpi.h 2008-04-04 11:21:10.000000000 > +0200 > @@ -31,6 +31,9 @@ > #include <acpi/pdc_intel.h> > > #include <asm/system.h> /* defines cmpxchg */ > +#ifdef CONFIG_XEN > +#include <xen/interface/platform.h> > +#endif > > #define COMPILER_DEPENDENT_INT64 long long > #define COMPILER_DEPENDENT_UINT64 unsigned long long > @@ -154,6 +157,27 @@ static inline void acpi_disable_pci(void > } > extern int acpi_irq_balance_set(char *str); > > +#ifdef CONFIG_XEN > +static inline int acpi_notify_hypervisor_state(u8 sleep_state, > + u32 pm1a_cnt_val, > + u32 pm1b_cnt_val) > +{ > + struct xen_platform_op op = { > + .cmd = XENPF_enter_acpi_sleep, > + .interface_version = XENPF_INTERFACE_VERSION, > + .u = { > + .enter_acpi_sleep = { > + .pm1a_cnt_val = pm1a_cnt_val, > + .pm1b_cnt_val = pm1b_cnt_val, > + .sleep_state = sleep_state, > + }, > + }, > + }; > + > + return HYPERVISOR_platform_op(&op); > +} > +#endif /* CONFIG_XEN */ > + > #else /* !CONFIG_ACPI */ > > #define acpi_lapic 0 > @@ -175,10 +199,6 @@ extern unsigned long acpi_wakeup_address > /* early initialization routine */ > extern void acpi_reserve_bootmem(void); > > -#ifdef CONFIG_ACPI_PV_SLEEP > -extern int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt); > -#endif /* CONFIG_ACPI_PV_SLEEP */ > #endif /*CONFIG_ACPI_SLEEP*/ > > extern u8 x86_acpiid_to_apicid[]; > Index: sle10sp2-2008-03-31/include/asm-x86_64/acpi.h > ==================================================================> --- sle10sp2-2008-03-31.orig/include/asm-x86_64/acpi.h 2008-01-31 > 11:23:28.000000000 +0100 > +++ sle10sp2-2008-03-31/include/asm-x86_64/acpi.h 2008-04-04 > 11:20:51.000000000 +0200 > @@ -28,6 +28,9 @@ > > #ifdef __KERNEL__ > > +#ifdef CONFIG_XEN > +#include <xen/interface/platform.h> > +#endif > #include <acpi/pdc_intel.h> > > #define COMPILER_DEPENDENT_INT64 long long > @@ -129,6 +132,27 @@ static inline void acpi_disable_pci(void > } > extern int acpi_irq_balance_set(char *str); > > +#ifdef CONFIG_XEN > +static inline int acpi_notify_hypervisor_state(u8 sleep_state, > + u32 pm1a_cnt_val, > + u32 pm1b_cnt_val) > +{ > + struct xen_platform_op op = { > + .cmd = XENPF_enter_acpi_sleep, > + .interface_version = XENPF_INTERFACE_VERSION, > + .u = { > + .enter_acpi_sleep = { > + .pm1a_cnt_val = pm1a_cnt_val, > + .pm1b_cnt_val = pm1b_cnt_val, > + .sleep_state = sleep_state, > + }, > + }, > + }; > + > + return HYPERVISOR_platform_op(&op); > +} > +#endif /* CONFIG_XEN */ > + > #else /* !CONFIG_ACPI */ > > #define acpi_lapic 0 > @@ -152,11 +176,6 @@ extern unsigned long acpi_wakeup_address > > /* early initialization routine */ > extern void acpi_reserve_bootmem(void); > - > -#ifdef CONFIG_ACPI_PV_SLEEP > -extern int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt); > -#endif /* CONFIG_ACPI_PV_SLEEP */ > #endif /*CONFIG_ACPI_SLEEP*/ > > #define boot_cpu_physical_apicid boot_cpu_id > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Apr-04 12:15 UTC
Re: [Xen-devel] [PATCH] linux/x86: fix powering off certainmachines
The code to call acpi_notify_hypervisor_state() was conditional upon CONFIG_ACPI_PV_SLEEP, which isn''t necessarily set when CONFIG_XEN is. Therefore, the conditional needed to be converted to CONFIG_XEN, and consequently the implementation of the function needed to move out of sleep-xen.c (which doesn''t always get build without CONFIG_ACPI_SLEEP). So without the change, and without CONFIG_ACPI_PV_SLEEP, the native power off code was used, which didn''t work on that particular machine (including Xen versions pre-dating S3 support, where the problem wouldn''t have been easily fixable at all). Jan>>> Keir Fraser <keir.fraser@eu.citrix.com> 04.04.08 13:55 >>>The patch seems to mostly move code around. What does it actually fix, and how? -- Keir On 4/4/08 12:44, "Jan Beulich" <jbeulich@novell.com> wrote:> Dell''s Precision490, for example, fails to properly power off without > going through the full sequence of operations in the hypervisor. > > Also fix a compiler warning and the improper use of a hypervisor > return value as ACPI status. > > As usual, written and tested on 2.6.25-rc8 and 2.6.16.60 and made apply > to the 2.6.18 tree without further testing. > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > > Index: sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c > ==================================================================> --- sle10sp2-2008-03-31.orig/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04 > 11:12:58.000000000 +0200 > +++ sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04 > 11:14:46.000000000 +0200 > @@ -110,25 +110,4 @@ static int __init acpisleep_dmi_init(voi > } > > core_initcall(acpisleep_dmi_init); > - > -#else /* CONFIG_ACPI_PV_SLEEP */ > -#include <asm/hypervisor.h> > -#include <xen/interface/platform.h> > -int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt) > -{ > - struct xen_platform_op op = { > - .cmd = XENPF_enter_acpi_sleep, > - .interface_version = XENPF_INTERFACE_VERSION, > - .u = { > - .enter_acpi_sleep = { > - .pm1a_cnt_val = (u16)pm1a_cnt, > - .pm1b_cnt_val = (u16)pm1b_cnt, > - .sleep_state = sleep_state, > - }, > - }, > - }; > - > - return HYPERVISOR_platform_op(&op); > -} > #endif /* CONFIG_ACPI_PV_SLEEP */ > Index: sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c > ==================================================================> --- sle10sp2-2008-03-31.orig/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04 > 11:12:58.000000000 +0200 > +++ sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04 > 11:15:23.000000000 +0200 > @@ -137,29 +137,6 @@ static int __init acpi_sleep_setup(char > } > > __setup("acpi_sleep=", acpi_sleep_setup); > - > -#else /* CONFIG_ACPI_PV_SLEEP */ > -#include <asm/hypervisor.h> > -#include <xen/interface/platform.h> > -int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt) > -{ > - struct xen_platform_op op = { > - .cmd = XENPF_enter_acpi_sleep, > - .interface_version = XENPF_INTERFACE_VERSION, > - .u = { > - .enter_acpi_sleep = { > - .pm1a_cnt_val = (u16)pm1a_cnt, > - .pm1b_cnt_val = (u16)pm1b_cnt, > - .sleep_state = sleep_state, > - }, > - }, > - }; > - > - return HYPERVISOR_platform_op(&op); > -} > -#endif /* CONFIG_ACPI_PV_SLEEP */ > - > #endif /*CONFIG_ACPI_SLEEP */ > > void acpi_pci_link_exit(void) > Index: sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c > ==================================================================> --- sle10sp2-2008-03-31.orig/drivers/acpi/hardware/hwsleep.c 2008-04-04 > 11:12:58.000000000 +0200 > +++ sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c 2008-04-04 > 11:16:18.000000000 +0200 > @@ -227,7 +227,11 @@ acpi_status asmlinkage acpi_enter_sleep_ > u32 PM1Bcontrol; > struct acpi_bit_register_info *sleep_type_reg_info; > struct acpi_bit_register_info *sleep_enable_reg_info; > +#ifndef CONFIG_XEN > u32 in_value; > +#else > + int err; > +#endif > acpi_status status; > > ACPI_FUNCTION_TRACE(acpi_enter_sleep_state); > @@ -327,7 +329,7 @@ acpi_status asmlinkage acpi_enter_sleep_ > > ACPI_FLUSH_CPU_CACHE(); > > -#ifndef CONFIG_ACPI_PV_SLEEP > +#ifndef CONFIG_XEN > status = acpi_hw_register_write(ACPI_MTX_DO_NOT_LOCK, > ACPI_REGISTER_PM1A_CONTROL, > PM1Acontrol); > @@ -377,17 +379,18 @@ acpi_status asmlinkage acpi_enter_sleep_ > /* Spin until we wake */ > > } while (!in_value); > - > - return_ACPI_STATUS(AE_OK); > #else > /* PV ACPI just need check hypercall return value */ > - status = acpi_notify_hypervisor_state(sleep_state, > + err = acpi_notify_hypervisor_state(sleep_state, > PM1Acontrol, PM1Bcontrol); > - if (ACPI_FAILURE(status)) > - return_ACPI_STATUS(status); > - else > - return_ACPI_STATUS(AE_OK); > + if (err) { > + ACPI_DEBUG_PRINT((ACPI_DB_ERROR, > + "Hypervisor failure [%d]\n", err)); > + return_ACPI_STATUS(AE_ERROR); > + } > #endif > + > + return_ACPI_STATUS(AE_OK); > } > > ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state) > Index: sle10sp2-2008-03-31/include/asm-i386/acpi.h > ==================================================================> --- sle10sp2-2008-03-31.orig/include/asm-i386/acpi.h 2007-12-10 > 11:30:46.000000000 +0100 > +++ sle10sp2-2008-03-31/include/asm-i386/acpi.h 2008-04-04 11:21:10.000000000 > +0200 > @@ -31,6 +31,9 @@ > #include <acpi/pdc_intel.h> > > #include <asm/system.h> /* defines cmpxchg */ > +#ifdef CONFIG_XEN > +#include <xen/interface/platform.h> > +#endif > > #define COMPILER_DEPENDENT_INT64 long long > #define COMPILER_DEPENDENT_UINT64 unsigned long long > @@ -154,6 +157,27 @@ static inline void acpi_disable_pci(void > } > extern int acpi_irq_balance_set(char *str); > > +#ifdef CONFIG_XEN > +static inline int acpi_notify_hypervisor_state(u8 sleep_state, > + u32 pm1a_cnt_val, > + u32 pm1b_cnt_val) > +{ > + struct xen_platform_op op = { > + .cmd = XENPF_enter_acpi_sleep, > + .interface_version = XENPF_INTERFACE_VERSION, > + .u = { > + .enter_acpi_sleep = { > + .pm1a_cnt_val = pm1a_cnt_val, > + .pm1b_cnt_val = pm1b_cnt_val, > + .sleep_state = sleep_state, > + }, > + }, > + }; > + > + return HYPERVISOR_platform_op(&op); > +} > +#endif /* CONFIG_XEN */ > + > #else /* !CONFIG_ACPI */ > > #define acpi_lapic 0 > @@ -175,10 +199,6 @@ extern unsigned long acpi_wakeup_address > /* early initialization routine */ > extern void acpi_reserve_bootmem(void); > > -#ifdef CONFIG_ACPI_PV_SLEEP > -extern int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt); > -#endif /* CONFIG_ACPI_PV_SLEEP */ > #endif /*CONFIG_ACPI_SLEEP*/ > > extern u8 x86_acpiid_to_apicid[]; > Index: sle10sp2-2008-03-31/include/asm-x86_64/acpi.h > ==================================================================> --- sle10sp2-2008-03-31.orig/include/asm-x86_64/acpi.h 2008-01-31 > 11:23:28.000000000 +0100 > +++ sle10sp2-2008-03-31/include/asm-x86_64/acpi.h 2008-04-04 > 11:20:51.000000000 +0200 > @@ -28,6 +28,9 @@ > > #ifdef __KERNEL__ > > +#ifdef CONFIG_XEN > +#include <xen/interface/platform.h> > +#endif > #include <acpi/pdc_intel.h> > > #define COMPILER_DEPENDENT_INT64 long long > @@ -129,6 +132,27 @@ static inline void acpi_disable_pci(void > } > extern int acpi_irq_balance_set(char *str); > > +#ifdef CONFIG_XEN > +static inline int acpi_notify_hypervisor_state(u8 sleep_state, > + u32 pm1a_cnt_val, > + u32 pm1b_cnt_val) > +{ > + struct xen_platform_op op = { > + .cmd = XENPF_enter_acpi_sleep, > + .interface_version = XENPF_INTERFACE_VERSION, > + .u = { > + .enter_acpi_sleep = { > + .pm1a_cnt_val = pm1a_cnt_val, > + .pm1b_cnt_val = pm1b_cnt_val, > + .sleep_state = sleep_state, > + }, > + }, > + }; > + > + return HYPERVISOR_platform_op(&op); > +} > +#endif /* CONFIG_XEN */ > + > #else /* !CONFIG_ACPI */ > > #define acpi_lapic 0 > @@ -152,11 +176,6 @@ extern unsigned long acpi_wakeup_address > > /* early initialization routine */ > extern void acpi_reserve_bootmem(void); > - > -#ifdef CONFIG_ACPI_PV_SLEEP > -extern int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt); > -#endif /* CONFIG_ACPI_PV_SLEEP */ > #endif /*CONFIG_ACPI_SLEEP*/ > > #define boot_cpu_physical_apicid boot_cpu_id > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Apr-04 12:23 UTC
Re: [Xen-devel] [PATCH] linux/x86: fix powering off certainmachines
Makes sense, thanks. -- Keir On 4/4/08 13:15, "Jan Beulich" <jbeulich@novell.com> wrote:> The code to call acpi_notify_hypervisor_state() was conditional upon > CONFIG_ACPI_PV_SLEEP, which isn''t necessarily set when CONFIG_XEN > is. Therefore, the conditional needed to be converted to CONFIG_XEN, > and consequently the implementation of the function needed to move > out of sleep-xen.c (which doesn''t always get build without > CONFIG_ACPI_SLEEP). So without the change, and without > CONFIG_ACPI_PV_SLEEP, the native power off code was used, which > didn''t work on that particular machine (including Xen versions pre-dating > S3 support, where the problem wouldn''t have been easily fixable at all). > > Jan > >>>> Keir Fraser <keir.fraser@eu.citrix.com> 04.04.08 13:55 >>> > The patch seems to mostly move code around. What does it actually fix, and > how? > > -- Keir > > On 4/4/08 12:44, "Jan Beulich" <jbeulich@novell.com> wrote: > >> Dell''s Precision490, for example, fails to properly power off without >> going through the full sequence of operations in the hypervisor. >> >> Also fix a compiler warning and the improper use of a hypervisor >> return value as ACPI status. >> >> As usual, written and tested on 2.6.25-rc8 and 2.6.16.60 and made apply >> to the 2.6.18 tree without further testing. >> >> Signed-off-by: Jan Beulich <jbeulich@novell.com> >> >> Index: sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c >> ==================================================================>> --- sle10sp2-2008-03-31.orig/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04 >> 11:12:58.000000000 +0200 >> +++ sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c 2008-04-04 >> 11:14:46.000000000 +0200 >> @@ -110,25 +110,4 @@ static int __init acpisleep_dmi_init(voi >> } >> >> core_initcall(acpisleep_dmi_init); >> - >> -#else /* CONFIG_ACPI_PV_SLEEP */ >> -#include <asm/hypervisor.h> >> -#include <xen/interface/platform.h> >> -int acpi_notify_hypervisor_state(u8 sleep_state, >> - u32 pm1a_cnt, u32 pm1b_cnt) >> -{ >> - struct xen_platform_op op = { >> - .cmd = XENPF_enter_acpi_sleep, >> - .interface_version = XENPF_INTERFACE_VERSION, >> - .u = { >> - .enter_acpi_sleep = { >> - .pm1a_cnt_val = (u16)pm1a_cnt, >> - .pm1b_cnt_val = (u16)pm1b_cnt, >> - .sleep_state = sleep_state, >> - }, >> - }, >> - }; >> - >> - return HYPERVISOR_platform_op(&op); >> -} >> #endif /* CONFIG_ACPI_PV_SLEEP */ >> Index: sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c >> ==================================================================>> --- sle10sp2-2008-03-31.orig/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04 >> 11:12:58.000000000 +0200 >> +++ sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c 2008-04-04 >> 11:15:23.000000000 +0200 >> @@ -137,29 +137,6 @@ static int __init acpi_sleep_setup(char >> } >> >> __setup("acpi_sleep=", acpi_sleep_setup); >> - >> -#else /* CONFIG_ACPI_PV_SLEEP */ >> -#include <asm/hypervisor.h> >> -#include <xen/interface/platform.h> >> -int acpi_notify_hypervisor_state(u8 sleep_state, >> - u32 pm1a_cnt, u32 pm1b_cnt) >> -{ >> - struct xen_platform_op op = { >> - .cmd = XENPF_enter_acpi_sleep, >> - .interface_version = XENPF_INTERFACE_VERSION, >> - .u = { >> - .enter_acpi_sleep = { >> - .pm1a_cnt_val = (u16)pm1a_cnt, >> - .pm1b_cnt_val = (u16)pm1b_cnt, >> - .sleep_state = sleep_state, >> - }, >> - }, >> - }; >> - >> - return HYPERVISOR_platform_op(&op); >> -} >> -#endif /* CONFIG_ACPI_PV_SLEEP */ >> - >> #endif /*CONFIG_ACPI_SLEEP */ >> >> void acpi_pci_link_exit(void) >> Index: sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c >> ==================================================================>> --- sle10sp2-2008-03-31.orig/drivers/acpi/hardware/hwsleep.c 2008-04-04 >> 11:12:58.000000000 +0200 >> +++ sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c 2008-04-04 >> 11:16:18.000000000 +0200 >> @@ -227,7 +227,11 @@ acpi_status asmlinkage acpi_enter_sleep_ >> u32 PM1Bcontrol; >> struct acpi_bit_register_info *sleep_type_reg_info; >> struct acpi_bit_register_info *sleep_enable_reg_info; >> +#ifndef CONFIG_XEN >> u32 in_value; >> +#else >> + int err; >> +#endif >> acpi_status status; >> >> ACPI_FUNCTION_TRACE(acpi_enter_sleep_state); >> @@ -327,7 +329,7 @@ acpi_status asmlinkage acpi_enter_sleep_ >> >> ACPI_FLUSH_CPU_CACHE(); >> >> -#ifndef CONFIG_ACPI_PV_SLEEP >> +#ifndef CONFIG_XEN >> status = acpi_hw_register_write(ACPI_MTX_DO_NOT_LOCK, >> ACPI_REGISTER_PM1A_CONTROL, >> PM1Acontrol); >> @@ -377,17 +379,18 @@ acpi_status asmlinkage acpi_enter_sleep_ >> /* Spin until we wake */ >> >> } while (!in_value); >> - >> - return_ACPI_STATUS(AE_OK); >> #else >> /* PV ACPI just need check hypercall return value */ >> - status = acpi_notify_hypervisor_state(sleep_state, >> + err = acpi_notify_hypervisor_state(sleep_state, >> PM1Acontrol, PM1Bcontrol); >> - if (ACPI_FAILURE(status)) >> - return_ACPI_STATUS(status); >> - else >> - return_ACPI_STATUS(AE_OK); >> + if (err) { >> + ACPI_DEBUG_PRINT((ACPI_DB_ERROR, >> + "Hypervisor failure [%d]\n", err)); >> + return_ACPI_STATUS(AE_ERROR); >> + } >> #endif >> + >> + return_ACPI_STATUS(AE_OK); >> } >> >> ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state) >> Index: sle10sp2-2008-03-31/include/asm-i386/acpi.h >> ==================================================================>> --- sle10sp2-2008-03-31.orig/include/asm-i386/acpi.h 2007-12-10 >> 11:30:46.000000000 +0100 >> +++ sle10sp2-2008-03-31/include/asm-i386/acpi.h 2008-04-04 11:21:10.000000000 >> +0200 >> @@ -31,6 +31,9 @@ >> #include <acpi/pdc_intel.h> >> >> #include <asm/system.h> /* defines cmpxchg */ >> +#ifdef CONFIG_XEN >> +#include <xen/interface/platform.h> >> +#endif >> >> #define COMPILER_DEPENDENT_INT64 long long >> #define COMPILER_DEPENDENT_UINT64 unsigned long long >> @@ -154,6 +157,27 @@ static inline void acpi_disable_pci(void >> } >> extern int acpi_irq_balance_set(char *str); >> >> +#ifdef CONFIG_XEN >> +static inline int acpi_notify_hypervisor_state(u8 sleep_state, >> + u32 pm1a_cnt_val, >> + u32 pm1b_cnt_val) >> +{ >> + struct xen_platform_op op = { >> + .cmd = XENPF_enter_acpi_sleep, >> + .interface_version = XENPF_INTERFACE_VERSION, >> + .u = { >> + .enter_acpi_sleep = { >> + .pm1a_cnt_val = pm1a_cnt_val, >> + .pm1b_cnt_val = pm1b_cnt_val, >> + .sleep_state = sleep_state, >> + }, >> + }, >> + }; >> + >> + return HYPERVISOR_platform_op(&op); >> +} >> +#endif /* CONFIG_XEN */ >> + >> #else /* !CONFIG_ACPI */ >> >> #define acpi_lapic 0 >> @@ -175,10 +199,6 @@ extern unsigned long acpi_wakeup_address >> /* early initialization routine */ >> extern void acpi_reserve_bootmem(void); >> >> -#ifdef CONFIG_ACPI_PV_SLEEP >> -extern int acpi_notify_hypervisor_state(u8 sleep_state, >> - u32 pm1a_cnt, u32 pm1b_cnt); >> -#endif /* CONFIG_ACPI_PV_SLEEP */ >> #endif /*CONFIG_ACPI_SLEEP*/ >> >> extern u8 x86_acpiid_to_apicid[]; >> Index: sle10sp2-2008-03-31/include/asm-x86_64/acpi.h >> ==================================================================>> --- sle10sp2-2008-03-31.orig/include/asm-x86_64/acpi.h 2008-01-31 >> 11:23:28.000000000 +0100 >> +++ sle10sp2-2008-03-31/include/asm-x86_64/acpi.h 2008-04-04 >> 11:20:51.000000000 +0200 >> @@ -28,6 +28,9 @@ >> >> #ifdef __KERNEL__ >> >> +#ifdef CONFIG_XEN >> +#include <xen/interface/platform.h> >> +#endif >> #include <acpi/pdc_intel.h> >> >> #define COMPILER_DEPENDENT_INT64 long long >> @@ -129,6 +132,27 @@ static inline void acpi_disable_pci(void >> } >> extern int acpi_irq_balance_set(char *str); >> >> +#ifdef CONFIG_XEN >> +static inline int acpi_notify_hypervisor_state(u8 sleep_state, >> + u32 pm1a_cnt_val, >> + u32 pm1b_cnt_val) >> +{ >> + struct xen_platform_op op = { >> + .cmd = XENPF_enter_acpi_sleep, >> + .interface_version = XENPF_INTERFACE_VERSION, >> + .u = { >> + .enter_acpi_sleep = { >> + .pm1a_cnt_val = pm1a_cnt_val, >> + .pm1b_cnt_val = pm1b_cnt_val, >> + .sleep_state = sleep_state, >> + }, >> + }, >> + }; >> + >> + return HYPERVISOR_platform_op(&op); >> +} >> +#endif /* CONFIG_XEN */ >> + >> #else /* !CONFIG_ACPI */ >> >> #define acpi_lapic 0 >> @@ -152,11 +176,6 @@ extern unsigned long acpi_wakeup_address >> >> /* early initialization routine */ >> extern void acpi_reserve_bootmem(void); >> - >> -#ifdef CONFIG_ACPI_PV_SLEEP >> -extern int acpi_notify_hypervisor_state(u8 sleep_state, >> - u32 pm1a_cnt, u32 pm1b_cnt); >> -#endif /* CONFIG_ACPI_PV_SLEEP */ >> #endif /*CONFIG_ACPI_SLEEP*/ >> >> #define boot_cpu_physical_apicid boot_cpu_id >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel