Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

[RFC PATCH v1] ACPI S3 to work under Xen.

Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.

The relationship that Xen has with Linux kernel is symbiotic. 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). The realm of
ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
values - which the hypervisor has to do) and then restore them.

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.

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.

What do you guys think? One thought is to use the paravirt interface to
deal with cr3, cr8, idt, gdt for suspend/resume case.. But that is a lot
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

[PATCH 6/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep

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 <at> intel.com> initial idea.
[ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
change c68699484a65 ]

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/x86/include/asm/xen/hypercall.h |    8 ++++++++
 arch/x86/xen/enlighten.c             |    3 +++
 drivers/xen/Makefile                 |    2 +-
 drivers/xen/acpi.c                   |   25 +++++++++++++++++++++++++
 include/xen/acpi.h                   |   26 ++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 1 deletions(-)
 create mode 100644 drivers/xen/acpi.c
 create mode 100644 include/xen/acpi.h

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index d240ea9..0c9894e 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
 <at>  <at>  -45,6 +45,7  <at>  <at> 
 #include <xen/interface/xen.h>
 #include <xen/interface/sched.h>
 #include <xen/interface/physdev.h>
+#include <xen/interface/platform.h>

(Continue reading)

Cihula, Joseph | 7 Sep 06:36 2011
Picon

Re: [PATCH 6/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> 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 <at> intel.com> initial idea.
> [ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
> change c68699484a65 ]
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> ---
>  arch/x86/include/asm/xen/hypercall.h |    8 ++++++++
>  arch/x86/xen/enlighten.c             |    3 +++
>  drivers/xen/Makefile                 |    2 +-
>  drivers/xen/acpi.c                   |   25 +++++++++++++++++++++++++
>  include/xen/acpi.h                   |   26 ++++++++++++++++++++++++++
>  5 files changed, 63 insertions(+), 1 deletions(-)  create mode 100644 drivers/xen/acpi.c  create
> mode 100644 include/xen/acpi.h
> 
> diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
> index d240ea9..0c9894e 100644
> --- a/arch/x86/include/asm/xen/hypercall.h
> +++ b/arch/x86/include/asm/xen/hypercall.h
>  <at>  <at>  -45,6 +45,7  <at>  <at> 
>  #include <xen/interface/xen.h>
>  #include <xen/interface/sched.h>
>  #include <xen/interface/physdev.h>
> +#include <xen/interface/platform.h>
> 
(Continue reading)

Cihula, Joseph | 7 Sep 06:36 2011
Picon

RE: [PATCH 6/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> 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 <at> intel.com> initial idea.
> [ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
> change c68699484a65 ]
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> ---
>  arch/x86/include/asm/xen/hypercall.h |    8 ++++++++
>  arch/x86/xen/enlighten.c             |    3 +++
>  drivers/xen/Makefile                 |    2 +-
>  drivers/xen/acpi.c                   |   25 +++++++++++++++++++++++++
>  include/xen/acpi.h                   |   26 ++++++++++++++++++++++++++
>  5 files changed, 63 insertions(+), 1 deletions(-)  create mode 100644 drivers/xen/acpi.c  create
> mode 100644 include/xen/acpi.h
> 
> diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
> index d240ea9..0c9894e 100644
> --- a/arch/x86/include/asm/xen/hypercall.h
> +++ b/arch/x86/include/asm/xen/hypercall.h
>  <at>  <at>  -45,6 +45,7  <at>  <at> 
>  #include <xen/interface/xen.h>
>  #include <xen/interface/sched.h>
>  #include <xen/interface/physdev.h>
> +#include <xen/interface/platform.h>
> 
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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

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 <at> linutronix.de>
CC: "H. Peter Anvin" <hpa <at> zytor.com>
CC: x86 <at> kernel.org
CC: Len Brown <len.brown <at> intel.com>
CC: Joseph Cihula <joseph.cihula <at> intel.com>
CC: Shane Wang <shane.wang <at> intel.com>
CC: linux-pm <at> lists.linux-foundation.org
CC: linux-acpi <at> vger.kernel.org
CC: Len Brown <len.brown <at> intel.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
Signed-off-by: Liang Tang <liang.tang <at> oracle.com>
---
 arch/x86/include/asm/acpi.h  |    2 +-
 arch/x86/kernel/acpi/boot.c  |    2 ++
 arch/x86/kernel/acpi/sleep.c |    4 ++--
 arch/x86/kernel/acpi/sleep.h |    2 ++
 drivers/acpi/sleep.c         |    2 ++
 5 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 49864a1..a5f0b73 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
 <at>  <at>  -118,7 +118,7  <at>  <at>  static inline void acpi_disable_pci(void)
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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 <at> oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 include/xen/acpi.h |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/include/xen/acpi.h b/include/xen/acpi.h
index e414f14..0409919 100644
--- a/include/xen/acpi.h
+++ b/include/xen/acpi.h
 <at>  <at>  -12,10 +12,22  <at>  <at>  int xen_acpi_notify_hypervisor_state(u8 sleep_state,
 				     u32 pm1a_cnt, u32 pm1b_cnd,
 				     bool *skip_rest);

+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);
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/x86/pci/xen.c              |   12 ++++++++++++
 include/xen/interface/physdev.h |    7 +++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index f567965..f140999 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
 <at>  <at>  -241,6 +241,17  <at>  <at>  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;
+	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
 #endif
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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 <at> kernel.org
Cc: Thomas Gleixner <tglx <at> linutronix.de>
Cc: "H. Peter Anvin" <hpa <at> zytor.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> 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
 <at>  <at>  -112,19 +112,28  <at>  <at>  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
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

[PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

From: Yu Ke <ke.yu <at> intel.com>

This patches implements the xen_platform_op hypercall, to pass the parsed
ACPI info to hypervisor.

Signed-off-by: Yu Ke <ke.yu <at> intel.com>
Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
[v1: Added DEFINE_GUEST.. in appropiate headers]
[v2: Ripped out typedefs]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/ia64/include/asm/xen/interface.h |    1 +
 arch/x86/include/asm/xen/interface.h  |    1 +
 include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
 include/xen/interface/xen.h           |    1 +
 4 files changed, 323 insertions(+), 0 deletions(-)
 create mode 100644 include/xen/interface/platform.h

diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
index e951e74..1d2427d 100644
--- a/arch/ia64/include/asm/xen/interface.h
+++ b/arch/ia64/include/asm/xen/interface.h
 <at>  <at>  -76,6 +76,7  <at>  <at>  DEFINE_GUEST_HANDLE(char);
 DEFINE_GUEST_HANDLE(int);
 DEFINE_GUEST_HANDLE(long);
 DEFINE_GUEST_HANDLE(void);
+DEFINE_GUEST_HANDLE(uint64_t);

 typedef unsigned long xen_pfn_t;
(Continue reading)

Cihula, Joseph | 7 Sep 07:50 2011
Picon

Re: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> From: Yu Ke <ke.yu <at> intel.com>
> 
> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
> 
> Signed-off-by: Yu Ke <ke.yu <at> intel.com>
> Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
> [v1: Added DEFINE_GUEST.. in appropiate headers]
> [v2: Ripped out typedefs]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> ---
>  arch/ia64/include/asm/xen/interface.h |    1 +
>  arch/x86/include/asm/xen/interface.h  |    1 +
>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
>  include/xen/interface/xen.h           |    1 +
>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
> include/xen/interface/platform.h
> 
> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> index e951e74..1d2427d 100644
> --- a/arch/ia64/include/asm/xen/interface.h
> +++ b/arch/ia64/include/asm/xen/interface.h
>  <at>  <at>  -76,6 +76,7  <at>  <at>  DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
> +DEFINE_GUEST_HANDLE(uint64_t);
> 
>  typedef unsigned long xen_pfn_t;
(Continue reading)

Cihula, Joseph | 7 Sep 07:50 2011
Picon

RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> From: Yu Ke <ke.yu <at> intel.com>
> 
> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
> 
> Signed-off-by: Yu Ke <ke.yu <at> intel.com>
> Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
> [v1: Added DEFINE_GUEST.. in appropiate headers]
> [v2: Ripped out typedefs]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> ---
>  arch/ia64/include/asm/xen/interface.h |    1 +
>  arch/x86/include/asm/xen/interface.h  |    1 +
>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
>  include/xen/interface/xen.h           |    1 +
>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
> include/xen/interface/platform.h
> 
> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> index e951e74..1d2427d 100644
> --- a/arch/ia64/include/asm/xen/interface.h
> +++ b/arch/ia64/include/asm/xen/interface.h
>  <at>  <at>  -76,6 +76,7  <at>  <at>  DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
> +DEFINE_GUEST_HANDLE(uint64_t);
> 
>  typedef unsigned long xen_pfn_t;
(Continue reading)

Jeremy Fitzhardinge | 7 Sep 19:29 2011

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
>> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
>> Sent: Wednesday, August 31, 2011 11:31 AM
>>
>> From: Yu Ke <ke.yu <at> intel.com>
>>
>> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
>>
>> Signed-off-by: Yu Ke <ke.yu <at> intel.com>
>> Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
>> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
>> [v1: Added DEFINE_GUEST.. in appropiate headers]
>> [v2: Ripped out typedefs]
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
>> ---
>>  arch/ia64/include/asm/xen/interface.h |    1 +
>>  arch/x86/include/asm/xen/interface.h  |    1 +
>>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
>>  include/xen/interface/xen.h           |    1 +
>>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
>> include/xen/interface/platform.h
>>
>> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
>> index e951e74..1d2427d 100644
>> --- a/arch/ia64/include/asm/xen/interface.h
>> +++ b/arch/ia64/include/asm/xen/interface.h
>>  <at>  <at>  -76,6 +76,7  <at>  <at>  DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
>> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
>> +DEFINE_GUEST_HANDLE(uint64_t);
>>
(Continue reading)

Cihula, Joseph | 7 Sep 19:43 2011
Picon

RE: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

> From: Jeremy Fitzhardinge [mailto:jeremy <at> goop.org]
> Sent: Wednesday, September 07, 2011 10:29 AM
> 
> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> From: Yu Ke <ke.yu <at> intel.com>
> >>
> >> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to
> hypervisor.
> >>
> >> Signed-off-by: Yu Ke <ke.yu <at> intel.com>
> >> Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
> >> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
> >> [v1: Added DEFINE_GUEST.. in appropiate headers]
> >> [v2: Ripped out typedefs]
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> >> ---
> >>  arch/ia64/include/asm/xen/interface.h |    1 +
> >>  arch/x86/include/asm/xen/interface.h  |    1 +
> >>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
> >>  include/xen/interface/xen.h           |    1 +
> >>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode
> >> 100644 include/xen/interface/platform.h
> >>
> >> diff --git a/arch/ia64/include/asm/xen/interface.h
> >> b/arch/ia64/include/asm/xen/interface.h
> >> index e951e74..1d2427d 100644
> >> --- a/arch/ia64/include/asm/xen/interface.h
(Continue reading)

Konrad Rzeszutek Wilk | 7 Sep 21:06 2011
Picon

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

> > >> +#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. */
> > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > 
> > Does ACPI define them as 32 or 16 bit?
> 
> The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b, which
your other patch is truncating for this call.

Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
and also address all the other comments (thanks for looking at it) you pointed
out.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Konrad Rzeszutek Wilk | 8 Sep 15:38 2011
Picon

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

On Wed, Sep 07, 2011 at 03:06:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > > >> +#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. */
> > > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > > 
> > > Does ACPI define them as 32 or 16 bit?
> > 
> > The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b,
which your other patch is truncating for this call.
> 
> Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
> and also address all the other comments (thanks for looking at it) you pointed
> out.

So read up the ACPI spec and it says that the minimum is 2 bytes and does not
say anything about the maximum. The list of what the bits do stops at 16-bits
(the last two are reserved) so I think we are actually OK.

Albeit if the spec starts using more of them - then yes we will need to revist
this Xen ABI and potentially add a new call.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Konrad Rzeszutek Wilk | 22 Sep 14:51 2011
Picon

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

On Thu, Sep 08, 2011 at 09:38:48AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 07, 2011 at 03:06:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > >> +#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. */
> > > > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > > > 
> > > > Does ACPI define them as 32 or 16 bit?
> > > 
> > > The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b,
which your other patch is truncating for this call.
> > 
> > Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
> > and also address all the other comments (thanks for looking at it) you pointed
> > out.
> 
> So read up the ACPI spec and it says that the minimum is 2 bytes and does not
> say anything about the maximum. The list of what the bits do stops at 16-bits
> (the last two are reserved) so I think we are actually OK.

Perhaps a better way of doing this is in the hypercall (in the Linux kernel)
check if the other 16-bits have any value. And if so WARN (with an appropiate
message to email xen-devel) and also bail out on making the hypercall. That
way we can find out about this.

> 
> Albeit if the spec starts using more of them - then yes we will need to revist
> this Xen ABI and potentially add a new call.
(Continue reading)

Konrad Rzeszutek Wilk | 8 Sep 15:38 2011
Picon

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

On Wed, Sep 07, 2011 at 03:06:59PM -0400, Konrad Rzeszutek Wilk wrote:
> > > >> +#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. */
> > > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > > 
> > > Does ACPI define them as 32 or 16 bit?
> > 
> > The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b,
which your other patch is truncating for this call.
> 
> Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
> and also address all the other comments (thanks for looking at it) you pointed
> out.

So read up the ACPI spec and it says that the minimum is 2 bytes and does not
say anything about the maximum. The list of what the bits do stops at 16-bits
(the last two are reserved) so I think we are actually OK.

Albeit if the spec starts using more of them - then yes we will need to revist
this Xen ABI and potentially add a new call.
Konrad Rzeszutek Wilk | 7 Sep 21:06 2011
Picon

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

> > >> +#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. */
> > > These are uint32_t in native Linux--why truncate in the API and not at use?
> > 
> > Does ACPI define them as 32 or 16 bit?
> 
> The spec indicates that the length is variable and could be up to 32 bits (AFAICT).  And Linux uses 32b, which
your other patch is truncating for this call.

Yikes! Well, looks like we need to fix the Xen ABI too. Lets get that fixed
and also address all the other comments (thanks for looking at it) you pointed
out.
Konrad Rzeszutek Wilk | 21 Sep 21:29 2011
Picon

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

On Wed, Sep 07, 2011 at 10:29:11AM -0700, Jeremy Fitzhardinge wrote:
> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> From: Yu Ke <ke.yu <at> intel.com>
> >>
> >> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
> >>
> >> Signed-off-by: Yu Ke <ke.yu <at> intel.com>
> >> Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
> >> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
> >> [v1: Added DEFINE_GUEST.. in appropiate headers]
> >> [v2: Ripped out typedefs]
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> >> ---
> >>  arch/ia64/include/asm/xen/interface.h |    1 +
> >>  arch/x86/include/asm/xen/interface.h  |    1 +
> >>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
> >>  include/xen/interface/xen.h           |    1 +
> >>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
> >> include/xen/interface/platform.h
> >>
> >> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
> >> index e951e74..1d2427d 100644
> >> --- a/arch/ia64/include/asm/xen/interface.h
> >> +++ b/arch/ia64/include/asm/xen/interface.h
> >>  <at>  <at>  -76,6 +76,7  <at>  <at>  DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
> >> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
> >> +DEFINE_GUEST_HANDLE(uint64_t);
(Continue reading)

Jeremy Fitzhardinge | 22 Sep 00:42 2011

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

On 09/21/2011 12:29 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Sep 07, 2011 at 10:29:11AM -0700, Jeremy Fitzhardinge wrote:
>> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
>>>> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
>>>> Sent: Wednesday, August 31, 2011 11:31 AM
>>>>
>>>> From: Yu Ke <ke.yu <at> intel.com>
>>>>
>>>> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to hypervisor.
>>>>
>>>> Signed-off-by: Yu Ke <ke.yu <at> intel.com>
>>>> Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
>>>> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
>>>> [v1: Added DEFINE_GUEST.. in appropiate headers]
>>>> [v2: Ripped out typedefs]
>>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
>>>> ---
>>>>  arch/ia64/include/asm/xen/interface.h |    1 +
>>>>  arch/x86/include/asm/xen/interface.h  |    1 +
>>>>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
>>>>  include/xen/interface/xen.h           |    1 +
>>>>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode 100644
>>>> include/xen/interface/platform.h
>>>>
>>>> diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
>>>> index e951e74..1d2427d 100644
>>>> --- a/arch/ia64/include/asm/xen/interface.h
>>>> +++ b/arch/ia64/include/asm/xen/interface.h
>>>>  <at>  <at>  -76,6 +76,7  <at>  <at>  DEFINE_GUEST_HANDLE(char);  DEFINE_GUEST_HANDLE(int);
>>>> DEFINE_GUEST_HANDLE(long);  DEFINE_GUEST_HANDLE(void);
(Continue reading)

Cihula, Joseph | 7 Sep 19:43 2011
Picon

Re: [Xen-devel] RE: [PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

> From: Jeremy Fitzhardinge [mailto:jeremy <at> goop.org]
> Sent: Wednesday, September 07, 2011 10:29 AM
> 
> On 09/06/2011 10:50 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> From: Yu Ke <ke.yu <at> intel.com>
> >>
> >> This patches implements the xen_platform_op hypercall, to pass the parsed ACPI info to
> hypervisor.
> >>
> >> Signed-off-by: Yu Ke <ke.yu <at> intel.com>
> >> Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
> >> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
> >> [v1: Added DEFINE_GUEST.. in appropiate headers]
> >> [v2: Ripped out typedefs]
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> >> ---
> >>  arch/ia64/include/asm/xen/interface.h |    1 +
> >>  arch/x86/include/asm/xen/interface.h  |    1 +
> >>  include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
> >>  include/xen/interface/xen.h           |    1 +
> >>  4 files changed, 323 insertions(+), 0 deletions(-)  create mode
> >> 100644 include/xen/interface/platform.h
> >>
> >> diff --git a/arch/ia64/include/asm/xen/interface.h
> >> b/arch/ia64/include/asm/xen/interface.h
> >> index e951e74..1d2427d 100644
> >> --- a/arch/ia64/include/asm/xen/interface.h
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

[PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

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 <at> linutronix.de>
CC: "H. Peter Anvin" <hpa <at> zytor.com>
CC: x86 <at> kernel.org
CC: Len Brown <len.brown <at> intel.com>
CC: Joseph Cihula <joseph.cihula <at> intel.com>
CC: Shane Wang <shane.wang <at> intel.com>
CC: xen-devel <at> lists.xensource.com
CC: linux-pm <at> lists.linux-foundation.org
CC: tboot-devel <at> lists.sourceforge.net
CC: linux-acpi <at> vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/x86/include/asm/acpi.h   |    3 +++
 arch/x86/kernel/acpi/boot.c   |    3 +++
 arch/x86/kernel/tboot.c       |   13 +++++++++----
 drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
 include/linux/tboot.h         |    3 ++-
 5 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 610001d..49864a1 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
 <at>  <at>  -98,6 +98,9  <at>  <at>  void acpi_pic_sci_set_trigger(unsigned int, u16);
 extern int (*__acpi_register_gsi)(struct device *dev, u32 gsi,
 				  int trigger, int polarity);
(Continue reading)

Cihula, Joseph | 7 Sep 06:20 2011
Picon

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

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> 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 <at> linutronix.de>
> CC: "H. Peter Anvin" <hpa <at> zytor.com>
> CC: x86 <at> kernel.org
> CC: Len Brown <len.brown <at> intel.com>
> CC: Joseph Cihula <joseph.cihula <at> intel.com>
> CC: Shane Wang <shane.wang <at> intel.com>
> CC: xen-devel <at> lists.xensource.com
> CC: linux-pm <at> lists.linux-foundation.org
> CC: tboot-devel <at> lists.sourceforge.net
> CC: linux-acpi <at> vger.kernel.org
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> ---
>  arch/x86/include/asm/acpi.h   |    3 +++
>  arch/x86/kernel/acpi/boot.c   |    3 +++
>  arch/x86/kernel/tboot.c       |   13 +++++++++----
>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
>  include/linux/tboot.h         |    3 ++-
>  5 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 610001d..49864a1
> 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
>  <at>  <at>  -98,6 +98,9  <at>  <at>  void acpi_pic_sci_set_trigger(unsigned int, u16);  extern int
(Continue reading)

Cihula, Joseph | 7 Sep 06:20 2011
Picon

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

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Wednesday, August 31, 2011 11:31 AM
> 
> 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 <at> linutronix.de>
> CC: "H. Peter Anvin" <hpa <at> zytor.com>
> CC: x86 <at> kernel.org
> CC: Len Brown <len.brown <at> intel.com>
> CC: Joseph Cihula <joseph.cihula <at> intel.com>
> CC: Shane Wang <shane.wang <at> intel.com>
> CC: xen-devel <at> lists.xensource.com
> CC: linux-pm <at> lists.linux-foundation.org
> CC: tboot-devel <at> lists.sourceforge.net
> CC: linux-acpi <at> vger.kernel.org
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> ---
>  arch/x86/include/asm/acpi.h   |    3 +++
>  arch/x86/kernel/acpi/boot.c   |    3 +++
>  arch/x86/kernel/tboot.c       |   13 +++++++++----
>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
>  include/linux/tboot.h         |    3 ++-
>  5 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 610001d..49864a1
> 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
>  <at>  <at>  -98,6 +98,9  <at>  <at>  void acpi_pic_sci_set_trigger(unsigned int, u16);  extern int
(Continue reading)

Jeremy Fitzhardinge | 7 Sep 19:27 2011

Re: [Xen-devel] RE: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

On 09/06/2011 09:20 PM, Cihula, Joseph wrote:
>> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
>> Sent: Wednesday, August 31, 2011 11:31 AM
>>
>> 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 <at> linutronix.de>
>> CC: "H. Peter Anvin" <hpa <at> zytor.com>
>> CC: x86 <at> kernel.org
>> CC: Len Brown <len.brown <at> intel.com>
>> CC: Joseph Cihula <joseph.cihula <at> intel.com>
>> CC: Shane Wang <shane.wang <at> intel.com>
>> CC: xen-devel <at> lists.xensource.com
>> CC: linux-pm <at> lists.linux-foundation.org
>> CC: tboot-devel <at> lists.sourceforge.net
>> CC: linux-acpi <at> vger.kernel.org
>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
>> ---
>>  arch/x86/include/asm/acpi.h   |    3 +++
>>  arch/x86/kernel/acpi/boot.c   |    3 +++
>>  arch/x86/kernel/tboot.c       |   13 +++++++++----
>>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
>>  include/linux/tboot.h         |    3 ++-
>>  5 files changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index 610001d..49864a1
>> 100644
>> --- a/arch/x86/include/asm/acpi.h
>> +++ b/arch/x86/include/asm/acpi.h
(Continue reading)

Cihula, Joseph | 7 Sep 19:55 2011
Picon

RE: [Xen-devel] RE: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

> From: Jeremy Fitzhardinge [mailto:jeremy <at> goop.org]
> Sent: Wednesday, September 07, 2011 10:27 AM
> 
> On 09/06/2011 09:20 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> 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 <at> linutronix.de>
> >> CC: "H. Peter Anvin" <hpa <at> zytor.com>
> >> CC: x86 <at> kernel.org
> >> CC: Len Brown <len.brown <at> intel.com>
> >> CC: Joseph Cihula <joseph.cihula <at> intel.com>
> >> CC: Shane Wang <shane.wang <at> intel.com>
> >> CC: xen-devel <at> lists.xensource.com
> >> CC: linux-pm <at> lists.linux-foundation.org
> >> CC: tboot-devel <at> lists.sourceforge.net
> >> CC: linux-acpi <at> vger.kernel.org
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> >> ---
> >>  arch/x86/include/asm/acpi.h   |    3 +++
> >>  arch/x86/kernel/acpi/boot.c   |    3 +++
> >>  arch/x86/kernel/tboot.c       |   13 +++++++++----
> >>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
> >>  include/linux/tboot.h         |    3 ++-
> >>  5 files changed, 27 insertions(+), 7 deletions(-)
> >>
(Continue reading)

Cihula, Joseph | 7 Sep 19:55 2011
Picon

Re: [Xen-devel] RE: [PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

> From: Jeremy Fitzhardinge [mailto:jeremy <at> goop.org]
> Sent: Wednesday, September 07, 2011 10:27 AM
> 
> On 09/06/2011 09:20 PM, Cihula, Joseph wrote:
> >> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> >> Sent: Wednesday, August 31, 2011 11:31 AM
> >>
> >> 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 <at> linutronix.de>
> >> CC: "H. Peter Anvin" <hpa <at> zytor.com>
> >> CC: x86 <at> kernel.org
> >> CC: Len Brown <len.brown <at> intel.com>
> >> CC: Joseph Cihula <joseph.cihula <at> intel.com>
> >> CC: Shane Wang <shane.wang <at> intel.com>
> >> CC: xen-devel <at> lists.xensource.com
> >> CC: linux-pm <at> lists.linux-foundation.org
> >> CC: tboot-devel <at> lists.sourceforge.net
> >> CC: linux-acpi <at> vger.kernel.org
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
> >> ---
> >>  arch/x86/include/asm/acpi.h   |    3 +++
> >>  arch/x86/kernel/acpi/boot.c   |    3 +++
> >>  arch/x86/kernel/tboot.c       |   13 +++++++++----
> >>  drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
> >>  include/linux/tboot.h         |    3 ++-
> >>  5 files changed, 27 insertions(+), 7 deletions(-)
> >>
(Continue reading)

Tian, Kevin | 1 Sep 08:58 2011
Picon

Re: [RFC PATCH v1] ACPI S3 to work under Xen.

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Thursday, September 01, 2011 2:31 AM
> 
> Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.
> 
> The relationship that Xen has with Linux kernel is symbiotic. 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). The realm of
> ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
> values - which the hypervisor has to do) and then restore them.
> 
> 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.

that's not a problem for Xen. On bare metal the processor state handled off
from BIOS after resume (e.g. real mode in many cases) doesn't match the
execution environment expected by Linux. So Linux has to save original context
and then restore it after resume.

However for Xen, Linux doesn't talk to BIOS directly. Instead it simply walks
through necessary ACPI methods, and then issues a hypercall to Xen which
then further completes the remaining suspend steps. So here ACPI S3 is
exactly same as any other hypercall path, where processor context will be
saved/restored by Xen automatically.

> 
> So the attached patches do exactly that - there are two entry points
(Continue reading)

Stefano Stabellini | 2 Sep 13:52 2011

Re: [Xen-devel] [RFC PATCH v1] ACPI S3 to work under Xen.

On Wed, 31 Aug 2011, Konrad Rzeszutek Wilk wrote:
> Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.
> 
> The relationship that Xen has with Linux kernel is symbiotic. 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). The realm of
> ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
> values - which the hypervisor has to do) and then restore them.
> 
> 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.
> 
> 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.
> 
> What do you guys think? One thought is to use the paravirt interface to
(Continue reading)

Tian, Kevin | 1 Sep 08:58 2011
Picon

RE: [RFC PATCH v1] ACPI S3 to work under Xen.

> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk <at> oracle.com]
> Sent: Thursday, September 01, 2011 2:31 AM
> 
> Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.
> 
> The relationship that Xen has with Linux kernel is symbiotic. 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). The realm of
> ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
> values - which the hypervisor has to do) and then restore them.
> 
> 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.

