[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-dev
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface. |
Date: |
Mon, 09 Dec 2013 11:09:02 +0200 |
On Fri, 2013-12-06 at 18:03 +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> hw/pci/pci.c | 70 +++++++++++++++++++++++++++++++++------------
> include/hw/pci/pci.h | 10 ------
> include/hw/pci/pci_bus.h | 2 -
> 3 files changed, 51 insertions(+), 31 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 49eca95..26229de 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -35,6 +35,7 @@
> #include "hw/pci/msi.h"
> #include "hw/pci/msix.h"
> #include "exec/address-spaces.h"
> +#include "hw/hotplug.h"
>
> //#define DEBUG_PCI
> #ifdef DEBUG_PCI
> @@ -348,13 +349,6 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
> pci_map_irq_fn map_irq,
> bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> }
>
> -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *qdev)
> -{
> - bus->qbus.allow_hotplug = 1;
> - bus->hotplug = hotplug;
> - bus->hotplug_qdev = qdev;
> -}
> -
> PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> @@ -1720,6 +1714,8 @@ static int pci_qdev_init(DeviceState *qdev)
> {
> PCIDevice *pci_dev = (PCIDevice *)qdev;
> PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
> + DeviceState *hotplug_dev;
> + Error *local_err = NULL;
> PCIBus *bus;
> int rc;
> bool is_default_rom;
> @@ -1756,32 +1752,68 @@ static int pci_qdev_init(DeviceState *qdev)
> }
> pci_add_option_rom(pci_dev, is_default_rom);
>
> - if (bus->hotplug) {
> - /* Let buses differentiate between hotplug and when device is
> - * enabled during qemu machine creation. */
> - rc = bus->hotplug(bus->hotplug_qdev, pci_dev,
> - qdev->hotplugged ? PCI_HOTPLUG_ENABLED:
> - PCI_COLDPLUG_ENABLED);
> - if (rc != 0) {
> - int r = pci_unregister_device(&pci_dev->qdev);
> - assert(!r);
> - return rc;
> + hotplug_dev = DEVICE(object_property_get_link(OBJECT(bus),
> "hotplug-device",
> + &local_err));
> + if (error_is_set(&local_err)) {
> + goto error_exit;
> + }
> + if (hotplug_dev) {
It seems that object_property_get_link never returns NULL
if no error, meaning that if you go for this link
you *must* be sure that the link exists.
If the above is the case, I think you can loose the "if" statement.
Otherwise, if NULL is an option, you would abort the device init unnecessary.
> + HotplugDeviceClass *hdc = HOTPLUG_DEVICE_GET_CLASS(hotplug_dev);
> +
> + /* handler can differentiate between hotplug and when device is
> + * enabled during qemu machine creation by inspecting
> + * dev->hotplugged field. */
> + if (hdc->hotplug) {
> + hdc->hotplug(hotplug_dev, qdev, &local_err);
> + if (error_is_set(&local_err)) {
> + int r = pci_unregister_device(&pci_dev->qdev);
> + assert(!r);
> + goto error_exit;
> + }
> }
> }
> return 0;
> +
> +error_exit:
> + qerror_report_err(local_err);
> + error_free(local_err);
> + return -1;
> }
>
> static int pci_unplug_device(DeviceState *qdev)
> {
> PCIDevice *dev = PCI_DEVICE(qdev);
> PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> + BusState *bus = qdev_get_parent_bus(qdev);
> + DeviceState *hotplug_dev;
> + Error *local_err = NULL;
>
> if (pc->no_hotplug) {
> qerror_report(QERR_DEVICE_NO_HOTPLUG,
> object_get_typename(OBJECT(dev)));
> return -1;
> }
> - return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
> - PCI_HOTPLUG_DISABLED);
> +
> + hotplug_dev = DEVICE(object_property_get_link(OBJECT(bus),
> "hotplug-device",
> + &local_err));
> + if (error_is_set(&local_err)) {
> + goto error_exit;
> + }
> + if (hotplug_dev)
Same as above,
I hope it helped,
Thanks,
Marcel
> + HotplugDeviceClass *hdc = HOTPLUG_DEVICE_GET_CLASS(hotplug_dev);
> +
> + if (hdc->hot_unplug) {
> + hdc->hot_unplug(hotplug_dev, qdev, &local_err);
> + if (error_is_set(&local_err)) {
> + goto error_exit;
> + }
> + }
> + }
> + return 0;
> +
> +error_exit:
> + qerror_report_err(local_err);
> + error_free(local_err);
> + return -1;
> }
>
> PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool
> multifunction,
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index b783e68..33dec40 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -330,15 +330,6 @@ typedef void (*pci_set_irq_fn)(void *opaque, int
> irq_num, int level);
> typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
>
> -typedef enum {
> - PCI_HOTPLUG_DISABLED,
> - PCI_HOTPLUG_ENABLED,
> - PCI_COLDPLUG_ENABLED,
> -} PCIHotplugState;
> -
> -typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev,
> - PCIHotplugState state);
> -
> #define TYPE_PCI_BUS "PCI"
> #define PCI_BUS(obj) OBJECT_CHECK(PCIBus, (obj), TYPE_PCI_BUS)
> #define TYPE_PCIE_BUS "PCIE"
> @@ -357,7 +348,6 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn
> map_irq,
> void *irq_opaque, int nirq);
> int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
> /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 9df1788..fabaeee 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -16,8 +16,6 @@ struct PCIBus {
> pci_set_irq_fn set_irq;
> pci_map_irq_fn map_irq;
> pci_route_irq_fn route_intx_to_irq;
> - pci_hotplug_fn hotplug;
> - DeviceState *hotplug_qdev;
> void *irq_opaque;
> PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
> PCIDevice *parent_dev;
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., (continued)
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Igor Mammedov, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Paolo Bonzini, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Igor Mammedov, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Paolo Bonzini, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Igor Mammedov, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Paolo Bonzini, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Igor Mammedov, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Paolo Bonzini, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Igor Mammedov, 2013/12/09
- Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface., Paolo Bonzini, 2013/12/09
Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface.,
Marcel Apfelbaum <=
[Qemu-devel] [PATCH 6/7] pci/pcie: convert PCIE hotplug to use hotplug-device interface, Igor Mammedov, 2013/12/06
Re: [Qemu-devel] [PATCH 0/7] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API, Paolo Bonzini, 2013/12/06
[Qemu-devel] [PATCH 2/7] qdev: add to BusState "hotplug-device" link, Igor Mammedov, 2013/12/06
[Qemu-devel] [PATCH 1/7] define hotplug interface, Igor Mammedov, 2013/12/06