[PATCH] hvmloader: add "wakeup from S4 on RTC alarm" bit to ACPI FADT

[PATCH] hvmloader: add "wakeup from S4 on RTC alarm" bit to ACPI FADT

[Paolo Bonzini]

# HG changeset patch
# User Paolo Bonzini <pbonzini@redhat.com>
# Date 1296827299 -3600
# Node ID c923816861a96328aaf3d5ba8939fd9b4c441cd0
# Parent  4cd3bd56fc0343031085b7e466e3022816b1b5dc
hvmloader: add "wakeup from S4 on RTC alarm" bit to ACPI FADT

Testing Xen with Microsoft's Windows logo tests reveals a non-fatal
failure in the "CHAOS" test (more precisely in the PwrTest subjob).

The test expects to be able to hibernate the machine and have it
power up again via the RTC.  We do not really care about implementing
the exact timing in the management (and neither does MS) so their
request can be easily by manually restarting the VM or even via
on_poweroff="restart" (granted, that's not something you'd usually do).

Ok?

Paolo

diff --git a/tools/firmware/hvmloader/acpi/static_tables.c b/tools/firmware/hvmloader/acpi/static_tables.c
--- a/tools/firmware/hvmloader/acpi/static_tables.c
+++ b/tools/firmware/hvmloader/acpi/static_tables.c
@@ -71,7 +71,8 @@ struct acpi_20_fadt Fadt = {
     .flags = (ACPI_PROC_C1 |
               ACPI_WBINVD |
               ACPI_FIX_RTC | ACPI_TMR_VAL_EXT |
-              ACPI_USE_PLATFORM_CLOCK),
+              ACPI_USE_PLATFORM_CLOCK |
+              ACPI_RTC_S4),
 
     .reset_reg = {
         .address_space_id    = ACPI_SYSTEM_IO,

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Keir Fraser]

On 04/02/2011 14:52, "Paolo Bonzini" <pbonzini@redhat.com> wrote:

> hvmloader: add "wakeup from S4 on RTC alarm" bit to ACPI FADT
> 
> Testing Xen with Microsoft's Windows logo tests reveals a non-fatal
> failure in the "CHAOS" test (more precisely in the PwrTest subjob).
> 
> The test expects to be able to hibernate the machine and have it
> power up again via the RTC.  We do not really care about implementing
> the exact timing in the management (and neither does MS) so their
> request can be easily by manually restarting the VM or even via
> on_poweroff="restart" (granted, that's not something you'd usually do).
> 
> Ok?

Does a non-fatal error actually matter to passing a Windows logo test? After
all it seems more correct to not advertise a feature we don't actually
support, regardless of what Microsoft would prefer.

 -- Keir

RE: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Paul Durrant]

I believe the test *requires* S4 support. Some others require S3 support, with similar RTC wake.

  Paul

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-
> bounces@lists.xensource.com] On Behalf Of Keir Fraser
> Sent: 04 February 2011 15:05
> To: Paolo Bonzini; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] hvmloader: add "wakeup from S4 on
> RTC alarm"bit to ACPI FADT
> 
> On 04/02/2011 14:52, "Paolo Bonzini" <pbonzini@redhat.com> wrote:
> 
> > hvmloader: add "wakeup from S4 on RTC alarm" bit to ACPI FADT
> >
> > Testing Xen with Microsoft's Windows logo tests reveals a non-
> fatal
> > failure in the "CHAOS" test (more precisely in the PwrTest
> subjob).
> >
> > The test expects to be able to hibernate the machine and have it
> power
> > up again via the RTC.  We do not really care about implementing
> the
> > exact timing in the management (and neither does MS) so their
> request
> > can be easily by manually restarting the VM or even via
> > on_poweroff="restart" (granted, that's not something you'd usually
> do).
> >
> > Ok?
> 
> Does a non-fatal error actually matter to passing a Windows logo
> test? After all it seems more correct to not advertise a feature we
> don't actually support, regardless of what Microsoft would prefer.
> 
>  -- Keir
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Paolo Bonzini]

On 02/04/2011 04:04 PM, Keir Fraser wrote:
> Does a non-fatal error actually matter to passing a Windows logo test?

No, it doesn't, though I expect the error to become fatal in future 
releases of the tests.

Paolo

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Paolo Bonzini]

On 02/04/2011 04:22 PM, Paul Durrant wrote:
> I believe the test *requires* S4 support. Some others require S3 support, with similar RTC wake.

