[Qemu-devel] [PULL 0/4] wakeup: make wakeup events guest-configurable

[Qemu-devel] [PULL 0/4] wakeup: make wakeup events guest-configurable

[Gerd Hoffmann]

  Hi,

This patch series allows the guest to enable and disable s3 wakeup
events per device via acpi.  It also adds support for waking up
the guest from uhci.  Each device gets a GPE pin assigned.

With this patch series applied and seabios updated (for the dsdt updates
needed to tell guests about the GPE assignments) /proc/acpi/wakeup
looks like this:

[root@fedora ~]# cat /proc/acpi/wakeup 
Device	S-state	  Status   Sysfs node
UHCI	  S3	*enabled   pci:0000:00:01.2
KBD	  S3	*enabled   pnp:00:02
MOU	  S3	*disabled  pnp:00:03
COM1	  S3	*disabled  pnp:00:05

Patches have been posted for review a while back, pull req got delayed
due to vacation and 1.2 freeze, now I'm finally aiming for merge.

seabios changes needed are available from
  git://git.kraxel.org/seabios wakeup

please pull,
  Gerd

The following changes since commit 8db972cfa469b4e4afd9c65e54e796b83b5ce3a2:

  Update version for 1.2.0 (2012-09-05 07:50:01 -0500)

are available in the git repository at:
  git://git.kraxel.org/qemu wakeup.1

Gerd Hoffmann (4):
      wakeup: add acpi gpe wakeup reasons
      wakeup: make ps/2 configurable
      wakeup: make serial configurable
      wakeup: uhci support

 hw/acpi.c         |   26 ++++++++++++++++++++++++++
 hw/ps2.c          |    4 ++--
 hw/serial.c       |   10 +++++++++-
 hw/usb/hcd-uhci.c |   35 +++++++++++++++++++++++++++++++++++
 sysemu.h          |    4 ++++
 5 files changed, 76 insertions(+), 3 deletions(-)

[Qemu-devel] [PATCH 1/4] wakeup: add acpi gpe wakeup reasons

[Gerd Hoffmann]

Allocate four acpi gpe bits (0x08 -> 0x0b) for s3 wakeup
configuration and notification.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/acpi.c |   26 ++++++++++++++++++++++++++
 sysemu.h  |    4 ++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index f7950be..4d6bc71 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -263,6 +263,22 @@ static void acpi_notify_wakeup(Notifier *notifier, void *data)
         ar->pm1.evt.sts |=
             (ACPI_BITMASK_WAKE_STATUS | ACPI_BITMASK_TIMER_STATUS);
         break;
+    case QEMU_WAKEUP_REASON_GPE_8:
+        ar->pm1.evt.sts |= ACPI_BITMASK_WAKE_STATUS;
+        ar->gpe.sts[1] |= (1 << 0);
+        break;
+    case QEMU_WAKEUP_REASON_GPE_9:
+        ar->pm1.evt.sts |= ACPI_BITMASK_WAKE_STATUS;
+        ar->gpe.sts[1] |= (1 << 1);
+        break;
+    case QEMU_WAKEUP_REASON_GPE_a:
+        ar->pm1.evt.sts |= ACPI_BITMASK_WAKE_STATUS;
+        ar->gpe.sts[1] |= (1 << 2);
+        break;
+    case QEMU_WAKEUP_REASON_GPE_b:
+        ar->pm1.evt.sts |= ACPI_BITMASK_WAKE_STATUS;
+        ar->gpe.sts[1] |= (1 << 3);
+        break;
     case QEMU_WAKEUP_REASON_OTHER:
     default:
         /* ACPI_BITMASK_WAKE_STATUS should be set on resume.
@@ -428,6 +444,10 @@ void acpi_gpe_reset(ACPIREGS *ar)
 {
     memset(ar->gpe.sts, 0, ar->gpe.len / 2);
     memset(ar->gpe.en, 0, ar->gpe.len / 2);
+    qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_8, 0);
+    qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_9, 0);
+    qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_a, 0);
+    qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_b, 0);
 }
 
 static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
@@ -457,6 +477,12 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
     } else if (addr < ar->gpe.len) {
         /* GPE_EN */
         *cur = val;
+        if (addr == ar->gpe.len / 2 + 1) {
+            qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_8, val & (1 << 0));
+            qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_9, val & (1 << 1));
+            qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_a, val & (1 << 2));
+            qemu_system_wakeup_enable(QEMU_WAKEUP_REASON_GPE_b, val & (1 << 3));
+        }
     } else {
         abort();
     }
