From 1553d4f1fc6b7455234de93d105e7b46bc7a0844 Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 5 Aug 2011 18:22:03 +0900 Subject: [PATCH 1/4] pcie/slot: fix hotplug event When slot status register is cleared, PCIDevice::exp.hpev_notify needs to be cleared. Otherwise, PCIDevice::exp.hpev_notify is never set to false resulting in no more hot plug event once it's raised. Signed-off-by: Isaku Yamahata Signed-off-by: Michael S. Tsirkin --- hw/pcie.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/pcie.c b/hw/pcie.c index 39607bf31a..5c9eb2f0ac 100644 --- a/hw/pcie.c +++ b/hw/pcie.c @@ -175,6 +175,14 @@ static void hotplug_event_notify(PCIDevice *dev) } } +static void hotplug_event_clear(PCIDevice *dev) +{ + hotplug_event_update_event_status(dev); + if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) { + qemu_set_irq(dev->irq[dev->exp.hpev_intx], 0); + } +} + /* * A PCI Express Hot-Plug Event has occurred, so update slot status register * and notify OS of the event if necessary. @@ -320,6 +328,10 @@ void pcie_cap_slot_write_config(PCIDevice *dev, uint8_t *exp_cap = dev->config + pos; uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA); + if (ranges_overlap(addr, len, pos + PCI_EXP_SLTSTA, 2)) { + hotplug_event_clear(dev); + } + if (!ranges_overlap(addr, len, pos + PCI_EXP_SLTCTL, 2)) { return; } From 74d63b65479f4d8a72c1bba54256eee029cd5d5f Mon Sep 17 00:00:00 2001 From: Isaku Yamahata Date: Fri, 5 Aug 2011 18:22:06 +0900 Subject: [PATCH 2/4] pcie/aer: fix inject aer error command various fixes to make aer inject error command work. - wrong assert - command line parser - err.status needs initialization Signed-off-by: Isaku Yamahata Signed-off-by: Michael S. Tsirkin --- hw/pcie_aer.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c index 2ae65ec807..62c06eafd6 100644 --- a/hw/pcie_aer.c +++ b/hw/pcie_aer.c @@ -415,7 +415,7 @@ static void pcie_aer_update_log(PCIDevice *dev, const PCIEAERErr *err) int i; assert(err->status); - assert(err->status & (err->status - 1)); + assert(!(err->status & (err->status - 1))); errcap &= ~(PCI_ERR_CAP_FEP_MASK | PCI_ERR_CAP_TLP); errcap |= PCI_ERR_CAP_FEP(first_bit); @@ -495,7 +495,7 @@ static int pcie_aer_record_error(PCIDevice *dev, int fep = PCI_ERR_CAP_FEP(errcap); assert(err->status); - assert(err->status & (err->status - 1)); + assert(!(err->status & (err->status - 1))); if (errcap & PCI_ERR_CAP_MHRE && (pci_get_long(aer_cap + PCI_ERR_UNCOR_STATUS) & (1U << fep))) { @@ -979,20 +979,21 @@ int do_pcie_aer_inejct_error(Monitor *mon, if (pcie_aer_parse_error_string(error_name, &error_status, &correctable)) { char *e = NULL; error_status = strtoul(error_name, &e, 0); - correctable = !!qdict_get_int(qdict, "correctable"); + correctable = qdict_get_try_bool(qdict, "correctable", 0); if (!e || *e != '\0') { monitor_printf(mon, "invalid error status value. \"%s\"", error_name); return -EINVAL; } } + err.status = error_status; err.source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn; err.flags = 0; if (correctable) { err.flags |= PCIE_AER_ERR_IS_CORRECTABLE; } - if (qdict_get_int(qdict, "advisory_non_fatal")) { + if (qdict_get_try_bool(qdict, "advisory_non_fatal", 0)) { err.flags |= PCIE_AER_ERR_MAYBE_ADVISORY; } if (qdict_haskey(qdict, "header0")) { From c9abe111209abca1b910e35c6ca9888aced5f183 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 24 Aug 2011 14:29:30 +0200 Subject: [PATCH 3/4] pci: Error on PCI capability collisions Nothing good can happen when we overlap capabilities. This may happen when plugging in assigned devices or when devices models contain bugs. Detect the overlap and report it. Based on qemu-kvm commit by Alex Williamson. Signed-off-by: Jan Kiszka Acked-by: Don Dutile Signed-off-by: Michael S. Tsirkin --- hw/pci.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/hw/pci.c b/hw/pci.c index 6124790f01..57ff7b1098 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1811,6 +1811,25 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, return next; } +static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) +{ + uint8_t next, prev, found = 0; + + if (!(pdev->used[offset])) { + return 0; + } + + assert(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST); + + for (prev = PCI_CAPABILITY_LIST; (next = pdev->config[prev]); + prev = next + PCI_CAP_LIST_NEXT) { + if (next <= offset && next > found) { + found = next; + } + } + return found; +} + /* Patch the PCI vendor and device ids in a PCI rom image if necessary. This is needed for an option rom which is used for more than one device. */ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size) @@ -1952,11 +1971,30 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size) { uint8_t *config; + int i, overlapping_cap; + if (!offset) { offset = pci_find_space(pdev, size); if (!offset) { return -ENOSPC; } + } else { + /* Verify that capabilities don't overlap. Note: device assignment + * depends on this check to verify that the device is not broken. + * Should never trigger for emulated devices, but it's helpful + * for debugging these. */ + for (i = offset; i < offset + size; i++) { + overlapping_cap = pci_find_capability_at_offset(pdev, i); + if (overlapping_cap) { + fprintf(stderr, "ERROR: %04x:%02x:%02x.%x " + "Attempt to add PCI capability %x at offset " + "%x overlaps existing capability %x at offset %x\n", + pci_find_domain(pdev->bus), pci_bus_num(pdev->bus), + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), + cap_id, offset, overlapping_cap, i); + return -EINVAL; + } + } } config = pdev->config + offset; From b0b3db79559e57db340b292621c397e7a6cdbdc5 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 11 Aug 2011 10:21:18 +0300 Subject: [PATCH 4/4] vhost-net: cleanup host notifiers at last step When the vhost notifier is disabled, the userspace handler runs immediately: virtio_pci_set_host_notifier_internal might call virtio_queue_notify_vq. Since the VQ state and the tap backend state aren't recovered yet, this causes "Guest moved used index from XXX to YYY" assertions. The solution is to split out host notifier handling from vhost VQ setup and disable notifiers as our last step when we stop vhost-net. For symmetry enable them first thing on start. Signed-off-by: Michael S. Tsirkin --- hw/vhost.c | 74 +++++++++++++++++++++++++++++++++++++------------- hw/vhost.h | 2 ++ hw/vhost_net.c | 16 +++++++++-- 3 files changed, 70 insertions(+), 22 deletions(-) diff --git a/hw/vhost.c b/hw/vhost.c index 18860678ba..0870cb7d85 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -515,11 +515,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, }; struct VirtQueue *vvq = virtio_get_queue(vdev, idx); - if (!vdev->binding->set_host_notifier) { - fprintf(stderr, "binding does not support host notifiers\n"); - return -ENOSYS; - } - vq->num = state.num = virtio_queue_get_num(vdev, idx); r = ioctl(dev->control, VHOST_SET_VRING_NUM, &state); if (r) { @@ -567,12 +562,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, r = -errno; goto fail_alloc; } - r = vdev->binding->set_host_notifier(vdev->binding_opaque, idx, true); - if (r < 0) { - fprintf(stderr, "Error binding host notifier: %d\n", -r); - goto fail_host_notifier; - } - file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq)); r = ioctl(dev->control, VHOST_SET_VRING_KICK, &file); if (r) { @@ -591,8 +580,6 @@ static int vhost_virtqueue_init(struct vhost_dev *dev, fail_call: fail_kick: - vdev->binding->set_host_notifier(vdev->binding_opaque, idx, false); -fail_host_notifier: fail_alloc: cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx), 0, 0); @@ -618,12 +605,6 @@ static void vhost_virtqueue_cleanup(struct vhost_dev *dev, .index = idx, }; int r; - r = vdev->binding->set_host_notifier(vdev->binding_opaque, idx, false); - if (r < 0) { - fprintf(stderr, "vhost VQ %d host cleanup failed: %d\n", idx, r); - fflush(stderr); - } - assert (r >= 0); r = ioctl(dev->control, VHOST_GET_VRING_BASE, &state); if (r < 0) { fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r); @@ -697,6 +678,60 @@ bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev) hdev->force; } +/* Stop processing guest IO notifications in qemu. + * Start processing them in vhost in kernel. + */ +int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) +{ + int i, r; + if (!vdev->binding->set_host_notifier) { + fprintf(stderr, "binding does not support host notifiers\n"); + r = -ENOSYS; + goto fail; + } + + for (i = 0; i < hdev->nvqs; ++i) { + r = vdev->binding->set_host_notifier(vdev->binding_opaque, i, true); + if (r < 0) { + fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i, -r); + goto fail_vq; + } + } + + return 0; +fail_vq: + while (--i >= 0) { + r = vdev->binding->set_host_notifier(vdev->binding_opaque, i, false); + if (r < 0) { + fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i, -r); + fflush(stderr); + } + assert (r >= 0); + } +fail: + return r; +} + +/* Stop processing guest IO notifications in vhost. + * Start processing them in qemu. + * This might actually run the qemu handlers right away, + * so virtio in qemu must be completely setup when this is called. + */ +void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev) +{ + int i, r; + + for (i = 0; i < hdev->nvqs; ++i) { + r = vdev->binding->set_host_notifier(vdev->binding_opaque, i, false); + if (r < 0) { + fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i, -r); + fflush(stderr); + } + assert (r >= 0); + } +} + +/* Host notifiers must be enabled at this point. */ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) { int i, r; @@ -762,6 +797,7 @@ fail: return r; } +/* Host notifiers must be enabled at this point. */ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev) { int i, r; diff --git a/hw/vhost.h b/hw/vhost.h index c8c595a147..c9452f0732 100644 --- a/hw/vhost.h +++ b/hw/vhost.h @@ -46,5 +46,7 @@ void vhost_dev_cleanup(struct vhost_dev *hdev); bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev); int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev); void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev); +int vhost_dev_enable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev); +void vhost_dev_disable_notifiers(struct vhost_dev *hdev, VirtIODevice *vdev); #endif diff --git a/hw/vhost_net.c b/hw/vhost_net.c index a55981200d..950a6b8d99 100644 --- a/hw/vhost_net.c +++ b/hw/vhost_net.c @@ -139,16 +139,22 @@ int vhost_net_start(struct vhost_net *net, { struct vhost_vring_file file = { }; int r; + + net->dev.nvqs = 2; + net->dev.vqs = net->vqs; + + r = vhost_dev_enable_notifiers(&net->dev, dev); + if (r < 0) { + goto fail_notifiers; + } if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) { tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr_mrg_rxbuf)); } - net->dev.nvqs = 2; - net->dev.vqs = net->vqs; r = vhost_dev_start(&net->dev, dev); if (r < 0) { - return r; + goto fail_start; } net->vc->info->poll(net->vc, false); @@ -173,6 +179,9 @@ fail: if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) { tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr)); } +fail_start: + vhost_dev_disable_notifiers(&net->dev, dev); +fail_notifiers: return r; } @@ -190,6 +199,7 @@ void vhost_net_stop(struct vhost_net *net, if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) { tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr)); } + vhost_dev_disable_notifiers(&net->dev, dev); } void vhost_net_cleanup(struct vhost_net *net)