[PATCH] ACPI cleanup's and enablement for Xen ACPI S3 [v4].

[PATCH] ACPI cleanup's and enablement for Xen ACPI S3 [v4].

[Konrad Rzeszutek Wilk]

Attached is an [v4] set of patches to enable S3 to work with the Xen hypervisor
and also do some cleanups. Posting just before holidays so that when folks are
tired of unwrapping presents, drinking eggnog, etc, they can read some
exciting patches!

Changes since v3: [http://lkml.org/lkml/2011/11/15/340]:
 - Redid the acpi_os_prepare_sleep fnc per Rafael's suggestions.
 - Fixed compile issues on other platforms, various config options.
Changes since v2: [https://lkml.org/lkml/2011/9/29/408]
 - Moved tboot_sleep out to the osl.c code.
 - Dropped some patches.
since the RFC posting [http://comments.gmane.org/gmane.linux.acpi.devel/50701]:
 - Per review comments added: __unused__ attribute, support for PM1A/B if more than 16-bit,
   copyright/license.
 - Added support for PHYSDEVOP_restore_msi_ext call.

The first two patches can be considered independently as cleanup - they move
the tboot_sleep out of the ACPI code and move it in the OS part. That is
the only OSPM code changes done.

The more complex ones are in the ACPI x86 code. I was not sure how to
post the patches so I grouped them in "functionality" parts.

1). Use the acpi_os_prepare_sleep to register a variant of it. The reason
    for the need for this is explained in more details below. The patches are:

 [PATCH 1/7] x86, acpi, tboot: Have a ACPI os prepare sleep instead
 [PATCH 2/7] tboot: Add return values for tboot_sleep
 [PATCH 3/7] xen/acpi/sleep: Enable ACPI sleep via the

2). Expand x86_msi_ops. Every time we resume, we end up calling write_msi_irq
    to resume the MSI vectors. But when using Xen, we would write the MSI
    vectors using the other x86_msi_ops - hence we expand the x86_msi_ops
    indirection mechanism to take resume in account. The paches are:

 [PATCH 4/7] x86: Expand the x86_msi_ops to have a restore MSIs.
 [PATCH 5/7] xen/pci: Utilize the restore_msi_irqs hook.

3). Make acpi_suspend_lowlevel be a function pointer instead of a function.
    Details of why we want to omit the lowlevel values is explained below.
    Originally I was thinking that perhaps doing it via a registration
    function would be better? But not sure what folks leanings are
    in this case. The patches are:

 [PATCH 6/7] x86/acpi/sleep: Provide registration for
 [PATCH 7/7] xen/acpi/sleep: Register to the acpi_suspend_lowlevel a

Details of what I said in the first postings:

The Xen ACPI S3 functionality requires help from the Linux kernel. The Linux
kernel does the ACPI "stuff" and tells the hypervisor to do the low-level
stuff (such as program the IOAPIC, setup vectors, etc). Naturally do it correctly
the Xen hypervisor must be programmed with correct values that are extracted
as part of parsing the ACPI.

The ACPI code used during suspend is mostly all in hwsleep.c and there is one
particular case where 'hwsleep.c' is calling in the tboot.c code. This is replaced
by making the call go through the OS part of the ACPI code. The reason for
doing this is two fold: 1) cleanup, 2) for Xen case, it needs to make a hypercall
so that the hypervisor can write the PM1A/PM1B bits.

The major difficulties we hit was with 'acpi_suspend_lowlevel' - which tweaks
a lot of lowlevel values and some of them are not properly handled by Xen.
Liang Tang has figured which ones of them we trip over (read below) - and he
suggested that perhaps we can provide a registration mechanism to abstract
this away. The reason for all of this is that Linux does not talk to the
BIOS directly - instead it simply walks through the necessary ACPI methods
and then issues hypercall to Xen which then further completes the remaining
suspend steps.

So the attached patches do exactly that - there are two entry points
in the ACPI.

 1). For S3: acpi_suspend_lowlevel -> .. lots of code -> acpi_enter_sleep_state
 2). For S1/S4/S5: acpi_enter_sleep_state

The first naive idea was of abstracting away in the 'acpi_enter_sleep_state'
function the tboot_sleep code so that we can use it too. And low-behold - it
worked splendidly for powering off (S5 I believe)

For S3 that did not work - during suspend the hypervisor tripped over when
saving cr8. During resume it tripped over at restoring the cr3, cr8, idt,
and gdt values.

When I posted the RFC, the feedback I got was to use a higher upper
interface to make the call to the hypervisor. Instead of doing it at the
lower pv-ops case for cr3, cr8, idt, gdt, etc. The code is much nicer
this way, I've to say.

Anyhow, please take a look!

The patches are also located at

git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git devel/acpi-s3.v7

 arch/x86/include/asm/acpi.h     |    2 +-
 arch/x86/include/asm/pci.h      |    9 +++++
 arch/x86/include/asm/x86_init.h |    1 +
 arch/x86/kernel/acpi/boot.c     |    7 ++++
 arch/x86/kernel/acpi/sleep.c    |    4 +-
 arch/x86/kernel/acpi/sleep.h    |    2 +
 arch/x86/kernel/tboot.c         |    9 +++--
 arch/x86/kernel/x86_init.c      |    1 +
 arch/x86/pci/xen.c              |   27 ++++++++++++++
 arch/x86/xen/enlighten.c        |    3 ++
 drivers/acpi/acpica/hwsleep.c   |   10 ++++--
 drivers/acpi/osl.c              |   24 +++++++++++++
 drivers/acpi/sleep.c            |    2 +
 drivers/pci/msi.c               |   29 ++++++++++++++-
 drivers/xen/Makefile            |    2 +-
 drivers/xen/acpi.c              |   62 +++++++++++++++++++++++++++++++++
 include/acpi/acexcep.h          |    1 +
 include/linux/acpi.h            |   10 +++++
 include/linux/tboot.h           |    1 -
 include/xen/acpi.h              |   72 +++++++++++++++++++++++++++++++++++++++
 include/xen/interface/physdev.h |    7 ++++
 21 files changed, 272 insertions(+), 13 deletions(-)

Konrad Rzeszutek Wilk (5):
      tboot: Add return values for tboot_sleep
      x86/acpi/sleep: Provide registration for acpi_suspend_lowlevel.
      xen/acpi/sleep: Enable ACPI sleep via the __acpi_os_prepare_sleep
      xen/acpi/sleep: Register to the acpi_suspend_lowlevel a callback.
      x86: Expand the x86_msi_ops to have a restore MSIs.

Tang Liang (2):
      x86, acpi, tboot: Have a ACPI os prepare sleep instead of calling tboot_sleep.
      xen: Utilize the restore_msi_irqs hook.

[PATCH 1/7] x86, acpi, tboot: Have a ACPI os prepare sleep instead of calling tboot_sleep.

[Konrad Rzeszutek Wilk]

From: Tang Liang <liang.tang@oracle.com>

The ACPI suspend path makes a call to tboot_sleep right before
it writes the PM1A, PM1B values. We replace the direct call to
tboot via an registration callback similar to __acpi_register_gsi.

CC: Thomas Gleixner <tglx@linutronix.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Len Brown <len.brown@intel.com>
CC: Joseph Cihula <joseph.cihula@intel.com>
CC: Shane Wang <shane.wang@intel.com>
CC: xen-devel@lists.xensource.com
CC: linux-pm@lists.linux-foundation.org
CC: tboot-devel@lists.sourceforge.net
CC: linux-acpi@vger.kernel.org
[v1: Added __attribute__ ((unused))]
[v2: Introduced a wrapper instead of changing tboot_sleep return values]
[v3: Added return value AE_CTRL_SKIP for acpi_os_sleep_prepare]
Signed-off-by: Tang Liang <liang.tang@oracle.com>
[v1: Fix compile issues on IA64 and PPC64]
[v2: Fix where __acpi_os_prepare_sleep==NULL and did not go in sleep properly]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/tboot.c       |    8 ++++++++
 drivers/acpi/acpica/hwsleep.c |   10 +++++++---
 drivers/acpi/osl.c            |   24 ++++++++++++++++++++++++
 include/acpi/acexcep.h        |    1 +
 include/linux/acpi.h          |   10 ++++++++++
 include/linux/tboot.h         |    1 -
 6 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index e2410e2..1a4ab7d 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -297,6 +297,12 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
 }
+static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
+			       u32 pm1b_control)
+{
+	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
+	return 0;
+}
 
 static atomic_t ap_wfs_count;
 
@@ -345,6 +351,8 @@ static __init int tboot_late_init(void)
 
 	atomic_set(&ap_wfs_count, 0);
 	register_hotcpu_notifier(&tboot_cpu_notifier);
+
+	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
 	return 0;
 }
 
diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c
index d52da30..992359a 100644
--- a/drivers/acpi/acpica/hwsleep.c
+++ b/drivers/acpi/acpica/hwsleep.c
@@ -43,9 +43,9 @@
  */
 
 #include <acpi/acpi.h>
+#include <linux/acpi.h>
 #include "accommon.h"
 #include "actables.h"
-#include <linux/tboot.h>
 #include <linux/module.h>
 
 #define _COMPONENT          ACPI_HARDWARE
@@ -344,8 +344,12 @@ acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state)
 
 	ACPI_FLUSH_CPU_CACHE();
 
-	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
-
+	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
+				       pm1b_control);
+	if (ACPI_SKIP(status))
+		return_ACPI_STATUS(AE_OK);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
 	/* Write #2: Write both SLP_TYP + SLP_EN */
 
 	status = acpi_hw_write_pm1_control(pm1a_control, pm1b_control);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f31c5c5..f3aae4b 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -76,6 +76,9 @@ 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 acpi_osd_handler acpi_irq_handler;
 static void *acpi_irq_context;
 static struct workqueue_struct *kacpid_wq;
@@ -1659,3 +1662,24 @@ acpi_status acpi_os_terminate(void)
 
 	return AE_OK;
 }