diff --git a/sysemu.h b/sysemu.h
index 65552ac..af04941 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -42,6 +42,10 @@ typedef enum WakeupReason {
     QEMU_WAKEUP_REASON_OTHER = 0,
     QEMU_WAKEUP_REASON_RTC,
     QEMU_WAKEUP_REASON_PMTIMER,
+    QEMU_WAKEUP_REASON_GPE_8,
+    QEMU_WAKEUP_REASON_GPE_9,
+    QEMU_WAKEUP_REASON_GPE_a,
+    QEMU_WAKEUP_REASON_GPE_b,
 } WakeupReason;
 
 void qemu_system_reset_request(void);
-- 
1.7.1

[Qemu-devel] [PATCH 2/4] wakeup: make ps/2 configurable

[Gerd Hoffmann]

ps/2 gets gpe bits 0x08 (keyboard) and 0x09 (mouse).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/ps2.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ps2.c b/hw/ps2.c
index f93cd24..a3cd124 100644
--- a/hw/ps2.c
+++ b/hw/ps2.c
@@ -155,7 +155,7 @@ static void ps2_put_keycode(void *opaque, int keycode)
 {
     PS2KbdState *s = opaque;
 
-    qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+    qemu_system_wakeup_request(QEMU_WAKEUP_REASON_GPE_8);
     /* XXX: add support for scancode set 1 */
     if (!s->translate && keycode < 0xe0 && s->scancode_set > 1) {
         if (keycode & 0x80) {
@@ -371,7 +371,7 @@ static void ps2_mouse_event(void *opaque,
     s->mouse_buttons = buttons_state;
 
     if (buttons_state) {
-        qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+        qemu_system_wakeup_request(QEMU_WAKEUP_REASON_GPE_9);
     }
 
     if (!(s->mouse_status & MOUSE_STATUS_REMOTE) &&
-- 
1.7.1

[Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Gerd Hoffmann]

serial port #1 gets gpe 0x0a.  Now that the wakeup is guest-configurable
via acpi we also enable it unconditionally.

Other serial ports are unchanged: they continue to use the "other" exit
reason and are disabled unless explicitly enabled via wakeup property.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/serial.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/hw/serial.c b/hw/serial.c
index a421d1e..06f7e28 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -140,6 +140,7 @@ struct SerialState {
     int baudbase;
     int tsr_retry;
     uint32_t wakeup;
+    WakeupReason reason;
 
     uint64_t last_xmit_ts;              /* Time when the last byte was successfully sent out of the tsr */
     SerialFIFO recv_fifo;
@@ -641,7 +642,7 @@ static void serial_receive1(void *opaque, const uint8_t *buf, int size)
     SerialState *s = opaque;
 
     if (s->wakeup) {
-        qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+        qemu_system_wakeup_request(s->reason);
     }
     if(s->fcr & UART_FCR_FE) {
         int i;
@@ -789,6 +790,13 @@ static int serial_isa_initfn(ISADevice *dev)
         isa->isairq = isa_serial_irq[isa->index];
     index++;
 
+    if (isa->iobase == 0x3f8) {
+        s->reason = QEMU_WAKEUP_REASON_GPE_a;
+        s->wakeup = 1;
+    } else {
+        s->reason = QEMU_WAKEUP_REASON_OTHER;
+    }
+
     s->baudbase = 115200;
     isa_init_irq(dev, &s->irq, isa->isairq);
     serial_init_core(s);
-- 
1.7.1

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Peter Maydell]

On 6 September 2012 08:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
> @@ -789,6 +790,13 @@ static int serial_isa_initfn(ISADevice *dev)
>          isa->isairq = isa_serial_irq[isa->index];
>      index++;
>
> +    if (isa->iobase == 0x3f8) {
> +        s->reason = QEMU_WAKEUP_REASON_GPE_a;
> +        s->wakeup = 1;
> +    } else {
> +        s->reason = QEMU_WAKEUP_REASON_OTHER;
> +    }
> +

It seems a bit odd that this is done in the ISA serial model
itself and not by the next level up wiring up some output
of the ISA serial device to some appropriate input...

-- PMM

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Gerd Hoffmann]

On 09/06/12 09:48, Peter Maydell wrote:
> On 6 September 2012 08:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> @@ -789,6 +790,13 @@ static int serial_isa_initfn(ISADevice *dev)
>>          isa->isairq = isa_serial_irq[isa->index];
>>      index++;
>>
>> +    if (isa->iobase == 0x3f8) {
>> +        s->reason = QEMU_WAKEUP_REASON_GPE_a;
>> +        s->wakeup = 1;
>> +    } else {
>> +        s->reason = QEMU_WAKEUP_REASON_OTHER;
>> +    }
>> +
> 
> It seems a bit odd that this is done in the ISA serial model
> itself and not by the next level up wiring up some output
> of the ISA serial device to some appropriate input...

Suggestions how to do that are welcome.  Preferably some which don't
break on 'qemu -nodefault -device isa-serial,chardev=foo'.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Blue Swirl]

On Thu, Sep 6, 2012 at 10:47 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 09/06/12 09:48, Peter Maydell wrote:
>> On 6 September 2012 08:42, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>> @@ -789,6 +790,13 @@ static int serial_isa_initfn(ISADevice *dev)
>>>          isa->isairq = isa_serial_irq[isa->index];
>>>      index++;
>>>
>>> +    if (isa->iobase == 0x3f8) {
>>> +        s->reason = QEMU_WAKEUP_REASON_GPE_a;
>>> +        s->wakeup = 1;
>>> +    } else {
>>> +        s->reason = QEMU_WAKEUP_REASON_OTHER;
>>> +    }
>>> +
>>
>> It seems a bit odd that this is done in the ISA serial model
>> itself and not by the next level up wiring up some output
>> of the ISA serial device to some appropriate input...
>
> Suggestions how to do that are welcome.  Preferably some which don't
> break on 'qemu -nodefault -device isa-serial,chardev=foo'.

Add a qdev property? The base address check can't be correct, the
serial device could be the only one in the board and wired to wakeup
but still use a different iobase.

One way could be to check if chr == serial_hds[0] or rather, pass the
wakeup reason code from board level based on this check.

>
> cheers,
>   Gerd
>
>

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Paolo Bonzini]

Il 08/09/2012 09:15, Blue Swirl ha scritto:
>> Preferably some which don't
>> > break on 'qemu -nodefault -device isa-serial,chardev=foo'.
> Add a qdev property? The base address check can't be correct, the
> serial device could be the only one in the board and wired to wakeup
> but still use a different iobase.

Could work, but the default value for the property would still be
"depending on the iobase".

> One way could be to check if chr == serial_hds[0] or rather, pass the
> wakeup reason code from board level based on this check.

That doesn't work for -device.

Paolo

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 08/09/2012 09:15, Blue Swirl ha scritto:
>>> Preferably some which don't
>>> > break on 'qemu -nodefault -device isa-serial,chardev=foo'.
>> Add a qdev property? The base address check can't be correct, the
>> serial device could be the only one in the board and wired to wakeup
>> but still use a different iobase.
>
> Could work, but the default value for the property would still be
> "depending on the iobase".
>
>> One way could be to check if chr == serial_hds[0] or rather, pass the
>> wakeup reason code from board level based on this check.
>
> That doesn't work for -device.

I think the wiring is wrong here.  Why would a device assert the wakeup
reason?

I assume on bare metal, the device asserts a single line and then
something devices how to treat the output from a given device.

IOW, shouldn't it be something more like, SerialState exports a qemu_irq
and then the machine code decides whether to route that irq to a call to
qemu_system_wakeup().  Then depending on whether this is coming from
serial[1], determines which reason is used.

Regards,

Anthony Liguori

>
> Paolo

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Paolo Bonzini]

Il 10/09/2012 18:02, Anthony Liguori ha scritto:
> I think the wiring is wrong here.  Why would a device assert the wakeup
> reason?
> 
> I assume on bare metal, the device asserts a single line and then
> something devices how to treat the output from a given device.
> 
> IOW, shouldn't it be something more like, SerialState exports a qemu_irq
> and then the machine code decides whether to route that irq to a call to
> qemu_system_wakeup().  Then depending on whether this is coming from
> serial[1], determines which reason is used.

But the only way to do it, that works for -device, is to look at the
SerialState's iobase.  So it would still be a hack, only in ACPI rather
than isa-serial.c.

Paolo

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Peter Maydell]

On 10 September 2012 17:07, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 10/09/2012 18:02, Anthony Liguori ha scritto:
>> IOW, shouldn't it be something more like, SerialState exports a qemu_irq
>> and then the machine code decides whether to route that irq to a call to
>> qemu_system_wakeup().  Then depending on whether this is coming from
>> serial[1], determines which reason is used.
>
> But the only way to do it, that works for -device, is to look at the
> SerialState's iobase.

This just says that -device is broken (which we kinda already knew :-))

