Patchwork [1/3] split acpi pci hotplug code into separate file

login
register
mail settings
Submitter Gerd Hoffmann
Date Jan. 9, 2013, 3:41 p.m.
Message ID <1357746097-23338-2-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/210735/
State New
Headers show

Comments

Gerd Hoffmann - Jan. 9, 2013, 3:41 p.m.
Also some minor code restructions along the way:

 * create a new struct for acpi pci hotplug state
 * rename functions so they no longer carry piix in the name

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/Makefile.objs      |    2 +-
 hw/acpi_pci_hotplug.c |  214 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/acpi_pci_hotplug.h |   26 ++++++
 hw/acpi_piix4.c       |  202 ++--------------------------------------------
 4 files changed, 250 insertions(+), 194 deletions(-)
 create mode 100644 hw/acpi_pci_hotplug.c
 create mode 100644 hw/acpi_pci_hotplug.h
Blue Swirl - Jan. 9, 2013, 9:13 p.m.
On Wed, Jan 9, 2013 at 3:41 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Also some minor code restructions along the way:
>
>  * create a new struct for acpi pci hotplug state
>  * rename functions so they no longer carry piix in the name
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/Makefile.objs      |    2 +-
>  hw/acpi_pci_hotplug.c |  214 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/acpi_pci_hotplug.h |   26 ++++++
>  hw/acpi_piix4.c       |  202 ++--------------------------------------------
>  4 files changed, 250 insertions(+), 194 deletions(-)
>  create mode 100644 hw/acpi_pci_hotplug.c
>  create mode 100644 hw/acpi_pci_hotplug.h
>
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 6b8a68c..dc1479f 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -31,7 +31,7 @@ common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
>  common-obj-$(CONFIG_PCSPK) += pcspk.o
>  common-obj-$(CONFIG_PCKBD) += pckbd.o
>  common-obj-$(CONFIG_FDC) += fdc.o
> -common-obj-$(CONFIG_ACPI) += acpi.o acpi_piix4.o acpi_ich9.o smbus_ich9.o
> +common-obj-$(CONFIG_ACPI) += acpi.o acpi_pci_hotplug.o acpi_piix4.o acpi_ich9.o smbus_ich9.o
>  common-obj-$(CONFIG_APM) += pm_smbus.o apm.o
>  common-obj-$(CONFIG_DMA) += dma.o
>  common-obj-$(CONFIG_I82374) += i82374.o
> diff --git a/hw/acpi_pci_hotplug.c b/hw/acpi_pci_hotplug.c
> new file mode 100644
> index 0000000..cff21bd
> --- /dev/null
> +++ b/hw/acpi_pci_hotplug.c
> @@ -0,0 +1,214 @@
> +/*
> + * ACPI implementation
> + *
> + * Copyright (c) 2006 Fabrice Bellard
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License version 2 as published by the Free Software Foundation.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> + *
> + * Contributions after 2012-01-13 are licensed under the terms of the
> + * GNU GPL, version 2 or (at your option) any later version.
> + */
> +
> +#include "hw.h"
> +#include "pci/pci.h"
> +
> +#include "acpi_pci_hotplug.h"
> +
> +//#define DEBUG
> +
> +#ifdef DEBUG
> +# define DPRINTF(format, ...)     printf(format, ## __VA_ARGS__)
> +#else
> +# define DPRINTF(format, ...)     do { } while (0)
> +#endif
> +
> +static void acpi_pci_hotplug_eject_slot(ACPIPCI *s, unsigned slots)
> +{
> +    BusChild *kid, *next;
> +    BusState *bus = s->bus;
> +    int slot = ffs(slots) - 1;
> +    bool slot_free = true;
> +
> +    /* Mark request as complete */
> +    s->pci0_status.down &= ~(1U << slot);
> +
> +    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
> +        DeviceState *qdev = kid->child;
> +        PCIDevice *dev = PCI_DEVICE(qdev);
> +        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> +        if (PCI_SLOT(dev->devfn) == slot) {
> +            if (pc->no_hotplug) {
> +                slot_free = false;
> +            } else {
> +                qdev_free(qdev);
> +            }
> +        }
> +    }
> +    if (slot_free) {
> +        s->pci0_slot_device_present &= ~(1U << slot);
> +    }
> +}
> +
> +static uint32_t pci_up_read(void *opaque, uint32_t addr)
> +{
> +    ACPIPCI *s = opaque;
> +    uint32_t val;
> +
> +    /* Manufacture an "up" value to cause a device check on any hotplug
> +     * slot with a device.  Extra device checks are harmless. */
> +    val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
> +
> +    DPRINTF("pci_up_read %x\n", val);
> +    return val;
> +}
> +
> +static uint32_t pci_down_read(void *opaque, uint32_t addr)
> +{
> +    ACPIPCI *s = opaque;
> +    uint32_t val = s->pci0_status.down;
> +
> +    DPRINTF("pci_down_read %x\n", val);
> +    return val;
> +}
> +
> +static uint32_t pci_features_read(void *opaque, uint32_t addr)
> +{
> +    /* No feature defined yet */
> +    DPRINTF("pci_features_read %x\n", 0);
> +    return 0;
> +}
> +
> +static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    acpi_pci_hotplug_eject_slot(opaque, val);
> +
> +    DPRINTF("pciej write %x <== %d\n", addr, val);
> +}
> +
> +static uint32_t pcirmv_read(void *opaque, uint32_t addr)
> +{
> +    ACPIPCI *s = opaque;
> +
> +    return s->pci0_hotplug_enable;
> +}
> +
> +static const MemoryRegionOps piix4_pci_ops = {
> +    .old_portio = (MemoryRegionPortio[]) {
> +        {
> +            .offset = PCI_UP_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
> +            .read = pci_up_read,
> +        },{
> +            .offset = PCI_DOWN_BASE - PCI_HOTPLUG_ADDR, .len = 4, .size = 4,
> +            .read = pci_down_read,
> +        },{
> +            .offset = PCI_EJ_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
> +            .read = pci_features_read,
> +            .write = pciej_write,
> +        },{
> +            .offset = PCI_RMV_BASE - PCI_HOTPLUG_ADDR,  .len = 4, .size = 4,
> +            .read = pcirmv_read,
> +        },
> +        PORTIO_END_OF_LIST()
> +    },
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static void vmstate_pci_status_pre_save(void *opaque)
> +{
> +    struct pci_status *pci0_status = opaque;
> +    ACPIPCI *s = container_of(pci0_status, ACPIPCI, pci0_status);
> +
> +    /* We no longer track up, so build a safe value for migrating
> +     * to a version that still does... of course these might get lost
> +     * by an old buggy implementation, but we try. */
> +    pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable;
> +}
> +
> +const VMStateDescription vmstate_pci_status = {
> +    .name = "pci_status",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .pre_save = vmstate_pci_status_pre_save,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT32(up, struct pci_status),
> +        VMSTATE_UINT32(down, struct pci_status),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +void acpi_pci_hotplug_update(ACPIPCI *s)
> +{
> +    BusState *bus = s->bus;
> +    BusChild *kid, *next;
> +
> +    /* Execute any pending removes during reset */
> +    while (s->pci0_status.down) {
> +        acpi_pci_hotplug_eject_slot(s, s->pci0_status.down);
> +    }
> +
> +    s->pci0_hotplug_enable = ~0;
> +    s->pci0_slot_device_present = 0;
> +
> +    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
> +        DeviceState *qdev = kid->child;
> +        PCIDevice *pdev = PCI_DEVICE(qdev);
> +        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
> +        int slot = PCI_SLOT(pdev->devfn);
> +
> +        if (pc->no_hotplug) {
> +            s->pci0_hotplug_enable &= ~(1U << slot);
> +        }
> +
> +        s->pci0_slot_device_present |= (1U << slot);
> +    }
> +}
> +
> +void acpi_pci_hotplug_init(ACPIPCI *s, DeviceState *qdev)
> +{
> +    s->bus = qdev_get_parent_bus(qdev);
> +    memory_region_init_io(&s->io, &piix4_pci_ops, s, "apci-pci-hotplug",
> +                          PCI_HOTPLUG_SIZE);
> +}
> +
> +
> +static void enable_device(ACPIPCI *s, int slot)
> +{
> +    s->pci0_slot_device_present |= (1U << slot);
> +}
> +
> +static void disable_device(ACPIPCI *s, int slot)
> +{
> +    s->pci0_status.down |= (1U << slot);
> +}
> +
> +int acpi_pci_hotplug_device(ACPIPCI *s, PCIDevice *dev,
> +                            PCIHotplugState state)
> +{
> +    int slot = PCI_SLOT(dev->devfn);
> +
> +    /* Don't send event when device is enabled during qemu machine creation:
> +     * it is present on boot, no hotplug event is necessary. We do send an
> +     * event when the device is disabled later. */
> +    if (state == PCI_COLDPLUG_ENABLED) {
> +        s->pci0_slot_device_present |= (1U << slot);
> +        return 0;
> +    }
> +
> +    if (state == PCI_HOTPLUG_ENABLED) {
> +        enable_device(s, slot);
> +    } else {
> +        disable_device(s, slot);
> +    }
> +    return 1;
> +}
> diff --git a/hw/acpi_pci_hotplug.h b/hw/acpi_pci_hotplug.h
> new file mode 100644
> index 0000000..b97403d
> --- /dev/null
> +++ b/hw/acpi_pci_hotplug.h
> @@ -0,0 +1,26 @@
> +#define PCI_HOTPLUG_ADDR 0xae00
> +#define PCI_HOTPLUG_SIZE 0x000f
> +#define PCI_UP_BASE 0xae00
> +#define PCI_DOWN_BASE 0xae04
> +#define PCI_EJ_BASE 0xae08
> +#define PCI_RMV_BASE 0xae0c
> +
> +struct pci_status {

This should be renamed to PCIStatus and the typedef added. Perhaps
with a separate patch.

> +    uint32_t up; /* deprecated, maintained for migration compatibility */
> +    uint32_t down;
> +};
> +
> +typedef struct ACPIPCI {
> +    MemoryRegion io;
> +    BusState *bus;
> +    struct pci_status pci0_status;
> +    uint32_t pci0_hotplug_enable;
> +    uint32_t pci0_slot_device_present;
> +} ACPIPCI;
> +
> +extern const VMStateDescription vmstate_pci_status;
> +
> +void acpi_pci_hotplug_update(ACPIPCI *s);
> +void acpi_pci_hotplug_init(ACPIPCI *s, DeviceState *qdev);
> +int acpi_pci_hotplug_device(ACPIPCI *s, PCIDevice *dev,
> +                            PCIHotplugState state);
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 06a8aca..1374116 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -29,6 +29,7 @@
>  #include "exec/ioport.h"
>  #include "fw_cfg.h"
>  #include "exec/address-spaces.h"
> +#include "acpi_pci_hotplug.h"
>
>  //#define DEBUG
>
> @@ -41,27 +42,15 @@
>  #define GPE_BASE 0xafe0
>  #define GPE_LEN 4
>
> -#define PCI_HOTPLUG_ADDR 0xae00
> -#define PCI_HOTPLUG_SIZE 0x000f
> -#define PCI_UP_BASE 0xae00
> -#define PCI_DOWN_BASE 0xae04
> -#define PCI_EJ_BASE 0xae08
> -#define PCI_RMV_BASE 0xae0c
> -
>  #define PIIX4_PCI_HOTPLUG_STATUS 2
>
> -struct pci_status {
> -    uint32_t up; /* deprecated, maintained for migration compatibility */
> -    uint32_t down;
> -};
> -
>  typedef struct PIIX4PMState {
>      PCIDevice dev;
>
>      MemoryRegion io;
>      MemoryRegion io_gpe;
> -    MemoryRegion io_pci;
>      ACPIREGS ar;
> +    ACPIPCI pci;
>
>      APMState apm;
>
> @@ -74,11 +63,6 @@ typedef struct PIIX4PMState {
>      Notifier machine_ready;
>      Notifier powerdown_notifier;
>
> -    /* for pci hotplug */
> -    struct pci_status pci0_status;
> -    uint32_t pci0_hotplug_enable;
> -    uint32_t pci0_slot_device_present;
> -
>      uint8_t disable_s3;
>      uint8_t disable_s4;
>      uint8_t s4_val;
> @@ -167,17 +151,6 @@ static void pm_write_config(PCIDevice *d,
>      }
>  }
>
> -static void vmstate_pci_status_pre_save(void *opaque)
> -{
> -    struct pci_status *pci0_status = opaque;
> -    PIIX4PMState *s = container_of(pci0_status, PIIX4PMState, pci0_status);
> -
> -    /* We no longer track up, so build a safe value for migrating
> -     * to a version that still does... of course these might get lost
> -     * by an old buggy implementation, but we try. */
> -    pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable;
> -}
> -
>  static int vmstate_acpi_post_load(void *opaque, int version_id)
>  {
>      PIIX4PMState *s = opaque;
> @@ -208,19 +181,6 @@ static const VMStateDescription vmstate_gpe = {
>      }
>  };
>
> -static const VMStateDescription vmstate_pci_status = {
> -    .name = "pci_status",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> -    .minimum_version_id_old = 1,
> -    .pre_save = vmstate_pci_status_pre_save,
> -    .fields      = (VMStateField []) {
> -        VMSTATE_UINT32(up, struct pci_status),
> -        VMSTATE_UINT32(down, struct pci_status),
> -        VMSTATE_END_OF_LIST()
> -    }
> -};
> -
>  static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
>  {
>      PIIX4PMState *s = opaque;
> @@ -279,67 +239,12 @@ static const VMStateDescription vmstate_acpi = {
>          VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState),
>          VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState),
>          VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
> -        VMSTATE_STRUCT(pci0_status, PIIX4PMState, 2, vmstate_pci_status,
> +        VMSTATE_STRUCT(pci.pci0_status, PIIX4PMState, 2, vmstate_pci_status,
>                         struct pci_status),
>          VMSTATE_END_OF_LIST()
>      }
>  };
>
> -static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
> -{
> -    BusChild *kid, *next;
> -    BusState *bus = qdev_get_parent_bus(&s->dev.qdev);
> -    int slot = ffs(slots) - 1;
> -    bool slot_free = true;
> -
> -    /* Mark request as complete */
> -    s->pci0_status.down &= ~(1U << slot);
> -
> -    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
> -        DeviceState *qdev = kid->child;
> -        PCIDevice *dev = PCI_DEVICE(qdev);
> -        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> -        if (PCI_SLOT(dev->devfn) == slot) {
> -            if (pc->no_hotplug) {
> -                slot_free = false;
> -            } else {
> -                qdev_free(qdev);
> -            }
> -        }
> -    }
> -    if (slot_free) {
> -        s->pci0_slot_device_present &= ~(1U << slot);
> -    }
> -}
> -
> -static void piix4_update_hotplug(PIIX4PMState *s)
> -{
> -    PCIDevice *dev = &s->dev;
> -    BusState *bus = qdev_get_parent_bus(&dev->qdev);
> -    BusChild *kid, *next;
> -
> -    /* Execute any pending removes during reset */
> -    while (s->pci0_status.down) {
> -        acpi_piix_eject_slot(s, s->pci0_status.down);
> -    }
> -
> -    s->pci0_hotplug_enable = ~0;
> -    s->pci0_slot_device_present = 0;
> -
> -    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
> -        DeviceState *qdev = kid->child;
> -        PCIDevice *pdev = PCI_DEVICE(qdev);
> -        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
> -        int slot = PCI_SLOT(pdev->devfn);
> -
> -        if (pc->no_hotplug) {
> -            s->pci0_hotplug_enable &= ~(1U << slot);
> -        }
> -
> -        s->pci0_slot_device_present |= (1U << slot);
> -    }
> -}
> -
>  static void piix4_reset(void *opaque)
>  {
>      PIIX4PMState *s = opaque;
> @@ -357,7 +262,7 @@ static void piix4_reset(void *opaque)
>          /* Mark SMM as already inited (until KVM supports SMM). */
>          pci_conf[0x5B] = 0x02;
>      }
> -    piix4_update_hotplug(s);
> +    acpi_pci_hotplug_update(&s->pci);
>  }
>
>  static void piix4_pm_powerdown_req(Notifier *n, void *opaque)
> @@ -531,70 +436,6 @@ static const MemoryRegionOps piix4_gpe_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>
> -static uint32_t pci_up_read(void *opaque, uint32_t addr)
> -{
> -    PIIX4PMState *s = opaque;
> -    uint32_t val;
> -
> -    /* Manufacture an "up" value to cause a device check on any hotplug
> -     * slot with a device.  Extra device checks are harmless. */
> -    val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
> -
> -    PIIX4_DPRINTF("pci_up_read %x\n", val);
> -    return val;
> -}
> -
> -static uint32_t pci_down_read(void *opaque, uint32_t addr)
> -{
> -    PIIX4PMState *s = opaque;
> -    uint32_t val = s->pci0_status.down;
> -
> -    PIIX4_DPRINTF("pci_down_read %x\n", val);
> -    return val;
> -}
> -
> -static uint32_t pci_features_read(void *opaque, uint32_t addr)
> -{
> -    /* No feature defined yet */
> -    PIIX4_DPRINTF("pci_features_read %x\n", 0);
> -    return 0;
> -}
> -
> -static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
> -{
> -    acpi_piix_eject_slot(opaque, val);
> -
> -    PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val);
> -}
> -
> -static uint32_t pcirmv_read(void *opaque, uint32_t addr)
> -{
> -    PIIX4PMState *s = opaque;
> -
> -    return s->pci0_hotplug_enable;
> -}
> -
> -static const MemoryRegionOps piix4_pci_ops = {
> -    .old_portio = (MemoryRegionPortio[]) {
> -        {
> -            .offset = PCI_UP_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
> -            .read = pci_up_read,
> -        },{
> -            .offset = PCI_DOWN_BASE - PCI_HOTPLUG_ADDR, .len = 4, .size = 4,
> -            .read = pci_down_read,
> -        },{
> -            .offset = PCI_EJ_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
> -            .read = pci_features_read,
> -            .write = pciej_write,
> -        },{
> -            .offset = PCI_RMV_BASE - PCI_HOTPLUG_ADDR,  .len = 4, .size = 4,
> -            .read = pcirmv_read,
> -        },
> -        PORTIO_END_OF_LIST()
> -    },
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> -
>  static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
>                                  PCIHotplugState state);
>
> @@ -605,47 +446,22 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
>                            GPE_LEN);
>      memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
>
> -    memory_region_init_io(&s->io_pci, &piix4_pci_ops, s, "apci-pci-hotplug",
> -                          PCI_HOTPLUG_SIZE);
> +    acpi_pci_hotplug_init(&s->pci, &s->dev.qdev);
>      memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR,
> -                                &s->io_pci);
> +                                &s->pci.io);
>      pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
>  }
>
> -static void enable_device(PIIX4PMState *s, int slot)
> -{
> -    s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
> -    s->pci0_slot_device_present |= (1U << slot);
> -}
> -
> -static void disable_device(PIIX4PMState *s, int slot)
> -{
> -    s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
> -    s->pci0_status.down |= (1U << slot);
> -}
> -
>  static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
>                                 PCIHotplugState state)
>  {
> -    int slot = PCI_SLOT(dev->devfn);
>      PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev,
>                                  PCI_DEVICE(qdev));
>
> -    /* Don't send event when device is enabled during qemu machine creation:
> -     * it is present on boot, no hotplug event is necessary. We do send an
> -     * event when the device is disabled later. */
> -    if (state == PCI_COLDPLUG_ENABLED) {
> -        s->pci0_slot_device_present |= (1U << slot);
> -        return 0;
> +    if (acpi_pci_hotplug_device(&s->pci, dev, state)) {
> +        s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
> +        pm_update_sci(s);
>      }
>
> -    if (state == PCI_HOTPLUG_ENABLED) {
> -        enable_device(s, slot);
> -    } else {
> -        disable_device(s, slot);
> -    }
> -
> -    pm_update_sci(s);
> -
>      return 0;
>  }
> --
> 1.7.1
>
>