+
+acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
+				  u32 pm1b_control)
+{
+	int rc = 0;
+	if (__acpi_os_prepare_sleep)
+		rc = __acpi_os_prepare_sleep(sleep_state,
+					     pm1a_control, pm1b_control);
+	if (rc < 0)
+		return AE_ERROR;
+	else if (rc > 0)
+		return AE_CTRL_SKIP;
+
+	return AE_OK;
+}
+
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
+			       u32 pm1a_ctrl, u32 pm1b_ctrl))
+{
+	__acpi_os_prepare_sleep = func;
+}
diff --git a/include/acpi/acexcep.h b/include/acpi/acexcep.h
index 5b6c391..fa0d22c 100644
--- a/include/acpi/acexcep.h
+++ b/include/acpi/acexcep.h
@@ -57,6 +57,7 @@
 #define ACPI_SUCCESS(a)                 (!(a))
 #define ACPI_FAILURE(a)                 (a)
 
+#define ACPI_SKIP(a)                    (a == AE_CTRL_SKIP)
 #define AE_OK                           (acpi_status) 0x0000
 
 /*
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6001b4da..fccd017 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -359,4 +359,14 @@ static inline int suspend_nvs_register(unsigned long a, unsigned long b)
 }
 #endif
 
+#ifdef CONFIG_ACPI
+void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
+			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
+
+acpi_status acpi_os_prepare_sleep(u8 sleep_state,
+				  u32 pm1a_control, u32 pm1b_control);
+#else
+#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
+#endif
+
 #endif	/*_LINUX_ACPI_H*/
diff --git a/include/linux/tboot.h b/include/linux/tboot.h
index 1dba6ee..c75128b 100644
--- a/include/linux/tboot.h
+++ b/include/linux/tboot.h
@@ -143,7 +143,6 @@ static inline int tboot_enabled(void)
 
 extern void tboot_probe(void);
 extern void tboot_shutdown(u32 shutdown_type);
-extern void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control);
 extern struct acpi_table_header *tboot_get_dmar_table(
 				      struct acpi_table_header *dmar_tbl);
 extern int tboot_force_iommu(void);
-- 
1.7.7.4

[PATCH 2/7] tboot: Add return values for tboot_sleep

[Konrad Rzeszutek Wilk]

. as appropiately. As tboot_sleep now returns values.
remove tboot_sleep_wrapper.

Suggested-by: "Rafael J. Wysocki" <rjw@sisk.pl>
CC: Joseph Cihula <joseph.cihula@intel.com>
[v1: Return -1/0/+1 instead of ACPI_xx values]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/tboot.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 1a4ab7d..6410744 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -272,7 +272,7 @@ static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -281,7 +281,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 		/* S5: */ TB_SHUTDOWN_S5 };
 
 	if (!tboot_enabled())
-		return;
+		return 0;
 
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
@@ -292,15 +292,10 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 	if (sleep_state >= ACPI_S_STATE_COUNT ||
 	    acpi_shutdown_map[sleep_state] == -1) {
 		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
-		return;
+		return -1;
 	}
 
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
-}
-static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
-			       u32 pm1b_control)
-{
-	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
 	return 0;
 }
 
@@ -352,7 +347,7 @@ static __init int tboot_late_init(void)
 	atomic_set(&ap_wfs_count, 0);
 	register_hotcpu_notifier(&tboot_cpu_notifier);
 
-	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
+	acpi_os_set_prepare_sleep(&tboot_sleep);
 	return 0;
 }
 
-- 
1.7.7.4

[PATCH 3/7] x86/acpi/sleep: Provide registration for acpi_suspend_lowlevel.

[Konrad Rzeszutek Wilk]

Which by default will be x86_acpi_suspend_lowlevel.
This registration allows us to register another callback
if there is a need to use another platform specific callback.

CC: Thomas Gleixner <tglx@linutronix.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: x86@kernel.org
CC: Len Brown <len.brown@intel.com>
CC: Joseph Cihula <joseph.cihula@intel.com>
CC: Shane Wang <shane.wang@intel.com>
CC: linux-pm@lists.linux-foundation.org
CC: linux-acpi@vger.kernel.org
CC: Len Brown <len.brown@intel.com>
Signed-off-by: Liang Tang <liang.tang@oracle.com>
[v1: Fix when CONFIG_ACPI_SLEEP is not set]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/include/asm/acpi.h  |    2 +-
 arch/x86/kernel/acpi/boot.c  |    7 +++++++
 arch/x86/kernel/acpi/sleep.c |    4 ++--
 arch/x86/kernel/acpi/sleep.h |    2 ++
 drivers/acpi/sleep.c         |    2 ++
 5 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 610001d..68cf060 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -115,7 +115,7 @@ static inline void acpi_disable_pci(void)
 }
 
 /* Low-level suspend routine. */
-extern int acpi_suspend_lowlevel(void);
+extern int (*acpi_suspend_lowlevel)(void);
 
 extern const unsigned char acpi_wakeup_code[];
 #define acpi_wakeup_address (__pa(TRAMPOLINE_SYM(acpi_wakeup_code)))
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 4558f0d..90e06ef 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -44,6 +44,7 @@
 #include <asm/mpspec.h>
 #include <asm/smp.h>
 
+#include "sleep.h" /* To include x86_acpi_suspend_lowlevel */
 static int __initdata acpi_force = 0;
 u32 acpi_rsdt_forced;
 int acpi_disabled;
@@ -552,6 +553,12 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
 int (*__acpi_register_gsi)(struct device *dev, u32 gsi,
 			   int trigger, int polarity) = acpi_register_gsi_pic;
 
+#ifdef CONFIG_ACPI_SLEEP
+int (*acpi_suspend_lowlevel)(void) = x86_acpi_suspend_lowlevel;
+#else
+int (*acpi_suspend_lowlevel)(void);
+#endif
+
 /*
  * success: return IRQ number (>=0)
  * failure: return < 0
diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index 103b6ab..4d2d0b1 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -25,12 +25,12 @@ static char temp_stack[4096];
 #endif
 
 /**
- * acpi_suspend_lowlevel - save kernel state
+ * x86_acpi_suspend_lowlevel - save kernel state
  *
  * Create an identity mapped page table and copy the wakeup routine to
  * low memory.
  */
-int acpi_suspend_lowlevel(void)
+int x86_acpi_suspend_lowlevel(void)
 {
 	struct wakeup_header *header;
 	/* address in low memory of the wakeup routine. */
diff --git a/arch/x86/kernel/acpi/sleep.h b/arch/x86/kernel/acpi/sleep.h
index 416d4be..4d3feb5 100644
--- a/arch/x86/kernel/acpi/sleep.h
+++ b/arch/x86/kernel/acpi/sleep.h
@@ -13,3 +13,5 @@ extern unsigned long acpi_copy_wakeup_routine(unsigned long);
 extern void wakeup_long64(void);
 
 extern void do_suspend_lowlevel(void);
+
+extern int x86_acpi_suspend_lowlevel(void);
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index 6d9a3ab..355fa4b 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -254,6 +254,8 @@ static int acpi_suspend_enter(suspend_state_t pm_state)
 		break;
 
 	case ACPI_STATE_S3:
+		if (!acpi_suspend_lowlevel)
+			return -ENOSYS;
 		error = acpi_suspend_lowlevel();
 		if (error)
 			return error;
-- 
1.7.7.4

[PATCH 4/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_os_prepare_sleep

[Konrad Rzeszutek Wilk]

Provide the registration callback to call in the Xen's
ACPI sleep functionality. This means that during S3/S5
we make a hypercall XENPF_enter_acpi_sleep with the
proper PM1A/PM1B registers.

Based of Ke Yu's <ke.yu@intel.com> initial idea.
[ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
change c68699484a65 ]

[v1: Added Copyright and license]
[v2: Added check if PM1A/B the 16-bits MSB contain something. The spec
     only uses 16-bits but might have more in future]
Signed-off-by: Liang Tang <liang.tang@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/enlighten.c |    3 ++
 drivers/xen/Makefile     |    2 +-
 drivers/xen/acpi.c       |   62 ++++++++++++++++++++++++++++++++++++++++++++++
 include/xen/acpi.h       |   58 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 124 insertions(+), 1 deletions(-)
 create mode 100644 drivers/xen/acpi.c
 create mode 100644 include/xen/acpi.h

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 1f92865..d7a44cc 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -42,6 +42,7 @@
 #include <xen/page.h>
 #include <xen/hvm.h>
 #include <xen/hvc-console.h>
+#include <xen/acpi.h>
 
 #include <asm/paravirt.h>
 #include <asm/apic.h>
@@ -1277,6 +1278,8 @@ asmlinkage void __init xen_start_kernel(void)
 
 		/* Make sure ACS will be enabled */
 		pci_request_acs();
+
+		xen_acpi_sleep_register();
 	}
 		
 
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 974fffd..0435996 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_XEN_SYS_HYPERVISOR)	+= sys-hypervisor.o
 obj-$(CONFIG_XEN_PVHVM)			+= platform-pci.o
 obj-$(CONFIG_XEN_TMEM)			+= tmem.o
 obj-$(CONFIG_SWIOTLB_XEN)		+= swiotlb-xen.o
-obj-$(CONFIG_XEN_DOM0)			+= pci.o
+obj-$(CONFIG_XEN_DOM0)			+= pci.o acpi.o
 obj-$(CONFIG_XEN_PCIDEV_BACKEND)	+= xen-pciback/
 
 xen-evtchn-y				:= evtchn.o