-- PMM

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 10 September 2012 17:07, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 10/09/2012 18:02, Anthony Liguori ha scritto:
>>> IOW, shouldn't it be something more like, SerialState exports a qemu_irq
>>> and then the machine code decides whether to route that irq to a call to
>>> qemu_system_wakeup().  Then depending on whether this is coming from
>>> serial[1], determines which reason is used.
>>
>> But the only way to do it, that works for -device, is to look at the
>> SerialState's iobase.
>
> This just says that -device is broken (which we kinda already knew
> :-))

There are opposing forces here.

libvirt wants to use -device because we don't provide a way for machine
creates devices to be configurable.  We can't fix this until we separate
realization from initialization.  After that point, libvirt could
address properties on machine-created devices directly.

I think the best short term solution is to have a post '-device' handler
in machines that could be used to make the appropriate connections.

Power et al. need this anyway for dealing with device tree creation so
it seems to be inevitable.

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] [PATCH 3/4] wakeup: make serial configurable

[Gerd Hoffmann]

  Hi,

>>> It seems a bit odd that this is done in the ISA serial model
>>> itself and not by the next level up wiring up some output
>>> of the ISA serial device to some appropriate input...
>>
>> Suggestions how to do that are welcome.  Preferably some which don't
>> break on 'qemu -nodefault -device isa-serial,chardev=foo'.
> 
> Add a qdev property?

