All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
@ 2013-03-11  9:48 Jan Beulich
  2013-03-11 13:21 ` Konrad Rzeszutek Wilk
  2013-03-11 13:21 ` [Xen-devel] " Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11  9:48 UTC (permalink / raw)
  To: Len Brown, Konrad Rzeszutek Wilk
  Cc: Gang Wei, richard.l.maliszewski, Shane Wang, tboot-devel,
	xen-devel, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 8056 bytes --]

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 



[-- Attachment #2: linux-3.9-rc2-xen-ACPI-v5-sleep.patch --]
[-- Type: text/plain, Size: 8122 bytes --]

Xen/ACPI: support sleep state entering on hardware reduced systems

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 

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

* Re: [Xen-devel] [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11  9:48 [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems Jan Beulich
  2013-03-11 13:21 ` Konrad Rzeszutek Wilk
@ 2013-03-11 13:21 ` Konrad Rzeszutek Wilk
  2013-03-11 13:32   ` Jan Beulich
  2013-03-11 13:32   ` [Xen-devel] " Jan Beulich
  1 sibling, 2 replies; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-03-11 13:21 UTC (permalink / raw)
  To: Jan Beulich, rjw; +Cc: xen-devel, linux-acpi, tboot-devel, Len Brown

On Mon, Mar 11, 2013 at 09:48:46AM +0000, Jan Beulich wrote:
> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> reduced hardware sleep support, and the two changes didn't get
> synchronized: The new code doesn't call the hook function (if so
> requested). Fix this, requiring a boolean parameter to be added to the
> hook function to distinguish "extended" from "legacy" sleep.

Ooops.
> 
> This requires adjusting TXT, but the adjustments only go as far as
> failing the extended mode call (since, looking at the TXT interface,
> there doesn't even appear to be precautions to deal with that
> alternative interface).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>

CC: Rafael since he is the ACPI maintainer nowadays. Len
is concentrating on Intel-Idle.

> Cc: Gang Wei <gang.wei@intel.com>
> Cc: Shane Wang <shane.wang@intel.com>
> 
> ---
>  arch/x86/kernel/tboot.c          |    6 +++++-
>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>  drivers/acpi/osl.c               |   16 ++++++++--------
>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>  include/linux/acpi.h             |   10 +++++-----
>  include/xen/acpi.h               |    4 ++--
>  include/xen/interface/platform.h |    7 ++++---
>  8 files changed, 46 insertions(+), 33 deletions(-)
> 
> --- 3.9-rc2/arch/x86/kernel/tboot.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>  }
>  
> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> +		       bool extended)
>  {
>  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
>  		/* S0,1,2: */ -1, -1, -1,
> @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
>  	if (!tboot_enabled())
>  		return 0;
>  
> +	if (extended)
> +		return -1;
> +
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
> --- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
> @@ -43,6 +43,7 @@
>   */
>  
>  #include <acpi/acpi.h>
> +#include <linux/acpi.h>
>  #include "accommon.h"
>  
>  #define _COMPONENT          ACPI_HARDWARE
> @@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
>  
>  	ACPI_FLUSH_CPU_CACHE();
>  
> +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> +				       acpi_gbl_sleep_type_b, true);
> +	if (ACPI_SKIP(status))
> +		return_ACPI_STATUS(AE_OK);
> +	if (ACPI_FAILURE(status))
> +		return_ACPI_STATUS(status);
> +
>  	/*
>  	 * Set the SLP_TYP and SLP_EN bits.
>  	 *
> --- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
> @@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>  	ACPI_FLUSH_CPU_CACHE();
>  
>  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> -				       pm1b_control);
> +				       pm1b_control, false);
>  	if (ACPI_SKIP(status))
>  		return_ACPI_STATUS(AE_OK);
>  	if (ACPI_FAILURE(status))
> --- 3.9-rc2/drivers/acpi/osl.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
> @@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
>  extern char line_buf[80];
>  #endif				/*ENABLE_DEBUGGER */
>  
> -static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> -				      u32 pm1b_ctrl);
> +static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
> +				      bool extended);
>  
>  static acpi_osd_handler acpi_irq_handler;
>  static void *acpi_irq_context;
> @@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
>  	return AE_OK;
>  }
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> -				  u32 pm1b_control)
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended)
>  {
>  	int rc = 0;
>  	if (__acpi_os_prepare_sleep)
> -		rc = __acpi_os_prepare_sleep(sleep_state,
> -					     pm1a_control, pm1b_control);
> +		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
> +					     extended);
>  	if (rc < 0)
>  		return AE_ERROR;
>  	else if (rc > 0)
> @@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
>  	return AE_OK;
>  }
>  
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl, u32 pm1b_ctrl))
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended))
>  {
>  	__acpi_os_prepare_sleep = func;
>  }
> --- 3.9-rc2/drivers/xen/acpi.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
> @@ -35,27 +35,27 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnt)
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended)
>  {
> +	unsigned int bits = extended ? 8 : 16;
> +
>  	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,
> -			},
> +		.u.enter_acpi_sleep = {
> +			.val_a = (u16)val_a,
> +			.val_b = (u16)val_b,
> +			.sleep_state = sleep_state,
> +			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
>  		},
>  	};
>  
> -	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
> -		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
> -		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
> -		     pm1a_cnt, pm1b_cnt);
> +	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
> +		 "Using more than %u bits of sleep control values %#x/%#x!"
> +		 "Email xen-devel@lists.xen.org - Thank you.\n", \
> +		 bits, val_a, val_b))
>  		return -1;
> -	}
>  
>  	HYPERVISOR_dom0_op(&op);
>  	return 1;
> --- 3.9-rc2/include/linux/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
> @@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
>  #endif	/* !CONFIG_ACPI */
>  
>  #ifdef CONFIG_ACPI
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended));
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state,
> -				  u32 pm1a_control, u32 pm1b_control);
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended);
>  #ifdef CONFIG_X86
>  void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
>  #else
> @@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
>  }
>  #endif /* CONFIG_X86 */
>  #else
> -#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
> +#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
>  #endif
>  
>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
> --- 3.9-rc2/include/xen/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
> @@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
>  	return -ENXIO;
>  }
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnd);
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended);
>  
>  static inline void xen_acpi_sleep_register(void)
>  {
> --- 3.9-rc2/include/xen/interface/platform.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
> @@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
>  #define XENPF_enter_acpi_sleep    51
>  struct xenpf_enter_acpi_sleep {
>  	/* IN variables */
> -	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> -	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> +	uint16_t val_a;             /* PM1a control / sleep type A. */
> +	uint16_t val_b;             /* PM1b control / sleep type B. */
>  	uint32_t sleep_state;       /* Which state to enter (Sn). */
> -	uint32_t flags;             /* Must be zero. */
> +#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
> +	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */

Could the commit should also mention what version of the Xen hypervisor
supports this new flag and takes action on it?


Besides that little nit-pick it looks OK to me. Are there any changes
neccessary on the IA64 or ARM side for this?

>  };
>  DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
>  
> 
> 

------------------------------------------------------------------------------
Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester  
Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the  
endpoint security space. For insight on selecting the right partner to 
tackle endpoint security challenges, access the full report. 
http://p.sf.net/sfu/symantec-dev2dev

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

* Re: [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11  9:48 [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems Jan Beulich
@ 2013-03-11 13:21 ` Konrad Rzeszutek Wilk
  2013-03-11 13:21 ` [Xen-devel] " Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-03-11 13:21 UTC (permalink / raw)
  To: Jan Beulich, rjw
  Cc: Shane Wang, xen-devel, linux-acpi, tboot-devel,
	richard.l.maliszewski, Gang Wei, Len Brown

On Mon, Mar 11, 2013 at 09:48:46AM +0000, Jan Beulich wrote:
> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> reduced hardware sleep support, and the two changes didn't get
> synchronized: The new code doesn't call the hook function (if so
> requested). Fix this, requiring a boolean parameter to be added to the
> hook function to distinguish "extended" from "legacy" sleep.

Ooops.
> 
> This requires adjusting TXT, but the adjustments only go as far as
> failing the extended mode call (since, looking at the TXT interface,
> there doesn't even appear to be precautions to deal with that
> alternative interface).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>

CC: Rafael since he is the ACPI maintainer nowadays. Len
is concentrating on Intel-Idle.

> Cc: Gang Wei <gang.wei@intel.com>
> Cc: Shane Wang <shane.wang@intel.com>
> 
> ---
>  arch/x86/kernel/tboot.c          |    6 +++++-
>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>  drivers/acpi/osl.c               |   16 ++++++++--------
>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>  include/linux/acpi.h             |   10 +++++-----
>  include/xen/acpi.h               |    4 ++--
>  include/xen/interface/platform.h |    7 ++++---
>  8 files changed, 46 insertions(+), 33 deletions(-)
> 
> --- 3.9-rc2/arch/x86/kernel/tboot.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>  }
>  
> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> +		       bool extended)
>  {
>  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
>  		/* S0,1,2: */ -1, -1, -1,
> @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
>  	if (!tboot_enabled())
>  		return 0;
>  
> +	if (extended)
> +		return -1;
> +
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
> --- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
> @@ -43,6 +43,7 @@
>   */
>  
>  #include <acpi/acpi.h>
> +#include <linux/acpi.h>
>  #include "accommon.h"
>  
>  #define _COMPONENT          ACPI_HARDWARE
> @@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
>  
>  	ACPI_FLUSH_CPU_CACHE();
>  
> +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> +				       acpi_gbl_sleep_type_b, true);
> +	if (ACPI_SKIP(status))
> +		return_ACPI_STATUS(AE_OK);
> +	if (ACPI_FAILURE(status))
> +		return_ACPI_STATUS(status);
> +
>  	/*
>  	 * Set the SLP_TYP and SLP_EN bits.
>  	 *
> --- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
> @@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>  	ACPI_FLUSH_CPU_CACHE();
>  
>  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> -				       pm1b_control);
> +				       pm1b_control, false);
>  	if (ACPI_SKIP(status))
>  		return_ACPI_STATUS(AE_OK);
>  	if (ACPI_FAILURE(status))
> --- 3.9-rc2/drivers/acpi/osl.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
> @@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
>  extern char line_buf[80];
>  #endif				/*ENABLE_DEBUGGER */
>  
> -static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> -				      u32 pm1b_ctrl);
> +static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
> +				      bool extended);
>  
>  static acpi_osd_handler acpi_irq_handler;
>  static void *acpi_irq_context;
> @@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
>  	return AE_OK;
>  }
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> -				  u32 pm1b_control)
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended)
>  {
>  	int rc = 0;
>  	if (__acpi_os_prepare_sleep)
> -		rc = __acpi_os_prepare_sleep(sleep_state,
> -					     pm1a_control, pm1b_control);
> +		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
> +					     extended);
>  	if (rc < 0)
>  		return AE_ERROR;
>  	else if (rc > 0)
> @@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
>  	return AE_OK;
>  }
>  
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl, u32 pm1b_ctrl))
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended))
>  {
>  	__acpi_os_prepare_sleep = func;
>  }
> --- 3.9-rc2/drivers/xen/acpi.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
> @@ -35,27 +35,27 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnt)
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended)
>  {
> +	unsigned int bits = extended ? 8 : 16;
> +
>  	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,
> -			},
> +		.u.enter_acpi_sleep = {
> +			.val_a = (u16)val_a,
> +			.val_b = (u16)val_b,
> +			.sleep_state = sleep_state,
> +			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
>  		},
>  	};
>  
> -	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
> -		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
> -		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
> -		     pm1a_cnt, pm1b_cnt);
> +	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
> +		 "Using more than %u bits of sleep control values %#x/%#x!"
> +		 "Email xen-devel@lists.xen.org - Thank you.\n", \
> +		 bits, val_a, val_b))
>  		return -1;
> -	}
>  
>  	HYPERVISOR_dom0_op(&op);
>  	return 1;
> --- 3.9-rc2/include/linux/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
> @@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
>  #endif	/* !CONFIG_ACPI */
>  
>  #ifdef CONFIG_ACPI
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended));
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state,
> -				  u32 pm1a_control, u32 pm1b_control);
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended);
>  #ifdef CONFIG_X86
>  void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
>  #else
> @@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
>  }
>  #endif /* CONFIG_X86 */
>  #else
> -#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
> +#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
>  #endif
>  
>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
> --- 3.9-rc2/include/xen/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
> @@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
>  	return -ENXIO;
>  }
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnd);
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended);
>  
>  static inline void xen_acpi_sleep_register(void)
>  {
> --- 3.9-rc2/include/xen/interface/platform.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
> @@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
>  #define XENPF_enter_acpi_sleep    51
>  struct xenpf_enter_acpi_sleep {
>  	/* IN variables */
> -	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> -	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> +	uint16_t val_a;             /* PM1a control / sleep type A. */
> +	uint16_t val_b;             /* PM1b control / sleep type B. */
>  	uint32_t sleep_state;       /* Which state to enter (Sn). */
> -	uint32_t flags;             /* Must be zero. */
> +#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
> +	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */

Could the commit should also mention what version of the Xen hypervisor
supports this new flag and takes action on it?


Besides that little nit-pick it looks OK to me. Are there any changes
neccessary on the IA64 or ARM side for this?

>  };
>  DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
>  
> 
> 

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

* Re: [Xen-devel] [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:21 ` [Xen-devel] " Konrad Rzeszutek Wilk
  2013-03-11 13:32   ` Jan Beulich
@ 2013-03-11 13:32   ` Jan Beulich
  2013-03-11 13:53     ` Konrad Rzeszutek Wilk
  2013-03-11 13:53     ` [PATCH] " Konrad Rzeszutek Wilk
  1 sibling, 2 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 13:32 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Gang Wei, richard.l.maliszewski, Shane Wang, Len Brown,
	tboot-devel, xen-devel, rjw, linux-acpi

>>> On 11.03.13 at 14:21, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> Could the commit should also mention what version of the Xen hypervisor
> supports this new flag and takes action on it?

Do you really think this is needed? I generally try to stay away
from doing such, since backports can easily invalidate such
statements.

> Besides that little nit-pick it looks OK to me. Are there any changes
> neccessary on the IA64 or ARM side for this?

Xen on IA64 is dead, so I don't see any need for further action
there. And I don't think ARM has any ACPI enablement so far (on
the Xen side at least).

Jan


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

* Re: [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:21 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2013-03-11 13:32   ` Jan Beulich
  2013-03-11 13:32   ` [Xen-devel] " Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 13:32 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Shane Wang, xen-devel, rjw, linux-acpi, tboot-devel,
	richard.l.maliszewski, Gang Wei, Len Brown

>>> On 11.03.13 at 14:21, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> Could the commit should also mention what version of the Xen hypervisor
> supports this new flag and takes action on it?

Do you really think this is needed? I generally try to stay away
from doing such, since backports can easily invalidate such
statements.

> Besides that little nit-pick it looks OK to me. Are there any changes
> neccessary on the IA64 or ARM side for this?

Xen on IA64 is dead, so I don't see any need for further action
there. And I don't think ARM has any ACPI enablement so far (on
the Xen side at least).

Jan

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

* Re: [Xen-devel] [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:32   ` [Xen-devel] " Jan Beulich
@ 2013-03-11 13:53     ` Konrad Rzeszutek Wilk
  2013-03-11 14:02       ` Jan Beulich
                         ` (3 more replies)
  2013-03-11 13:53     ` [PATCH] " Konrad Rzeszutek Wilk
  1 sibling, 4 replies; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-03-11 13:53 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, rjw, linux-acpi, tboot-devel, Len Brown

On Mon, Mar 11, 2013 at 01:32:17PM +0000, Jan Beulich wrote:
> >>> On 11.03.13 at 14:21, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > Could the commit should also mention what version of the Xen hypervisor
> > supports this new flag and takes action on it?
> 
> Do you really think this is needed? I generally try to stay away
> from doing such, since backports can easily invalidate such
> statements.

Perhaps just mention the title of the patch in the Xen tree?

> 
> > Besides that little nit-pick it looks OK to me. Are there any changes
> > neccessary on the IA64 or ARM side for this?
> 
> Xen on IA64 is dead, so I don't see any need for further action

You are touching generic ACPI code. My question was in terms of
the Linux ACPI IA64 code.

> there. And I don't think ARM has any ACPI enablement so far (on
> the Xen side at least).

And also on the Linux side by just doing a quick grep. Surely
that will change at some point.

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

------------------------------------------------------------------------------
Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester  
Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the  
endpoint security space. For insight on selecting the right partner to 
tackle endpoint security challenges, access the full report. 
http://p.sf.net/sfu/symantec-dev2dev

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

* Re: [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:32   ` [Xen-devel] " Jan Beulich
  2013-03-11 13:53     ` Konrad Rzeszutek Wilk
@ 2013-03-11 13:53     ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 28+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-03-11 13:53 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Shane Wang, xen-devel, rjw, linux-acpi, tboot-devel,
	richard.l.maliszewski, Gang Wei, Len Brown

On Mon, Mar 11, 2013 at 01:32:17PM +0000, Jan Beulich wrote:
> >>> On 11.03.13 at 14:21, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > Could the commit should also mention what version of the Xen hypervisor
> > supports this new flag and takes action on it?
> 
> Do you really think this is needed? I generally try to stay away
> from doing such, since backports can easily invalidate such
> statements.

Perhaps just mention the title of the patch in the Xen tree?

> 
> > Besides that little nit-pick it looks OK to me. Are there any changes
> > neccessary on the IA64 or ARM side for this?
> 
> Xen on IA64 is dead, so I don't see any need for further action

You are touching generic ACPI code. My question was in terms of
the Linux ACPI IA64 code.

> there. And I don't think ARM has any ACPI enablement so far (on
> the Xen side at least).

And also on the Linux side by just doing a quick grep. Surely
that will change at some point.

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

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

* Re: [Xen-devel] [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:53     ` Konrad Rzeszutek Wilk
  2013-03-11 14:02       ` Jan Beulich
@ 2013-03-11 14:02       ` Jan Beulich
  2013-03-11 14:06       ` [PATCH v2] " Jan Beulich
  2013-03-11 14:06       ` Jan Beulich
  3 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 14:02 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Gang Wei, richard.l.maliszewski, Shane Wang, Len Brown,
	tboot-devel, xen-devel, rjw, linux-acpi

>>> On 11.03.13 at 14:53, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Mon, Mar 11, 2013 at 01:32:17PM +0000, Jan Beulich wrote:
>> >>> On 11.03.13 at 14:21, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> > Could the commit should also mention what version of the Xen hypervisor
>> > supports this new flag and takes action on it?
>> 
>> Do you really think this is needed? I generally try to stay away
>> from doing such, since backports can easily invalidate such
>> statements.
> 
> Perhaps just mention the title of the patch in the Xen tree?

Okay, will send you a v2 with the hypervisor size change
referenced. Sigh.

>> > Besides that little nit-pick it looks OK to me. Are there any changes
>> > neccessary on the IA64 or ARM side for this?
>> 
>> Xen on IA64 is dead, so I don't see any need for further action
> 
> You are touching generic ACPI code. My question was in terms of
> the Linux ACPI IA64 code.

The changes there are completely benign to IA64.

Jan


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

* Re: [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:53     ` Konrad Rzeszutek Wilk
@ 2013-03-11 14:02       ` Jan Beulich
  2013-03-11 14:02       ` [Xen-devel] " Jan Beulich
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 14:02 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Shane Wang, xen-devel, rjw, linux-acpi, tboot-devel,
	richard.l.maliszewski, Gang Wei, Len Brown

>>> On 11.03.13 at 14:53, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Mon, Mar 11, 2013 at 01:32:17PM +0000, Jan Beulich wrote:
>> >>> On 11.03.13 at 14:21, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> > Could the commit should also mention what version of the Xen hypervisor
>> > supports this new flag and takes action on it?
>> 
>> Do you really think this is needed? I generally try to stay away
>> from doing such, since backports can easily invalidate such
>> statements.
> 
> Perhaps just mention the title of the patch in the Xen tree?

Okay, will send you a v2 with the hypervisor size change
referenced. Sigh.

>> > Besides that little nit-pick it looks OK to me. Are there any changes
>> > neccessary on the IA64 or ARM side for this?
>> 
>> Xen on IA64 is dead, so I don't see any need for further action
> 
> You are touching generic ACPI code. My question was in terms of
> the Linux ACPI IA64 code.

The changes there are completely benign to IA64.

Jan

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

* [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:53     ` Konrad Rzeszutek Wilk
                         ` (2 preceding siblings ...)
  2013-03-11 14:06       ` [PATCH v2] " Jan Beulich
@ 2013-03-11 14:06       ` Jan Beulich
  2013-03-11 14:51         ` Rafael J. Wysocki
                           ` (3 more replies)
  3 siblings, 4 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 14:06 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Rafael J. Wysocki
  Cc: Gang Wei, richard.l.maliszewski, Shane Wang, tboot-devel,
	xen-devel, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 8299 bytes --]

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
v5 (reduced HW) sleep interface") on the master branch of
git://xenbits.xen.org/xen.git.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>
---
v2: Extend description to include reference to hypervisor side change.

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 



[-- Attachment #2: linux-3.9-rc2-xen-ACPI-v5-sleep.patch --]
[-- Type: text/plain, Size: 8365 bytes --]

Xen/ACPI: support sleep state entering on hardware reduced systems

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
v5 (reduced HW) sleep interface") on the master branch of
git://xenbits.xen.org/xen.git.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>
---
v2: Extend description to include reference to hypervisor side change.

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 

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

* [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 13:53     ` Konrad Rzeszutek Wilk
  2013-03-11 14:02       ` Jan Beulich
  2013-03-11 14:02       ` [Xen-devel] " Jan Beulich
@ 2013-03-11 14:06       ` Jan Beulich
  2013-03-11 14:06       ` Jan Beulich
  3 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 14:06 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Rafael J. Wysocki
  Cc: Shane Wang, xen-devel, linux-acpi, tboot-devel,
	richard.l.maliszewski, Gang Wei

[-- Attachment #1: Type: text/plain, Size: 8299 bytes --]

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
v5 (reduced HW) sleep interface") on the master branch of
git://xenbits.xen.org/xen.git.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>
---
v2: Extend description to include reference to hypervisor side change.

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 



[-- Attachment #2: linux-3.9-rc2-xen-ACPI-v5-sleep.patch --]
[-- Type: text/plain, Size: 8365 bytes --]

Xen/ACPI: support sleep state entering on hardware reduced systems

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
v5 (reduced HW) sleep interface") on the master branch of
git://xenbits.xen.org/xen.git.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>
---
v2: Extend description to include reference to hypervisor side change.

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:06       ` Jan Beulich
  2013-03-11 14:51         ` Rafael J. Wysocki
@ 2013-03-11 14:51         ` Rafael J. Wysocki
  2013-03-11 16:12           ` Jan Beulich
                             ` (3 more replies)
  2013-03-12  6:39         ` Wei, Gang
  2013-03-12  6:39         ` Wei, Gang
  3 siblings, 4 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2013-03-11 14:51 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Konrad Rzeszutek Wilk, Gang Wei, richard.l.maliszewski,
	Shane Wang, tboot-devel, xen-devel, linux-acpi, Bob Moore,
	Lv Zheng

On Monday, March 11, 2013 02:06:51 PM Jan Beulich wrote:
> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> reduced hardware sleep support, and the two changes didn't get
> synchronized: The new code doesn't call the hook function (if so
> requested). Fix this, requiring a boolean parameter to be added to the
> hook function to distinguish "extended" from "legacy" sleep.
> 
> This requires adjusting TXT, but the adjustments only go as far as
> failing the extended mode call (since, looking at the TXT interface,
> there doesn't even appear to be precautions to deal with that
> alternative interface).
> 
> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
> v5 (reduced HW) sleep interface") on the master branch of
> git://xenbits.xen.org/xen.git.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
> Cc: Gang Wei <gang.wei@intel.com>
> Cc: Shane Wang <shane.wang@intel.com>
> ---
> v2: Extend description to include reference to hypervisor side change.
> 
> ---
>  arch/x86/kernel/tboot.c          |    6 +++++-
>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>  drivers/acpi/osl.c               |   16 ++++++++--------
>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>  include/linux/acpi.h             |   10 +++++-----
>  include/xen/acpi.h               |    4 ++--
>  include/xen/interface/platform.h |    7 ++++---
>  8 files changed, 46 insertions(+), 33 deletions(-)

ACPICA changes (hwesleep.c and hwsleep.c) need to go separately and through the
upstream ACPICA before we can take them into the kernel.  Sorry about that.

Thanks,
Rafael


> --- 3.9-rc2/arch/x86/kernel/tboot.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>  }
>  
> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> +		       bool extended)
>  {
>  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
>  		/* S0,1,2: */ -1, -1, -1,
> @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
>  	if (!tboot_enabled())
>  		return 0;
>  
> +	if (extended)
> +		return -1;
> +
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
> --- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
> @@ -43,6 +43,7 @@
>   */
>  
>  #include <acpi/acpi.h>
> +#include <linux/acpi.h>
>  #include "accommon.h"
>  
>  #define _COMPONENT          ACPI_HARDWARE
> @@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
>  
>  	ACPI_FLUSH_CPU_CACHE();
>  
> +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> +				       acpi_gbl_sleep_type_b, true);
> +	if (ACPI_SKIP(status))
> +		return_ACPI_STATUS(AE_OK);
> +	if (ACPI_FAILURE(status))
> +		return_ACPI_STATUS(status);
> +
>  	/*
>  	 * Set the SLP_TYP and SLP_EN bits.
>  	 *
> --- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
> @@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>  	ACPI_FLUSH_CPU_CACHE();
>  
>  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> -				       pm1b_control);
> +				       pm1b_control, false);
>  	if (ACPI_SKIP(status))
>  		return_ACPI_STATUS(AE_OK);
>  	if (ACPI_FAILURE(status))
> --- 3.9-rc2/drivers/acpi/osl.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
> @@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
>  extern char line_buf[80];
>  #endif				/*ENABLE_DEBUGGER */
>  
> -static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> -				      u32 pm1b_ctrl);
> +static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
> +				      bool extended);
>  
>  static acpi_osd_handler acpi_irq_handler;
>  static void *acpi_irq_context;
> @@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
>  	return AE_OK;
>  }
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> -				  u32 pm1b_control)
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended)
>  {
>  	int rc = 0;
>  	if (__acpi_os_prepare_sleep)
> -		rc = __acpi_os_prepare_sleep(sleep_state,
> -					     pm1a_control, pm1b_control);
> +		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
> +					     extended);
>  	if (rc < 0)
>  		return AE_ERROR;
>  	else if (rc > 0)
> @@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
>  	return AE_OK;
>  }
>  
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl, u32 pm1b_ctrl))
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended))
>  {
>  	__acpi_os_prepare_sleep = func;
>  }
> --- 3.9-rc2/drivers/xen/acpi.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
> @@ -35,27 +35,27 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnt)
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended)
>  {
> +	unsigned int bits = extended ? 8 : 16;
> +
>  	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,
> -			},
> +		.u.enter_acpi_sleep = {
> +			.val_a = (u16)val_a,
> +			.val_b = (u16)val_b,
> +			.sleep_state = sleep_state,
> +			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
>  		},
>  	};
>  
> -	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
> -		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
> -		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
> -		     pm1a_cnt, pm1b_cnt);
> +	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
> +		 "Using more than %u bits of sleep control values %#x/%#x!"
> +		 "Email xen-devel@lists.xen.org - Thank you.\n", \
> +		 bits, val_a, val_b))
>  		return -1;
> -	}
>  
>  	HYPERVISOR_dom0_op(&op);
>  	return 1;
> --- 3.9-rc2/include/linux/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
> @@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
>  #endif	/* !CONFIG_ACPI */
>  
>  #ifdef CONFIG_ACPI
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended));
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state,
> -				  u32 pm1a_control, u32 pm1b_control);
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended);
>  #ifdef CONFIG_X86
>  void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
>  #else
> @@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
>  }
>  #endif /* CONFIG_X86 */
>  #else
> -#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
> +#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
>  #endif
>  
>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
> --- 3.9-rc2/include/xen/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
> @@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
>  	return -ENXIO;
>  }
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnd);
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended);
>  
>  static inline void xen_acpi_sleep_register(void)
>  {
> --- 3.9-rc2/include/xen/interface/platform.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
> @@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
>  #define XENPF_enter_acpi_sleep    51
>  struct xenpf_enter_acpi_sleep {
>  	/* IN variables */
> -	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> -	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> +	uint16_t val_a;             /* PM1a control / sleep type A. */
> +	uint16_t val_b;             /* PM1b control / sleep type B. */
>  	uint32_t sleep_state;       /* Which state to enter (Sn). */
> -	uint32_t flags;             /* Must be zero. */
> +#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
> +	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
>  };
>  DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
>  
> 
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:06       ` Jan Beulich
@ 2013-03-11 14:51         ` Rafael J. Wysocki
  2013-03-11 14:51         ` Rafael J. Wysocki
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2013-03-11 14:51 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Konrad Rzeszutek Wilk, Shane Wang, Bob Moore, xen-devel,
	linux-acpi, tboot-devel, Lv Zheng, richard.l.maliszewski,
	Gang Wei

On Monday, March 11, 2013 02:06:51 PM Jan Beulich wrote:
> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> reduced hardware sleep support, and the two changes didn't get
> synchronized: The new code doesn't call the hook function (if so
> requested). Fix this, requiring a boolean parameter to be added to the
> hook function to distinguish "extended" from "legacy" sleep.
> 
> This requires adjusting TXT, but the adjustments only go as far as
> failing the extended mode call (since, looking at the TXT interface,
> there doesn't even appear to be precautions to deal with that
> alternative interface).
> 
> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
> v5 (reduced HW) sleep interface") on the master branch of
> git://xenbits.xen.org/xen.git.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
> Cc: Gang Wei <gang.wei@intel.com>
> Cc: Shane Wang <shane.wang@intel.com>
> ---
> v2: Extend description to include reference to hypervisor side change.
> 
> ---
>  arch/x86/kernel/tboot.c          |    6 +++++-
>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>  drivers/acpi/osl.c               |   16 ++++++++--------
>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>  include/linux/acpi.h             |   10 +++++-----
>  include/xen/acpi.h               |    4 ++--
>  include/xen/interface/platform.h |    7 ++++---
>  8 files changed, 46 insertions(+), 33 deletions(-)

ACPICA changes (hwesleep.c and hwsleep.c) need to go separately and through the
upstream ACPICA before we can take them into the kernel.  Sorry about that.

Thanks,
Rafael


> --- 3.9-rc2/arch/x86/kernel/tboot.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>  }
>  
> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
> +		       bool extended)
>  {
>  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
>  		/* S0,1,2: */ -1, -1, -1,
> @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
>  	if (!tboot_enabled())
>  		return 0;
>  
> +	if (extended)
> +		return -1;
> +
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
> --- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
> @@ -43,6 +43,7 @@
>   */
>  
>  #include <acpi/acpi.h>
> +#include <linux/acpi.h>
>  #include "accommon.h"
>  
>  #define _COMPONENT          ACPI_HARDWARE
> @@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
>  
>  	ACPI_FLUSH_CPU_CACHE();
>  
> +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> +				       acpi_gbl_sleep_type_b, true);
> +	if (ACPI_SKIP(status))
> +		return_ACPI_STATUS(AE_OK);
> +	if (ACPI_FAILURE(status))
> +		return_ACPI_STATUS(status);
> +
>  	/*
>  	 * Set the SLP_TYP and SLP_EN bits.
>  	 *
> --- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
> @@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>  	ACPI_FLUSH_CPU_CACHE();
>  
>  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> -				       pm1b_control);
> +				       pm1b_control, false);
>  	if (ACPI_SKIP(status))
>  		return_ACPI_STATUS(AE_OK);
>  	if (ACPI_FAILURE(status))
> --- 3.9-rc2/drivers/acpi/osl.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
> @@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
>  extern char line_buf[80];
>  #endif				/*ENABLE_DEBUGGER */
>  
> -static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> -				      u32 pm1b_ctrl);
> +static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
> +				      bool extended);
>  
>  static acpi_osd_handler acpi_irq_handler;
>  static void *acpi_irq_context;
> @@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
>  	return AE_OK;
>  }
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> -				  u32 pm1b_control)
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended)
>  {
>  	int rc = 0;
>  	if (__acpi_os_prepare_sleep)
> -		rc = __acpi_os_prepare_sleep(sleep_state,
> -					     pm1a_control, pm1b_control);
> +		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
> +					     extended);
>  	if (rc < 0)
>  		return AE_ERROR;
>  	else if (rc > 0)
> @@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
>  	return AE_OK;
>  }
>  
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl, u32 pm1b_ctrl))
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended))
>  {
>  	__acpi_os_prepare_sleep = func;
>  }
> --- 3.9-rc2/drivers/xen/acpi.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
> @@ -35,27 +35,27 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnt)
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended)
>  {
> +	unsigned int bits = extended ? 8 : 16;
> +
>  	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,
> -			},
> +		.u.enter_acpi_sleep = {
> +			.val_a = (u16)val_a,
> +			.val_b = (u16)val_b,
> +			.sleep_state = sleep_state,
> +			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
>  		},
>  	};
>  
> -	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
> -		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
> -		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
> -		     pm1a_cnt, pm1b_cnt);
> +	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
> +		 "Using more than %u bits of sleep control values %#x/%#x!"
> +		 "Email xen-devel@lists.xen.org - Thank you.\n", \
> +		 bits, val_a, val_b))
>  		return -1;
> -	}
>  
>  	HYPERVISOR_dom0_op(&op);
>  	return 1;
> --- 3.9-rc2/include/linux/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
> @@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
>  #endif	/* !CONFIG_ACPI */
>  
>  #ifdef CONFIG_ACPI
> -void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> -			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
> +					   u32 val_b, bool extended));
>  
> -acpi_status acpi_os_prepare_sleep(u8 sleep_state,
> -				  u32 pm1a_control, u32 pm1b_control);
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> +				  bool extended);
>  #ifdef CONFIG_X86
>  void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
>  #else
> @@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
>  }
>  #endif /* CONFIG_X86 */
>  #else
> -#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
> +#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
>  #endif
>  
>  #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
> --- 3.9-rc2/include/xen/acpi.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
> @@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
>  	return -ENXIO;
>  }
>  
> -int xen_acpi_notify_hypervisor_state(u8 sleep_state,
> -				     u32 pm1a_cnt, u32 pm1b_cnd);
> +int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
> +				     bool extended);
>  
>  static inline void xen_acpi_sleep_register(void)
>  {
> --- 3.9-rc2/include/xen/interface/platform.h
> +++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
> @@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
>  #define XENPF_enter_acpi_sleep    51
>  struct xenpf_enter_acpi_sleep {
>  	/* IN variables */
> -	uint16_t pm1a_cnt_val;      /* PM1a control value. */
> -	uint16_t pm1b_cnt_val;      /* PM1b control value. */
> +	uint16_t val_a;             /* PM1a control / sleep type A. */
> +	uint16_t val_b;             /* PM1b control / sleep type B. */
>  	uint32_t sleep_state;       /* Which state to enter (Sn). */
> -	uint32_t flags;             /* Must be zero. */
> +#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
> +	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
>  };
>  DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
>  
> 
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:51         ` Rafael J. Wysocki
  2013-03-11 16:12           ` Jan Beulich
@ 2013-03-11 16:12           ` Jan Beulich
  2013-03-11 16:58             ` Rafael J. Wysocki
  2013-03-11 16:58             ` Rafael J. Wysocki
  2013-03-12  0:59           ` Zheng, Lv
  2013-03-12  0:59           ` Zheng, Lv
  3 siblings, 2 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 16:12 UTC (permalink / raw)
  To: Bob Moore, Rafael J. Wysocki
  Cc: Gang Wei, Lv Zheng, richard.l.maliszewski, Shane Wang,
	tboot-devel, xen-devel, Konrad Rzeszutek Wilk, linux-acpi

>>> On 11.03.13 at 15:51, "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Monday, March 11, 2013 02:06:51 PM Jan Beulich wrote:
>> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
>> reduced hardware sleep support, and the two changes didn't get
>> synchronized: The new code doesn't call the hook function (if so
>> requested). Fix this, requiring a boolean parameter to be added to the
>> hook function to distinguish "extended" from "legacy" sleep.
>> 
>> This requires adjusting TXT, but the adjustments only go as far as
>> failing the extended mode call (since, looking at the TXT interface,
>> there doesn't even appear to be precautions to deal with that
>> alternative interface).
>> 
>> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
>> v5 (reduced HW) sleep interface") on the master branch of
>> git://xenbits.xen.org/xen.git.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
>> Cc: Gang Wei <gang.wei@intel.com>
>> Cc: Shane Wang <shane.wang@intel.com>
>> ---
>> v2: Extend description to include reference to hypervisor side change.
>> 
>> ---
>>  arch/x86/kernel/tboot.c          |    6 +++++-
>>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>>  drivers/acpi/osl.c               |   16 ++++++++--------
>>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>>  include/linux/acpi.h             |   10 +++++-----
>>  include/xen/acpi.h               |    4 ++--
>>  include/xen/interface/platform.h |    7 ++++---
>>  8 files changed, 46 insertions(+), 33 deletions(-)
> 
> ACPICA changes (hwesleep.c and hwsleep.c) need to go separately and through 
> the
> upstream ACPICA before we can take them into the kernel.  Sorry about that.