It requires S4 support, but that's not a problem.  S4 is just hibernate 
and works nicely.

For the test, RTC wake is not needed (you can simply restart the machine 
yourself), but the test fails to run if it is not available.

Paolo

RE: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Paul Durrant]

Yes, you can restart the VM yourself if you want to wait around and watch the test! Yes, ACPI does have to be set up correctly for the test to run at all though.

  Paul

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: 04 February 2011 15:57
> To: Paul Durrant
> Cc: Keir Fraser; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] hvmloader: add "wakeup from S4 on
> RTC alarm"bit to ACPI FADT
> 
> On 02/04/2011 04:22 PM, Paul Durrant wrote:
> > I believe the test *requires* S4 support. Some others require S3
> support, with similar RTC wake.
> 
> It requires S4 support, but that's not a problem.  S4 is just
> hibernate and works nicely.
> 
> For the test, RTC wake is not needed (you can simply restart the
> machine yourself), but the test fails to run if it is not available.
> 
> Paolo

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm" bit to ACPI FADT

[Paolo Bonzini]

On 02/04/2011 05:00 PM, Paul Durrant wrote:
> Yes, you can restart the VM yourself if you want to wait around and watch the test!

That's why I mentioned the slighly unorthodox alternative of 
on_poweroff="restart", which is as close as Xen management gets to RTC 
wake...

Paolo

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Tim Deegan]

Please don't top-post. 

At 16:00 +0000 on 04 Feb (1296835213), Paul Durrant wrote:
> Yes, you can restart the VM yourself if you want to wait around and
> watch the test! Yes, ACPI does have to be set up correctly for the
> test to run at all though.

XenServer has code in the qemu acpi path to actually perform timed wake
from S4 by pulling the RTC state out of the hypervisor.  I thought this
had gone upstream to qemu-xen already; now that we're transitioning to
upstream qemu I'm not sure where it should go.  It will certainly need
tidying for upstream qemu as it's very xen-specific.

Cheers,

Tim.

>   Paul
> 
> > -----Original Message-----
> > From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> > Sent: 04 February 2011 15:57
> > To: Paul Durrant
> > Cc: Keir Fraser; xen-devel@lists.xensource.com
> > Subject: Re: [Xen-devel] [PATCH] hvmloader: add "wakeup from S4 on
> > RTC alarm"bit to ACPI FADT
> > 
> > On 02/04/2011 04:22 PM, Paul Durrant wrote:
> > > I believe the test *requires* S4 support. Some others require S3
> > support, with similar RTC wake.
> > 
> > It requires S4 support, but that's not a problem.  S4 is just
> > hibernate and works nicely.
> > 
> > For the test, RTC wake is not needed (you can simply restart the
> > machine yourself), but the test fails to run if it is not available.
> > 
> > Paolo
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Paolo Bonzini]

On 02/04/2011 05:17 PM, Tim Deegan wrote:
> XenServer has code in the qemu acpi path to actually perform timed wake
> from S4 by pulling the RTC state out of the hypervisor.  I thought this
> had gone upstream to qemu-xen already; now that we're transitioning to
> upstream qemu I'm not sure where it should go.  It will certainly need
> tidying for upstream qemu as it's very xen-specific.

There's nothing like that indeed in qemu-xen-unstable.git.

Paolo

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Tim Deegan]

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

At 16:22 +0000 on 04 Feb (1296836548), Paolo Bonzini wrote:
> On 02/04/2011 05:17 PM, Tim Deegan wrote:
> > XenServer has code in the qemu acpi path to actually perform timed wake
> > from S4 by pulling the RTC state out of the hypervisor.  I thought this
> > had gone upstream to qemu-xen already; now that we're transitioning to
> > upstream qemu I'm not sure where it should go.  It will certainly need
> > tidying for upstream qemu as it's very xen-specific.
> 
> There's nothing like that indeed in qemu-xen-unstable.git.

Grrr. :(  I've attached a patch extracted from XenServer source trees,
against qemu-xen of Xen 3.4 vintage.  This is just as RFC, since I'm
sure it will need work even to apply to current qemu-xen.

Ian, what's the best thing to do with this?  Is it worth rebasing it to
qemu-xen tip or should it wait for the switch to upstream? 

Tim

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: acpi-s4-wake-timer --]
[-- Type: text/plain, Size: 5673 bytes --]

