Re: [Qemu-devel] [PULL 41/43] pcihp: generalization of piix4 acpi

[Anthony Liguori]

"Michael S. Tsirkin" <address@hidden> writes:

> Add ACPI based PCI hotplug library with bridge hotplug
> support.
> Design
>    - each bus gets assigned "bsel" property.
>    - ACPI code writes this number
>      to a new BNUM register, then uses existing
>      UP/DOWN registers to probe slot status;
>      to eject, write number to BNUM register,
>      then slot into existing EJ.
>
> The interface is actually backwards-compatible with
> existing PIIX4 ACPI (though not migration compatible).
>
> This is split out from PIIX4 codebase so we can
> reuse it for Q35 as well.

So we break migration for PIIX4 so the code can be reused for Q35?

That doesn't seem acceptable to me.

Regards,

Anthony Liguori

>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  include/hw/acpi/pcihp.h |  72 +++++++++++
>  hw/acpi/pcihp.c         | 312 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/acpi/Makefile.objs   |   2 +-
>  3 files changed, 385 insertions(+), 1 deletion(-)
>  create mode 100644 include/hw/acpi/pcihp.h
>  create mode 100644 hw/acpi/pcihp.c
>
> diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h
> new file mode 100644
> index 0000000..6230e60
> --- /dev/null
> +++ b/include/hw/acpi/pcihp.h
> @@ -0,0 +1,72 @@
> +/*
> + * QEMU<->ACPI BIOS PCI hotplug interface
> + *
> + * QEMU supports PCI hotplug via ACPI. This module
> + * implements the interface between QEMU and the ACPI BIOS.
> + * Interface specification - see docs/specs/acpi_pci_hotplug.txt
> + *
> + * Copyright (c) 2013, Red Hat Inc, Michael S. Tsirkin (address@hidden)
> + * 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.
> + */
> +
> +#ifndef HW_ACPI_PCIHP_H
> +#define HW_ACPI_PCIHP_H
> +
> +#include <inttypes.h>
> +#include <qemu/typedefs.h>
> +#include "hw/pci/pci.h" /* for PCIHotplugState */
> +
> +typedef struct AcpiPciHpPciStatus {
> +    uint32_t up; /* deprecated, maintained for migration compatibility */
> +    uint32_t down;
> +    uint32_t hotplug_enable;
> +    uint32_t device_present;
> +} AcpiPciHpPciStatus;
> +
> +#define ACPI_PCIHP_PROP_BSEL "acpi-pcihp-bsel"
> +#define ACPI_PCIHP_MAX_HOTPLUG_BUS 256
> +
> +typedef struct AcpiPciHpState {
> +    AcpiPciHpPciStatus acpi_pcihp_pci_status[ACPI_PCIHP_MAX_HOTPLUG_BUS];
> +    uint32_t hotplug_select;
> +    PCIBus *root;
> +    MemoryRegion io;
> +} AcpiPciHpState;
> +
> +void acpi_pcihp_init(AcpiPciHpState *, PCIBus *root,
> +                     MemoryRegion *address_space_io);
> +
> +/* Invoke on device hotplug */
> +int acpi_pcihp_device_hotplug(AcpiPciHpState *, PCIDevice *,
> +                              PCIHotplugState state);
> +
> +/* Called on reset */
> +void acpi_pcihp_reset(AcpiPciHpState *s);
> +
> +extern const VMStateDescription vmstate_acpi_pcihp_pci_status;
> +
> +#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp) \
> +        VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
> +                            test_pcihp), \
> +        VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \
> +                                  ACPI_PCIHP_MAX_HOTPLUG_BUS, \
> +                                  test_pcihp, 1, \
> +                                  vmstate_acpi_pcihp_pci_status, \
> +                                  AcpiPciHpPciStatus)
> +
> +#endif
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> new file mode 100644
> index 0000000..13938e6
> --- /dev/null
> +++ b/hw/acpi/pcihp.c
> @@ -0,0 +1,312 @@
> +/*
> + * QEMU<->ACPI BIOS PCI hotplug interface
> + *
> + * QEMU supports PCI hotplug via ACPI. This module
> + * implements the interface between QEMU and the ACPI BIOS.
> + * Interface specification - see docs/specs/acpi_pci_hotplug.txt
> + *
> + * Copyright (c) 2013, Red Hat Inc, Michael S. Tsirkin (address@hidden)
> + * 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/acpi/pcihp.h"
> +
> +#include "hw/hw.h"
> +#include "hw/i386/pc.h"
> +#include "hw/pci/pci.h"
> +#include "hw/acpi/acpi.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/range.h"
> +#include "exec/ioport.h"
> +#include "exec/address-spaces.h"
> +#include "hw/pci/pci_bus.h"
> +#include "qom/qom-qobject.h"
> +#include "qapi/qmp/qint.h"
> +
> +//#define DEBUG
> +
> +#ifdef DEBUG
> +# define ACPI_PCIHP_DPRINTF(format, ...)     printf(format, ## __VA_ARGS__)
> +#else
> +# define ACPI_PCIHP_DPRINTF(format, ...)     do { } while (0)
> +#endif
> +
> +#define PCI_HOTPLUG_ADDR 0xae00
> +#define PCI_HOTPLUG_SIZE 0x0014
> +#define PCI_UP_BASE 0xae00
> +#define PCI_DOWN_BASE 0xae04
> +#define PCI_EJ_BASE 0xae08
> +#define PCI_RMV_BASE 0xae0c
> +#define PCI_SEL_BASE 0xae10
> +
> +typedef struct AcpiPciHpFind {
> +    int bsel;
> +    PCIBus *bus;
> +} AcpiPciHpFind;
> +
> +static int acpi_pcihp_get_bsel(PCIBus *bus)
> +{
> +    QObject *o = object_property_get_qobject(OBJECT(bus),
> +                                             ACPI_PCIHP_PROP_BSEL, NULL);
> +    int64_t bsel = -1;
> +    if (o) {
> +        bsel = qint_get_int(qobject_to_qint(o));
> +    }
> +    if (bsel < 0) {
> +        return -1;
> +    }
> +    return bsel;
> +}
> +
> +static void acpi_pcihp_test_hotplug_bus(PCIBus *bus, void *opaque)
> +{
> +    AcpiPciHpFind *find = opaque;
> +    if (find->bsel == acpi_pcihp_get_bsel(bus)) {
> +        find->bus = bus;
> +    }
> +}
> +
> +static PCIBus *acpi_pcihp_find_hotplug_bus(AcpiPciHpState *s, int bsel)
> +{
> +    AcpiPciHpFind find = { .bsel = bsel, .bus = NULL };
> +
> +    if (bsel < 0) {
> +        return NULL;
> +    }
> +
> +    pci_for_each_bus(s->root, acpi_pcihp_test_hotplug_bus, &find);
> +
> +    if (!bsel && !find.bus) {
> +        find.bus = s->root;
> +    }
> +    return find.bus;
> +}
> +
> +static bool acpi_pcihp_pc_no_hotplug(AcpiPciHpState *s, PCIDevice *dev)
> +{
> +    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> +    /*
> +     * ACPI doesn't allow hotplug of bridge devices.  Don't allow
> +     * hot-unplug of bridge devices unless they were added by hotplug
> +     * (and so, not described by acpi).
> +     */
> +    return (pc->is_bridge && !dev->qdev.hotplugged) || pc->no_hotplug;
> +}
> +
> +static void acpi_pcihp_eject_slot(AcpiPciHpState *s, unsigned bsel, unsigned 
> slots)
> +{
> +    BusChild *kid, *next;
> +    int slot = ffs(slots) - 1;
> +    bool slot_free = true;
> +    PCIBus *bus = acpi_pcihp_find_hotplug_bus(s, bsel);
> +
> +    if (!bus) {
> +        return;
> +    }
> +
> +    /* Mark request as complete */
> +    s->acpi_pcihp_pci_status[bsel].down &= ~(1U << slot);
> +
> +    QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {
> +        DeviceState *qdev = kid->child;
> +        PCIDevice *dev = PCI_DEVICE(qdev);
> +        if (PCI_SLOT(dev->devfn) == slot) {
> +            if (acpi_pcihp_pc_no_hotplug(s, dev)) {
> +                slot_free = false;
> +            } else {
> +                qdev_free(qdev);
> +            }
> +        }
> +    }
> +    if (slot_free) {
> +        s->acpi_pcihp_pci_status[bsel].device_present &= ~(1U << slot);
> +    }
> +}
> +
> +static void acpi_pcihp_update_hotplug_bus(AcpiPciHpState *s, int bsel)
> +{
> +    BusChild *kid, *next;
> +    PCIBus *bus = acpi_pcihp_find_hotplug_bus(s, bsel);
> +
> +    /* Execute any pending removes during reset */
> +    while (s->acpi_pcihp_pci_status[bsel].down) {
> +        acpi_pcihp_eject_slot(s, bsel, s->acpi_pcihp_pci_status[bsel].down);
> +    }
> +
> +    s->acpi_pcihp_pci_status[bsel].hotplug_enable = ~0;
> +    s->acpi_pcihp_pci_status[bsel].device_present = 0;
> +
> +    if (!bus) {
> +        return;
> +    }
> +    QTAILQ_FOREACH_SAFE(kid, &bus->qbus.children, sibling, next) {
> +        DeviceState *qdev = kid->child;
> +        PCIDevice *pdev = PCI_DEVICE(qdev);
> +        int slot = PCI_SLOT(pdev->devfn);
> +
> +        if (acpi_pcihp_pc_no_hotplug(s, pdev)) {
> +            s->acpi_pcihp_pci_status[bsel].hotplug_enable &= ~(1U << slot);
> +        }
> +
> +        s->acpi_pcihp_pci_status[bsel].device_present |= (1U << slot);
> +    }
> +}
> +
> +static void acpi_pcihp_update(AcpiPciHpState *s)
> +{
> +    int i;
> +
> +    for (i = 0; i < ACPI_PCIHP_MAX_HOTPLUG_BUS; ++i) {
> +        acpi_pcihp_update_hotplug_bus(s, i);
> +    }
> +}
> +
> +void acpi_pcihp_reset(AcpiPciHpState *s)
> +{
> +    acpi_pcihp_update(s);
> +}
> +
> +static void enable_device(AcpiPciHpState *s, unsigned bsel, int slot)
> +{
> +    s->acpi_pcihp_pci_status[bsel].device_present |= (1U << slot);
> +}
> +
> +static void disable_device(AcpiPciHpState *s, unsigned bsel, int slot)
> +{
> +    s->acpi_pcihp_pci_status[bsel].down |= (1U << slot);
> +}
> +
> +int acpi_pcihp_device_hotplug(AcpiPciHpState *s, PCIDevice *dev,
> +                              PCIHotplugState state)
> +{
> +    int slot = PCI_SLOT(dev->devfn);
> +    int bsel = acpi_pcihp_get_bsel(dev->bus);
> +    if (bsel < 0) {
> +        return -1;
> +    }
> +
> +    /* 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->acpi_pcihp_pci_status[bsel].device_present |= (1U << slot);
> +        return 0;
> +    }
> +
> +    if (state == PCI_HOTPLUG_ENABLED) {
> +        enable_device(s, bsel, slot);
> +    } else {
> +        disable_device(s, bsel, slot);
> +    }
> +
> +    return 0;
> +}
> +
> +static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    AcpiPciHpState *s = opaque;
> +    uint32_t val = 0;
> +    int bsel = s->hotplug_select;
> +
> +    if (bsel < 0 || bsel > ACPI_PCIHP_MAX_HOTPLUG_BUS) {
> +        return 0;
> +    }
> +
> +    switch (addr) {
> +    case PCI_UP_BASE - PCI_HOTPLUG_ADDR:
> +        /* Manufacture an "up" value to cause a device check on any hotplug
> +         * slot with a device.  Extra device checks are harmless. */
> +        val = s->acpi_pcihp_pci_status[bsel].device_present &
> +            s->acpi_pcihp_pci_status[bsel].hotplug_enable;
> +        ACPI_PCIHP_DPRINTF("pci_up_read %" PRIu32 "\n", val);
> +        break;
> +    case PCI_DOWN_BASE - PCI_HOTPLUG_ADDR:
> +        val = s->acpi_pcihp_pci_status[bsel].down;
> +        ACPI_PCIHP_DPRINTF("pci_down_read %" PRIu32 "\n", val);
> +        break;
> +    case PCI_EJ_BASE - PCI_HOTPLUG_ADDR:
> +        /* No feature defined yet */
> +        ACPI_PCIHP_DPRINTF("pci_features_read %" PRIu32 "\n", val);
> +        break;
> +    case PCI_RMV_BASE - PCI_HOTPLUG_ADDR:
> +        val = s->acpi_pcihp_pci_status[bsel].hotplug_enable;
> +        ACPI_PCIHP_DPRINTF("pci_rmv_read %" PRIu32 "\n", val);
> +        break;
> +    case PCI_SEL_BASE - PCI_HOTPLUG_ADDR:
> +        val = s->hotplug_select;
> +        ACPI_PCIHP_DPRINTF("pci_sel_read %" PRIu32 "\n", val);
> +    default:
> +        break;
> +    }
> +
> +    return val;
> +}
> +
> +static void pci_write(void *opaque, hwaddr addr, uint64_t data,
> +                      unsigned int size)
> +{
> +    AcpiPciHpState *s = opaque;
> +    switch (addr) {
> +    case PCI_EJ_BASE - PCI_HOTPLUG_ADDR:
> +        if (s->hotplug_select >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
> +            break;
> +        }
> +        acpi_pcihp_eject_slot(s, s->hotplug_select, data);
> +        ACPI_PCIHP_DPRINTF("pciej write %" HWADDR_PRIx " <== %" PRIu64 "\n",
> +                      addr, data);
> +        break;
> +    case PCI_SEL_BASE - PCI_HOTPLUG_ADDR:
> +        s->hotplug_select = data;
> +        ACPI_PCIHP_DPRINTF("pcisel write %" HWADDR_PRIx " <== %" PRIu64 "\n",
> +                      addr, data);
> +    default:
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps acpi_pcihp_io_ops = {
> +    .read = pci_read,
> +    .write = pci_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +
> +void acpi_pcihp_init(AcpiPciHpState *s, PCIBus *root_bus,
> +                     MemoryRegion *address_space_io)
> +{
> +    s->root= root_bus;
> +    memory_region_init_io(&s->io, NULL, &acpi_pcihp_io_ops, s,
> +                          "acpi-pci-hotplug",
> +                          PCI_HOTPLUG_SIZE);
> +    memory_region_add_subregion(address_space_io, PCI_HOTPLUG_ADDR, &s->io);
> +}
> +
> +const VMStateDescription vmstate_acpi_pcihp_pci_status = {
> +    .name = "acpi_pcihp_pci_status",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField []) {
> +        VMSTATE_UINT32(up, AcpiPciHpPciStatus),
> +        VMSTATE_UINT32(down, AcpiPciHpPciStatus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index a0b63b5..0a47a53 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -1,2 +1,2 @@
> -common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o
> +common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o
>  
> -- 
> MST