Looking at 09f98a8 (the change originally introducing the hook)
and comparing with the rest of the changes to hwsleep.c, I would
think this one didn't come from the ACPICA tree either.

But anyway - if I really need to do that, are there any pointers as
to where to submit this to, and how to make sure it gets picked up
rather sooner than later?

Jan


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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:51         ` Rafael J. Wysocki
@ 2013-03-11 16:12           ` Jan Beulich
  2013-03-11 16:12           ` Jan Beulich
                             ` (2 subsequent siblings)
  3 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11 16:12 UTC (permalink / raw)
  To: Bob Moore, Rafael J. Wysocki
  Cc: Konrad Rzeszutek Wilk, Shane Wang, xen-devel, linux-acpi,
	tboot-devel, Lv Zheng, richard.l.maliszewski, Gang Wei

>>> On 11.03.13 at 15:51, "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Monday, March 11, 2013 02:06:51 PM Jan Beulich wrote:
>> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
>> reduced hardware sleep support, and the two changes didn't get
>> synchronized: The new code doesn't call the hook function (if so
>> requested). Fix this, requiring a boolean parameter to be added to the
>> hook function to distinguish "extended" from "legacy" sleep.
>> 
>> This requires adjusting TXT, but the adjustments only go as far as
>> failing the extended mode call (since, looking at the TXT interface,
>> there doesn't even appear to be precautions to deal with that
>> alternative interface).
>> 
>> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
>> v5 (reduced HW) sleep interface") on the master branch of
>> git://xenbits.xen.org/xen.git.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
>> Cc: Gang Wei <gang.wei@intel.com>
>> Cc: Shane Wang <shane.wang@intel.com>
>> ---
>> v2: Extend description to include reference to hypervisor side change.
>> 
>> ---
>>  arch/x86/kernel/tboot.c          |    6 +++++-
>>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>>  drivers/acpi/osl.c               |   16 ++++++++--------
>>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>>  include/linux/acpi.h             |   10 +++++-----
>>  include/xen/acpi.h               |    4 ++--
>>  include/xen/interface/platform.h |    7 ++++---
>>  8 files changed, 46 insertions(+), 33 deletions(-)
> 
> ACPICA changes (hwesleep.c and hwsleep.c) need to go separately and through 
> the
> upstream ACPICA before we can take them into the kernel.  Sorry about that.

