Patchwork [3/3] piix4 acpi xen support

login
register
mail settings
Submitter John Baboval
Date Oct. 28, 2011, 7:38 p.m.
Message ID <4EAB04B8.2030505@virtualcomputer.com>
Download mbox | patch
Permalink /patch/122491/
State New
Headers show

Comments

John Baboval - Oct. 28, 2011, 7:38 p.m.
When in xen mode, handle the view of pm ioport appropriately.

I'm not entirely comfortable with this patch, since it relies on values
that are hard coded into the DSDT that is shipped with Xen. There has to 
be a better way to handle it, but I haven't thought of what that might 
be yet... Perhaps there should be an acpi_xen.c. Or perhaps the Xen 
table should be modified to match the device model. Or perhaps there is 
a good way to match them up dynamically.


Signed-off-by: John V. Baboval <john.baboval@virtualcomputer.com>
Signed-off-by: Tom Goetz <tom.goetz@virtualcomputer.com>
---
  hw/acpi_piix4.c |   82 
+++++++++++++++++++++++++++++++++++++++++++++++++------
  1 files changed, 73 insertions(+), 9 deletions(-)

                        (unsigned)addr, width, (unsigned)val);
      }
  +    if (xen_enabled()) {
+    /*
+     * Only the PM control register is emulated in Qemu under Xen. The
+     * remaining registers are emulated by the hypervisor.
+     */
+        int sus_typ;
+        s->pm1_cnt.cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
+        if (val & ACPI_BITMASK_SLEEP_ENABLE) {
+            /* change suspend type */
+            sus_typ = (val >> 10) & 7;
+            switch(sus_typ) {
+            case 6: /* soft power off */
+            case 7: /* soft power off */
+                qemu_system_shutdown_request();
+                break;
+            case 5:
+                /* ACPI_BITMASK_WAKE_STATUS should be set on resume.
+                   Pretend that resume was caused by power button */
+                qemu_system_reset_request();
+                if (s->pm1_cnt.cmos_s3) {
+                    qemu_irq_raise(s->pm1_cnt.cmos_s3);
+                }
+            default:
+                break;
+            }
+        }
+        return;
+    }
+
      switch(addr) {
      case 0x00:
          acpi_pm1_evt_write_sts(&s->pm1a, &s->tmr, val);
@@ -136,6 +166,15 @@ static void pm_ioport_read(IORange *ioport, 
uint64_t addr, unsigned width,
      PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
      uint32_t val;
  +    if (xen_enabled()) {
+    /*
+     * Only the PM control register is emulated in Qemu under Xen. The
+     * remaining registers are emulated by the hypervisor.
+     */
+        val = s->pm1_cnt.cnt;
+        return;
+    }
+
      switch(addr) {
      case 0x00:
          val = acpi_pm1_evt_get_sts(&s->pm1a, s->tmr.overflow_time);
@@ -181,19 +220,28 @@ static void acpi_dbg_writel(void *opaque, uint32_t 
addr, uint32_t val)
      PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
  }
  +#define PMCNTRL	0x04
  static void pm_io_space_update(PIIX4PMState *s)
  {
-    uint32_t pm_io_base;
+    uint32_t pm_io_base, size;
+
+    if (!(s->dev.config[0x80] & 1) && !xen_enabled()) {
+        return;
+    }
  -    if (s->dev.config[0x80] & 1) {
-        pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
-        pm_io_base &= 0xffc0;
+    pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
+    pm_io_base &= 0xffc0;
+    size = 16;
  -        /* XXX: need to improve memory and ioport allocation */
-        PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
-        iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
-        ioport_register(&s->ioport);
+    if (xen_enabled()) {
+        size = 2;
+        pm_io_base += PMCNTRL;
      }
+
+    /* XXX: need to improve memory and ioport allocation */
+    PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
+    iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, size);
+    ioport_register(&s->ioport);
  }
   static void pm_write_config(PCIDevice *d,
@@ -326,6 +374,13 @@ static void piix4_pm_machine_ready(Notifier *n, 
void *opaque)
   }
  +#define    PIIX4_BASE_IOADDR       0x1f40
+#define    PIIX4_BASE_IOADDR_LO    ((PIIX4_BASE_IOADDR) & 0xff)
+#define    PIIX4_BASE_IOADDR_HI    ((PIIX4_BASE_IOADDR)>>8)
+
+/* PM1a_CNT bits, as defined in the ACPI specification. */
+#define SCI_EN            (1 <<  0)
+
  static int piix4_pm_initfn(PCIDevice *dev)
  {
      PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
@@ -337,7 +392,13 @@ static int piix4_pm_initfn(PCIDevice *dev)
      pci_conf[0x09] = 0x00;
      pci_conf[0x3d] = 0x01; // interrupt pin 1
  -    pci_conf[0x40] = 0x01; /* PM io base read only bit */
+    if (!xen_enabled()) {
+        pci_conf[0x40] = 0x01; /* PM io base read only bit */
+    } else {
+        pci_conf[0x40] = PIIX4_BASE_IOADDR_LO | 0x01; /* Special 
device-specific BAR at 0x40 */
+        pci_conf[0x41] = PIIX4_BASE_IOADDR_HI;
+        s->pm1_cnt.cnt = SCI_EN;
+    }
       /* APM */
      apm_init(&s->apm, apm_ctrl_changed, s);
@@ -369,6 +430,9 @@ static int piix4_pm_initfn(PCIDevice *dev)
      qemu_register_reset(piix4_reset, s);
      piix4_acpi_system_hot_add_init(dev->bus, s);
  +    if (xen_enabled())
+        pm_io_space_update(s);
+
      return 0;
  }
  -- 1.7.4.1
Ian Campbell - Oct. 31, 2011, 12:41 p.m.
Please CC xen-devel on patches which impact the Xen support in qemu.

On Fri, 2011-10-28 at 15:38 -0400, John Baboval wrote:
> When in xen mode, handle the view of pm ioport appropriately.
> 
> I'm not entirely comfortable with this patch, since it relies on values
> that are hard coded into the DSDT that is shipped with Xen. There has to 
> be a better way to handle it, but I haven't thought of what that might 
> be yet... Perhaps there should be an acpi_xen.c. Or perhaps the Xen 
> table should be modified to match the device model. Or perhaps there is 
> a good way to match them up dynamically.

Anthony Perard posted a patch to xen-devel last week (series entitled
"hvmloader/DSDT change to handle PCI hotplug with QEMU upstream") which
makes the Xen ACPI tables compatible with the upstream QEMU PM registers
etc. I think that covers this issue too?

Ian.
> 
> 
> Signed-off-by: John V. Baboval <john.baboval@virtualcomputer.com>
> Signed-off-by: Tom Goetz <tom.goetz@virtualcomputer.com>
> ---
>   hw/acpi_piix4.c |   82 
> +++++++++++++++++++++++++++++++++++++++++++++++++------
>   1 files changed, 73 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 29f0f76..277ae9f 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -24,6 +24,7 @@
>   #include "sysemu.h"
>   #include "range.h"
>   #include "ioport.h"
> +#include "xen.h"
>    //#define DEBUG
>   @@ -111,6 +112,35 @@ static void pm_ioport_write(IORange *ioport, 
> uint64_t addr, unsigned width,
>                         (unsigned)addr, width, (unsigned)val);
>       }
>   +    if (xen_enabled()) {
> +    /*
> +     * Only the PM control register is emulated in Qemu under Xen. The
> +     * remaining registers are emulated by the hypervisor.
> +     */
> +        int sus_typ;
> +        s->pm1_cnt.cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
> +        if (val & ACPI_BITMASK_SLEEP_ENABLE) {
> +            /* change suspend type */
> +            sus_typ = (val >> 10) & 7;
> +            switch(sus_typ) {
> +            case 6: /* soft power off */
> +            case 7: /* soft power off */
> +                qemu_system_shutdown_request();
> +                break;
> +            case 5:
> +                /* ACPI_BITMASK_WAKE_STATUS should be set on resume.
> +                   Pretend that resume was caused by power button */
> +                qemu_system_reset_request();
> +                if (s->pm1_cnt.cmos_s3) {
> +                    qemu_irq_raise(s->pm1_cnt.cmos_s3);
> +                }
> +            default:
> +                break;
> +            }
> +        }
> +        return;
> +    }
> +
>       switch(addr) {
>       case 0x00:
>           acpi_pm1_evt_write_sts(&s->pm1a, &s->tmr, val);
> @@ -136,6 +166,15 @@ static void pm_ioport_read(IORange *ioport, 
> uint64_t addr, unsigned width,
>       PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>       uint32_t val;
>   +    if (xen_enabled()) {
> +    /*
> +     * Only the PM control register is emulated in Qemu under Xen. The
> +     * remaining registers are emulated by the hypervisor.
> +     */
> +        val = s->pm1_cnt.cnt;
> +        return;
> +    }
> +
>       switch(addr) {
>       case 0x00:
>           val = acpi_pm1_evt_get_sts(&s->pm1a, s->tmr.overflow_time);
> @@ -181,19 +220,28 @@ static void acpi_dbg_writel(void *opaque, uint32_t 
> addr, uint32_t val)
>       PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
>   }
>   +#define PMCNTRL	0x04
>   static void pm_io_space_update(PIIX4PMState *s)
>   {
> -    uint32_t pm_io_base;
> +    uint32_t pm_io_base, size;
> +
> +    if (!(s->dev.config[0x80] & 1) && !xen_enabled()) {
> +        return;
> +    }
>   -    if (s->dev.config[0x80] & 1) {
> -        pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
> -        pm_io_base &= 0xffc0;
> +    pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
> +    pm_io_base &= 0xffc0;
> +    size = 16;
>   -        /* XXX: need to improve memory and ioport allocation */
> -        PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
> -        iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
> -        ioport_register(&s->ioport);
> +    if (xen_enabled()) {
> +        size = 2;
> +        pm_io_base += PMCNTRL;
>       }
> +
> +    /* XXX: need to improve memory and ioport allocation */
> +    PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
> +    iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, size);
> +    ioport_register(&s->ioport);
>   }
>    static void pm_write_config(PCIDevice *d,
> @@ -326,6 +374,13 @@ static void piix4_pm_machine_ready(Notifier *n, 
> void *opaque)
>    }
>   +#define    PIIX4_BASE_IOADDR       0x1f40
> +#define    PIIX4_BASE_IOADDR_LO    ((PIIX4_BASE_IOADDR) & 0xff)
> +#define    PIIX4_BASE_IOADDR_HI    ((PIIX4_BASE_IOADDR)>>8)
> +
> +/* PM1a_CNT bits, as defined in the ACPI specification. */
> +#define SCI_EN            (1 <<  0)
> +
>   static int piix4_pm_initfn(PCIDevice *dev)
>   {
>       PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
> @@ -337,7 +392,13 @@ static int piix4_pm_initfn(PCIDevice *dev)
>       pci_conf[0x09] = 0x00;
>       pci_conf[0x3d] = 0x01; // interrupt pin 1
>   -    pci_conf[0x40] = 0x01; /* PM io base read only bit */
> +    if (!xen_enabled()) {
> +        pci_conf[0x40] = 0x01; /* PM io base read only bit */
> +    } else {
> +        pci_conf[0x40] = PIIX4_BASE_IOADDR_LO | 0x01; /* Special 
> device-specific BAR at 0x40 */
> +        pci_conf[0x41] = PIIX4_BASE_IOADDR_HI;
> +        s->pm1_cnt.cnt = SCI_EN;
> +    }
>        /* APM */
>       apm_init(&s->apm, apm_ctrl_changed, s);
> @@ -369,6 +430,9 @@ static int piix4_pm_initfn(PCIDevice *dev)
>       qemu_register_reset(piix4_reset, s);
>       piix4_acpi_system_hot_add_init(dev->bus, s);
>   +    if (xen_enabled())
> +        pm_io_space_update(s);
> +
>       return 0;
>   }
>   -- 1.7.4.1
> 
> 
>
Anthony PERARD - Oct. 31, 2011, 1:18 p.m.
On Mon, Oct 31, 2011 at 12:41, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> Please CC xen-devel on patches which impact the Xen support in qemu.
>
> On Fri, 2011-10-28 at 15:38 -0400, John Baboval wrote:
>> When in xen mode, handle the view of pm ioport appropriately.
>>
>> I'm not entirely comfortable with this patch, since it relies on values
>> that are hard coded into the DSDT that is shipped with Xen. There has to
>> be a better way to handle it, but I haven't thought of what that might
>> be yet... Perhaps there should be an acpi_xen.c. Or perhaps the Xen
>> table should be modified to match the device model. Or perhaps there is
>> a good way to match them up dynamically.
>
> Anthony Perard posted a patch to xen-devel last week (series entitled
> "hvmloader/DSDT change to handle PCI hotplug with QEMU upstream") which
> makes the Xen ACPI tables compatible with the upstream QEMU PM registers
> etc. I think that covers this issue too?

Actually, Xen unstable should already have the necessary change to
cover everythings in this patch since a while. So QEMU does not need
to be modified.

If something does not work, could you tell me what?

Regards,

Patch

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 29f0f76..277ae9f 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -24,6 +24,7 @@ 
  #include "sysemu.h"
  #include "range.h"
  #include "ioport.h"
+#include "xen.h"
   //#define DEBUG
  @@ -111,6 +112,35 @@ static void pm_ioport_write(IORange *ioport, 
uint64_t addr, unsigned width,