And offload it to the user / management tool to get that correct?  Don't
think this is a good idea.

> The base address check can't be correct, the
> serial device could be the only one in the board and wired to wakeup
> but still use a different iobase.

We might want to add a check for TARGET_I386, but for x86 it actually
_is_ correct as this matches the acpi dsdt information.

> One way could be to check if chr == serial_hds[0] or rather, pass the
> wakeup reason code from board level based on this check.

Wouldn't work.

cheers,
  Gerd

[Qemu-devel] [PATCH 4/4] wakeup: uhci support

[Gerd Hoffmann]

Implement the (intel-specific) pci configuration register 0xc4, which is
a bitmask saying which ports are allowed to wakeup the system.

Also assign gpe bit 0x0b (used only in case uhci handles device 01.2).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index b0db921..cc0e2de 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -32,6 +32,7 @@
 #include "iov.h"
 #include "dma.h"
 #include "trace.h"
+#include "sysemu.h"
 
 //#define DEBUG
 //#define DEBUG_DUMP_DATA
@@ -129,6 +130,7 @@ struct UHCIState {
     uint32_t fl_base_addr; /* frame list base address */
     uint8_t sof_timing;
     uint8_t status2; /* bit 0 and 1 are used to generate UHCI_STS_USBINT */
+    uint8_t system_wakeup;
     int64_t expire_time;
     QEMUTimer *frame_timer;
     QEMUBH *bh;
@@ -677,6 +679,22 @@ static void uhci_wakeup(USBPort *port1)
     }
 }
 
+static void uhci_wakeup_endpoint(USBBus *bus, USBEndpoint *ep)
+{
+    USBPort *port1 = ep->dev->port;
+    UHCIState *s = port1->opaque;
+
+    if (!(s->system_wakeup & (1 << port1->index))) {
+        return;
+    }
+    if (s->dev.devfn == PCI_DEVFN(1, 2)) {
+        /* piix3/4 chipset uhci controller */
+        qemu_system_wakeup_request(QEMU_WAKEUP_REASON_GPE_b);
+    } else {
+        qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
+    }
+}
+
 static USBDevice *uhci_find_device(UHCIState *s, uint8_t addr)
 {
     USBDevice *dev;
@@ -1209,6 +1227,7 @@ static USBPortOps uhci_port_ops = {
 };
 
 static USBBusOps uhci_bus_ops = {
+    .wakeup_endpoint = uhci_wakeup_endpoint,
 };
 
 static int usb_uhci_common_initfn(PCIDevice *dev)
@@ -1270,6 +1289,17 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
     return 0;
 }
 
+static void usb_uhci_intel_write_config(PCIDevice *dev, uint32_t addr,
+                                        uint32_t val, int len)
+{
+    UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
+
+    pci_default_write_config(dev, addr, val, len);
+    if (addr == 0xc4) {
+        s->system_wakeup = val;
+    }
+}
+
 static int usb_uhci_vt82c686b_initfn(PCIDevice *dev)
 {
     UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
@@ -1306,6 +1336,7 @@ static void piix3_uhci_class_init(ObjectClass *klass, void *data)
 
     k->init = usb_uhci_common_initfn;
     k->exit = usb_uhci_exit;
+    k->config_write = usb_uhci_intel_write_config;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371SB_2;
     k->revision = 0x01;
@@ -1328,6 +1359,7 @@ static void piix4_uhci_class_init(ObjectClass *klass, void *data)
 
     k->init = usb_uhci_common_initfn;
     k->exit = usb_uhci_exit;
+    k->config_write = usb_uhci_intel_write_config;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82371AB_2;
     k->revision = 0x01;
@@ -1371,6 +1403,7 @@ static void ich9_uhci1_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->init = usb_uhci_common_initfn;
+    k->config_write = usb_uhci_intel_write_config;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1;
     k->revision = 0x03;
@@ -1392,6 +1425,7 @@ static void ich9_uhci2_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->init = usb_uhci_common_initfn;
+    k->config_write = usb_uhci_intel_write_config;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2;
     k->revision = 0x03;
@@ -1413,6 +1447,7 @@ static void ich9_uhci3_class_init(ObjectClass *klass, void *data)
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
     k->init = usb_uhci_common_initfn;
+    k->config_write = usb_uhci_intel_write_config;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3;
     k->revision = 0x03;
-- 
1.7.1