Looking at 09f98a8 (the change originally introducing the hook)
and comparing with the rest of the changes to hwsleep.c, I would
think this one didn't come from the ACPICA tree either.

But anyway - if I really need to do that, are there any pointers as
to where to submit this to, and how to make sure it gets picked up
rather sooner than later?

Jan

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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 16:12           ` Jan Beulich
@ 2013-03-11 16:58             ` Rafael J. Wysocki
  2013-03-11 16:58             ` Rafael J. Wysocki
  1 sibling, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2013-03-11 16:58 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Bob Moore, Gang Wei, Lv Zheng, richard.l.maliszewski, Shane Wang,
	tboot-devel, xen-devel, Konrad Rzeszutek Wilk, linux-acpi

On Monday, March 11, 2013 04:12:42 PM Jan Beulich wrote:
> >>> On 11.03.13 at 15:51, "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > On Monday, March 11, 2013 02:06:51 PM Jan Beulich wrote:
> >> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> >> reduced hardware sleep support, and the two changes didn't get
> >> synchronized: The new code doesn't call the hook function (if so
> >> requested). Fix this, requiring a boolean parameter to be added to the
> >> hook function to distinguish "extended" from "legacy" sleep.
> >> 
> >> This requires adjusting TXT, but the adjustments only go as far as
> >> failing the extended mode call (since, looking at the TXT interface,
> >> there doesn't even appear to be precautions to deal with that
> >> alternative interface).
> >> 
> >> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
> >> v5 (reduced HW) sleep interface") on the master branch of
> >> git://xenbits.xen.org/xen.git.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
> >> Cc: Gang Wei <gang.wei@intel.com>
> >> Cc: Shane Wang <shane.wang@intel.com>
> >> ---
> >> v2: Extend description to include reference to hypervisor side change.
> >> 
> >> ---
> >>  arch/x86/kernel/tboot.c          |    6 +++++-
> >>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
> >>  drivers/acpi/acpica/hwsleep.c    |    2 +-
> >>  drivers/acpi/osl.c               |   16 ++++++++--------
> >>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
> >>  include/linux/acpi.h             |   10 +++++-----
> >>  include/xen/acpi.h               |    4 ++--
> >>  include/xen/interface/platform.h |    7 ++++---
> >>  8 files changed, 46 insertions(+), 33 deletions(-)
> > 
> > ACPICA changes (hwesleep.c and hwsleep.c) need to go separately and through 
> > the
> > upstream ACPICA before we can take them into the kernel.  Sorry about that.
> 
> Looking at 09f98a8 (the change originally introducing the hook)
> and comparing with the rest of the changes to hwsleep.c, I would
> think this one didn't come from the ACPICA tree either.

No, it didn't, but the policy has changed since then.

> But anyway - if I really need to do that, are there any pointers as
> to where to submit this to, and how to make sure it gets picked up
> rather sooner than later?

You can submit it to linux-acpi as before, but as separate patches and please
CC me and Bob Moore on that submission.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 16:12           ` Jan Beulich
  2013-03-11 16:58             ` Rafael J. Wysocki
@ 2013-03-11 16:58             ` Rafael J. Wysocki
  1 sibling, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2013-03-11 16:58 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Konrad Rzeszutek Wilk, Shane Wang, Bob Moore, xen-devel,
	linux-acpi, tboot-devel, Lv Zheng, richard.l.maliszewski,
	Gang Wei

