From patchwork Tue Jul 21 16:54:00 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 36569 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n6LGtp4s012654 for ; Tue, 21 Jul 2009 16:55:51 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755622AbZGUQzA (ORCPT ); Tue, 21 Jul 2009 12:55:00 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755595AbZGUQzA (ORCPT ); Tue, 21 Jul 2009 12:55:00 -0400 Received: from mx2.redhat.com ([66.187.237.31]:50079 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755541AbZGUQy5 (ORCPT ); Tue, 21 Jul 2009 12:54:57 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n6LGsv5G005146 for ; Tue, 21 Jul 2009 12:54:57 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n6LGsuQJ029708; Tue, 21 Jul 2009 12:54:57 -0400 Received: from redhat.com (vpn-10-77.str.redhat.com [10.32.10.77]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n6LGsrEF017629; Tue, 21 Jul 2009 12:54:54 -0400 Date: Tue, 21 Jul 2009 19:54:00 +0300 From: "Michael S. Tsirkin" To: Amit Shah Cc: kvm@vger.kernel.org Subject: Re: qemu-kvm missing some msix capability check Message-ID: <20090721165400.GB4826@redhat.com> References: <20090717130439.GA10081@amit-x200.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20090717130439.GA10081@amit-x200.redhat.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, Jul 17, 2009 at 06:34:40PM +0530, Amit Shah wrote: > Hello, > > Using recent qemu-kvm userspace with a slightly older kernel module I > get this when using the virtio-net device: > > kvm_msix_add: kvm_get_irq_route_gsi failed: No space left on device > > ... and the guest doesn't use the net device. > > This goes away when using a newer kvm module. > > Amit Could you verify if the following helps please? Tested-by: Amit Shah --- virtio: retry on vector assignment failure virtio currently fails to find any vqs if the device supports msi-x but fails to assign it to a queue. Turns out, this actually happens for old host kernels which can't allocate a gsi for the vector. As a result guest does not use virtio net. Fix this by disabling msi on such failures and falling back on regular interrupts. Signed-off-by: Michael S. Tsirkin -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 9dcc368..567c972 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -273,26 +273,35 @@ static void vp_free_vectors(struct virtio_device *vdev) } static int vp_enable_msix(struct pci_dev *dev, struct msix_entry *entries, - int *options, int noptions) + int nvectors) { - int i; - for (i = 0; i < noptions; ++i) - if (!pci_enable_msix(dev, entries, options[i])) - return options[i]; - return -EBUSY; + int err = pci_enable_msix(dev, entries, nvectors); + if (err > 0) + err = -ENOSPC; + return err; } -static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs) +static int vp_request_irq(struct virtio_device *vdev) +{ + struct virtio_pci_device *vp_dev = to_vp_device(vdev); + int err; + /* Can't allocate enough MSI-X vectors, use regular interrupt */ + vp_dev->msix_vectors = 0; + err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, + IRQF_SHARED, dev_name(&vp_dev->vdev.dev), vp_dev); + if (err) + return err; + vp_dev->intx_enabled = 1; + return 0; +} + +static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs, + int nvectors) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); const char *name = dev_name(&vp_dev->vdev.dev); unsigned i, v; int err = -ENOMEM; - /* We want at most one vector per queue and one for config changes. - * Fallback to separate vectors for config and a shared for queues. - * Finally fall back to regular interrupts. */ - int options[] = { max_vqs + 1, 2 }; - int nvectors = max(options[0], options[1]); vp_dev->msix_entries = kmalloc(nvectors * sizeof *vp_dev->msix_entries, GFP_KERNEL); @@ -307,37 +316,29 @@ static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs) vp_dev->msix_entries[i].entry = i; err = vp_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries, - options, ARRAY_SIZE(options)); - if (err < 0) { - /* Can't allocate enough MSI-X vectors, use regular interrupt */ - vp_dev->msix_vectors = 0; - err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, - IRQF_SHARED, name, vp_dev); - if (err) - goto error_irq; - vp_dev->intx_enabled = 1; - } else { - vp_dev->msix_vectors = err; - vp_dev->msix_enabled = 1; - - /* Set the vector used for configuration */ - v = vp_dev->msix_used_vectors; - snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names, - "%s-config", name); - err = request_irq(vp_dev->msix_entries[v].vector, - vp_config_changed, 0, vp_dev->msix_names[v], - vp_dev); - if (err) - goto error_irq; - ++vp_dev->msix_used_vectors; - - iowrite16(v, vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); - /* Verify we had enough resources to assign the vector */ - v = ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); - if (v == VIRTIO_MSI_NO_VECTOR) { - err = -EBUSY; - goto error_irq; - } + nvectors); + if (err) + goto error_enable; + vp_dev->msix_vectors = nvectors; + vp_dev->msix_enabled = 1; + + /* Set the vector used for configuration */ + v = vp_dev->msix_used_vectors; + snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names, + "%s-config", name); + err = request_irq(vp_dev->msix_entries[v].vector, + vp_config_changed, 0, vp_dev->msix_names[v], + vp_dev); + if (err) + goto error_irq; + ++vp_dev->msix_used_vectors; + + iowrite16(v, vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); + /* Verify we had enough resources to assign the vector */ + v = ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR); + if (v == VIRTIO_MSI_NO_VECTOR) { + err = -EBUSY; + goto error_irq; } if (vp_dev->msix_vectors && vp_dev->msix_vectors != max_vqs + 1) { @@ -355,9 +356,12 @@ static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs) return 0; error_irq: vp_free_vectors(vdev); +error_enable: kfree(vp_dev->msix_names); + vp_dev->msix_names = NULL; error_names: kfree(vp_dev->msix_entries); + vp_dev->msix_entries = NULL; error_entries: return err; } @@ -499,24 +503,24 @@ static void vp_del_vqs(struct virtio_device *vdev) vp_free_vectors(vdev); kfree(vp_dev->msix_names); + vp_dev->msix_names = NULL; kfree(vp_dev->msix_entries); + vp_dev->msix_entries = NULL; } -/* the config->find_vqs() implementation */ -static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs, - struct virtqueue *vqs[], - vq_callback_t *callbacks[], - const char *names[]) +static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, + struct virtqueue *vqs[], + vq_callback_t *callbacks[], + const char *names[], + int max_vectors, + int nvectors) { - int vectors = 0; int i, err; - /* How many vectors would we like? */ - for (i = 0; i < nvqs; ++i) - if (callbacks[i]) - ++vectors; - - err = vp_request_vectors(vdev, vectors); + if (nvectors) + err = vp_request_vectors(vdev, max_vectors, nvectors); + else + err = vp_request_irq(vdev); if (err) goto error_request; @@ -534,6 +538,34 @@ error_request: return PTR_ERR(vqs[i]); } +/* the config->find_vqs() implementation */ +static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs, + struct virtqueue *vqs[], + vq_callback_t *callbacks[], + const char *names[]) +{ + /* We want at most one vector per queue and one for config changes. + * Fallback to separate vectors for config and a shared for queues. + * Finally fall back to regular interrupts. */ + int options[] = { 1, 2, 0 }; + int vectors = 0; + int i, uninitialized_var(err); + + /* How many vectors would we like? */ + for (i = 0; i < nvqs; ++i) + if (callbacks[i]) + ++vectors; + options[0] = vectors + 1; + + for (i = 0; i < ARRAY_SIZE(options); ++i) { + err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names, + vectors, options[i]); + if (!err) + break; + } + return err; +} + static struct virtio_config_ops virtio_pci_config_ops = { .get = vp_get, .set = vp_set,