that's not a problem for Xen. On bare metal the processor state handled off
from BIOS after resume (e.g. real mode in many cases) doesn't match the
execution environment expected by Linux. So Linux has to save original context
and then restore it after resume.

However for Xen, Linux doesn't talk to BIOS directly. Instead it simply walks
through necessary ACPI methods, and then issues a hypercall to Xen which
then further completes the remaining suspend steps. So here ACPI S3 is
exactly same as any other hypercall path, where processor context will be
saved/restored by Xen automatically.

> 
> So the attached patches do exactly that - there are two entry points
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/x86/pci/xen.c              |   12 ++++++++++++
 include/xen/interface/physdev.h |    7 +++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index f567965..f140999 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
 <at>  <at>  -241,6 +241,17  <at>  <at>  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;
+	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
 #endif
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

[PATCH 6/7] xen/acpi/sleep: Enable ACPI sleep via the __acpi_override_sleep

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 <at> intel.com> initial idea.
[ From http://xenbits.xensource.com/linux-2.6.18-xen.hg
change c68699484a65 ]

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/x86/include/asm/xen/hypercall.h |    8 ++++++++
 arch/x86/xen/enlighten.c             |    3 +++
 drivers/xen/Makefile                 |    2 +-
 drivers/xen/acpi.c                   |   25 +++++++++++++++++++++++++
 include/xen/acpi.h                   |   26 ++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 1 deletions(-)
 create mode 100644 drivers/xen/acpi.c
 create mode 100644 include/xen/acpi.h

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index d240ea9..0c9894e 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
 <at>  <at>  -45,6 +45,7  <at>  <at> 
 #include <xen/interface/xen.h>
 #include <xen/interface/sched.h>
 #include <xen/interface/physdev.h>