On Monday, March 11, 2013 04:12:42 PM Jan Beulich wrote:
> >>> On 11.03.13 at 15:51, "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > On Monday, March 11, 2013 02:06:51 PM Jan Beulich wrote:
> >> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> >> reduced hardware sleep support, and the two changes didn't get
> >> synchronized: The new code doesn't call the hook function (if so
> >> requested). Fix this, requiring a boolean parameter to be added to the
> >> hook function to distinguish "extended" from "legacy" sleep.
> >> 
> >> This requires adjusting TXT, but the adjustments only go as far as
> >> failing the extended mode call (since, looking at the TXT interface,
> >> there doesn't even appear to be precautions to deal with that
> >> alternative interface).
> >> 
> >> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
> >> v5 (reduced HW) sleep interface") on the master branch of
> >> git://xenbits.xen.org/xen.git.
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
> >> Cc: Gang Wei <gang.wei@intel.com>
> >> Cc: Shane Wang <shane.wang@intel.com>
> >> ---
> >> v2: Extend description to include reference to hypervisor side change.
> >> 
> >> ---
> >>  arch/x86/kernel/tboot.c          |    6 +++++-
> >>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
> >>  drivers/acpi/acpica/hwsleep.c    |    2 +-
> >>  drivers/acpi/osl.c               |   16 ++++++++--------
> >>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
> >>  include/linux/acpi.h             |   10 +++++-----
> >>  include/xen/acpi.h               |    4 ++--
> >>  include/xen/interface/platform.h |    7 ++++---
> >>  8 files changed, 46 insertions(+), 33 deletions(-)
> > 
> > ACPICA changes (hwesleep.c and hwsleep.c) need to go separately and through 
> > the
> > upstream ACPICA before we can take them into the kernel.  Sorry about that.
> 
> Looking at 09f98a8 (the change originally introducing the hook)
> and comparing with the rest of the changes to hwsleep.c, I would
> think this one didn't come from the ACPICA tree either.

