msix_init() is a supporting function in PCI device initialization, in order to convert .init() to .realize(), it should be modified first. Also modify the callers.
Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com> --- hw/block/nvme.c | 32 +++++++++++++++++-------- hw/misc/ivshmem.c | 7 +++--- hw/net/rocker/rocker.c | 10 +++++--- hw/net/vmxnet3.c | 26 ++++++++++++++------ hw/pci/msix.c | 20 ++++++++++++---- hw/scsi/megasas.c | 20 ++++++++++++---- hw/usb/hcd-xhci.c | 7 ++++-- hw/vfio/pci.c | 8 +++---- hw/virtio/virtio-bus.c | 3 +++ hw/virtio/virtio-pci.c | 64 +++++++++++++++++++++++++------------------------- include/hw/pci/msix.h | 5 ++-- 11 files changed, 129 insertions(+), 73 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 169e4fa..21b6479 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -778,14 +778,27 @@ static const MemoryRegionOps nvme_mmio_ops = { }, }; +static void nvme_exit(PCIDevice *pci_dev) +{ + NvmeCtrl *n = NVME(pci_dev); + + nvme_clear_ctrl(n); + g_free(n->namespaces); + g_free(n->cq); + g_free(n->sq); + msix_uninit_exclusive_bar(pci_dev); +} + static int nvme_init(PCIDevice *pci_dev) { NvmeCtrl *n = NVME(pci_dev); NvmeIdCtrl *id = &n->id_ctrl; + Error *local_err = NULL; int i; int64_t bs_size; uint8_t *pci_conf; + int ret; if (!n->conf.blk) { return -1; @@ -822,7 +835,11 @@ static int nvme_init(PCIDevice *pci_dev) pci_register_bar(&n->parent_obj, 0, PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64, &n->iomem); - msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4); + ret = msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, &local_err); + if (ret) { + error_report_err(local_err); + goto cleanup_on_msix_fail; + } id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID)); @@ -872,17 +889,12 @@ static int nvme_init(PCIDevice *pci_dev) id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds); } return 0; -} -static void nvme_exit(PCIDevice *pci_dev) -{ - NvmeCtrl *n = NVME(pci_dev); +cleanup_on_msix_fail: + object_unref(OBJECT(&n->iomem)); + nvme_exit(pci_dev); - nvme_clear_ctrl(n); - g_free(n->namespaces); - g_free(n->cq); - g_free(n->sq); - msix_uninit_exclusive_bar(pci_dev); + return ret; } static Property nvme_props[] = { diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index f73f0c2..cfb210f 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -771,9 +771,9 @@ static void ivshmem_reset(DeviceState *d) ivshmem_use_msix(s); } -static int ivshmem_setup_msi(IVShmemState * s) +static int ivshmem_setup_msi(IVShmemState *s, Error **errp) { - if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) { + if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, errp)) { return -1; } @@ -950,8 +950,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Error **errp) s->server_chr->filename); if (ivshmem_has_feature(s, IVSHMEM_MSI) && - ivshmem_setup_msi(s)) { - error_setg(errp, "msix initialization failed"); + ivshmem_setup_msi(s, errp)) { return; } diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c index c57f1a6..64e4c2a 100644 --- a/hw/net/rocker/rocker.c +++ b/hw/net/rocker/rocker.c @@ -1244,7 +1244,7 @@ rollback: return err; } -static int rocker_msix_init(Rocker *r) +static int rocker_msix_init(Rocker *r, Error **errp) { PCIDevice *dev = PCI_DEVICE(r); int err; @@ -1254,7 +1254,7 @@ static int rocker_msix_init(Rocker *r) ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_TABLE_OFFSET, &r->msix_bar, ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET, - 0); + 0, errp); if (err) { return err; } @@ -1286,6 +1286,7 @@ static int pci_rocker_init(PCIDevice *dev) const MACAddr dflt = { .a = { 0x52, 0x54, 0x00, 0x12, 0x35, 0x01 } }; static int sw_index; int i, err = 0; + Error *local_err = NULL; /* allocate worlds */ @@ -1314,8 +1315,9 @@ static int pci_rocker_init(PCIDevice *dev) /* MSI-X init */ - err = rocker_msix_init(r); + err = rocker_msix_init(r, &local_err); if (err) { + error_report_err(local_err); goto err_msix_init; } @@ -1431,6 +1433,8 @@ err_duplicate: err_msix_init: object_unparent(OBJECT(&r->msix_bar)); object_unparent(OBJECT(&r->mmio)); + object_unref(OBJECT(&r->msix_bar)); + object_unref(OBJECT(&r->mmio)); err_world_alloc: for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) { if (r->worlds[i]) { diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index c373e77..b40d70c 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2078,37 +2078,42 @@ vmxnet3_unuse_msix_vectors(VMXNET3State *s, int num_vectors) } static bool -vmxnet3_use_msix_vectors(VMXNET3State *s, int num_vectors) +vmxnet3_use_msix_vectors(VMXNET3State *s, int num_vectors, Error **errp) { PCIDevice *d = PCI_DEVICE(s); int i; + for (i = 0; i < num_vectors; i++) { int res = msix_vector_use(d, i); if (0 > res) { VMW_WRPRN("Failed to use MSI-X vector %d, error %d", i, res); + error_setg(errp, "Failed to use MSI-X vector %d, error %d", + i, res); vmxnet3_unuse_msix_vectors(s, i); return false; } } + return true; } static bool -vmxnet3_init_msix(VMXNET3State *s) +vmxnet3_init_msix(VMXNET3State *s, Error **errp) { PCIDevice *d = PCI_DEVICE(s); + int res = msix_init(d, VMXNET3_MAX_INTRS, &s->msix_bar, VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE, &s->msix_bar, VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA, - 0); + 0, errp); if (0 > res) { VMW_WRPRN("Failed to initialize MSI-X, error %d", res); s->msix_used = false; } else { - if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) { + if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS, errp)) { VMW_WRPRN("Failed to use MSI-X vectors, error %d", res); msix_uninit(d, &s->msix_bar, &s->msix_bar); s->msix_used = false; @@ -2223,20 +2228,25 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) VMXNET3_MSIX_BAR_SIZE); pci_register_bar(pci_dev, VMXNET3_MSIX_BAR_IDX, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->msix_bar); - vmxnet3_reset_interrupt_states(s); /* Interrupt pin A */ pci_dev->config[PCI_INTERRUPT_PIN] = 0x01; - if (!vmxnet3_init_msix(s)) { + if (!vmxnet3_init_msix(s, errp)) { VMW_WRPRN("Failed to initialize MSI-X, configuration is inconsistent."); + goto cleanup_on_msix_fail; } vmxnet3_net_init(s); register_savevm(dev, "vmxnet3-msix", -1, 1, vmxnet3_msix_save, vmxnet3_msix_load, s); + +cleanup_on_msix_fail: + object_unref(OBJECT(&s->bar0)); + object_unref(OBJECT(&s->bar1)); + object_unref(OBJECT(&s->msix_bar)); } static void vmxnet3_instance_init(Object *obj) @@ -2453,13 +2463,15 @@ static int vmxnet3_post_load(void *opaque, int version_id) { VMXNET3State *s = opaque; PCIDevice *d = PCI_DEVICE(s); + Error *local_err = NULL; vmxnet_tx_pkt_init(&s->tx_pkt, s->max_tx_frags, s->peer_has_vhdr); vmxnet_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); if (s->msix_used) { - if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) { + if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS, &local_err)) { VMW_WRPRN("Failed to re-use MSI-X vectors"); + error_report_err(local_err); msix_uninit(d, &s->msix_bar, &s->msix_bar); s->msix_used = false; return -1; diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 64c93d8..44abd58 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -233,7 +233,8 @@ static void msix_mask_all(struct PCIDevice *dev, unsigned nentries) int msix_init(struct PCIDevice *dev, unsigned short nentries, MemoryRegion *table_bar, uint8_t table_bar_nr, unsigned table_offset, MemoryRegion *pba_bar, - uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos) + uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos, + Error **errp) { int cap; unsigned table_size, pba_size; @@ -241,10 +242,13 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, /* Nothing to do if MSI is not supported by interrupt controller */ if (!msi_supported) { + error_setg(errp, "MSI-X is not supported by interrupt controller"); return -ENOTSUP; } if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1) { + error_setg(errp, "Vector number %d invalid, should be 1 ~ %d", + nentries, PCI_MSIX_FLAGS_QSIZE + 1); return -EINVAL; } @@ -257,10 +261,12 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, table_offset + table_size > memory_region_size(table_bar) || pba_offset + pba_size > memory_region_size(pba_bar) || (table_offset | pba_offset) & PCI_MSIX_FLAGS_BIRMASK) { + error_setg(errp, "Arguments invalid. Please check them carefully"); return -EINVAL; } - cap = pci_add_capability(dev, PCI_CAP_ID_MSIX, cap_pos, MSIX_CAP_LENGTH); + cap = pci_add_capability2(dev, PCI_CAP_ID_MSIX, cap_pos, + MSIX_CAP_LENGTH, errp); if (cap < 0) { return cap; } @@ -297,7 +303,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, } int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, - uint8_t bar_nr) + uint8_t bar_nr, Error **errp) { int ret; char *name; @@ -329,15 +335,19 @@ int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, bar_nr, 0, &dev->msix_exclusive_bar, bar_nr, bar_pba_offset, - 0); + 0, errp); if (ret) { - return ret; + goto cleanup_on_fail; } pci_register_bar(dev, bar_nr, PCI_BASE_ADDRESS_SPACE_MEMORY, &dev->msix_exclusive_bar); return 0; + +cleanup_on_fail: + object_unref(OBJECT(&dev->msix_exclusive_bar)); + return ret; } static void msix_free_irq_entries(PCIDevice *dev) diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index ba25b3a..465fadd 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -2357,11 +2357,18 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp) memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s, "megasas-queue", 0x40000); - if (megasas_use_msix(s) && - msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000, - &s->mmio_io, b->mmio_bar, 0x3800, 0x68)) { - s->flags &= ~MEGASAS_MASK_USE_MSIX; + if (megasas_use_msix(s)) { + int ret; + + ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000, + &s->mmio_io, b->mmio_bar, 0x3800, 0x68, errp); + if (ret > 0) { + s->flags &= ~MEGASAS_MASK_USE_MSIX; + } else { + goto cleanup_on_msix_fail; + } } + if (pci_is_express(dev)) { pcie_endpoint_cap_init(dev, 0xa0); } @@ -2419,6 +2426,11 @@ static void megasas_scsi_realize(PCIDevice *dev, Error **errp) if (!d->hotplugged) { scsi_bus_legacy_handle_cmdline(&s->bus, errp); } + +cleanup_on_msix_fail: + object_unref(OBJECT(&s->mmio_io)); + object_unref(OBJECT(&s->port_io)); + object_unref(OBJECT(&s->queue_io)); } static Property megasas_properties_gen1[] = { diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 7cd5f6c..c7d2ea7 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -3686,10 +3686,13 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp) } } if (xhci_get_flag(xhci, XHCI_FLAG_USE_MSI_X)) { - msix_init(dev, xhci->numintrs, + ret = msix_init(dev, xhci->numintrs, &xhci->mem, 0, OFF_MSIX_TABLE, &xhci->mem, 0, OFF_MSIX_PBA, - 0x90); + 0x90, errp); + if (ret > 0) { + return; + } } cleanup_on_msi_fail: diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 633642e..fe88d57 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1247,7 +1247,7 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev) return 0; } -static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos) +static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp) { int ret; @@ -1255,12 +1255,12 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos) &vdev->bars[vdev->msix->table_bar].region.mem, vdev->msix->table_bar, vdev->msix->table_offset, &vdev->bars[vdev->msix->pba_bar].region.mem, - vdev->msix->pba_bar, vdev->msix->pba_offset, pos); + vdev->msix->pba_bar, vdev->msix->pba_offset, pos, + errp); if (ret < 0) { if (ret == -ENOTSUP) { return 0; } - error_report("vfio: msix_init failed"); return ret; } @@ -1702,7 +1702,7 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp) ret = vfio_setup_pcie_cap(vdev, pos, size); break; case PCI_CAP_ID_MSIX: - ret = vfio_msix_setup(vdev, pos); + ret = vfio_msix_setup(vdev, pos, errp); break; case PCI_CAP_ID_PM: vfio_check_pm_reset(vdev, pos); diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index 81c7cdd..d36edac 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -50,6 +50,9 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error **errp) if (klass->device_plugged != NULL) { klass->device_plugged(qbus->parent, errp); + if (*errp) { + return; + } } /* Get the features of the plugged device. */ diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 94667e6..f50f77f 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1619,6 +1619,25 @@ static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy, ®ion->mr); } +static void virtio_pci_device_unplugged(DeviceState *d) +{ + VirtIOPCIProxy *proxy = VIRTIO_PCI(d); + bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); + bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; + + virtio_pci_stop_ioeventfd(proxy); + + if (modern) { + virtio_pci_modern_mem_region_unmap(proxy, &proxy->common); + virtio_pci_modern_mem_region_unmap(proxy, &proxy->isr); + virtio_pci_modern_mem_region_unmap(proxy, &proxy->device); + virtio_pci_modern_mem_region_unmap(proxy, &proxy->notify); + if (modern_pio) { + virtio_pci_modern_io_region_unmap(proxy, &proxy->notify_pio); + } + } +} + /* This is called by virtio-bus just after the device is plugged. */ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) { @@ -1651,6 +1670,19 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) } config[PCI_INTERRUPT_PIN] = 1; + if (proxy->nvectors) { + int err = msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, + proxy->msix_bar, errp); + if (err) { + /* Notice when a system that supports MSIx can't initialize it. */ + if (err != -ENOTSUP) { + error_report("unable to init msix vectors to %" PRIu32, + proxy->nvectors); + } + proxy->nvectors = 0; + return; + } + } if (modern) { struct virtio_pci_cap cap = { @@ -1705,19 +1737,6 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) pci_set_long(cfg_mask->pci_cfg_data, ~0x0); } - if (proxy->nvectors) { - int err = msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, - proxy->msix_bar); - if (err) { - /* Notice when a system that supports MSIx can't initialize it. */ - if (err != -ENOTSUP) { - error_report("unable to init msix vectors to %" PRIu32, - proxy->nvectors); - } - proxy->nvectors = 0; - } - } - proxy->pci_dev.config_write = virtio_write_config; proxy->pci_dev.config_read = virtio_read_config; @@ -1741,25 +1760,6 @@ static void virtio_pci_device_plugged(DeviceState *d, Error **errp) virtio_add_feature(&vdev->host_features, VIRTIO_F_BAD_FEATURE); } -static void virtio_pci_device_unplugged(DeviceState *d) -{ - VirtIOPCIProxy *proxy = VIRTIO_PCI(d); - bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); - bool modern_pio = proxy->flags & VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY; - - virtio_pci_stop_ioeventfd(proxy); - - if (modern) { - virtio_pci_modern_mem_region_unmap(proxy, &proxy->common); - virtio_pci_modern_mem_region_unmap(proxy, &proxy->isr); - virtio_pci_modern_mem_region_unmap(proxy, &proxy->device); - virtio_pci_modern_mem_region_unmap(proxy, &proxy->notify); - if (modern_pio) { - virtio_pci_modern_io_region_unmap(proxy, &proxy->notify_pio); - } - } -} - static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) { VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h index 72e5f93..7f2b8cc 100644 --- a/include/hw/pci/msix.h +++ b/include/hw/pci/msix.h @@ -9,9 +9,10 @@ MSIMessage msix_get_message(PCIDevice *dev, unsigned int vector); int msix_init(PCIDevice *dev, unsigned short nentries, MemoryRegion *table_bar, uint8_t table_bar_nr, unsigned table_offset, MemoryRegion *pba_bar, - uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos); + uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos, + Error **errp); int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries, - uint8_t bar_nr); + uint8_t bar_nr, Error **errp); void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len); -- 2.1.0