diff --git a/drivers/xen/acpi.c b/drivers/xen/acpi.c
new file mode 100644
index 0000000..119d42a
--- /dev/null
+++ b/drivers/xen/acpi.c
@@ -0,0 +1,62 @@
+/******************************************************************************
+ * acpi.c
+ * acpi file for domain 0 kernel
+ *
+ * Copyright (c) 2011 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
+ * Copyright (c) 2011 Yu Ke ke.yu@intel.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <xen/acpi.h>
+#include <xen/interface/platform.h>
+#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)
+{
+	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,
+			},
+		},
+	};
+
+	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);
+		return -1;
+	}
+
+	HYPERVISOR_dom0_op(&op);
+	return 1;
+}
diff --git a/include/xen/acpi.h b/include/xen/acpi.h
new file mode 100644
index 0000000..48a9c01
--- /dev/null
+++ b/include/xen/acpi.h
@@ -0,0 +1,58 @@
+/******************************************************************************
+ * acpi.h
+ * acpi file for domain 0 kernel
+ *
+ * Copyright (c) 2011 Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
+ * Copyright (c) 2011 Yu Ke <ke.yu@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef _XEN_ACPI_H
+#define _XEN_ACPI_H
+
+#include <linux/types.h>
+
+#ifdef CONFIG_XEN_DOM0
+#include <asm/xen/hypervisor.h>
+#include <xen/xen.h>
+#include <linux/acpi.h>
+
+int xen_acpi_notify_hypervisor_state(u8 sleep_state,
+				     u32 pm1a_cnt, u32 pm1b_cnd);
+
+static inline void xen_acpi_sleep_register(void)
+{
+	if (xen_initial_domain())
+		acpi_os_set_prepare_sleep(
+			&xen_acpi_notify_hypervisor_state);
+}
+#else
+static inline void xen_acpi_sleep_register(void)
+{
+}
+#endif
+
+#endif	/* _XEN_ACPI_H */
-- 
1.7.7.4

[PATCH 5/7] xen/acpi/sleep: Register to the acpi_suspend_lowlevel a callback.

[Konrad Rzeszutek Wilk]

We piggyback on "x86/acpi: Provide registration for acpi_suspend_lowlevel."
to register a Xen version of the callback. The callback does not
do anything special - except it omits the x86_acpi_suspend_lowlevel.
It does that b/c during suspend it tries to save cr8 values (which
the hypervisor does not support), and then on resume path the
cr3, cr8, idt, and gdt are all resumed which clashes with what
the hypervisor has set up for the guest.

Signed-off-by: Liang Tang <liang.tang@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 include/xen/acpi.h |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/include/xen/acpi.h b/include/xen/acpi.h
index 48a9c01..ebaabbb 100644
--- a/include/xen/acpi.h
+++ b/include/xen/acpi.h
@@ -43,11 +43,25 @@
 int xen_acpi_notify_hypervisor_state(u8 sleep_state,
 				     u32 pm1a_cnt, u32 pm1b_cnd);
 
+static inline int xen_acpi_suspend_lowlevel(void)
+{
+	/*
+	* Xen will save and restore CPU context, so
+	* we can skip that and just go straight to
+	* the suspend.
+	*/
+	acpi_enter_sleep_state(ACPI_STATE_S3);
+	return 0;
+}
+
 static inline void xen_acpi_sleep_register(void)
 {
-	if (xen_initial_domain())
+	if (xen_initial_domain()) {
 		acpi_os_set_prepare_sleep(
 			&xen_acpi_notify_hypervisor_state);
+
+		acpi_suspend_lowlevel = xen_acpi_suspend_lowlevel;
+	}
 }
 #else
 static inline void xen_acpi_sleep_register(void)
-- 
1.7.7.4

[PATCH 6/7] x86: Expand the x86_msi_ops to have a restore MSIs.

[Konrad Rzeszutek Wilk]

The MSI restore function will become a function pointer in an
x86_msi_ops struct. It defaults to the implementation in the
io_apic.c and msi.c. We piggyback on the indirection mechanism
introduced by "x86: Introduce x86_msi_ops".

Cc: x86@kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/include/asm/pci.h      |    9 +++++++++
 arch/x86/include/asm/x86_init.h |    1 +
 arch/x86/kernel/x86_init.c      |    1 +
 drivers/pci/msi.c               |   29 +++++++++++++++++++++++++++--
 4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index d498943..df75d07 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -112,19 +112,28 @@ static inline void x86_teardown_msi_irq(unsigned int irq)
 {
 	x86_msi.teardown_msi_irq(irq);
 }
+static inline void x86_restore_msi_irqs(struct pci_dev *dev, int irq)
+{
+	x86_msi.restore_msi_irqs(dev, irq);
+}
 #define arch_setup_msi_irqs x86_setup_msi_irqs
 #define arch_teardown_msi_irqs x86_teardown_msi_irqs
 #define arch_teardown_msi_irq x86_teardown_msi_irq
+#define arch_restore_msi_irqs x86_restore_msi_irqs
 /* implemented in arch/x86/kernel/apic/io_apic. */
 int native_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
 void native_teardown_msi_irq(unsigned int irq);
+void native_restore_msi_irqs(struct pci_dev *dev, int irq);
 /* default to the implementation in drivers/lib/msi.c */
 #define HAVE_DEFAULT_MSI_TEARDOWN_IRQS
+#define HAVE_DEFAULT_MSI_RESTORE_IRQS
 void default_teardown_msi_irqs(struct pci_dev *dev);
+void default_restore_msi_irqs(struct pci_dev *dev, int irq);
 #else
 #define native_setup_msi_irqs		NULL
 #define native_teardown_msi_irq		NULL
 #define default_teardown_msi_irqs	NULL
+#define default_restore_msi_irqs	NULL
 #endif
 
 #define PCI_DMA_BUS_IS_PHYS (dma_ops->is_phys)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 1971e65..cd52084 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -177,6 +177,7 @@ struct x86_msi_ops {
 	int (*setup_msi_irqs)(struct pci_dev *dev, int nvec, int type);
 	void (*teardown_msi_irq)(unsigned int irq);
 	void (*teardown_msi_irqs)(struct pci_dev *dev);
+	void (*restore_msi_irqs)(struct pci_dev *dev, int irq);
 };
 
 extern struct x86_init_ops x86_init;
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index c1d6cd5..83b05ad 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -114,4 +114,5 @@ struct x86_msi_ops x86_msi = {
 	.setup_msi_irqs = native_setup_msi_irqs,
 	.teardown_msi_irq = native_teardown_msi_irq,
 	.teardown_msi_irqs = default_teardown_msi_irqs,
+	.restore_msi_irqs = default_restore_msi_irqs,
 };
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 0e6d04d..ba2ea4e 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -86,6 +86,31 @@ void default_teardown_msi_irqs(struct pci_dev *dev)
 }
 #endif
 
+#ifndef arch_restore_msi_irqs
+# define arch_restore_msi_irqs default_restore_msi_irqs
+# define HAVE_DEFAULT_MSI_RESTORE_IRQS
+#endif
+
+#ifdef HAVE_DEFAULT_MSI_RESTORE_IRQS
+void default_restore_msi_irqs(struct pci_dev *dev, int irq)
+{
+	struct msi_desc *entry;
+
+	entry = NULL;
+	if (dev->msix_enabled) {
+		list_for_each_entry(entry, &dev->msi_list, list) {
+			if (irq == entry->irq)
+				break;
+		}
+	} else if (dev->msi_enabled)  {
+		entry = irq_get_msi_desc(irq);
+	}
+
+	if (entry)
+		write_msi_msg(irq, &entry->msg);
+}
+#endif
+
 static void msi_set_enable(struct pci_dev *dev, int pos, int enable)
 {
 	u16 control;
@@ -360,7 +385,7 @@ static void __pci_restore_msi_state(struct pci_dev *dev)
 
 	pci_intx_for_msi(dev, 0);
 	msi_set_enable(dev, pos, 0);
-	write_msi_msg(dev->irq, &entry->msg);
+	arch_restore_msi_irqs(dev, dev->irq);
 
 	pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
 	msi_mask_irq(entry, msi_capable_mask(control), entry->masked);
@@ -388,7 +413,7 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
 	pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
 
 	list_for_each_entry(entry, &dev->msi_list, list) {
-		write_msi_msg(entry->irq, &entry->msg);
+		arch_restore_msi_irqs(dev, entry->irq);
 		msix_mask_irq(entry, entry->masked);
 	}
 
-- 
1.7.7.4

[PATCH 7/7] xen: Utilize the restore_msi_irqs hook.

[Konrad Rzeszutek Wilk]

From: Tang Liang <liang.tang@oracle.com>

to make a hypercall to restore the vectors in the MSI/MSI-X
configuration space.

Signed-off-by: Tang Liang <liang.tang@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/pci/xen.c              |   27 +++++++++++++++++++++++++++
 include/xen/interface/physdev.h |    7 +++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 492ade8..249a5ae 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -324,6 +324,32 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 out:
 	return ret;
 }
+
+static void xen_initdom_restore_msi_irqs(struct pci_dev *dev, int irq)
+{
+	int ret = 0;
+
+	if (pci_seg_supported) {
+		struct physdev_pci_device restore_ext;
+
+		restore_ext.seg = pci_domain_nr(dev->bus);
+		restore_ext.bus = dev->bus->number;
+		restore_ext.devfn = dev->devfn;
+		ret = HYPERVISOR_physdev_op(PHYSDEVOP_restore_msi_ext,
+					&restore_ext);
+		if (ret == -ENOSYS)
+			pci_seg_supported = false;
+		WARN(ret && ret != -ENOSYS, "restore_msi_ext -> %d\n", ret);
+	}
+	if (!pci_seg_supported) {
+		struct physdev_restore_msi restore;
+
+		restore.bus = dev->bus->number;
+		restore.devfn = dev->devfn;
+		ret = HYPERVISOR_physdev_op(PHYSDEVOP_restore_msi, &restore);
+		WARN(ret && ret != -ENOSYS, "restore_msi -> %d\n", ret);
+	}
+}
 #endif
 
 static void xen_teardown_msi_irqs(struct pci_dev *dev)