diff -r c4cccfde855f hw/piix4acpi.c
--- a/hw/piix4acpi.c	Wed Jun 17 15:13:19 2009 +0100
+++ b/hw/piix4acpi.c	Wed Jun 17 16:05:31 2009 +0100
@@ -32,6 +32,7 @@
 
 #include <xen/hvm/ioreq.h>
 #include <xen/hvm/params.h>
+#include <xen/hvm/save.h>
 
 /* PM1a_CNT bits, as defined in the ACPI specification. */
 #define SCI_EN            (1 <<  0)
@@ -53,6 +54,8 @@
 
 /* The bit in GPE0_STS/EN to notify the pci hotplug event */
 #define ACPI_PHP_GPE_BIT 3
+
+#define RTC_EN            (1 << 10)
 
 typedef struct AcpiDeviceState AcpiDeviceState;
 AcpiDeviceState *acpi_device_table;
@@ -108,6 +111,138 @@
     return 0;
 }
 
+static inline int unbcd(int coded, int val)
+{
+    if (!coded) 
+        return val;
+    else
+        return ((val >> 4) * 10) + (val & 0x0f);
+}
+
+static void acpi_s4(void)
+{
+    ssize_t rec_len;
+    void *buf = NULL;
+    struct hvm_save_descriptor *d;
+    struct hvm_hw_pmtimer *pt;    
+    struct hvm_hw_rtc *rtc;
+    int wait = -1;
+    int wake_enabled = 0;
+
+    fprintf(stderr, "ACPI: entering S4\n");
+
+    xc_domain_pause(xc_handle, domid);
+
+    /* Extract the HVM state from Xen */
+    rec_len = xc_domain_hvm_getcontext(xc_handle, domid, 0, 0);
+    if (rec_len < 0) {
+        fprintf(stderr, "Can't get HVM state length\n");
+        goto no_hvm_state;
+    }
+    if ((buf = malloc(rec_len)) == NULL) {
+        fprintf(stderr, "Can't allocate HVM state buffer\n");
+        goto no_hvm_state;
+    }
+    if ((rec_len = xc_domain_hvm_getcontext(xc_handle, domid, buf, 
+                                            rec_len)) == -1) {
+        fprintf(stderr, "Can't get HVM state\n");
+        goto no_hvm_state;
+    }
+    
+    for (d = buf;
+         ((void *)d < (buf + rec_len)) && d->typecode != HVM_SAVE_CODE(END);
+         d = (void *)d + sizeof (*d) + d->length) {
+        switch (d->typecode) {
+            
+        case HVM_SAVE_CODE(PMTIMER):
+            pt = (struct hvm_hw_pmtimer *)(d + 1);
+            wake_enabled = !!(pt->pm1a_en & RTC_EN);
+            break;
+            
+        case HVM_SAVE_CODE(RTC):
+            rtc = (struct hvm_hw_rtc *)(d + 1);
+            if (rtc->cmos_data[0xB] & 0x20) { /* Alarm interrupt enabled */
+                unsigned int now, alarm, ch, cm, cs, h, m, s, pm, bcd;
+
+                bcd = !(rtc->cmos_data[0xB] & 0x04);
+
+                cs = unbcd(bcd, rtc->cmos_data[0x0]);  /* Clock secs */
+                cm = unbcd(bcd, rtc->cmos_data[0x2]);  /* Clock minutes */
+                ch = unbcd(bcd, rtc->cmos_data[0x4] & 0x7f);  /* Clock hours */
+                if (!(rtc->cmos_data[0xB] & 0x02)  /* 12-hour clock */
+                    && (rtc->cmos_data[0x4] & 0x80)) /* PM */
+                    ch += 12;
+                
+                now = cs + cm * 60 + ch * 3600;
+
+                s = rtc->cmos_data[0x1];  /* Alarm secs */
+                m = rtc->cmos_data[0x3];  /* Alarm minutes */
+                h = rtc->cmos_data[0x5];  /* Alarm hours */
+                pm = (!(rtc->cmos_data[0xB] & 0x02)  /* 12-hour clock */
+                      && (rtc->cmos_data[0x5] & 0x80)) ? 43200 : 0; /* PM */
+
+                alarm = 0;
+                /* Add alarm h/m/s.  If an entry is wildcarded (>= 0xc0), 
+                 * use the one from the current time. */
+                if (s < 0xc0) alarm += unbcd(bcd, s); 
+                else alarm += cs;
+                if (m < 0xc0) alarm += unbcd(bcd, m) * 60;
+                else alarm += cm * 60;
+                if (h < 0xc0) alarm += unbcd(bcd, h & 0x7f) * 3600 + pm;
+                else alarm += ch * 3600;
+
+                wait = (int) alarm - (int) now; 
+                while (wait < 0) wait += 86400;
+
+                /* Handle wildcarded h/m/s entries */
+                if (h >= 0xc0) { 
+                    while (wait > 3600) wait -= 3600;
+                    if (m >= 0xc0) {
+                        while (wait > 60) wait -= 60;
+                        if (s >= 0xc0)
+                            wait = 0;
+                    } else if (s >= 0xc0 && wait > cs)
+                        wait -= cs;
+                } else {
+                    if (m >= 0xc0 && wait > cm * 60)
+                        wait -= cm * 60;
+                    if (s >= 0xc0 && wait > cs)
+                        wait -= cs;
+                }
+
+                fprintf(stderr, "S4: RTC now %02i:%02i:%02i, "
+                        "alarm %02i:%02i:%02i (+%i secs)\n", 
+                        ch, cm, cs,
+                        (h < 0xc0) ? unbcd(bcd, h & 0x7f) : -1,
+                        (m < 0xc0) ? unbcd(bcd, m) : -1,
+                        (s < 0xc0) ? unbcd(bcd, s) : -1, wait);
+            }
+            break;
+
+        default:
+            break;
+        }
+    } 
+    
+ no_hvm_state:
+    free(buf);
+
+    if (!wake_enabled) {
+        fprintf(stderr, "S4: RTC wake not enabled: halting\n");
+    } else if (wait < 0) {
+        fprintf(stderr, "S4: RTC alarm not set: halting\n");
+    } else {
+        sleep(wait);
+        fprintf(stderr, "S4: RTC alarm fired: rebooting\n");
+        xc_domain_unpause(xc_handle, domid);
+        qemu_system_reset_request();
+        return;
+    }
+
+    xc_domain_unpause(xc_handle, domid);
+    qemu_system_shutdown_request();
+}
+
 static void acpiPm1Control_writeb(void *opaque, uint32_t addr, uint32_t val)
 {
     PCIAcpiState *s = opaque;
@@ -135,6 +270,8 @@
         xc_set_hvm_param(xc_handle, domid, HVM_PARAM_ACPI_S_STATE, 3);
         break;
     case SLP_TYP_S4:
+        acpi_s4();
+        break;
     case SLP_TYP_S5:
         qemu_system_shutdown_request();
         break;

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

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

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Ian Jackson]