No, it didn't, but the policy has changed since then.

> But anyway - if I really need to do that, are there any pointers as
> to where to submit this to, and how to make sure it gets picked up
> rather sooner than later?

You can submit it to linux-acpi as before, but as separate patches and please
CC me and Bob Moore on that submission.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* RE: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:51         ` Rafael J. Wysocki
  2013-03-11 16:12           ` Jan Beulich
  2013-03-11 16:12           ` Jan Beulich
@ 2013-03-12  0:59           ` Zheng, Lv
  2013-03-12  7:57             ` Jan Beulich
  2013-03-12  7:57             ` Jan Beulich
  2013-03-12  0:59           ` Zheng, Lv
  3 siblings, 2 replies; 28+ messages in thread
From: Zheng, Lv @ 2013-03-12  0:59 UTC (permalink / raw)
  To: Rafael J. Wysocki, Jan Beulich
  Cc: Konrad Rzeszutek Wilk, Wei, Gang, Maliszewski, Richard L, Wang,
	Shane, tboot-devel, xen-devel, linux-acpi, Moore, Robert

> > +#include <linux/acpi.h>

This line shouldn't appear in ACPICA codes (drivers/acpi/acpica).
Please try to declare OSL interfaces via include/acpi/platform/aclinux.h.

> >
> > +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> > +				       acpi_gbl_sleep_type_b, true);

bool is not used in ACPICA, please try u8 instead.

> >  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> > -				       pm1b_control);
> > +				       pm1b_control, false);

Likewise.

> > -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> > -				  u32 pm1b_control)
> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> > +				  bool extended)

Is this an ACPICA OSL interface? Then it should not include bool parameter.

Thanks
-Lv


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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:51         ` Rafael J. Wysocki
                             ` (2 preceding siblings ...)
  2013-03-12  0:59           ` Zheng, Lv
@ 2013-03-12  0:59           ` Zheng, Lv
  3 siblings, 0 replies; 28+ messages in thread
From: Zheng, Lv @ 2013-03-12  0:59 UTC (permalink / raw)
  To: Rafael J. Wysocki, Jan Beulich
  Cc: Konrad Rzeszutek Wilk, Wang, Shane, Moore, Robert, xen-devel,
	linux-acpi, tboot-devel, Maliszewski, Richard L, Wei, Gang

> > +#include <linux/acpi.h>

This line shouldn't appear in ACPICA codes (drivers/acpi/acpica).
Please try to declare OSL interfaces via include/acpi/platform/aclinux.h.

> >
> > +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> > +				       acpi_gbl_sleep_type_b, true);

bool is not used in ACPICA, please try u8 instead.

> >  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> > -				       pm1b_control);
> > +				       pm1b_control, false);

Likewise.

> > -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> > -				  u32 pm1b_control)
> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> > +				  bool extended)

Is this an ACPICA OSL interface? Then it should not include bool parameter.

Thanks
-Lv

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

* RE: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:06       ` Jan Beulich
                           ` (2 preceding siblings ...)
  2013-03-12  6:39         ` Wei, Gang
@ 2013-03-12  6:39         ` Wei, Gang
  2013-03-12  7:58           ` Jan Beulich
  2013-03-12  7:58           ` Jan Beulich
  3 siblings, 2 replies; 28+ messages in thread
From: Wei, Gang @ 2013-03-12  6:39 UTC (permalink / raw)
  To: Jan Beulich, Konrad Rzeszutek Wilk, Rafael J. Wysocki
  Cc: Maliszewski, Richard L, Wang, Shane, tboot-devel, xen-devel,
	linux-acpi, Wei, Gang

[-- Attachment #1: Type: text/plain, Size: 2566 bytes --]

Jan Beulich wrote on 2013-03-11:
> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> reduced hardware sleep support, and the two changes didn't get
> synchronized: The new code doesn't call the hook function (if so
> requested). Fix this, requiring a boolean parameter to be added to the
> hook function to distinguish "extended" from "legacy" sleep.
> 
> This requires adjusting TXT, but the adjustments only go as far as
> failing the extended mode call (since, looking at the TXT interface,
> there doesn't even appear to be precautions to deal with that
> alternative interface).
> 
> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
> v5 (reduced HW) sleep interface") on the master branch of
> git://xenbits.xen.org/xen.git.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
> Cc: Gang Wei <gang.wei@intel.com>
> Cc: Shane Wang <shane.wang@intel.com>
> ---
> v2: Extend description to include reference to hypervisor side change.
> 
> ---
>  arch/x86/kernel/tboot.c          |    6 +++++-
>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>  drivers/acpi/osl.c               |   16 ++++++++--------
>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>  include/linux/acpi.h             |   10 +++++-----
>  include/xen/acpi.h               |    4 ++--
>  include/xen/interface/platform.h |    7 ++++---
>  8 files changed, 46 insertions(+), 33 deletions(-)
> --- 3.9-rc2/arch/x86/kernel/tboot.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>  }
> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
pm1b_control,
> +		       bool extended)
>  { 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = { 		/*
S0,1,2: */
>  -1, -1, -1, @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state,
>  u 	if (!tboot_enabled()) 		return 0;
> +	if (extended)
> +		return -1;
> +
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;

So looks like to make the extended way go further with TXT case other than
failing, tboot & its' interface have to be modified to support Reduced
Hardware sleeping first, is that true?

Jimmy

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 8586 bytes --]

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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-11 14:06       ` Jan Beulich
  2013-03-11 14:51         ` Rafael J. Wysocki
  2013-03-11 14:51         ` Rafael J. Wysocki
