Xen devel - Oct 2011 - Re: [Qemu-devel] [PATCH 3/3] piix4 acpi xen support

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
> 
> 
> 


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

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,

-- 
Anthony PERARD

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