linux/x86: fix powering off certain machines

PATCH In review Jan Beulich 2 participants 4 messages Apr 4, 2008 ACPI

Share & tools

Jan Beulich

author

2008-04-04 11:44 UTC #0 lore raw

PATCH 1/1 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 <[email protected]>

arch/i386/kernel/acpi/sleep-xen.c +0 ~~-21~~

Index: sle10sp2-2008-03-31/arch/i386/kernel/acpi/sleep-xen.c
===================================================================
@@ -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 */

arch/x86_64/kernel/acpi/sleep-xen.c +0 ~~-23~~

Index: sle10sp2-2008-03-31/arch/x86_64/kernel/acpi/sleep-xen.c
===================================================================
@@ -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)

drivers/acpi/hardware/hwsleep.c +13 -8

Index: sle10sp2-2008-03-31/drivers/acpi/hardware/hwsleep.c
===================================================================
@@ -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)

include/asm-i386/acpi.h +24 -4

Index: sle10sp2-2008-03-31/include/asm-i386/acpi.h
===================================================================
@@ -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[];

include/asm-x86_64/acpi.h +24 -5

Index: sle10sp2-2008-03-31/include/asm-x86_64/acpi.h
===================================================================
@@ -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

Keir Fraser

2008-04-04 11:55 UTC #1 lore raw

The patch seems to mostly move code around. What does it actually fix, and how?

-- Keir

Quoted 250 lines · Jan Beulich

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 <[email protected]>

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

arch/i386/kernel/acpi/sleep-xen.c +0 ~~-21~~

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

arch/x86_64/kernel/acpi/sleep-xen.c +0 ~~-23~~

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

drivers/acpi/hardware/hwsleep.c +13 -8

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

include/asm-i386/acpi.h +24 -4

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

include/asm-x86_64/acpi.h +24 -5

@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Jan Beulich

author

2008-04-04 12:15 UTC #2 lore raw

Re: [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

Quoted 1 line

Quoted 1 line
Quoted 1 line
Keir Fraser <[email protected]> 04.04.08 13:55 >>>

The patch seems to mostly move code around. What does it actually fix, and how?

-- Keir

Quoted 250 lines · Jan Beulich

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 <[email protected]>

arch/i386/kernel/acpi/sleep-xen.c +0 ~~-21~~

@@ -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 */

arch/x86_64/kernel/acpi/sleep-xen.c +0 ~~-23~~

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

drivers/acpi/hardware/hwsleep.c +13 -8

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

include/asm-i386/acpi.h +24 -4

@@ -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[];

include/asm-x86_64/acpi.h +24 -5

@@ -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
[email protected]
http://lists.xensource.com/xen-devel

Keir Fraser

2008-04-04 12:23 UTC #3 lore raw

Re: [PATCH] linux/x86: fix powering off certainmachines

Makes sense, thanks.

-- Keir

Quoted 272 lines · Jan Beulich

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
Quoted 1 line
Quoted 1 line
Quoted 1 line
Keir Fraser <[email protected]> 04.04.08 13:55 >>>
The patch seems to mostly move code around. What does it actually fix, and how?
-- Keir
Quoted 250 lines · Jan Beulich
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 <[email protected]>
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
arch/i386/kernel/acpi/sleep-xen.c +0 ~~-21~~
@@ -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
arch/x86_64/kernel/acpi/sleep-xen.c +0 ~~-23~~
@@ -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
drivers/acpi/hardware/hwsleep.c +13 -8
@@ -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
include/asm-i386/acpi.h +24 -4
@@ -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
include/asm-x86_64/acpi.h +24 -5
@@ -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
[email protected]
http://lists.xensource.com/xen-devel