@ 2013-03-12  6:39         ` Wei, Gang
  2013-03-12  6:39         ` Wei, Gang
  3 siblings, 0 replies; 28+ messages in thread
From: Wei, Gang @ 2013-03-12  6:39 UTC (permalink / raw)
  To: Jan Beulich, Konrad Rzeszutek Wilk, Rafael J. Wysocki
  Cc: Wang, Shane, xen-devel, linux-acpi, tboot-devel, Maliszewski,
	Richard L, Wei, Gang


[-- Attachment #1.1: Type: text/plain, Size: 2566 bytes --]

Jan Beulich wrote on 2013-03-11:
> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
> reduced hardware sleep support, and the two changes didn't get
> synchronized: The new code doesn't call the hook function (if so
> requested). Fix this, requiring a boolean parameter to be added to the
> hook function to distinguish "extended" from "legacy" sleep.
> 
> This requires adjusting TXT, but the adjustments only go as far as
> failing the extended mode call (since, looking at the TXT interface,
> there doesn't even appear to be precautions to deal with that
> alternative interface).
> 
> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
> v5 (reduced HW) sleep interface") on the master branch of
> git://xenbits.xen.org/xen.git.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
> Cc: Gang Wei <gang.wei@intel.com>
> Cc: Shane Wang <shane.wang@intel.com>
> ---
> v2: Extend description to include reference to hypervisor side change.
> 
> ---
>  arch/x86/kernel/tboot.c          |    6 +++++-
>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>  drivers/acpi/osl.c               |   16 ++++++++--------
>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>  include/linux/acpi.h             |   10 +++++-----
>  include/xen/acpi.h               |    4 ++--
>  include/xen/interface/platform.h |    7 ++++---
>  8 files changed, 46 insertions(+), 33 deletions(-)
> --- 3.9-rc2/arch/x86/kernel/tboot.c
> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>  }
> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
pm1b_control,
> +		       bool extended)
>  { 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = { 		/*
S0,1,2: */
>  -1, -1, -1, @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state,
>  u 	if (!tboot_enabled()) 		return 0;
> +	if (extended)
> +		return -1;
> +
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;

So looks like to make the extended way go further with TXT case other than
failing, tboot & its' interface have to be modified to support Reduced
Hardware sleeping first, is that true?

Jimmy

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 8586 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

* RE: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-12  0:59           ` Zheng, Lv
@ 2013-03-12  7:57             ` Jan Beulich
  2013-03-18  1:46               ` Zheng, Lv
  2013-03-18  1:46               ` Zheng, Lv
  2013-03-12  7:57             ` Jan Beulich
  1 sibling, 2 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-12  7:57 UTC (permalink / raw)
  To: Lv Zheng, Rafael J. Wysocki
  Cc: Gang Wei, Richard L Maliszewski, Robert Moore, Shane Wang,
	tboot-devel, xen-devel, Konrad Rzeszutek Wilk, linux-acpi

>>> On 12.03.13 at 01:59, "Zheng, Lv" <lv.zheng@intel.com> wrote:
>> > +#include <linux/acpi.h>
> 
> This line shouldn't appear in ACPICA codes (drivers/acpi/acpica).
> Please try to declare OSL interfaces via include/acpi/platform/aclinux.h.

I'm sorry, but why am I not permitted to follow code that's
already there:
- hwsleep.c already includes linux/acpi.h
- acpi_os_set_prepare_sleep() and acpi_os_prepare_sleep() are
  both declared/stubbed out in linux/acpi.h

I'm particularly not intending to do cleanup work on ACPICA just
so that I can subsequently fix incomplete code.

Rafael, with that in mind, your request to submit the ACPICA
changes separately is impossible: The hook functions just can't
have a declaration in the non-Linux ACPI headers (or else they
wouldn't currently live in linux/acpi.h). Hence breaking up the
patch in the way you asked me to will not even compile without
first cleaning up the existing code. And again - I'm sorry, no,
that's not something I was looking forward to do.

>> >
>> > +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
>> > +				       acpi_gbl_sleep_type_b, true);
> 
> bool is not used in ACPICA, please try u8 instead.

Can do, but again - why am I not permitted to follow what's
already there? acglobal.h has

bool ACPI_INIT_GLOBAL(acpi_gbl_enable_aml_debug_object, FALSE);

The more that the declaration, as pointed out above, lives in
linux/acpi.h (and did so even _before_ this patch), i.e. isn't an
ACPICA one.

>> >  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
>> > -				       pm1b_control);
>> > +				       pm1b_control, false);
> 
> Likewise.
> 
>> > -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
>> > -				  u32 pm1b_control)
>> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
>> > +				  bool extended)
> 
> Is this an ACPICA OSL interface? Then it should not include bool parameter.

Returning the question: Given the current placement of
declarations, it isn't. But if the current layout is wrong, than that
needs fixing first. Which in turn likely means that the inconsistency
will continue to persist for the next (or the next few) kernel
version(s).

Jan


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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-12  0:59           ` Zheng, Lv
  2013-03-12  7:57             ` Jan Beulich
@ 2013-03-12  7:57             ` Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-12  7:57 UTC (permalink / raw)
  To: Lv Zheng, Rafael J. Wysocki
  Cc: Konrad Rzeszutek Wilk, Shane Wang, Robert Moore, xen-devel,
	linux-acpi, tboot-devel, Richard L Maliszewski, Gang Wei

>>> On 12.03.13 at 01:59, "Zheng, Lv" <lv.zheng@intel.com> wrote:
>> > +#include <linux/acpi.h>
> 
> This line shouldn't appear in ACPICA codes (drivers/acpi/acpica).
> Please try to declare OSL interfaces via include/acpi/platform/aclinux.h.

I'm sorry, but why am I not permitted to follow code that's
already there:
- hwsleep.c already includes linux/acpi.h
- acpi_os_set_prepare_sleep() and acpi_os_prepare_sleep() are
  both declared/stubbed out in linux/acpi.h

I'm particularly not intending to do cleanup work on ACPICA just
so that I can subsequently fix incomplete code.

Rafael, with that in mind, your request to submit the ACPICA
changes separately is impossible: The hook functions just can't
have a declaration in the non-Linux ACPI headers (or else they
wouldn't currently live in linux/acpi.h). Hence breaking up the
patch in the way you asked me to will not even compile without
first cleaning up the existing code. And again - I'm sorry, no,
that's not something I was looking forward to do.

>> >
>> > +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
>> > +				       acpi_gbl_sleep_type_b, true);
> 
> bool is not used in ACPICA, please try u8 instead.

Can do, but again - why am I not permitted to follow what's
already there? acglobal.h has

bool ACPI_INIT_GLOBAL(acpi_gbl_enable_aml_debug_object, FALSE);

The more that the declaration, as pointed out above, lives in
linux/acpi.h (and did so even _before_ this patch), i.e. isn't an
ACPICA one.

>> >  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
>> > -				       pm1b_control);
>> > +				       pm1b_control, false);
> 
> Likewise.
> 
>> > -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
>> > -				  u32 pm1b_control)
>> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
>> > +				  bool extended)
> 
> Is this an ACPICA OSL interface? Then it should not include bool parameter.

Returning the question: Given the current placement of
declarations, it isn't. But if the current layout is wrong, than that
needs fixing first. Which in turn likely means that the inconsistency
will continue to persist for the next (or the next few) kernel
version(s).

Jan

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

* RE: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-12  6:39         ` Wei, Gang
  2013-03-12  7:58           ` Jan Beulich
@ 2013-03-12  7:58           ` Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-12  7:58 UTC (permalink / raw)
  To: Gang Wei
  Cc: Richard L Maliszewski, Shane Wang, tboot-devel, xen-devel,
	Konrad Rzeszutek Wilk, Rafael J. Wysocki, linux-acpi

>>> On 12.03.13 at 07:39, "Wei, Gang" <gang.wei@intel.com> wrote:
> Jan Beulich wrote on 2013-03-11:
>> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
>> reduced hardware sleep support, and the two changes didn't get
>> synchronized: The new code doesn't call the hook function (if so
>> requested). Fix this, requiring a boolean parameter to be added to the
>> hook function to distinguish "extended" from "legacy" sleep.
>> 
>> This requires adjusting TXT, but the adjustments only go as far as
>> failing the extended mode call (since, looking at the TXT interface,
>> there doesn't even appear to be precautions to deal with that
>> alternative interface).
>> 
>> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
>> v5 (reduced HW) sleep interface") on the master branch of
>> git://xenbits.xen.org/xen.git.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
>> Cc: Gang Wei <gang.wei@intel.com>
>> Cc: Shane Wang <shane.wang@intel.com>
>> ---
>> v2: Extend description to include reference to hypervisor side change.
>> 
>> ---
>>  arch/x86/kernel/tboot.c          |    6 +++++-
>>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>>  drivers/acpi/osl.c               |   16 ++++++++--------
>>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>>  include/linux/acpi.h             |   10 +++++-----
>>  include/xen/acpi.h               |    4 ++--
>>  include/xen/interface/platform.h |    7 ++++---
>>  8 files changed, 46 insertions(+), 33 deletions(-)
>> --- 3.9-rc2/arch/x86/kernel/tboot.c
>> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
>> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>>  }
>> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
> pm1b_control)
>> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
> pm1b_control,
>> +		       bool extended)
>>  { 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = { 		/*
> S0,1,2: */
>>  -1, -1, -1, @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state,
>>  u 	if (!tboot_enabled()) 		return 0;
>> +	if (extended)
>> +		return -1;
>> +
>>  	tboot_copy_fadt(&acpi_gbl_FADT);
>>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
> 
> So looks like to make the extended way go further with TXT case other than
> failing, tboot & its' interface have to be modified to support Reduced
> Hardware sleeping first, is that true?

Yes, afaict.

Jan


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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-12  6:39         ` Wei, Gang
@ 2013-03-12  7:58           ` Jan Beulich
  2013-03-12  7:58           ` Jan Beulich
  1 sibling, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-12  7:58 UTC (permalink / raw)
  To: Gang Wei
  Cc: Konrad Rzeszutek Wilk, Shane Wang, xen-devel, Rafael J. Wysocki,
	linux-acpi, tboot-devel, Richard L Maliszewski