@@ -446,6 +472,7 @@ int __init pci_xen_initial_domain(void)
 #ifdef CONFIG_PCI_MSI
 	x86_msi.setup_msi_irqs = xen_initdom_setup_msi_irqs;
 	x86_msi.teardown_msi_irq = xen_teardown_msi_irq;
+	x86_msi.restore_msi_irqs = xen_initdom_restore_msi_irqs;
 #endif
 	xen_setup_acpi_sci();
 	__acpi_register_gsi = acpi_register_gsi_xen;
diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h
index c1080d9..0c28989 100644
--- a/include/xen/interface/physdev.h
+++ b/include/xen/interface/physdev.h
@@ -145,6 +145,13 @@ struct physdev_manage_pci {
 	uint8_t devfn;
 };
 
+#define PHYSDEVOP_restore_msi            19
+struct physdev_restore_msi {
+	/* IN */
+	uint8_t bus;
+	uint8_t devfn;
+};
+
 #define PHYSDEVOP_manage_pci_add_ext	20
 struct physdev_manage_pci_ext {
 	/* IN */
-- 
1.7.7.4

Re: [PATCH 1/7] x86, acpi, tboot: Have a ACPI os prepare sleep instead of calling tboot_sleep.

[Konrad Rzeszutek Wilk]

On Fri, Dec 16, 2011 at 05:38:13PM -0500, Konrad Rzeszutek Wilk wrote:
> From: Tang Liang <liang.tang@oracle.com>

Hey Joseph,

I remember you acked the initial rfc patches I had posted.
But I was wondering if these ones are to your liking (I think they are -
they aren't that much different, but I didn't want to blindly sticking
'Acked-by' as they did change a bit).

Thanks!
> 
> The ACPI suspend path makes a call to tboot_sleep right before
> it writes the PM1A, PM1B values. We replace the direct call to
> tboot via an registration callback similar to __acpi_register_gsi.
> 
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: x86@kernel.org
> CC: Len Brown <len.brown@intel.com>
> CC: Joseph Cihula <joseph.cihula@intel.com>
> CC: Shane Wang <shane.wang@intel.com>
> CC: xen-devel@lists.xensource.com
> CC: linux-pm@lists.linux-foundation.org
> CC: tboot-devel@lists.sourceforge.net
> CC: linux-acpi@vger.kernel.org
> [v1: Added __attribute__ ((unused))]
> [v2: Introduced a wrapper instead of changing tboot_sleep return values]
> [v3: Added return value AE_CTRL_SKIP for acpi_os_sleep_prepare]
> Signed-off-by: Tang Liang <liang.tang@oracle.com>
> [v1: Fix compile issues on IA64 and PPC64]
> [v2: Fix where __acpi_os_prepare_sleep==NULL and did not go in sleep properly]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/kernel/tboot.c       |    8 ++++++++
>  drivers/acpi/acpica/hwsleep.c |   10 +++++++---
>  drivers/acpi/osl.c            |   24 ++++++++++++++++++++++++
>  include/acpi/acexcep.h        |    1 +
>  include/linux/acpi.h          |   10 ++++++++++
>  include/linux/tboot.h         |    1 -
>  6 files changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
> index e2410e2..1a4ab7d 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -297,6 +297,12 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
>  
>  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
>  }
> +static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
> +			       u32 pm1b_control)
> +{
> +	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> +	return 0;
> +}
>  
>  static atomic_t ap_wfs_count;
>  
> @@ -345,6 +351,8 @@ static __init int tboot_late_init(void)
>  
>  	atomic_set(&ap_wfs_count, 0);
>  	register_hotcpu_notifier(&tboot_cpu_notifier);
> +
> +	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
>  	return 0;
>  }
>  
> diff --git a/drivers/acpi/acpica/hwsleep.c b/drivers/acpi/acpica/hwsleep.c
> index d52da30..992359a 100644
> --- a/drivers/acpi/acpica/hwsleep.c
> +++ b/drivers/acpi/acpica/hwsleep.c
> @@ -43,9 +43,9 @@
>   */
>  
>  #include <acpi/acpi.h>
> +#include <linux/acpi.h>
>  #include "accommon.h"
>  #include "actables.h"
> -#include <linux/tboot.h>
>  #include <linux/module.h>
>  
>  #define _COMPONENT          ACPI_HARDWARE
> @@ -344,8 +344,12 @@ acpi_status asmlinkage acpi_enter_sleep_state(u8 sleep_state)
>  
>  	ACPI_FLUSH_CPU_CACHE();
>  
> -	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> -
> +	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> +				       pm1b_control);
> +	if (ACPI_SKIP(status))
> +		return_ACPI_STATUS(AE_OK);
> +	if (ACPI_FAILURE(status))
> +		return_ACPI_STATUS(status);
>  	/* Write #2: Write both SLP_TYP + SLP_EN */
>  
>  	status = acpi_hw_write_pm1_control(pm1a_control, pm1b_control);
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index f31c5c5..f3aae4b 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -76,6 +76,9 @@ 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 acpi_osd_handler acpi_irq_handler;
>  static void *acpi_irq_context;
>  static struct workqueue_struct *kacpid_wq;
> @@ -1659,3 +1662,24 @@ acpi_status acpi_os_terminate(void)
>  
>  	return AE_OK;
>  }
> +
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> +				  u32 pm1b_control)
> +{
> +	int rc = 0;
> +	if (__acpi_os_prepare_sleep)
> +		rc = __acpi_os_prepare_sleep(sleep_state,
> +					     pm1a_control, pm1b_control);
> +	if (rc < 0)
> +		return AE_ERROR;
> +	else if (rc > 0)
> +		return AE_CTRL_SKIP;
> +
> +	return AE_OK;
> +}
> +
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> +			       u32 pm1a_ctrl, u32 pm1b_ctrl))
> +{
> +	__acpi_os_prepare_sleep = func;
> +}
> diff --git a/include/acpi/acexcep.h b/include/acpi/acexcep.h
> index 5b6c391..fa0d22c 100644
> --- a/include/acpi/acexcep.h
> +++ b/include/acpi/acexcep.h
> @@ -57,6 +57,7 @@
>  #define ACPI_SUCCESS(a)                 (!(a))
>  #define ACPI_FAILURE(a)                 (a)
>  
> +#define ACPI_SKIP(a)                    (a == AE_CTRL_SKIP)
>  #define AE_OK                           (acpi_status) 0x0000
>  
>  /*
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6001b4da..fccd017 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -359,4 +359,14 @@ static inline int suspend_nvs_register(unsigned long a, unsigned long b)
>  }
>  #endif
>  
> +#ifdef CONFIG_ACPI
> +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> +			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
> +
> +acpi_status acpi_os_prepare_sleep(u8 sleep_state,
> +				  u32 pm1a_control, u32 pm1b_control);
> +#else
> +#define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
> +#endif
> +
>  #endif	/*_LINUX_ACPI_H*/
> diff --git a/include/linux/tboot.h b/include/linux/tboot.h
> index 1dba6ee..c75128b 100644
> --- a/include/linux/tboot.h
> +++ b/include/linux/tboot.h
> @@ -143,7 +143,6 @@ static inline int tboot_enabled(void)
>  
>  extern void tboot_probe(void);
>  extern void tboot_shutdown(u32 shutdown_type);
> -extern void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control);
>  extern struct acpi_table_header *tboot_get_dmar_table(
>  				      struct acpi_table_header *dmar_tbl);
>  extern int tboot_force_iommu(void);
> -- 
> 1.7.7.4

RE: [PATCH 1/7] x86, acpi, tboot: Have a ACPI os prepare sleep instead of calling tboot_sleep.

[Cihula, Joseph]

