From: "Jan Beulich" <jbeulich@novell.com>
To: Keir Fraser <keir.fraser@eu.citrix.com>
Cc: xen-devel@lists.xensource.com
Subject: Re: [PATCH] linux/x86: fix powering off certainmachines
Date: Fri, 04 Apr 2008 13:15:36 +0100 [thread overview]
Message-ID: <47F63808.76E4.0078.0@novell.com> (raw)
In-Reply-To: <C41BD5B6.1ECDA%keir.fraser@eu.citrix.com>
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
next prev parent reply other threads:[~2008-04-04 12:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-04-04 11:44 [PATCH] linux/x86: fix powering off certain machines Jan Beulich
2008-04-04 11:55 ` Keir Fraser
2008-04-04 12:15 ` Jan Beulich [this message]
2008-04-04 12:23 ` [PATCH] linux/x86: fix powering off certainmachines Keir Fraser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=47F63808.76E4.0078.0@novell.com \
--to=jbeulich@novell.com \
--cc=keir.fraser@eu.citrix.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.