From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] linux/x86: fix powering off certainmachines Date: Fri, 04 Apr 2008 13:15:36 +0100 Message-ID: <47F63808.76E4.0078.0@novell.com> References: <47F630C3.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org 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 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" wrote: > Dell's Precision490, for example, fails to properly power off without > going through the full sequence of operations in the hypervisor. >=20 > Also fix a compiler warning and the improper use of a hypervisor > return value as ACPI status. >=20 > 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. >=20 > Signed-off-by: Jan Beulich >=20 > Index: sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 > } > =20 > core_initcall(acpisleep_dmi_init); > - > -#else /* CONFIG_ACPI_PV_SLEEP */ > -#include > -#include > -int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt) > -{ > - struct xen_platform_op op =3D { > - .cmd =3D XENPF_enter_acpi_sleep, > - .interface_version =3D XENPF_INTERFACE_VERSION, > - .u =3D { > - .enter_acpi_sleep =3D { > - .pm1a_cnt_val =3D (u16)pm1a_cnt, > - .pm1b_cnt_val =3D (u16)pm1b_cnt, > - .sleep_state =3D 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 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 > } > =20 > __setup("acpi_sleep=3D", acpi_sleep_setup); > - > -#else /* CONFIG_ACPI_PV_SLEEP */ > -#include > -#include > -int acpi_notify_hypervisor_state(u8 sleep_state, > - u32 pm1a_cnt, u32 pm1b_cnt) > -{ > - struct xen_platform_op op =3D { > - .cmd =3D XENPF_enter_acpi_sleep, > - .interface_version =3D XENPF_INTERFACE_VERSION, > - .u =3D { > - .enter_acpi_sleep =3D { > - .pm1a_cnt_val =3D (u16)pm1a_cnt, > - .pm1b_cnt_val =3D (u16)pm1b_cnt, > - .sleep_state =3D sleep_state, > - }, > - }, > - }; > - > - return HYPERVISOR_platform_op(&op); > -} > -#endif /* CONFIG_ACPI_PV_SLEEP */ > - > #endif /*CONFIG_ACPI_SLEEP */ > =20 > void acpi_pci_link_exit(void) > Index: sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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; > =20 > ACPI_FUNCTION_TRACE(acpi_enter_sleep_state); > @@ -327,7 +329,7 @@ acpi_status asmlinkage acpi_enter_sleep_ > =20 > ACPI_FLUSH_CPU_CACHE(); > =20 > -#ifndef CONFIG_ACPI_PV_SLEEP > +#ifndef CONFIG_XEN > status =3D 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 */ > =20 > } while (!in_value); > - > - return_ACPI_STATUS(AE_OK); > #else > /* PV ACPI just need check hypercall return value */ > - status =3D acpi_notify_hypervisor_state(sleep_state, > + err =3D 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); > } > =20 > ACPI_EXPORT_SYMBOL(acpi_enter_sleep_state) > Index: sle10sp2-2008-03-31/include/asm-i386/acpi.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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.00000= 0000 > +0200 > @@ -31,6 +31,9 @@ > #include > =20 > #include /* defines cmpxchg */ > +#ifdef CONFIG_XEN > +#include > +#endif > =20 > #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); > =20 > +#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 =3D { > + .cmd =3D XENPF_enter_acpi_sleep, > + .interface_version =3D XENPF_INTERFACE_VERSION, > + .u =3D { > + .enter_acpi_sleep =3D { > + .pm1a_cnt_val =3D pm1a_cnt_val, > + .pm1b_cnt_val =3D pm1b_cnt_val, > + .sleep_state =3D sleep_state, > + }, > + }, > + }; > + > + return HYPERVISOR_platform_op(&op); > +} > +#endif /* CONFIG_XEN */ > + > #else /* !CONFIG_ACPI */ > =20 > #define acpi_lapic 0 > @@ -175,10 +199,6 @@ extern unsigned long acpi_wakeup_address > /* early initialization routine */ > extern void acpi_reserve_bootmem(void); > =20 > -#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*/ > =20 > extern u8 x86_acpiid_to_apicid[]; > Index: sle10sp2-2008-03-31/include/asm-x86_64/acpi.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 @@ > =20 > #ifdef __KERNEL__ > =20 > +#ifdef CONFIG_XEN > +#include > +#endif > #include > =20 > #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); > =20 > +#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 =3D { > + .cmd =3D XENPF_enter_acpi_sleep, > + .interface_version =3D XENPF_INTERFACE_VERSION, > + .u =3D { > + .enter_acpi_sleep =3D { > + .pm1a_cnt_val =3D pm1a_cnt_val, > + .pm1b_cnt_val =3D pm1b_cnt_val, > + .sleep_state =3D sleep_state, > + }, > + }, > + }; > + > + return HYPERVISOR_platform_op(&op); > +} > +#endif /* CONFIG_XEN */ > + > #else /* !CONFIG_ACPI */ > =20 > #define acpi_lapic 0 > @@ -152,11 +176,6 @@ extern unsigned long acpi_wakeup_address > =20 > /* 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*/ > =20 > #define boot_cpu_physical_apicid boot_cpu_id >=20 >=20 >=20 > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com=20 > http://lists.xensource.com/xen-devel=20