Patch

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 6b8a68c..dc1479f 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -31,7 +31,7 @@  common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_PCSPK) += pcspk.o
 common-obj-$(CONFIG_PCKBD) += pckbd.o
 common-obj-$(CONFIG_FDC) += fdc.o
-common-obj-$(CONFIG_ACPI) += acpi.o acpi_piix4.o acpi_ich9.o smbus_ich9.o
+common-obj-$(CONFIG_ACPI) += acpi.o acpi_pci_hotplug.o acpi_piix4.o acpi_ich9.o smbus_ich9.o
 common-obj-$(CONFIG_APM) += pm_smbus.o apm.o
 common-obj-$(CONFIG_DMA) += dma.o
 common-obj-$(CONFIG_I82374) += i82374.o
diff --git a/hw/acpi_pci_hotplug.c b/hw/acpi_pci_hotplug.c
new file mode 100644
index 0000000..cff21bd
--- /dev/null
+++ b/hw/acpi_pci_hotplug.c
@@ -0,0 +1,214 @@ 
+/*
+ * ACPI implementation
+ *
+ * Copyright (c) 2006 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "hw.h"
+#include "pci/pci.h"
+
+#include "acpi_pci_hotplug.h"
+
+//#define DEBUG
+
+#ifdef DEBUG
+# define DPRINTF(format, ...)     printf(format, ## __VA_ARGS__)
+#else
+# define DPRINTF(format, ...)     do { } while (0)
+#endif
+
+static void acpi_pci_hotplug_eject_slot(ACPIPCI *s, unsigned slots)
+{
+    BusChild *kid, *next;
+    BusState *bus = s->bus;
+    int slot = ffs(slots) - 1;
+    bool slot_free = true;
+
+    /* Mark request as complete */
+    s->pci0_status.down &= ~(1U << slot);
+
+    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
+        DeviceState *qdev = kid->child;
+        PCIDevice *dev = PCI_DEVICE(qdev);
+        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+        if (PCI_SLOT(dev->devfn) == slot) {
+            if (pc->no_hotplug) {
+                slot_free = false;
+            } else {
+                qdev_free(qdev);
+            }
+        }
+    }
+    if (slot_free) {
+        s->pci0_slot_device_present &= ~(1U << slot);
+    }
+}
+
+static uint32_t pci_up_read(void *opaque, uint32_t addr)
+{
+    ACPIPCI *s = opaque;
+    uint32_t val;
+
+    /* Manufacture an "up" value to cause a device check on any hotplug
+     * slot with a device.  Extra device checks are harmless. */
+    val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
+
+    DPRINTF("pci_up_read %x\n", val);
+    return val;
+}
+
+static uint32_t pci_down_read(void *opaque, uint32_t addr)
+{
+    ACPIPCI *s = opaque;
+    uint32_t val = s->pci0_status.down;
+
+    DPRINTF("pci_down_read %x\n", val);
+    return val;
+}
+
+static uint32_t pci_features_read(void *opaque, uint32_t addr)
+{
+    /* No feature defined yet */
+    DPRINTF("pci_features_read %x\n", 0);
+    return 0;
+}
+
+static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    acpi_pci_hotplug_eject_slot(opaque, val);
+
+    DPRINTF("pciej write %x <== %d\n", addr, val);
+}
+
+static uint32_t pcirmv_read(void *opaque, uint32_t addr)
+{
+    ACPIPCI *s = opaque;
+
+    return s->pci0_hotplug_enable;
+}
+
+static const MemoryRegionOps piix4_pci_ops = {
+    .old_portio = (MemoryRegionPortio[]) {
+        {
+            .offset = PCI_UP_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
+            .read = pci_up_read,
+        },{
+            .offset = PCI_DOWN_BASE - PCI_HOTPLUG_ADDR, .len = 4, .size = 4,
+            .read = pci_down_read,
+        },{
+            .offset = PCI_EJ_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
+            .read = pci_features_read,
+            .write = pciej_write,
+        },{
+            .offset = PCI_RMV_BASE - PCI_HOTPLUG_ADDR,  .len = 4, .size = 4,
+            .read = pcirmv_read,
+        },
+        PORTIO_END_OF_LIST()
+    },
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void vmstate_pci_status_pre_save(void *opaque)
+{
+    struct pci_status *pci0_status = opaque;
+    ACPIPCI *s = container_of(pci0_status, ACPIPCI, pci0_status);
+
+    /* We no longer track up, so build a safe value for migrating
+     * to a version that still does... of course these might get lost
+     * by an old buggy implementation, but we try. */
+    pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable;
+}
+
+const VMStateDescription vmstate_pci_status = {
+    .name = "pci_status",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .pre_save = vmstate_pci_status_pre_save,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(up, struct pci_status),
+        VMSTATE_UINT32(down, struct pci_status),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+void acpi_pci_hotplug_update(ACPIPCI *s)
+{
+    BusState *bus = s->bus;
+    BusChild *kid, *next;
+
+    /* Execute any pending removes during reset */
+    while (s->pci0_status.down) {
+        acpi_pci_hotplug_eject_slot(s, s->pci0_status.down);
+    }
+
+    s->pci0_hotplug_enable = ~0;
+    s->pci0_slot_device_present = 0;
+
+    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
+        DeviceState *qdev = kid->child;
+        PCIDevice *pdev = PCI_DEVICE(qdev);
+        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
+        int slot = PCI_SLOT(pdev->devfn);
+
+        if (pc->no_hotplug) {
+            s->pci0_hotplug_enable &= ~(1U << slot);
+        }
+
+        s->pci0_slot_device_present |= (1U << slot);
+    }
+}
+
+void acpi_pci_hotplug_init(ACPIPCI *s, DeviceState *qdev)
+{
+    s->bus = qdev_get_parent_bus(qdev);
+    memory_region_init_io(&s->io, &piix4_pci_ops, s, "apci-pci-hotplug",
+                          PCI_HOTPLUG_SIZE);
+}
+
+
+static void enable_device(ACPIPCI *s, int slot)
+{
+    s->pci0_slot_device_present |= (1U << slot);
+}
+
+static void disable_device(ACPIPCI *s, int slot)
+{
+    s->pci0_status.down |= (1U << slot);
+}
+
+int acpi_pci_hotplug_device(ACPIPCI *s, PCIDevice *dev,
+                            PCIHotplugState state)
+{
+    int slot = PCI_SLOT(dev->devfn);
+
+    /* Don't send event when device is enabled during qemu machine creation:
+     * it is present on boot, no hotplug event is necessary. We do send an
+     * event when the device is disabled later. */
+    if (state == PCI_COLDPLUG_ENABLED) {
+        s->pci0_slot_device_present |= (1U << slot);
+        return 0;
+    }
+
+    if (state == PCI_HOTPLUG_ENABLED) {
+        enable_device(s, slot);
+    } else {
+        disable_device(s, slot);
+    }
+    return 1;
+}
diff --git a/hw/acpi_pci_hotplug.h b/hw/acpi_pci_hotplug.h
new file mode 100644
index 0000000..b97403d
--- /dev/null
+++ b/hw/acpi_pci_hotplug.h
@@ -0,0 +1,26 @@ 
+#define PCI_HOTPLUG_ADDR 0xae00
+#define PCI_HOTPLUG_SIZE 0x000f
+#define PCI_UP_BASE 0xae00
+#define PCI_DOWN_BASE 0xae04
+#define PCI_EJ_BASE 0xae08
+#define PCI_RMV_BASE 0xae0c
+
+struct pci_status {
+    uint32_t up; /* deprecated, maintained for migration compatibility */
+    uint32_t down;
+};
+
+typedef struct ACPIPCI {
+    MemoryRegion io;
+    BusState *bus;
+    struct pci_status pci0_status;
+    uint32_t pci0_hotplug_enable;
+    uint32_t pci0_slot_device_present;
+} ACPIPCI;
+
+extern const VMStateDescription vmstate_pci_status;
+
+void acpi_pci_hotplug_update(ACPIPCI *s);
+void acpi_pci_hotplug_init(ACPIPCI *s, DeviceState *qdev);
+int acpi_pci_hotplug_device(ACPIPCI *s, PCIDevice *dev,
+                            PCIHotplugState state);
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 06a8aca..1374116 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -29,6 +29,7 @@ 
 #include "exec/ioport.h"
 #include "fw_cfg.h"
 #include "exec/address-spaces.h"
+#include "acpi_pci_hotplug.h"
 
 //#define DEBUG
 
@@ -41,27 +42,15 @@ 
 #define GPE_BASE 0xafe0
 #define GPE_LEN 4
 
-#define PCI_HOTPLUG_ADDR 0xae00
-#define PCI_HOTPLUG_SIZE 0x000f
-#define PCI_UP_BASE 0xae00
-#define PCI_DOWN_BASE 0xae04
-#define PCI_EJ_BASE 0xae08
-#define PCI_RMV_BASE 0xae0c
-
 #define PIIX4_PCI_HOTPLUG_STATUS 2
 
-struct pci_status {
-    uint32_t up; /* deprecated, maintained for migration compatibility */
-    uint32_t down;
-};
-
 typedef struct PIIX4PMState {
     PCIDevice dev;
 
     MemoryRegion io;
     MemoryRegion io_gpe;
-    MemoryRegion io_pci;
     ACPIREGS ar;
+    ACPIPCI pci;
 
     APMState apm;
 
@@ -74,11 +63,6 @@  typedef struct PIIX4PMState {
     Notifier machine_ready;
     Notifier powerdown_notifier;
 
-    /* for pci hotplug */
-    struct pci_status pci0_status;
-    uint32_t pci0_hotplug_enable;
-    uint32_t pci0_slot_device_present;
-
     uint8_t disable_s3;
     uint8_t disable_s4;
     uint8_t s4_val;
@@ -167,17 +151,6 @@  static void pm_write_config(PCIDevice *d,
     }
 }
 
-static void vmstate_pci_status_pre_save(void *opaque)
-{
-    struct pci_status *pci0_status = opaque;
-    PIIX4PMState *s = container_of(pci0_status, PIIX4PMState, pci0_status);
-
-    /* We no longer track up, so build a safe value for migrating
-     * to a version that still does... of course these might get lost
-     * by an old buggy implementation, but we try. */
-    pci0_status->up = s->pci0_slot_device_present & s->pci0_hotplug_enable;
-}
-
 static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
     PIIX4PMState *s = opaque;
@@ -208,19 +181,6 @@  static const VMStateDescription vmstate_gpe = {
     }
 };
 
-static const VMStateDescription vmstate_pci_status = {
-    .name = "pci_status",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .pre_save = vmstate_pci_status_pre_save,
-    .fields      = (VMStateField []) {
-        VMSTATE_UINT32(up, struct pci_status),
-        VMSTATE_UINT32(down, struct pci_status),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
 {
     PIIX4PMState *s = opaque;
@@ -279,67 +239,12 @@  static const VMStateDescription vmstate_acpi = {
         VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState),
         VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState),
         VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
-        VMSTATE_STRUCT(pci0_status, PIIX4PMState, 2, vmstate_pci_status,
+        VMSTATE_STRUCT(pci.pci0_status, PIIX4PMState, 2, vmstate_pci_status,
                        struct pci_status),
         VMSTATE_END_OF_LIST()
     }
 };
 
-static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
-{
-    BusChild *kid, *next;
-    BusState *bus = qdev_get_parent_bus(&s->dev.qdev);
-    int slot = ffs(slots) - 1;
-    bool slot_free = true;
-
-    /* Mark request as complete */
-    s->pci0_status.down &= ~(1U << slot);
-
-    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
-        DeviceState *qdev = kid->child;
-        PCIDevice *dev = PCI_DEVICE(qdev);
-        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
-        if (PCI_SLOT(dev->devfn) == slot) {
-            if (pc->no_hotplug) {
-                slot_free = false;
-            } else {
-                qdev_free(qdev);
-            }
-        }
-    }
-    if (slot_free) {
-        s->pci0_slot_device_present &= ~(1U << slot);
-    }
-}
-
-static void piix4_update_hotplug(PIIX4PMState *s)
-{
-    PCIDevice *dev = &s->dev;
-    BusState *bus = qdev_get_parent_bus(&dev->qdev);
-    BusChild *kid, *next;
-
-    /* Execute any pending removes during reset */
-    while (s->pci0_status.down) {
-        acpi_piix_eject_slot(s, s->pci0_status.down);
-    }
-
-    s->pci0_hotplug_enable = ~0;
-    s->pci0_slot_device_present = 0;
-
-    QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
-        DeviceState *qdev = kid->child;
-        PCIDevice *pdev = PCI_DEVICE(qdev);
-        PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
-        int slot = PCI_SLOT(pdev->devfn);
-
-        if (pc->no_hotplug) {
-            s->pci0_hotplug_enable &= ~(1U << slot);
-        }
-
-        s->pci0_slot_device_present |= (1U << slot);
-    }
-}
-
 static void piix4_reset(void *opaque)
 {
     PIIX4PMState *s = opaque;
@@ -357,7 +262,7 @@  static void piix4_reset(void *opaque)
         /* Mark SMM as already inited (until KVM supports SMM). */
         pci_conf[0x5B] = 0x02;
     }
