All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] linux/x86: fix powering off certain machines
@ 2008-04-04 11:44 Jan Beulich
  2008-04-04 11:55 ` Keir Fraser
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2008-04-04 11:44 UTC (permalink / raw)
  To: xen-devel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] linux/x86: fix powering off certain machines
  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   ` [PATCH] linux/x86: fix powering off certainmachines Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Keir Fraser @ 2008-04-04 11:55 UTC (permalink / raw)
  To: Jan Beulich, xen-devel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] linux/x86: fix powering off certainmachines
  2008-04-04 11:55 ` Keir Fraser
@ 2008-04-04 12:15   ` Jan Beulich
  2008-04-04 12:23     ` Keir Fraser
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2008-04-04 12:15 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel

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 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] linux/x86: fix powering off certainmachines
  2008-04-04 12:15   ` [PATCH] linux/x86: fix powering off certainmachines Jan Beulich
@ 2008-04-04 12:23     ` Keir Fraser
  0 siblings, 0 replies; 4+ messages in thread
From: Keir Fraser @ 2008-04-04 12:23 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel

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
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-04-04 12:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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   ` [PATCH] linux/x86: fix powering off certainmachines Jan Beulich
2008-04-04 12:23     ` Keir Fraser

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.