From 133e9b228df16d11de01529c217417e78d1d9370 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 19 Jan 2015 15:52:28 +0100 Subject: pci: Convert core to realize Implement DeviceClass methods realize() and unrealize() instead of init() and exit(). The core's initialization errors now get propagated properly, and QMP sends them instead of an unspecific "Device initialization failed" error. Unrealize can't fail, so no change there. PCIDeviceClass is unchanged: it still provides init() and exit(). Therefore, device models' errors are still not propagated. Signed-off-by: Markus Armbruster Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Gonglei --- hw/pci/pci.c | 93 ++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 49 insertions(+), 44 deletions(-) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 31b222d8c0..fe697b1544 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -115,7 +115,7 @@ static const TypeInfo pcie_bus_info = { static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num); static void pci_update_mappings(PCIDevice *d); static void pci_irq_handler(void *opaque, int irq_num, int level); -static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom); +static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **); static void pci_del_option_rom(PCIDevice *pdev); static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET; @@ -726,7 +726,7 @@ static void pci_init_mask_bridge(PCIDevice *d) PCI_PREF_RANGE_TYPE_MASK); } -static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev) +static void pci_init_multifunction(PCIBus *bus, PCIDevice *dev, Error **errp) { uint8_t slot = PCI_SLOT(dev->devfn); uint8_t func; @@ -752,26 +752,25 @@ static int pci_init_multifunction(PCIBus *bus, PCIDevice *dev) PCIDevice *f0 = bus->devices[PCI_DEVFN(slot, 0)]; if (f0 && !(f0->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)) { /* function 0 should set multifunction bit */ - error_report("PCI: single function device can't be populated " - "in function %x.%x", slot, PCI_FUNC(dev->devfn)); - return -1; + error_setg(errp, "PCI: single function device can't be populated " + "in function %x.%x", slot, PCI_FUNC(dev->devfn)); + return; } - return 0; + return; } if (dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) { - return 0; + return; } /* function 0 indicates single function, so function > 0 must be NULL */ for (func = 1; func < PCI_FUNC_MAX; ++func) { if (bus->devices[PCI_DEVFN(slot, func)]) { - error_report("PCI: %x.0 indicates single function, " - "but %x.%x is already populated.", - slot, slot, func); - return -1; + error_setg(errp, "PCI: %x.0 indicates single function, " + "but %x.%x is already populated.", + slot, slot, func); + return; } } - return 0; } static void pci_config_alloc(PCIDevice *pci_dev) @@ -804,11 +803,13 @@ static void do_pci_unregister_device(PCIDevice *pci_dev) /* -1 for devfn means auto assign */ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, - const char *name, int devfn) + const char *name, int devfn, + Error **errp) { PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); PCIConfigReadFunc *config_read = pc->config_read; PCIConfigWriteFunc *config_write = pc->config_write; + Error *local_err = NULL; AddressSpace *dma_as; if (devfn < 0) { @@ -817,12 +818,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, if (!bus->devices[devfn]) goto found; } - error_report("PCI: no slot/function available for %s, all in use", name); + error_setg(errp, "PCI: no slot/function available for %s, all in use", + name); return NULL; found: ; } else if (bus->devices[devfn]) { - error_report("PCI: slot %d function %d not available for %s, in use by %s", - PCI_SLOT(devfn), PCI_FUNC(devfn), name, bus->devices[devfn]->name); + error_setg(errp, "PCI: slot %d function %d not available for %s," + " in use by %s", + PCI_SLOT(devfn), PCI_FUNC(devfn), name, + bus->devices[devfn]->name); return NULL; } @@ -866,7 +870,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, if (pc->is_bridge) { pci_init_mask_bridge(pci_dev); } - if (pci_init_multifunction(bus, pci_dev)) { + pci_init_multifunction(bus, pci_dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); do_pci_unregister_device(pci_dev); return NULL; } @@ -897,7 +903,7 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev) pci_unregister_vga(pci_dev); } -static int pci_unregister_device(DeviceState *dev) +static void pci_qdev_unrealize(DeviceState *dev, Error **errp) { PCIDevice *pci_dev = PCI_DEVICE(dev); PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); @@ -910,7 +916,6 @@ static int pci_unregister_device(DeviceState *dev) } do_pci_unregister_device(pci_dev); - return 0; } void pci_register_bar(PCIDevice *pci_dev, int region_num, @@ -1751,10 +1756,11 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn) return bus->devices[devfn]; } -static int pci_qdev_init(DeviceState *qdev) +static void pci_qdev_realize(DeviceState *qdev, Error **errp) { PCIDevice *pci_dev = (PCIDevice *)qdev; PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); + Error *local_err = NULL; PCIBus *bus; int rc; bool is_default_rom; @@ -1767,15 +1773,16 @@ static int pci_qdev_init(DeviceState *qdev) bus = PCI_BUS(qdev_get_parent_bus(qdev)); pci_dev = do_pci_register_device(pci_dev, bus, object_get_typename(OBJECT(qdev)), - pci_dev->devfn); + pci_dev->devfn, errp); if (pci_dev == NULL) - return -1; + return; if (pc->init) { rc = pc->init(pci_dev); if (rc != 0) { do_pci_unregister_device(pci_dev); - return rc; + error_setg(errp, "Device initialization failed"); + return; } } @@ -1786,13 +1793,12 @@ static int pci_qdev_init(DeviceState *qdev) is_default_rom = true; } - rc = pci_add_option_rom(pci_dev, is_default_rom); - if (rc != 0) { - pci_unregister_device(DEVICE(pci_dev)); - return rc; + pci_add_option_rom(pci_dev, is_default_rom, &local_err); + if (local_err) { + error_propagate(errp, local_err); + pci_qdev_unrealize(DEVICE(pci_dev), NULL); + return; } - - return 0; } PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction, @@ -1932,7 +1938,8 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size) } /* Add an option rom for the device */ -static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) +static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, + Error **errp) { int size; char *path; @@ -1941,9 +1948,9 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) const VMStateDescription *vmsd; if (!pdev->romfile) - return 0; + return; if (strlen(pdev->romfile) == 0) - return 0; + return; if (!pdev->rom_bar) { /* @@ -1957,7 +1964,9 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) * if the rom bar is disabled. */ if (DEVICE(pdev)->hotplugged) { - return -1; + error_setg(errp, "Hot-plugged device without ROM bar" + " can't have an option ROM"); + return; } if (class == 0x0300) { @@ -1965,7 +1974,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) } else { rom_add_option(pdev->romfile, -1); } - return 0; + return; } path = qemu_find_file(QEMU_FILE_TYPE_BIOS, pdev->romfile); @@ -1975,15 +1984,13 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) size = get_image_size(path); if (size < 0) { - error_report("%s: failed to find romfile \"%s\"", - __func__, pdev->romfile); + error_setg(errp, "failed to find romfile \"%s\"", pdev->romfile); g_free(path); - return -1; + return; } else if (size == 0) { - error_report("%s: ignoring empty romfile \"%s\"", - __func__, pdev->romfile); + error_setg(errp, "romfile \"%s\" is empty", pdev->romfile); g_free(path); - return -1; + return; } if (size & (size - 1)) { size = 1 << qemu_fls(size); @@ -2009,8 +2016,6 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom) } pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom); - - return 0; } static void pci_del_option_rom(PCIDevice *pdev) @@ -2291,8 +2296,8 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev) static void pci_device_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); - k->init = pci_qdev_init; - k->exit = pci_unregister_device; + k->realize = pci_qdev_realize; + k->unrealize = pci_qdev_unrealize; k->bus_type = TYPE_PCI_BUS; k->props = pci_props; } -- cgit v1.2.3-55-g7522 From 7ee6c1e182cca6ccf5253569fca3d05826efb4e9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 19 Jan 2015 15:52:29 +0100 Subject: pci: Permit incremental conversion of device models to realize Call the new PCIDeviceClass method realize(). Default it to pci_default_realize(), which calls old method init(). To convert a device model, make it implement realize() rather than init(). Signed-off-by: Markus Armbruster Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Gonglei --- hw/pci/pci.c | 24 +++++++++++++++++++----- include/hw/pci/pci.h | 3 ++- 2 files changed, 21 insertions(+), 6 deletions(-) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index fe697b1544..b1f3cea9af 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1762,7 +1762,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); Error *local_err = NULL; PCIBus *bus; - int rc; bool is_default_rom; /* initialize cap_present for pci_is_express() and pci_config_size() */ @@ -1777,11 +1776,11 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) if (pci_dev == NULL) return; - if (pc->init) { - rc = pc->init(pci_dev); - if (rc != 0) { + if (pc->realize) { + pc->realize(pci_dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); do_pci_unregister_device(pci_dev); - error_setg(errp, "Device initialization failed"); return; } } @@ -1801,6 +1800,18 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp) } } +static void pci_default_realize(PCIDevice *dev, Error **errp) +{ + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); + + if (pc->init) { + if (pc->init(dev) < 0) { + error_setg(errp, "Device initialization failed"); + return; + } + } +} + PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction, const char *name) { @@ -2296,10 +2307,13 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev) static void pci_device_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); + PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass); + k->realize = pci_qdev_realize; k->unrealize = pci_qdev_unrealize; k->bus_type = TYPE_PCI_BUS; k->props = pci_props; + pc->realize = pci_default_realize; } AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index bdee464e61..3164fc3a4f 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -185,7 +185,8 @@ typedef struct PCIINTxRoute { typedef struct PCIDeviceClass { DeviceClass parent_class; - int (*init)(PCIDevice *dev); + void (*realize)(PCIDevice *dev, Error **errp); + int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */ PCIUnregisterFunc *exit; PCIConfigReadFunc *config_read; PCIConfigWriteFunc *config_write; -- cgit v1.2.3-55-g7522 From 6dbcb81956b16d794c9c0257b94bd4c6feba713f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 26 Feb 2015 17:21:14 +0100 Subject: pci: Give a few helpers internal linkage None of them should be used in new code. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pci.c | 14 +++++++------- include/hw/pci/pci.h | 7 ------- 2 files changed, 7 insertions(+), 14 deletions(-) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index b1f3cea9af..cc5d946b8f 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -539,8 +539,8 @@ static void pci_set_default_subsystem_id(PCIDevice *pci_dev) * Parse [[:]:], return -1 on error if funcp == NULL * [[:]:]., return -1 on error */ -int pci_parse_devaddr(const char *addr, int *domp, int *busp, - unsigned int *slotp, unsigned int *funcp) +static int pci_parse_devaddr(const char *addr, int *domp, int *busp, + unsigned int *slotp, unsigned int *funcp) { const char *p; char *e; @@ -598,7 +598,8 @@ int pci_parse_devaddr(const char *addr, int *domp, int *busp, return 0; } -PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr) +static PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, + const char *devaddr) { int dom, bus; unsigned slot; @@ -1610,10 +1611,9 @@ static const char * const pci_nic_names[] = { }; /* Initialize a PCI NIC. */ -/* FIXME callers should check for failure, but don't */ -PCIDevice *pci_nic_init(NICInfo *nd, PCIBus *rootbus, - const char *default_model, - const char *default_devaddr) +static PCIDevice *pci_nic_init(NICInfo *nd, PCIBus *rootbus, + const char *default_model, + const char *default_devaddr) { const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr; PCIBus *bus; diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 3164fc3a4f..be2d9b8703 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -371,9 +371,6 @@ void pci_device_set_intx_routing_notifier(PCIDevice *dev, PCIINTxRoutingNotifier notifier); void pci_device_reset(PCIDevice *dev); -PCIDevice *pci_nic_init(NICInfo *nd, PCIBus *rootbus, - const char *default_model, - const char *default_devaddr); PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, const char *default_model, const char *default_devaddr); @@ -403,12 +400,8 @@ PCIBus *pci_device_root_bus(const PCIDevice *d); const char *pci_root_bus_path(PCIDevice *dev); PCIDevice *pci_find_device(PCIBus *bus, int bus_num, uint8_t devfn); int pci_qdev_find_device(const char *id, PCIDevice **pdev); -PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, const char *devaddr); void pci_bus_get_w64_range(PCIBus *bus, Range *range); -int pci_parse_devaddr(const char *addr, int *domp, int *busp, - unsigned int *slotp, unsigned int *funcp); - void pci_device_deassert_intx(PCIDevice *dev); typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int); -- cgit v1.2.3-55-g7522