-    piix4_update_hotplug(s);
+    acpi_pci_hotplug_update(&s->pci);
 }
 
 static void piix4_pm_powerdown_req(Notifier *n, void *opaque)
@@ -531,70 +436,6 @@  static const MemoryRegionOps piix4_gpe_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static uint32_t pci_up_read(void *opaque, uint32_t addr)
-{
-    PIIX4PMState *s = opaque;
-    uint32_t val;
-
-    /* Manufacture an "up" value to cause a device check on any hotplug
-     * slot with a device.  Extra device checks are harmless. */
-    val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
-
-    PIIX4_DPRINTF("pci_up_read %x\n", val);
-    return val;
-}
-
-static uint32_t pci_down_read(void *opaque, uint32_t addr)
-{
-    PIIX4PMState *s = opaque;
-    uint32_t val = s->pci0_status.down;
-
-    PIIX4_DPRINTF("pci_down_read %x\n", val);
-    return val;
-}
-
-static uint32_t pci_features_read(void *opaque, uint32_t addr)
-{
-    /* No feature defined yet */
-    PIIX4_DPRINTF("pci_features_read %x\n", 0);
-    return 0;
-}
-
-static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
-{
-    acpi_piix_eject_slot(opaque, val);
-
-    PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val);
-}
-
-static uint32_t pcirmv_read(void *opaque, uint32_t addr)
-{
-    PIIX4PMState *s = opaque;
-
-    return s->pci0_hotplug_enable;
-}
-
-static const MemoryRegionOps piix4_pci_ops = {
-    .old_portio = (MemoryRegionPortio[]) {
-        {
-            .offset = PCI_UP_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
-            .read = pci_up_read,
-        },{
-            .offset = PCI_DOWN_BASE - PCI_HOTPLUG_ADDR, .len = 4, .size = 4,
-            .read = pci_down_read,
-        },{
-            .offset = PCI_EJ_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
-            .read = pci_features_read,
-            .write = pciej_write,
-        },{
-            .offset = PCI_RMV_BASE - PCI_HOTPLUG_ADDR,  .len = 4, .size = 4,
-            .read = pcirmv_read,
-        },
-        PORTIO_END_OF_LIST()
-    },
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
                                 PCIHotplugState state);
 