+#include <xen/interface/platform.h>

(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

[PATCH 5/7] xen/acpi: Domain0 acpi parser related platform hypercall

From: Yu Ke <ke.yu <at> intel.com>

This patches implements the xen_platform_op hypercall, to pass the parsed
ACPI info to hypervisor.

Signed-off-by: Yu Ke <ke.yu <at> intel.com>
Signed-off-by: Tian Kevin <kevin.tian <at> intel.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge <at> citrix.com>
[v1: Added DEFINE_GUEST.. in appropiate headers]
[v2: Ripped out typedefs]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/ia64/include/asm/xen/interface.h |    1 +
 arch/x86/include/asm/xen/interface.h  |    1 +
 include/xen/interface/platform.h      |  320 +++++++++++++++++++++++++++++++++
 include/xen/interface/xen.h           |    1 +
 4 files changed, 323 insertions(+), 0 deletions(-)
 create mode 100644 include/xen/interface/platform.h

diff --git a/arch/ia64/include/asm/xen/interface.h b/arch/ia64/include/asm/xen/interface.h
index e951e74..1d2427d 100644
--- a/arch/ia64/include/asm/xen/interface.h
+++ b/arch/ia64/include/asm/xen/interface.h
 <at>  <at>  -76,6 +76,7  <at>  <at>  DEFINE_GUEST_HANDLE(char);
 DEFINE_GUEST_HANDLE(int);
 DEFINE_GUEST_HANDLE(long);
 DEFINE_GUEST_HANDLE(void);
+DEFINE_GUEST_HANDLE(uint64_t);

 typedef unsigned long xen_pfn_t;
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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 <at> oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 include/xen/acpi.h |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/include/xen/acpi.h b/include/xen/acpi.h
index e414f14..0409919 100644
--- a/include/xen/acpi.h
+++ b/include/xen/acpi.h
 <at>  <at>  -12,10 +12,22  <at>  <at>  int xen_acpi_notify_hypervisor_state(u8 sleep_state,
 				     u32 pm1a_cnt, u32 pm1b_cnd,
 				     bool *skip_rest);

+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);
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