ACK

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Tuesday, January 03, 2012 9:13 AM
> To: linux-kernel@vger.kernel.org; rjw@sisk.pl; x86@kernel.org; Brown, Len; Cihula, Joseph; linux-
> pm@lists.linux-foundation.org; tboot-devel@lists.sourceforge.net; linux-acpi@vger.kernel.org;
> liang.tang@oracle.com
> Cc: hpa@zytor.com; Thomas Gleixner; Wang, Shane; xen-devel@lists.xensource.com
> Subject: Re: [PATCH 1/7] x86, acpi, tboot: Have a ACPI os prepare sleep instead of calling
> tboot_sleep.
> 
> On Fri, Dec 16, 2011 at 05:38:13PM -0500, Konrad Rzeszutek Wilk wrote:
> > From: Tang Liang <liang.tang@oracle.com>
> 
> Hey Joseph,
> 
> I remember you acked the initial rfc patches I had posted.
> But I was wondering if these ones are to your liking (I think they are - they aren't that much
> different, but I didn't want to blindly sticking 'Acked-by' as they did change a bit).
> 
> Thanks!
> >
> > The ACPI suspend path makes a call to tboot_sleep right before it
> > writes the PM1A, PM1B values. We replace the direct call to tboot via
> > an registration callback similar to __acpi_register_gsi.
> >
> > CC: Thomas Gleixner <tglx@linutronix.de>
> > CC: "H. Peter Anvin" <hpa@zytor.com>
> > CC: x86@kernel.org
> > CC: Len Brown <len.brown@intel.com>
> > CC: Joseph Cihula <joseph.cihula@intel.com>
> > CC: Shane Wang <shane.wang@intel.com>
> > CC: xen-devel@lists.xensource.com
> > CC: linux-pm@lists.linux-foundation.org
> > CC: tboot-devel@lists.sourceforge.net
> > CC: linux-acpi@vger.kernel.org
> > [v1: Added __attribute__ ((unused))]
> > [v2: Introduced a wrapper instead of changing tboot_sleep return
> > values]
> > [v3: Added return value AE_CTRL_SKIP for acpi_os_sleep_prepare]
> > Signed-off-by: Tang Liang <liang.tang@oracle.com>
> > [v1: Fix compile issues on IA64 and PPC64]
> > [v2: Fix where __acpi_os_prepare_sleep==NULL and did not go in sleep
> > properly]
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  arch/x86/kernel/tboot.c       |    8 ++++++++
> >  drivers/acpi/acpica/hwsleep.c |   10 +++++++---
> >  drivers/acpi/osl.c            |   24 ++++++++++++++++++++++++
> >  include/acpi/acexcep.h        |    1 +
> >  include/linux/acpi.h          |   10 ++++++++++
> >  include/linux/tboot.h         |    1 -
> >  6 files changed, 50 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index
> > e2410e2..1a4ab7d 100644
> > --- a/arch/x86/kernel/tboot.c
> > +++ b/arch/x86/kernel/tboot.c
> > @@ -297,6 +297,12 @@ void tboot_sleep(u8 sleep_state, u32
> > pm1a_control, u32 pm1b_control)
> >
> >  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
> >  }
> > +static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
> > +			       u32 pm1b_control)
> > +{
> > +	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> > +	return 0;
> > +}
> >
> >  static atomic_t ap_wfs_count;
> >
> > @@ -345,6 +351,8 @@ static __init int tboot_late_init(void)
> >
> >  	atomic_set(&ap_wfs_count, 0);
> >  	register_hotcpu_notifier(&tboot_cpu_notifier);
> > +
> > +	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
> >  	return 0;
> >  }
> >
> > diff --git a/drivers/acpi/acpica/hwsleep.c
> > b/drivers/acpi/acpica/hwsleep.c index d52da30..992359a 100644
> > --- a/drivers/acpi/acpica/hwsleep.c
> > +++ b/drivers/acpi/acpica/hwsleep.c
> > @@ -43,9 +43,9 @@
> >   */
> >
> >  #include <acpi/acpi.h>
> > +#include <linux/acpi.h>
> >  #include "accommon.h"
> >  #include "actables.h"
> > -#include <linux/tboot.h>
> >  #include <linux/module.h>
> >
> >  #define _COMPONENT          ACPI_HARDWARE
> > @@ -344,8 +344,12 @@ acpi_status asmlinkage acpi_enter_sleep_state(u8
> > sleep_state)
> >
> >  	ACPI_FLUSH_CPU_CACHE();
> >
> > -	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> > -
> > +	status = acpi_os_prepare_sleep(sleep_state, pm1a_control,
> > +				       pm1b_control);
> > +	if (ACPI_SKIP(status))
> > +		return_ACPI_STATUS(AE_OK);
> > +	if (ACPI_FAILURE(status))
> > +		return_ACPI_STATUS(status);
> >  	/* Write #2: Write both SLP_TYP + SLP_EN */
> >
> >  	status = acpi_hw_write_pm1_control(pm1a_control, pm1b_control); diff
> > --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index f31c5c5..f3aae4b
> > 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -76,6 +76,9 @@ 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 acpi_osd_handler acpi_irq_handler;  static void
> > *acpi_irq_context;  static struct workqueue_struct *kacpid_wq; @@
> > -1659,3 +1662,24 @@ acpi_status acpi_os_terminate(void)
> >
> >  	return AE_OK;
> >  }
> > +
> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
> > +				  u32 pm1b_control)
> > +{
> > +	int rc = 0;
> > +	if (__acpi_os_prepare_sleep)
> > +		rc = __acpi_os_prepare_sleep(sleep_state,
> > +					     pm1a_control, pm1b_control);
> > +	if (rc < 0)
> > +		return AE_ERROR;
> > +	else if (rc > 0)
> > +		return AE_CTRL_SKIP;
> > +
> > +	return AE_OK;
> > +}
> > +
> > +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> > +			       u32 pm1a_ctrl, u32 pm1b_ctrl)) {
> > +	__acpi_os_prepare_sleep = func;
> > +}
> > diff --git a/include/acpi/acexcep.h b/include/acpi/acexcep.h index
> > 5b6c391..fa0d22c 100644
> > --- a/include/acpi/acexcep.h
> > +++ b/include/acpi/acexcep.h
> > @@ -57,6 +57,7 @@
> >  #define ACPI_SUCCESS(a)                 (!(a))
> >  #define ACPI_FAILURE(a)                 (a)
> >
> > +#define ACPI_SKIP(a)                    (a == AE_CTRL_SKIP)
> >  #define AE_OK                           (acpi_status) 0x0000
> >
> >  /*
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h index
> > 6001b4da..fccd017 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -359,4 +359,14 @@ static inline int suspend_nvs_register(unsigned
> > long a, unsigned long b)  }  #endif
> >
> > +#ifdef CONFIG_ACPI
> > +void acpi_os_set_prepare_sleep(int (*func)(u8 sleep_state,
> > +			       u32 pm1a_ctrl,  u32 pm1b_ctrl));
> > +
> > +acpi_status acpi_os_prepare_sleep(u8 sleep_state,
> > +				  u32 pm1a_control, u32 pm1b_control); #else #define
> > +acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while
> > +(0) #endif
> > +
> >  #endif	/*_LINUX_ACPI_H*/
> > diff --git a/include/linux/tboot.h b/include/linux/tboot.h index
> > 1dba6ee..c75128b 100644
> > --- a/include/linux/tboot.h
> > +++ b/include/linux/tboot.h
> > @@ -143,7 +143,6 @@ static inline int tboot_enabled(void)
> >
> >  extern void tboot_probe(void);
> >  extern void tboot_shutdown(u32 shutdown_type); -extern void
> > tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control);
> > extern struct acpi_table_header *tboot_get_dmar_table(
> >  				      struct acpi_table_header *dmar_tbl);  extern int
> > tboot_force_iommu(void);
> > --
> > 1.7.7.4

RE: [PATCH 2/7] tboot: Add return values for tboot_sleep

[Cihula, Joseph]

ACK, but tboot_sleep() calls tboot_shutdown() and there are error conditions in that which you are not reflecting upward--i.e. you should make tboot_shutdown() return an 'int' and propagate its errors through tboot_sleep().

Joe

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Friday, December 16, 2011 2:38 PM
> To: linux-kernel@vger.kernel.org; rjw@sisk.pl; x86@kernel.org; Brown, Len; Cihula, Joseph; linux-
> pm@lists.linux-foundation.org; tboot-devel@lists.sourceforge.net; linux-acpi@vger.kernel.org;
> liang.tang@oracle.com
> Cc: hpa@zytor.com; Konrad Rzeszutek Wilk
> Subject: [PATCH 2/7] tboot: Add return values for tboot_sleep
> 
> . as appropiately. As tboot_sleep now returns values.
> remove tboot_sleep_wrapper.
> 
> Suggested-by: "Rafael J. Wysocki" <rjw@sisk.pl>
> CC: Joseph Cihula <joseph.cihula@intel.com>
> [v1: Return -1/0/+1 instead of ACPI_xx values]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/kernel/tboot.c |   13 ++++---------
>  1 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 1a4ab7d..6410744 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -272,7 +272,7 @@ static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);  }
> 
> -void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
> +pm1b_control)
>  {
>  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
>  		/* S0,1,2: */ -1, -1, -1,
> @@ -281,7 +281,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
>  		/* S5: */ TB_SHUTDOWN_S5 };
> 
>  	if (!tboot_enabled())
> -		return;
> +		return 0;
> 
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control; @@ -292,15 +292,10 @@ void tboot_sleep(u8
> sleep_state, u32 pm1a_control, u32 pm1b_control)
>  	if (sleep_state >= ACPI_S_STATE_COUNT ||
>  	    acpi_shutdown_map[sleep_state] == -1) {
>  		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
> -		return;
> +		return -1;
>  	}
> 
>  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
> -}
> -static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
> -			       u32 pm1b_control)
> -{
> -	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
>  	return 0;
>  }
> 
> @@ -352,7 +347,7 @@ static __init int tboot_late_init(void)
>  	atomic_set(&ap_wfs_count, 0);
>  	register_hotcpu_notifier(&tboot_cpu_notifier);
> 
> -	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
> +	acpi_os_set_prepare_sleep(&tboot_sleep);
>  	return 0;
>  }
> 
> --
> 1.7.7.4

Re: [PATCH 2/7] tboot: Add return values for tboot_sleep

[Konrad Rzeszutek Wilk]

On Tue, Jan 10, 2012 at 08:10:53PM +0000, Cihula, Joseph wrote:
> ACK, but tboot_sleep() calls tboot_shutdown() and there are error conditions in that which you are not reflecting upward--i.e. you should make tboot_shutdown() return an 'int' and propagate its errors through tboot_sleep().

Hey Joe,

Thanks for looking at the patches.

Right now [with this patch applied] the code looks as so:

297 
298         tboot_shutdown(acpi_shutdown_map[sleep_state]);
299         return 0;
300 }


If we do make tboot_shutdown() return an int, there will be a need to modify
other callers: native_machine_emergency_restart, native_machine_halt, 
native_machine_power_off, and native_play_dead as well.

Perhaps that could be done in another patch and leave this one as is
where the 'tboot_sleep' would just return 0 instead of
'return tboot_shutdown(...)' ?

Perhaps another way to do this is to extract the common code of tboot_sleep
and tboot_shutdown?