@@ -605,47 +446,22 @@  static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
                           GPE_LEN);
     memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe);
 
-    memory_region_init_io(&s->io_pci, &piix4_pci_ops, s, "apci-pci-hotplug",
-                          PCI_HOTPLUG_SIZE);
+    acpi_pci_hotplug_init(&s->pci, &s->dev.qdev);
     memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR,
-                                &s->io_pci);
+                                &s->pci.io);
     pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
 }
 
-static void enable_device(PIIX4PMState *s, int slot)
-{
-    s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
-    s->pci0_slot_device_present |= (1U << slot);
-}
-
-static void disable_device(PIIX4PMState *s, int slot)
-{
-    s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
-    s->pci0_status.down |= (1U << slot);
-}
-
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
 				PCIHotplugState state)
 {
-    int slot = PCI_SLOT(dev->devfn);
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev,
                                 PCI_DEVICE(qdev));
 
-    /* Don't send event when device is enabled during qemu machine creation:
-     * it is present on boot, no hotplug event is necessary. We do send an
-     * event when the device is disabled later. */
-    if (state == PCI_COLDPLUG_ENABLED) {
-        s->pci0_slot_device_present |= (1U << slot);
-        return 0;
+    if (acpi_pci_hotplug_device(&s->pci, dev, state)) {
+        s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
+        pm_update_sci(s);
     }
 
-    if (state == PCI_HOTPLUG_ENABLED) {
-        enable_device(s, slot);
-    } else {
-        disable_device(s, slot);
-    }
-
-    pm_update_sci(s);
-
     return 0;
 }