Tim Deegan writes ("Re: [Xen-devel] [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT"):
> At 16:22 +0000 on 04 Feb (1296836548), Paolo Bonzini wrote:
> > There's nothing like that indeed in qemu-xen-unstable.git.
> 
> Grrr. :(  I've attached a patch extracted from XenServer source trees,
> against qemu-xen of Xen 3.4 vintage.  This is just as RFC, since I'm
> sure it will need work even to apply to current qemu-xen.

Hrm.

> Ian, what's the best thing to do with this?  Is it worth rebasing it to
> qemu-xen tip or should it wait for the switch to upstream? 

Well, it's definitely not going into 4.1 and in 4.2 we're hoping to
switch to upstream qemu.  So I wouldn't bother trying to rebase it to
current qemu-xen tip.

Ian.

Re: [PATCH] hvmloader: add "wakeup from S4 on RTC alarm"bit to ACPI FADT

[Stefano Stabellini]

On Mon, 7 Feb 2011, Tim Deegan wrote:
> At 16:22 +0000 on 04 Feb (1296836548), Paolo Bonzini wrote:
> > On 02/04/2011 05:17 PM, Tim Deegan wrote:
> > > XenServer has code in the qemu acpi path to actually perform timed wake
> > > from S4 by pulling the RTC state out of the hypervisor.  I thought this
> > > had gone upstream to qemu-xen already; now that we're transitioning to
> > > upstream qemu I'm not sure where it should go.  It will certainly need
> > > tidying for upstream qemu as it's very xen-specific.
> > 
> > There's nothing like that indeed in qemu-xen-unstable.git.
> 
> Grrr. :(  I've attached a patch extracted from XenServer source trees,
> against qemu-xen of Xen 3.4 vintage.  This is just as RFC, since I'm
> sure it will need work even to apply to current qemu-xen.
> 
> Ian, what's the best thing to do with this?  Is it worth rebasing it to
> qemu-xen tip or should it wait for the switch to upstream? 
> 

I personally think it could be rebased now because it is self contained.