Makes sense, thanks.
-- Keir
On 4/4/08 13:15, "Jan Beulich" <jbeulich@xxxxxxxxxx> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|