> 
> Joe
> 
> > -----Original Message-----
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > Sent: Friday, December 16, 2011 2:38 PM
> > To: linux-kernel@vger.kernel.org; rjw@sisk.pl; x86@kernel.org; Brown, Len; Cihula, Joseph; linux-
> > pm@lists.linux-foundation.org; tboot-devel@lists.sourceforge.net; linux-acpi@vger.kernel.org;
> > liang.tang@oracle.com
> > Cc: hpa@zytor.com; Konrad Rzeszutek Wilk
> > Subject: [PATCH 2/7] tboot: Add return values for tboot_sleep
> > 
> > . as appropiately. As tboot_sleep now returns values.
> > remove tboot_sleep_wrapper.
> > 
> > Suggested-by: "Rafael J. Wysocki" <rjw@sisk.pl>
> > CC: Joseph Cihula <joseph.cihula@intel.com>
> > [v1: Return -1/0/+1 instead of ACPI_xx values]
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  arch/x86/kernel/tboot.c |   13 ++++---------
> >  1 files changed, 4 insertions(+), 9 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 1a4ab7d..6410744 100644
> > --- a/arch/x86/kernel/tboot.c
> > +++ b/arch/x86/kernel/tboot.c
> > @@ -272,7 +272,7 @@ static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
> >  		offsetof(struct acpi_table_facs, firmware_waking_vector);  }
> > 
> > -void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> > +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
> > +pm1b_control)
> >  {
> >  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
> >  		/* S0,1,2: */ -1, -1, -1,
> > @@ -281,7 +281,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> >  		/* S5: */ TB_SHUTDOWN_S5 };
> > 
> >  	if (!tboot_enabled())
> > -		return;
> > +		return 0;
> > 
> >  	tboot_copy_fadt(&acpi_gbl_FADT);
> >  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control; @@ -292,15 +292,10 @@ void tboot_sleep(u8
> > sleep_state, u32 pm1a_control, u32 pm1b_control)
> >  	if (sleep_state >= ACPI_S_STATE_COUNT ||
> >  	    acpi_shutdown_map[sleep_state] == -1) {
> >  		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
> > -		return;
> > +		return -1;
> >  	}
> > 
> >  	tboot_shutdown(acpi_shutdown_map[sleep_state]);
> > -}
> > -static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
> > -			       u32 pm1b_control)
> > -{
> > -	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> >  	return 0;
> >  }
> > 
> > @@ -352,7 +347,7 @@ static __init int tboot_late_init(void)
> >  	atomic_set(&ap_wfs_count, 0);
> >  	register_hotcpu_notifier(&tboot_cpu_notifier);
> > 
> > -	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
> > +	acpi_os_set_prepare_sleep(&tboot_sleep);
> >  	return 0;
> >  }
> > 
> > --
> > 1.7.7.4

Re: [PATCH 2/7] tboot: Add return values for tboot_sleep

[Konrad Rzeszutek Wilk]

On Thu, Jan 12, 2012 at 09:49:58AM -0800, Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 10, 2012 at 08:10:53PM +0000, Cihula, Joseph wrote:
> > ACK, but tboot_sleep() calls tboot_shutdown() and there are error conditions in that which you are not reflecting upward--i.e. you should make tboot_shutdown() return an 'int' and propagate its errors through tboot_sleep().
> 
> Hey Joe,
> 
> Thanks for looking at the patches.
> 
> Right now [with this patch applied] the code looks as so:
> 
> 297 
> 298         tboot_shutdown(acpi_shutdown_map[sleep_state]);
> 299         return 0;
> 300 }
> 
> 
> If we do make tboot_shutdown() return an int, there will be a need to modify
> other callers: native_machine_emergency_restart, native_machine_halt, 
> native_machine_power_off, and native_play_dead as well.
> 
> Perhaps that could be done in another patch and leave this one as is
> where the 'tboot_sleep' would just return 0 instead of
> 'return tboot_shutdown(...)' ?
> 
> Perhaps another way to do this is to extract the common code of tboot_sleep
> and tboot_shutdown?

Like this (not yet tested):

diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 6410744..2f93a50 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -212,17 +212,16 @@ static int tboot_setup_sleep(void)
 {
 	/* S3 shutdown requested, but S3 not supported by the kernel... */
 	BUG();
-	return -1;
+	return -ENOSYS;
 }
 
 #endif
 
-void tboot_shutdown(u32 shutdown_type)
-{
-	void (*shutdown)(void);
+static int tboot_check(u32 shutdown_type) {
+
 
 	if (!tboot_enabled())
-		return;
+		return -ENODEV;
 
 	/*
 	 * if we're being called before the 1:1 mapping is set up then just
@@ -230,12 +229,21 @@ void tboot_shutdown(u32 shutdown_type)
 	 * due to very early panic()
 	 */
 	if (!tboot_pg_dir)
-		return;
+		return -EBUSY;
 
 	/* if this is S3 then set regions to MAC */
-	if (shutdown_type == TB_SHUTDOWN_S3)
-		if (tboot_setup_sleep())
-			return;
+	if (shutdown_type == TB_SHUTDOWN_S3) {
+		int err = tboot_setup_sleep();
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static void __tboot_shutdown(u32 shutdown_type)
+{
+	void (*shutdown)(void);
 
 	tboot->shutdown_type = shutdown_type;
 
@@ -248,6 +256,13 @@ void tboot_shutdown(u32 shutdown_type)
 	while (1)
 		halt();
 }
+void tboot_shutdown(u32 shutdown_type)
+{
+	if (!tboot_check(shutdown_type))
+		return;
+
+	__tboot_shutdown(shutdown_type);
+}
 
 static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
 {
@@ -279,9 +294,17 @@ static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 		/* S3: */ TB_SHUTDOWN_S3,
 		/* S4: */ TB_SHUTDOWN_S4,
 		/* S5: */ TB_SHUTDOWN_S5 };
+	int err;
 
-	if (!tboot_enabled())
-		return 0;
+	if (sleep_state >= ACPI_S_STATE_COUNT ||
+	    acpi_shutdown_map[sleep_state] == -1) {
+		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
+		return -ENOSYS;
+	}
+
+	err = tboot_check(acpi_shutdown_map[sleep_state]);
+	if (!err && err != EBUSY)
+		return err;
 
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
@@ -289,13 +312,7 @@ static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 	/* we always use the 32b wakeup vector */
 	tboot->acpi_sinfo.vector_width = 32;
 
-	if (sleep_state >= ACPI_S_STATE_COUNT ||
-	    acpi_shutdown_map[sleep_state] == -1) {
-		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
-		return -1;
-	}
-
-	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+	__tboot_shutdown((acpi_shutdown_map[sleep_state]));
 	return 0;
 }
 

RE: [PATCH 2/7] tboot: Add return values for tboot_sleep

[Cihula, Joseph]

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Thursday, January 12, 2012 12:23 PM
eturn values for tboot_sleep
> 
> On Thu, Jan 12, 2012 at 09:49:58AM -0800, Konrad Rzeszutek Wilk wrote:
> > On Tue, Jan 10, 2012 at 08:10:53PM +0000, Cihula, Joseph wrote:
> > > ACK, but tboot_sleep() calls tboot_shutdown() and there are error conditions in that which you
> are not reflecting upward--i.e. you should make tboot_shutdown() return an 'int' and propagate its
> errors through tboot_sleep().
> >
> > Hey Joe,
> >
> > Thanks for looking at the patches.
> >
> > Right now [with this patch applied] the code looks as so:
> >
> > 297
> > 298         tboot_shutdown(acpi_shutdown_map[sleep_state]);
> > 299         return 0;
> > 300 }
> >
> >
> > If we do make tboot_shutdown() return an int, there will be a need to
> > modify other callers: native_machine_emergency_restart,
> > native_machine_halt, native_machine_power_off, and native_play_dead as well.
> >
> > Perhaps that could be done in another patch and leave this one as is
> > where the 'tboot_sleep' would just return 0 instead of 'return
> > tboot_shutdown(...)' ?
> >
> > Perhaps another way to do this is to extract the common code of
> > tboot_sleep and tboot_shutdown?

I'd be happier to see the callers above changed to handle a tboot_shutdown() that returned an error than splitting this up and only checking the error in one path.

Joe

> 
> Like this (not yet tested):
> 
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 6410744..2f93a50 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -212,17 +212,16 @@ static int tboot_setup_sleep(void)  {
>  	/* S3 shutdown requested, but S3 not supported by the kernel... */
>  	BUG();
> -	return -1;
> +	return -ENOSYS;
>  }
> 
>  #endif
> 
> -void tboot_shutdown(u32 shutdown_type)
> -{
> -	void (*shutdown)(void);
> +static int tboot_check(u32 shutdown_type) {
> +
> 
>  	if (!tboot_enabled())
> -		return;
> +		return -ENODEV;
> 
>  	/*
>  	 * if we're being called before the 1:1 mapping is set up then just @@ -230,12 +229,21 @@
> void tboot_shutdown(u32 shutdown_type)
>  	 * due to very early panic()
>  	 */
>  	if (!tboot_pg_dir)
> -		return;
> +		return -EBUSY;
> 
>  	/* if this is S3 then set regions to MAC */
> -	if (shutdown_type == TB_SHUTDOWN_S3)
> -		if (tboot_setup_sleep())
> -			return;
> +	if (shutdown_type == TB_SHUTDOWN_S3) {
> +		int err = tboot_setup_sleep();
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static void __tboot_shutdown(u32 shutdown_type) {
> +	void (*shutdown)(void);
> 
>  	tboot->shutdown_type = shutdown_type;
> 
> @@ -248,6 +256,13 @@ void tboot_shutdown(u32 shutdown_type)
>  	while (1)
>  		halt();
>  }
> +void tboot_shutdown(u32 shutdown_type)
> +{
> +	if (!tboot_check(shutdown_type))
> +		return;
> +
> +	__tboot_shutdown(shutdown_type);
> +}
> 
>  static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)  { @@ -279,9 +294,17 @@ static
> int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
>  		/* S3: */ TB_SHUTDOWN_S3,
>  		/* S4: */ TB_SHUTDOWN_S4,
>  		/* S5: */ TB_SHUTDOWN_S5 };
> +	int err;
> 
> -	if (!tboot_enabled())
> -		return 0;
> +	if (sleep_state >= ACPI_S_STATE_COUNT ||
> +	    acpi_shutdown_map[sleep_state] == -1) {
> +		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
> +		return -ENOSYS;
> +	}
> +
> +	err = tboot_check(acpi_shutdown_map[sleep_state]);
> +	if (!err && err != EBUSY)
> +		return err;
> 
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control; @@ -289,13 +312,7 @@ static int
> tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
>  	/* we always use the 32b wakeup vector */
>  	tboot->acpi_sinfo.vector_width = 32;
> 
> -	if (sleep_state >= ACPI_S_STATE_COUNT ||
> -	    acpi_shutdown_map[sleep_state] == -1) {
> -		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
> -		return -1;
> -	}
> -
> -	tboot_shutdown(acpi_shutdown_map[sleep_state]);
> +	__tboot_shutdown((acpi_shutdown_map[sleep_state]));
>  	return 0;
>  }
> 

Re: [PATCH 2/7] tboot: Add return values for tboot_sleep

[Konrad Rzeszutek Wilk]

On Thu, Jan 26, 2012 at 12:16:39AM +0000, Cihula, Joseph wrote:
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > Sent: Thursday, January 12, 2012 12:23 PM
> eturn values for tboot_sleep
> > 
> > On Thu, Jan 12, 2012 at 09:49:58AM -0800, Konrad Rzeszutek Wilk wrote:
> > > On Tue, Jan 10, 2012 at 08:10:53PM +0000, Cihula, Joseph wrote:
> > > > ACK, but tboot_sleep() calls tboot_shutdown() and there are error conditions in that which you
> > are not reflecting upward--i.e. you should make tboot_shutdown() return an 'int' and propagate its
> > errors through tboot_sleep().
> > >
> > > Hey Joe,
> > >
> > > Thanks for looking at the patches.
> > >
> > > Right now [with this patch applied] the code looks as so:
> > >
> > > 297
> > > 298         tboot_shutdown(acpi_shutdown_map[sleep_state]);
> > > 299         return 0;
> > > 300 }
> > >
> > >
> > > If we do make tboot_shutdown() return an int, there will be a need to
> > > modify other callers: native_machine_emergency_restart,
> > > native_machine_halt, native_machine_power_off, and native_play_dead as well.
> > >
> > > Perhaps that could be done in another patch and leave this one as is
> > > where the 'tboot_sleep' would just return 0 instead of 'return
> > > tboot_shutdown(...)' ?
> > >
> > > Perhaps another way to do this is to extract the common code of
> > > tboot_sleep and tboot_shutdown?
> 
> I'd be happier to see the callers above changed to handle a tboot_shutdown() that returned an error than splitting this up and only checking the error in one path.

OK, so something like this then:

>From da60a441fd62aee83763758455c08c281893ae93 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 8 Dec 2011 17:14:08 +0800
Subject: [PATCH] tboot: Add return values for tboot_sleep and consequently
 tboot_shutdown

.. as appropiately. As tboot_sleep now returns values.
remove tboot_sleep_wrapper.

Suggested-by: "Rafael J. Wysocki" <rjw@sisk.pl>
Acked-by: Joseph Cihula <joseph.cihula@intel.com>
[v1: Return -1/0/+1 instead of ACPI_xx values]
[v2: Also make tboot_shutdown return error code]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/reboot.c  |    6 +++---
 arch/x86/kernel/smpboot.c |    2 +-
 arch/x86/kernel/tboot.c   |   33 +++++++++++++++------------------
 include/linux/tboot.h     |    4 ++--
 4 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 37a458b..b29326a 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -549,7 +549,7 @@ static void native_machine_emergency_restart(void)
 	if (reboot_emergency)
 		emergency_vmx_disable_all();
 
-	tboot_shutdown(TB_SHUTDOWN_REBOOT);
+	WARN_ON(tboot_shutdown(TB_SHUTDOWN_REBOOT));
 
 	/* Tell the BIOS if we want cold or warm reboot */
 	*((unsigned short *)__va(0x472)) = reboot_mode;
@@ -684,7 +684,7 @@ static void native_machine_halt(void)
 	/* stop other cpus and apics */
 	machine_shutdown();
 
-	tboot_shutdown(TB_SHUTDOWN_HALT);
+	WARN_ON(tboot_shutdown(TB_SHUTDOWN_HALT));
 
 	/* stop this cpu */
 	stop_this_cpu(NULL);
@@ -698,7 +698,7 @@ static void native_machine_power_off(void)
 		pm_power_off();
 	}
 	/* a fallback in case there is no PM info available */