[PATCH 2/7] x86, acpi, tboot: Have a ACPI sleep override instead of calling tboot_sleep.

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 <at> linutronix.de>
CC: "H. Peter Anvin" <hpa <at> zytor.com>
CC: x86 <at> kernel.org
CC: Len Brown <len.brown <at> intel.com>
CC: Joseph Cihula <joseph.cihula <at> intel.com>
CC: Shane Wang <shane.wang <at> intel.com>
CC: xen-devel <at> lists.xensource.com
CC: linux-pm <at> lists.linux-foundation.org
CC: tboot-devel <at> lists.sourceforge.net
CC: linux-acpi <at> vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
---
 arch/x86/include/asm/acpi.h   |    3 +++
 arch/x86/kernel/acpi/boot.c   |    3 +++
 arch/x86/kernel/tboot.c       |   13 +++++++++----
 drivers/acpi/acpica/hwsleep.c |   12 ++++++++++--
 include/linux/tboot.h         |    3 ++-
 5 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 610001d..49864a1 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
 <at>  <at>  -98,6 +98,9  <at>  <at>  void acpi_pic_sci_set_trigger(unsigned int, u16);
 extern int (*__acpi_register_gsi)(struct device *dev, u32 gsi,
 				  int trigger, int polarity);
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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

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 <at> linutronix.de>
CC: "H. Peter Anvin" <hpa <at> zytor.com>
CC: x86 <at> kernel.org
CC: Len Brown <len.brown <at> intel.com>
CC: Joseph Cihula <joseph.cihula <at> intel.com>
CC: Shane Wang <shane.wang <at> intel.com>
CC: linux-pm <at> lists.linux-foundation.org
CC: linux-acpi <at> vger.kernel.org
CC: Len Brown <len.brown <at> intel.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> oracle.com>
Signed-off-by: Liang Tang <liang.tang <at> oracle.com>
---
 arch/x86/include/asm/acpi.h  |    2 +-
 arch/x86/kernel/acpi/boot.c  |    2 ++
 arch/x86/kernel/acpi/sleep.c |    4 ++--
 arch/x86/kernel/acpi/sleep.h |    2 ++
 drivers/acpi/sleep.c         |    2 ++
 5 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 49864a1..a5f0b73 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
 <at>  <at>  -118,7 +118,7  <at>  <at>  static inline void acpi_disable_pci(void)
(Continue reading)

Konrad Rzeszutek Wilk | 31 Aug 20:31 2011
Picon

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

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 <at> kernel.org
Cc: Thomas Gleixner <tglx <at> linutronix.de>
Cc: "H. Peter Anvin" <hpa <at> zytor.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk <at> 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
 <at>  <at>  -112,19 +112,28  <at>  <at>  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
(Continue reading)

Stefano Stabellini | 2 Sep 13:52 2011

Re: [Xen-devel] [RFC PATCH v1] ACPI S3 to work under Xen.

On Wed, 31 Aug 2011, Konrad Rzeszutek Wilk wrote:
> Attached is an RFC set of patches to enable S3 to work with the Xen hypervisor.
> 
> The relationship that Xen has with Linux kernel is symbiotic. 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). The realm of
> ACPI S3 is more complex as we need to save the CPU state (and Intel TXT
> values - which the hypervisor has to do) and then restore them.
> 
> 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.
> 
> 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.
> 
> What do you guys think? One thought is to use the paravirt interface to
(Continue reading)


Gmane