>>> On 12.03.13 at 07:39, "Wei, Gang" <gang.wei@intel.com> wrote:
> Jan Beulich wrote on 2013-03-11:
>> In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
>> reduced hardware sleep support, and the two changes didn't get
>> synchronized: The new code doesn't call the hook function (if so
>> requested). Fix this, requiring a boolean parameter to be added to the
>> hook function to distinguish "extended" from "legacy" sleep.
>> 
>> This requires adjusting TXT, but the adjustments only go as far as
>> failing the extended mode call (since, looking at the TXT interface,
>> there doesn't even appear to be precautions to deal with that
>> alternative interface).
>> 
>> The hypervisor change underlying this is commit 62d1a69 ("ACPI: support
>> v5 (reduced HW) sleep interface") on the master branch of
>> git://xenbits.xen.org/xen.git.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
>> Cc: Gang Wei <gang.wei@intel.com>
>> Cc: Shane Wang <shane.wang@intel.com>
>> ---
>> v2: Extend description to include reference to hypervisor side change.
>> 
>> ---
>>  arch/x86/kernel/tboot.c          |    6 +++++-
>>  drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
>>  drivers/acpi/acpica/hwsleep.c    |    2 +-
>>  drivers/acpi/osl.c               |   16 ++++++++--------
>>  drivers/xen/acpi.c               |   26 +++++++++++++-------------
>>  include/linux/acpi.h             |   10 +++++-----
>>  include/xen/acpi.h               |    4 ++--
>>  include/xen/interface/platform.h |    7 ++++---
>>  8 files changed, 46 insertions(+), 33 deletions(-)
>> --- 3.9-rc2/arch/x86/kernel/tboot.c
>> +++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
>> @@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
>>  		offsetof(struct acpi_table_facs, firmware_waking_vector);
>>  }
>> -static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
> pm1b_control)
>> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
> pm1b_control,
>> +		       bool extended)
>>  { 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = { 		/*
> S0,1,2: */
>>  -1, -1, -1, @@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state,
>>  u 	if (!tboot_enabled()) 		return 0;
>> +	if (extended)
>> +		return -1;
>> +
>>  	tboot_copy_fadt(&acpi_gbl_FADT);
>>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
>>  	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
> 
> So looks like to make the extended way go further with TXT case other than
> failing, tboot & its' interface have to be modified to support Reduced
> Hardware sleeping first, is that true?

Yes, afaict.

Jan

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

* RE: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-12  7:57             ` Jan Beulich
  2013-03-18  1:46               ` Zheng, Lv
@ 2013-03-18  1:46               ` Zheng, Lv
  1 sibling, 0 replies; 28+ messages in thread
From: Zheng, Lv @ 2013-03-18  1:46 UTC (permalink / raw)
  To: Jan Beulich, Rafael J. Wysocki
  Cc: Wei, Gang, Maliszewski, Richard L, Moore, Robert, Wang, Shane,
	tboot-devel, xen-devel, Konrad Rzeszutek Wilk, linux-acpi

Sorry for delayed reply.

> >>> On 12.03.13 at 01:59, "Zheng, Lv" <lv.zheng@intel.com> wrote:
> >> > +#include <linux/acpi.h>
> >
> > This line shouldn't appear in ACPICA codes (drivers/acpi/acpica).
> > Please try to declare OSL interfaces via include/acpi/platform/aclinux.h.
> 
> I'm sorry, but why am I not permitted to follow code that's already there:
> - hwsleep.c already includes linux/acpi.h
> - acpi_os_set_prepare_sleep() and acpi_os_prepare_sleep() are
>   both declared/stubbed out in linux/acpi.h
> 
> I'm particularly not intending to do cleanup work on ACPICA just so that I can
> subsequently fix incomplete code.
> 
> Rafael, with that in mind, your request to submit the ACPICA changes
> separately is impossible: The hook functions just can't have a declaration in the
> non-Linux ACPI headers (or else they wouldn't currently live in linux/acpi.h).
> Hence breaking up the patch in the way you asked me to will not even compile
> without first cleaning up the existing code. And again - I'm sorry, no, that's not
> something I was looking forward to do.

Yes, you can try to use codes already there, but you still can find a way not introducing a new inclusion of <linux/acpi.h> in an ACPICA file.
Sorry for the inconvenience.

> >> > +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> >> > +				       acpi_gbl_sleep_type_b, true);
> >
> > bool is not used in ACPICA, please try u8 instead.
> 
> Can do, but again - why am I not permitted to follow what's already there?
> acglobal.h has
> 
> bool ACPI_INIT_GLOBAL(acpi_gbl_enable_aml_debug_object, FALSE);

ACPICA is a portable ACPI implementation, where there might be users using compilers not compatible with c99.

This difference is there for another reason:
It is exported as a sysfs entry, it could not be converted back to u8 without introducing new warnings during the current kernel compilation process.
At least for now, it is a committed difference.

Or you could wait until the "bool" portable layer to be implemented in the ACPICA.

> The more that the declaration, as pointed out above, lives in linux/acpi.h (and
> did so even _before_ this patch), i.e. isn't an ACPICA one.
> 
> >> >  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> >> > -				       pm1b_control);
> >> > +				       pm1b_control, false);
> >
> > Likewise.
> >
> >> > -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> >> > -				  u32 pm1b_control)
> >> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> >> > +				  bool extended)
> >
> > Is this an ACPICA OSL interface? Then it should not include bool parameter.
> 
> Returning the question: Given the current placement of declarations, it isn't.
> But if the current layout is wrong, than that needs fixing first. Which in turn
> likely means that the inconsistency will continue to persist for the next (or the
> next few) kernel version(s).

Yes, we may fix this ASAP.

Thanks
-Lv

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

* Re: [PATCH v2] Xen/ACPI: support sleep state entering on hardware reduced systems
  2013-03-12  7:57             ` Jan Beulich
@ 2013-03-18  1:46               ` Zheng, Lv
  2013-03-18  1:46               ` Zheng, Lv
  1 sibling, 0 replies; 28+ messages in thread
From: Zheng, Lv @ 2013-03-18  1:46 UTC (permalink / raw)
  To: Jan Beulich, Rafael J. Wysocki
  Cc: Konrad Rzeszutek Wilk, Wang, Shane, Moore, Robert, xen-devel,
	linux-acpi, tboot-devel, Maliszewski, Richard L, Wei, Gang

Sorry for delayed reply.

> >>> On 12.03.13 at 01:59, "Zheng, Lv" <lv.zheng@intel.com> wrote:
> >> > +#include <linux/acpi.h>
> >
> > This line shouldn't appear in ACPICA codes (drivers/acpi/acpica).
> > Please try to declare OSL interfaces via include/acpi/platform/aclinux.h.
> 
> I'm sorry, but why am I not permitted to follow code that's already there:
> - hwsleep.c already includes linux/acpi.h
> - acpi_os_set_prepare_sleep() and acpi_os_prepare_sleep() are
>   both declared/stubbed out in linux/acpi.h
> 
> I'm particularly not intending to do cleanup work on ACPICA just so that I can
> subsequently fix incomplete code.
> 
> Rafael, with that in mind, your request to submit the ACPICA changes
> separately is impossible: The hook functions just can't have a declaration in the
> non-Linux ACPI headers (or else they wouldn't currently live in linux/acpi.h).
> Hence breaking up the patch in the way you asked me to will not even compile
> without first cleaning up the existing code. And again - I'm sorry, no, that's not
> something I was looking forward to do.

Yes, you can try to use codes already there, but you still can find a way not introducing a new inclusion of <linux/acpi.h> in an ACPICA file.
Sorry for the inconvenience.

> >> > +	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
> >> > +				       acpi_gbl_sleep_type_b, true);
> >
> > bool is not used in ACPICA, please try u8 instead.
> 
> Can do, but again - why am I not permitted to follow what's already there?
> acglobal.h has
> 
> bool ACPI_INIT_GLOBAL(acpi_gbl_enable_aml_debug_object, FALSE);

ACPICA is a portable ACPI implementation, where there might be users using compilers not compatible with c99.

This difference is there for another reason:
It is exported as a sysfs entry, it could not be converted back to u8 without introducing new warnings during the current kernel compilation process.
At least for now, it is a committed difference.

Or you could wait until the "bool" portable layer to be implemented in the ACPICA.

> The more that the declaration, as pointed out above, lives in linux/acpi.h (and
> did so even _before_ this patch), i.e. isn't an ACPICA one.
> 
> >> >  	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> >> > -				       pm1b_control);
> >> > +				       pm1b_control, false);
> >
> > Likewise.
> >
> >> > -acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> >> > -				  u32 pm1b_control)
> >> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
> >> > +				  bool extended)
> >
> > Is this an ACPICA OSL interface? Then it should not include bool parameter.
> 
> Returning the question: Given the current placement of declarations, it isn't.
> But if the current layout is wrong, than that needs fixing first. Which in turn
> likely means that the inconsistency will continue to persist for the next (or the
> next few) kernel version(s).