-	tboot_shutdown(TB_SHUTDOWN_HALT);
+	WARN_ON(tboot_shutdown(TB_SHUTDOWN_HALT));
 }
 
 struct machine_ops machine_ops = {
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 66d250c..9aa903a 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1419,7 +1419,7 @@ static inline void hlt_play_dead(void)
 void native_play_dead(void)
 {
 	play_dead_common();
-	tboot_shutdown(TB_SHUTDOWN_WFS);
+	WARN_ON(tboot_shutdown(TB_SHUTDOWN_WFS));
 
 	mwait_play_dead();	/* Only returns on failure */
 	hlt_play_dead();
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 1a4ab7d..9504a7a 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -217,12 +217,12 @@ static int tboot_setup_sleep(void)
 
 #endif
 
-void tboot_shutdown(u32 shutdown_type)
+int tboot_shutdown(u32 shutdown_type)
 {
 	void (*shutdown)(void);
 
 	if (!tboot_enabled())
-		return;
+		return 0;
 
 	/*
 	 * if we're being called before the 1:1 mapping is set up then just
@@ -230,13 +230,14 @@ void tboot_shutdown(u32 shutdown_type)
 	 * due to very early panic()
 	 */
 	if (!tboot_pg_dir)
-		return;
+		return 0;
 
 	/* if this is S3 then set regions to MAC */
-	if (shutdown_type == TB_SHUTDOWN_S3)
-		if (tboot_setup_sleep())
-			return;
-
+	if (shutdown_type == TB_SHUTDOWN_S3) {
+		int rc = tboot_setup_sleep();
+		if (rc)
+			return rc;
+	}
 	tboot->shutdown_type = shutdown_type;
 
 	switch_to_tboot_pt();
@@ -247,6 +248,8 @@ void tboot_shutdown(u32 shutdown_type)
 	/* should not reach here */
 	while (1)
 		halt();
+
+	return 0;
 }
 
 static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
@@ -272,7 +275,7 @@ static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -281,7 +284,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 		/* S5: */ TB_SHUTDOWN_S5 };
 
 	if (!tboot_enabled())
-		return;
+		return 0;
 
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
@@ -292,16 +295,10 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 	if (sleep_state >= ACPI_S_STATE_COUNT ||
 	    acpi_shutdown_map[sleep_state] == -1) {
 		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
-		return;
+		return -1;
 	}
 
-	tboot_shutdown(acpi_shutdown_map[sleep_state]);
-}
-static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
-			       u32 pm1b_control)
-{
-	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
-	return 0;
+	return tboot_shutdown(acpi_shutdown_map[sleep_state]);
 }
 
 static atomic_t ap_wfs_count;
@@ -352,7 +349,7 @@ static __init int tboot_late_init(void)
 	atomic_set(&ap_wfs_count, 0);
 	register_hotcpu_notifier(&tboot_cpu_notifier);
 
-	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
+	acpi_os_set_prepare_sleep(&tboot_sleep);
 	return 0;
 }
 
diff --git a/include/linux/tboot.h b/include/linux/tboot.h
index c75128b..8f14fe0 100644
--- a/include/linux/tboot.h
+++ b/include/linux/tboot.h
@@ -142,7 +142,7 @@ static inline int tboot_enabled(void)
 }
 
 extern void tboot_probe(void);
-extern void tboot_shutdown(u32 shutdown_type);
+extern int tboot_shutdown(u32 shutdown_type);
 extern struct acpi_table_header *tboot_get_dmar_table(
 				      struct acpi_table_header *dmar_tbl);
 extern int tboot_force_iommu(void);
@@ -151,7 +151,7 @@ extern int tboot_force_iommu(void);
 
 #define tboot_enabled()			0
 #define tboot_probe()			do { } while (0)
-#define tboot_shutdown(shutdown_type)	do { } while (0)
+#define tboot_shutdown(shutdown_type)	0
 #define tboot_sleep(sleep_state, pm1a_control, pm1b_control)	\
 					do { } while (0)
 #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
-- 
1.7.7.5

RE: [PATCH 2/7] tboot: Add return values for tboot_sleep

[Cihula, Joseph]

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: Friday, February 03, 2012 12:05 PM
> To: Cihula, Joseph
> Cc: linux-kernel@vger.kernel.org; rjw@sisk.pl; x86@kernel.org; Brown, Len; linux-pm@lists.linux-
> foundation.org; tboot-devel@lists.sourceforge.net; linux-acpi@vger.kernel.org;
> liang.tang@oracle.com; hpa@zytor.com
> Subject: Re: [PATCH 2/7] tboot: Add return values for tboot_sleep
> 
> On Thu, Jan 26, 2012 at 12:16:39AM +0000, Cihula, Joseph wrote:
> > > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> > > Sent: Thursday, January 12, 2012 12:23 PM
> > eturn values for tboot_sleep
> > >
> > > On Thu, Jan 12, 2012 at 09:49:58AM -0800, Konrad Rzeszutek Wilk wrote:
> > > > On Tue, Jan 10, 2012 at 08:10:53PM +0000, Cihula, Joseph wrote:
> > > > > ACK, but tboot_sleep() calls tboot_shutdown() and there are
> > > > > error conditions in that which you
> > > are not reflecting upward--i.e. you should make tboot_shutdown()
> > > return an 'int' and propagate its errors through tboot_sleep().
> > > >
> > > > Hey Joe,
> > > >
> > > > Thanks for looking at the patches.
> > > >
> > > > Right now [with this patch applied] the code looks as so:
> > > >
> > > > 297
> > > > 298         tboot_shutdown(acpi_shutdown_map[sleep_state]);
> > > > 299         return 0;
> > > > 300 }
> > > >
> > > >
> > > > If we do make tboot_shutdown() return an int, there will be a need
> > > > to modify other callers: native_machine_emergency_restart,
> > > > native_machine_halt, native_machine_power_off, and native_play_dead as well.
> > > >
> > > > Perhaps that could be done in another patch and leave this one as
> > > > is where the 'tboot_sleep' would just return 0 instead of 'return
> > > > tboot_shutdown(...)' ?
> > > >
> > > > Perhaps another way to do this is to extract the common code of
> > > > tboot_sleep and tboot_shutdown?
> >
> > I'd be happier to see the callers above changed to handle a tboot_shutdown() that returned an
> error than splitting this up and only checking the error in one path.
> 
> OK, so something like this then:

