[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] split acpi pci hotplug code into separate f
|
From: |
Blue Swirl |
|
Subject: |
Re: [Qemu-devel] [PATCH 1/3] split acpi pci hotplug code into separate file |
|
Date: |
Wed, 9 Jan 2013 21:13:47 +0000 |
On Wed, Jan 9, 2013 at 3:41 PM, Gerd Hoffmann <address@hidden> 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 <address@hidden>
> ---
> 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
>
>