Yes, we may fix this ASAP.

Thanks
-Lv

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

* [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems
@ 2013-03-11  9:48 Jan Beulich
  0 siblings, 0 replies; 28+ messages in thread
From: Jan Beulich @ 2013-03-11  9:48 UTC (permalink / raw)
  To: Len Brown, Konrad Rzeszutek Wilk
  Cc: Shane Wang, xen-devel, linux-acpi, tboot-devel,
	richard.l.maliszewski, Gang Wei

[-- Attachment #1: Type: text/plain, Size: 8056 bytes --]

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 



[-- Attachment #2: linux-3.9-rc2-xen-ACPI-v5-sleep.patch --]
[-- Type: text/plain, Size: 8122 bytes --]

Xen/ACPI: support sleep state entering on hardware reduced systems

In version 3.4 acpi_os_prepare_sleep() got introduced in parallel with
reduced hardware sleep support, and the two changes didn't get
synchronized: The new code doesn't call the hook function (if so
requested). Fix this, requiring a boolean parameter to be added to the
hook function to distinguish "extended" from "legacy" sleep.

This requires adjusting TXT, but the adjustments only go as far as
failing the extended mode call (since, looking at the TXT interface,
there doesn't even appear to be precautions to deal with that
alternative interface).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Richard L Maliszewski <richard.l.maliszewski@intel.com>
Cc: Gang Wei <gang.wei@intel.com>
Cc: Shane Wang <shane.wang@intel.com>

---
 arch/x86/kernel/tboot.c          |    6 +++++-
 drivers/acpi/acpica/hwesleep.c   |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c    |    2 +-
 drivers/acpi/osl.c               |   16 ++++++++--------
 drivers/xen/acpi.c               |   26 +++++++++++++-------------
 include/linux/acpi.h             |   10 +++++-----
 include/xen/acpi.h               |    4 ++--
 include/xen/interface/platform.h |    7 ++++---
 8 files changed, 46 insertions(+), 33 deletions(-)

--- 3.9-rc2/arch/x86/kernel/tboot.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control,
+		       bool extended)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -284,6 +285,9 @@ static int tboot_sleep(u8 sleep_state, u
 	if (!tboot_enabled())
 		return 0;
 
+	if (extended)
+		return -1;
+
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
 	tboot->acpi_sinfo.pm1b_cnt_val = pm1b_control;
--- 3.9-rc2/drivers/acpi/acpica/hwesleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwesleep.c
@@ -43,6 +43,7 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -128,6 +129,13 @@ acpi_status acpi_hw_extended_sleep(u8 sl
 
 	ACPI_FLUSH_CPU_CACHE();
 
+	status = acpi_os_prepare_sleep(sleep_state, acpi_gbl_sleep_type_a,
+				       acpi_gbl_sleep_type_b, true);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
 	/*
 	 * Set the SLP_TYP and SLP_EN bits.
 	 *
--- 3.9-rc2/drivers/acpi/acpica/hwsleep.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/acpica/hwsleep.c
@@ -153,7 +153,7 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
 	ACPI_FLUSH_CPU_CACHE();
 
 	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
-				       pm1b_control);
+				       pm1b_control, false);
 	if (ACPI_SKIP(status))
 		return_ACPI_STATUS(AE_OK);
 	if (ACPI_FAILURE(status))
--- 3.9-rc2/drivers/acpi/osl.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/acpi/osl.c
@@ -77,8 +77,8 @@ EXPORT_SYMBOL(acpi_in_debugger);
 extern char line_buf[80];
 #endif				/*ENABLE_DEBUGGER */
 
-static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
-				      u32 pm1b_ctrl);
+static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 val_a, u32 val_b,
+				      bool extended);
 
 static acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
@@ -1757,13 +1757,13 @@ acpi_status acpi_os_terminate(void)
 	return AE_OK;
 }
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
-				  u32 pm1b_control)
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended)
 {
 	int rc = 0;
 	if (__acpi_os_prepare_sleep)
-		rc = __acpi_os_prepare_sleep(sleep_state,
-					     pm1a_control, pm1b_control);
+		rc = __acpi_os_prepare_sleep(sleep_state, val_a, val_b,
+					     extended);
 	if (rc < 0)
 		return AE_ERROR;
 	else if (rc > 0)
@@ -1772,8 +1772,8 @@ acpi_status acpi_os_prepare_sleep(u8 sle
 	return AE_OK;
 }
 
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended))
 {
 	__acpi_os_prepare_sleep = func;
 }
--- 3.9-rc2/drivers/xen/acpi.c
+++ 3.9-rc2-xen-ACPI-v5-sleep/drivers/xen/acpi.c
@@ -35,27 +35,27 @@
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnt)
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended)
 {
+	unsigned int bits = extended ? 8 : 16;
+
 	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,
-			},
+		.u.enter_acpi_sleep = {
+			.val_a = (u16)val_a,
+			.val_b = (u16)val_b,
+			.sleep_state = sleep_state,
+			.flags = extended ? XENPF_ACPI_SLEEP_EXTENDED : 0,
 		},
 	};
 
-	if ((pm1a_cnt & 0xffff0000) || (pm1b_cnt & 0xffff0000)) {
-		WARN(1, "Using more than 16bits of PM1A/B 0x%x/0x%x!"
-		     "Email xen-devel@lists.xensource.com  Thank you.\n", \
-		     pm1a_cnt, pm1b_cnt);
+	if (WARN((val_a & (~0 << bits)) || (val_b & (~0 << bits)),
+		 "Using more than %u bits of sleep control values %#x/%#x!"
+		 "Email xen-devel@lists.xen.org - Thank you.\n", \
+		 bits, val_a, val_b))
 		return -1;
-	}
 
 	HYPERVISOR_dom0_op(&op);
 	return 1;
--- 3.9-rc2/include/linux/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/linux/acpi.h
@@ -486,11 +486,11 @@ static inline bool acpi_driver_match_dev
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI
-void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
-			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state, u32 val_a,
+					   u32 val_b, bool extended));
 
-acpi_status acpi_os_prepare_sleep(u8 sleep_state,
-				  u32 pm1a_control, u32 pm1b_control);
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 val_a, u32 val_b,
+				  bool extended);
 #ifdef CONFIG_X86
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
@@ -500,7 +500,7 @@ static inline void arch_reserve_mem_area
 }
 #endif /* CONFIG_X86 */
 #else
-#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#define acpi_os_set_prepare_sleep(func, val_a, val_b, ext) do { } while (0)
 #endif
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM_RUNTIME)
--- 3.9-rc2/include/xen/acpi.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/acpi.h
@@ -75,8 +75,8 @@ static inline int xen_acpi_get_pxm(acpi_
 	return -ENXIO;
 }
 
-int xen_acpi_notify_hypervisor_state(u8 sleep_state,
-				     u32 pm1a_cnt, u32 pm1b_cnd);
+int xen_acpi_notify_hypervisor_state(u8 sleep_state, u32 val_a, u32 val_b,
+				     bool extended);
 
 static inline void xen_acpi_sleep_register(void)
 {
--- 3.9-rc2/include/xen/interface/platform.h
+++ 3.9-rc2-xen-ACPI-v5-sleep/include/xen/interface/platform.h
@@ -152,10 +152,11 @@ DEFINE_GUEST_HANDLE_STRUCT(xenpf_firmwar
 #define XENPF_enter_acpi_sleep    51
 struct xenpf_enter_acpi_sleep {
 	/* IN variables */
-	uint16_t pm1a_cnt_val;      /* PM1a control value. */
-	uint16_t pm1b_cnt_val;      /* PM1b control value. */
+	uint16_t val_a;             /* PM1a control / sleep type A. */
+	uint16_t val_b;             /* PM1b control / sleep type B. */
 	uint32_t sleep_state;       /* Which state to enter (Sn). */
-	uint32_t flags;             /* Must be zero. */
+#define XENPF_ACPI_SLEEP_EXTENDED 0x00000001
+	uint32_t flags;             /* XENPF_ACPI_SLEEP_*. */
 };
 DEFINE_GUEST_HANDLE_STRUCT(xenpf_enter_acpi_sleep_t);
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

end of thread, other threads:[~2013-03-18  1:46 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-11  9:48 [PATCH] Xen/ACPI: support sleep state entering on hardware reduced systems Jan Beulich
2013-03-11 13:21 ` Konrad Rzeszutek Wilk
2013-03-11 13:21 ` [Xen-devel] " Konrad Rzeszutek Wilk
2013-03-11 13:32   ` Jan Beulich
2013-03-11 13:32   ` [Xen-devel] " Jan Beulich
2013-03-11 13:53     ` Konrad Rzeszutek Wilk
2013-03-11 14:02       ` Jan Beulich
2013-03-11 14:02       ` [Xen-devel] " Jan Beulich
2013-03-11 14:06       ` [PATCH v2] " Jan Beulich
2013-03-11 14:06       ` Jan Beulich
2013-03-11 14:51         ` Rafael J. Wysocki
2013-03-11 14:51         ` Rafael J. Wysocki
2013-03-11 16:12           ` Jan Beulich
2013-03-11 16:12           ` Jan Beulich
2013-03-11 16:58             ` Rafael J. Wysocki
2013-03-11 16:58             ` Rafael J. Wysocki
2013-03-12  0:59           ` Zheng, Lv
2013-03-12  7:57             ` Jan Beulich
2013-03-18  1:46               ` Zheng, Lv
2013-03-18  1:46               ` Zheng, Lv
2013-03-12  7:57             ` Jan Beulich
2013-03-12  0:59           ` Zheng, Lv
2013-03-12  6:39         ` Wei, Gang
2013-03-12  6:39         ` Wei, Gang
2013-03-12  7:58           ` Jan Beulich
2013-03-12  7:58           ` Jan Beulich
2013-03-11 13:53     ` [PATCH] " Konrad Rzeszutek Wilk
2013-03-11  9:48 Jan Beulich

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.