Looks good.

Joe

> 
> From da60a441fd62aee83763758455c08c281893ae93 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Thu, 8 Dec 2011 17:14:08 +0800
> Subject: [PATCH] tboot: Add return values for tboot_sleep and consequently  tboot_shutdown
> 
> .. as appropiately. As tboot_sleep now returns values.
> remove tboot_sleep_wrapper.
> 
> Suggested-by: "Rafael J. Wysocki" <rjw@sisk.pl>
> Acked-by: Joseph Cihula <joseph.cihula@intel.com>
> [v1: Return -1/0/+1 instead of ACPI_xx values]
> [v2: Also make tboot_shutdown return error code]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/kernel/reboot.c  |    6 +++---
>  arch/x86/kernel/smpboot.c |    2 +-
>  arch/x86/kernel/tboot.c   |   33 +++++++++++++++------------------
>  include/linux/tboot.h     |    4 ++--
>  4 files changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c index 37a458b..b29326a 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -549,7 +549,7 @@ static void native_machine_emergency_restart(void)
>  	if (reboot_emergency)
>  		emergency_vmx_disable_all();
> 
> -	tboot_shutdown(TB_SHUTDOWN_REBOOT);
> +	WARN_ON(tboot_shutdown(TB_SHUTDOWN_REBOOT));
> 
>  	/* Tell the BIOS if we want cold or warm reboot */
>  	*((unsigned short *)__va(0x472)) = reboot_mode; @@ -684,7 +684,7 @@ static void
> native_machine_halt(void)
>  	/* stop other cpus and apics */
>  	machine_shutdown();
> 
> -	tboot_shutdown(TB_SHUTDOWN_HALT);
> +	WARN_ON(tboot_shutdown(TB_SHUTDOWN_HALT));
> 
>  	/* stop this cpu */
>  	stop_this_cpu(NULL);
> @@ -698,7 +698,7 @@ static void native_machine_power_off(void)
>  		pm_power_off();
>  	}
>  	/* a fallback in case there is no PM info available */
> -	tboot_shutdown(TB_SHUTDOWN_HALT);
> +	WARN_ON(tboot_shutdown(TB_SHUTDOWN_HALT));
>  }
> 
>  struct machine_ops machine_ops = {
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index 66d250c..9aa903a 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1419,7 +1419,7 @@ static inline void hlt_play_dead(void)  void native_play_dead(void)  {
>  	play_dead_common();
> -	tboot_shutdown(TB_SHUTDOWN_WFS);
> +	WARN_ON(tboot_shutdown(TB_SHUTDOWN_WFS));
> 
>  	mwait_play_dead();	/* Only returns on failure */
>  	hlt_play_dead();
> diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index 1a4ab7d..9504a7a 100644
> --- a/arch/x86/kernel/tboot.c
> +++ b/arch/x86/kernel/tboot.c
> @@ -217,12 +217,12 @@ static int tboot_setup_sleep(void)
> 
>  #endif
> 
> -void tboot_shutdown(u32 shutdown_type)
> +int tboot_shutdown(u32 shutdown_type)
>  {
>  	void (*shutdown)(void);
> 
>  	if (!tboot_enabled())
> -		return;
> +		return 0;
> 
>  	/*
>  	 * if we're being called before the 1:1 mapping is set up then just @@ -230,13 +230,14 @@
> void tboot_shutdown(u32 shutdown_type)
>  	 * due to very early panic()
>  	 */
>  	if (!tboot_pg_dir)
> -		return;
> +		return 0;
> 
>  	/* if this is S3 then set regions to MAC */
> -	if (shutdown_type == TB_SHUTDOWN_S3)
> -		if (tboot_setup_sleep())
> -			return;
> -
> +	if (shutdown_type == TB_SHUTDOWN_S3) {
> +		int rc = tboot_setup_sleep();
> +		if (rc)
> +			return rc;
> +	}
>  	tboot->shutdown_type = shutdown_type;
> 
>  	switch_to_tboot_pt();
> @@ -247,6 +248,8 @@ void tboot_shutdown(u32 shutdown_type)
>  	/* should not reach here */
>  	while (1)
>  		halt();
> +
> +	return 0;
>  }
> 
>  static void tboot_copy_fadt(const struct acpi_table_fadt *fadt) @@ -272,7 +275,7 @@ static void
> tboot_copy_fadt(const struct acpi_table_fadt *fadt)
>  		offsetof(struct acpi_table_facs, firmware_waking_vector);  }
> 
> -void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
> +static int tboot_sleep(u8 sleep_state, u32 pm1a_control, u32
> +pm1b_control)
>  {
>  	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
>  		/* S0,1,2: */ -1, -1, -1,
> @@ -281,7 +284,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
>  		/* S5: */ TB_SHUTDOWN_S5 };
> 
>  	if (!tboot_enabled())
> -		return;
> +		return 0;
> 
>  	tboot_copy_fadt(&acpi_gbl_FADT);
>  	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control; @@ -292,16 +295,10 @@ void tboot_sleep(u8
> sleep_state, u32 pm1a_control, u32 pm1b_control)
>  	if (sleep_state >= ACPI_S_STATE_COUNT ||
>  	    acpi_shutdown_map[sleep_state] == -1) {
>  		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
> -		return;
> +		return -1;
>  	}
> 
> -	tboot_shutdown(acpi_shutdown_map[sleep_state]);
> -}
> -static int tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
> -			       u32 pm1b_control)
> -{
> -	tboot_sleep(sleep_state, pm1a_control, pm1b_control);
> -	return 0;
> +	return tboot_shutdown(acpi_shutdown_map[sleep_state]);
>  }
> 
>  static atomic_t ap_wfs_count;
> @@ -352,7 +349,7 @@ static __init int tboot_late_init(void)
>  	atomic_set(&ap_wfs_count, 0);
>  	register_hotcpu_notifier(&tboot_cpu_notifier);
> 
> -	acpi_os_set_prepare_sleep(&tboot_sleep_wrapper);
> +	acpi_os_set_prepare_sleep(&tboot_sleep);
>  	return 0;
>  }
> 
> diff --git a/include/linux/tboot.h b/include/linux/tboot.h index c75128b..8f14fe0 100644
> --- a/include/linux/tboot.h
> +++ b/include/linux/tboot.h
> @@ -142,7 +142,7 @@ static inline int tboot_enabled(void)  }
> 
>  extern void tboot_probe(void);
> -extern void tboot_shutdown(u32 shutdown_type);
> +extern int tboot_shutdown(u32 shutdown_type);
>  extern struct acpi_table_header *tboot_get_dmar_table(
>  				      struct acpi_table_header *dmar_tbl);  extern int
> tboot_force_iommu(void); @@ -151,7 +151,7 @@ extern int tboot_force_iommu(void);
> 
>  #define tboot_enabled()			0
>  #define tboot_probe()			do { } while (0)
> -#define tboot_shutdown(shutdown_type)	do { } while (0)
> +#define tboot_shutdown(shutdown_type)	0
>  #define tboot_sleep(sleep_state, pm1a_control, pm1b_control)	\
>  					do { } while (0)
>  #define tboot_get_dmar_table(dmar_tbl)	(dmar_tbl)
> --
> 1.7.7.5

[PATCH 2/7] tboot: Add return values for tboot_sleep

[Konrad Rzeszutek Wilk]

. as appropiately. As tboot_sleep now returns values, we are free
to remove the tboot_sleep_wrapper function altogether.

Suggested-by: "Rafael J. Wysocki" <rjw@sisk.pl>
CC: Joseph Cihula <joseph.cihula@intel.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/tboot.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index 751d673..5ab5362 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -273,7 +273,8 @@ static void tboot_copy_fadt(const struct acpi_table_fadt *fadt)
 		offsetof(struct acpi_table_facs, firmware_waking_vector);
 }
 
-void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
+static acpi_status tboot_sleep(u8 sleep_state, u32 pm1a_control,
+			       u32 pm1b_control)
 {
 	static u32 acpi_shutdown_map[ACPI_S_STATE_COUNT] = {
 		/* S0,1,2: */ -1, -1, -1,
@@ -282,7 +283,7 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 		/* S5: */ TB_SHUTDOWN_S5 };
 
 	if (!tboot_enabled())
-		return;
+		return AE_OK;
 
 	tboot_copy_fadt(&acpi_gbl_FADT);
 	tboot->acpi_sinfo.pm1a_cnt_val = pm1a_control;
@@ -293,10 +294,12 @@ void tboot_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control)
 	if (sleep_state >= ACPI_S_STATE_COUNT ||
 	    acpi_shutdown_map[sleep_state] == -1) {
 		pr_warning("unsupported sleep state 0x%x\n", sleep_state);
-		return;
+		return AE_ERROR;
 	}
 
 	tboot_shutdown(acpi_shutdown_map[sleep_state]);
+
+	return AE_OK;
 }
 static acpi_status tboot_sleep_wrapper(u8 sleep_state, u32 pm1a_control,
 		u32 pm1b_control)
@@ -353,7 +356,7 @@ static __init int tboot_late_init(void)
 	atomic_set(&ap_wfs_count, 0);
 	register_hotcpu_notifier(&tboot_cpu_notifier);
 
-	acpi_os_prepare_sleep_register(&tboot_sleep_wrapper);
+	acpi_os_prepare_sleep_register(&tboot_sleep);
 	return 0;
 }
 
-- 
1.7.7.1