From d41795978c47fa87b6514a0f2238958b7e8319a0 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 15 Dec 2016 23:58:13 +0200 Subject: virtio: clean up handling of request_irq failure We call del_vqs twice when request_irq fails, this makes no sense. Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/virtio/virtio_pci_common.c') diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index d9a905827967..423f3d9c1548 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -360,10 +360,8 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, vring_interrupt, 0, vp_dev->msix_names[msix_vec], vqs[i]); - if (err) { - vp_del_vq(vqs[i]); + if (err) goto error_find; - } } return 0; -- cgit v1.2.3 From fa3a3279354e5f7a13ec16f2c9f5e39124745332 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 17 Nov 2016 11:43:13 +0100 Subject: virtio_pci: use pci_alloc_irq_vectors This avoids the separate allocation for the msix_entries structures, and instead allows us to use pci_irq_vector to find a given IRQ vector. Signed-off-by: Christoph Hellwig Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 42 +++++++++++++++----------------------- drivers/virtio/virtio_pci_common.h | 1 - 2 files changed, 17 insertions(+), 26 deletions(-) (limited to 'drivers/virtio/virtio_pci_common.c') diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 423f3d9c1548..2846872d5728 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -37,7 +37,7 @@ void vp_synchronize_vectors(struct virtio_device *vdev) synchronize_irq(vp_dev->pci_dev->irq); for (i = 0; i < vp_dev->msix_vectors; ++i) - synchronize_irq(vp_dev->msix_entries[i].vector); + synchronize_irq(pci_irq_vector(vp_dev->pci_dev, i)); } /* the notify function used when creating a virt queue */ @@ -113,7 +113,7 @@ static void vp_free_vectors(struct virtio_device *vdev) } for (i = 0; i < vp_dev->msix_used_vectors; ++i) - free_irq(vp_dev->msix_entries[i].vector, vp_dev); + free_irq(pci_irq_vector(vp_dev->pci_dev, i), vp_dev); for (i = 0; i < vp_dev->msix_vectors; i++) if (vp_dev->msix_affinity_masks[i]) @@ -123,7 +123,7 @@ static void vp_free_vectors(struct virtio_device *vdev) /* Disable the vector used for configuration */ vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR); - pci_disable_msix(vp_dev->pci_dev); + pci_free_irq_vectors(vp_dev->pci_dev); vp_dev->msix_enabled = 0; } @@ -131,8 +131,6 @@ static void vp_free_vectors(struct virtio_device *vdev) vp_dev->msix_used_vectors = 0; kfree(vp_dev->msix_names); vp_dev->msix_names = NULL; - kfree(vp_dev->msix_entries); - vp_dev->msix_entries = NULL; kfree(vp_dev->msix_affinity_masks); vp_dev->msix_affinity_masks = NULL; } @@ -147,10 +145,6 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, vp_dev->msix_vectors = nvectors; - vp_dev->msix_entries = kmalloc(nvectors * sizeof *vp_dev->msix_entries, - GFP_KERNEL); - if (!vp_dev->msix_entries) - goto error; vp_dev->msix_names = kmalloc(nvectors * sizeof *vp_dev->msix_names, GFP_KERNEL); if (!vp_dev->msix_names) @@ -165,12 +159,9 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, GFP_KERNEL)) goto error; - for (i = 0; i < nvectors; ++i) - vp_dev->msix_entries[i].entry = i; - - err = pci_enable_msix_exact(vp_dev->pci_dev, - vp_dev->msix_entries, nvectors); - if (err) + err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors, + PCI_IRQ_MSIX); + if (err < 0) goto error; vp_dev->msix_enabled = 1; @@ -178,7 +169,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, 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, + err = request_irq(pci_irq_vector(vp_dev->pci_dev, v), vp_config_changed, 0, vp_dev->msix_names[v], vp_dev); if (err) @@ -197,7 +188,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, v = vp_dev->msix_used_vectors; snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names, "%s-virtqueues", name); - err = request_irq(vp_dev->msix_entries[v].vector, + err = request_irq(pci_irq_vector(vp_dev->pci_dev, v), vp_vring_interrupt, 0, vp_dev->msix_names[v], vp_dev); if (err) @@ -276,14 +267,15 @@ void vp_del_vqs(struct virtio_device *vdev) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); struct virtqueue *vq, *n; - struct virtio_pci_vq_info *info; list_for_each_entry_safe(vq, n, &vdev->vqs, list) { - info = vp_dev->vqs[vq->index]; - if (vp_dev->per_vq_vectors && - info->msix_vector != VIRTIO_MSI_NO_VECTOR) - free_irq(vp_dev->msix_entries[info->msix_vector].vector, - vq); + if (vp_dev->per_vq_vectors) { + int v = vp_dev->vqs[vq->index]->msix_vector; + + if (v != VIRTIO_MSI_NO_VECTOR) + free_irq(pci_irq_vector(vp_dev->pci_dev, v), + vq); + } vp_del_vq(vq); } vp_dev->per_vq_vectors = false; @@ -356,7 +348,7 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, sizeof *vp_dev->msix_names, "%s-%s", dev_name(&vp_dev->vdev.dev), names[i]); - err = request_irq(vp_dev->msix_entries[msix_vec].vector, + err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), vring_interrupt, 0, vp_dev->msix_names[msix_vec], vqs[i]); @@ -417,7 +409,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu) if (vp_dev->msix_enabled) { mask = vp_dev->msix_affinity_masks[info->msix_vector]; - irq = vp_dev->msix_entries[info->msix_vector].vector; + irq = pci_irq_vector(vp_dev->pci_dev, info->msix_vector); if (cpu == -1) irq_set_affinity_hint(irq, NULL); else { diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h index 28263200ed42..b2f666250ae0 100644 --- a/drivers/virtio/virtio_pci_common.h +++ b/drivers/virtio/virtio_pci_common.h @@ -85,7 +85,6 @@ struct virtio_pci_device { /* MSI-X support */ int msix_enabled; int intx_enabled; - struct msix_entry *msix_entries; cpumask_var_t *msix_affinity_masks; /* Name strings for interrupts. This size should be enough, * and I'm too lazy to allocate each name separately. */ -- cgit v1.2.3 From 9f8196cc05caa9aba19f38f29c5f9a12722986c4 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 17 Nov 2016 11:43:14 +0100 Subject: virtio_pci: remove the call to vp_free_vectors in vp_request_msix_vectors vp_request_msix_vectors is only called by vp_try_to_find_vqs, which already calls vp_free_vectors through vp_del_vqs in the failure case. Signed-off-by: Christoph Hellwig Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/virtio/virtio_pci_common.c') diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 2846872d5728..5fc9d283a954 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -197,7 +197,6 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, } return 0; error: - vp_free_vectors(vdev); return err; } -- cgit v1.2.3 From 66f2f55542270c44d67f05533594f75404c1481a Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 17 Nov 2016 11:43:15 +0100 Subject: virtio_pci: merge vp_free_vectors into vp_del_vqs Signed-off-by: Christoph Hellwig Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 61 +++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 34 deletions(-) (limited to 'drivers/virtio/virtio_pci_common.c') diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index 5fc9d283a954..c93739eaeea0 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -102,39 +102,6 @@ static irqreturn_t vp_interrupt(int irq, void *opaque) return vp_vring_interrupt(irq, opaque); } -static void vp_free_vectors(struct virtio_device *vdev) -{ - struct virtio_pci_device *vp_dev = to_vp_device(vdev); - int i; - - if (vp_dev->intx_enabled) { - free_irq(vp_dev->pci_dev->irq, vp_dev); - vp_dev->intx_enabled = 0; - } - - for (i = 0; i < vp_dev->msix_used_vectors; ++i) - free_irq(pci_irq_vector(vp_dev->pci_dev, i), vp_dev); - - for (i = 0; i < vp_dev->msix_vectors; i++) - if (vp_dev->msix_affinity_masks[i]) - free_cpumask_var(vp_dev->msix_affinity_masks[i]); - - if (vp_dev->msix_enabled) { - /* Disable the vector used for configuration */ - vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR); - - pci_free_irq_vectors(vp_dev->pci_dev); - vp_dev->msix_enabled = 0; - } - - vp_dev->msix_vectors = 0; - vp_dev->msix_used_vectors = 0; - kfree(vp_dev->msix_names); - vp_dev->msix_names = NULL; - kfree(vp_dev->msix_affinity_masks); - vp_dev->msix_affinity_masks = NULL; -} - static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, bool per_vq_vectors) { @@ -266,6 +233,7 @@ void vp_del_vqs(struct virtio_device *vdev) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); struct virtqueue *vq, *n; + int i; list_for_each_entry_safe(vq, n, &vdev->vqs, list) { if (vp_dev->per_vq_vectors) { @@ -279,7 +247,32 @@ void vp_del_vqs(struct virtio_device *vdev) } vp_dev->per_vq_vectors = false; - vp_free_vectors(vdev); + if (vp_dev->intx_enabled) { + free_irq(vp_dev->pci_dev->irq, vp_dev); + vp_dev->intx_enabled = 0; + } + + for (i = 0; i < vp_dev->msix_used_vectors; ++i) + free_irq(pci_irq_vector(vp_dev->pci_dev, i), vp_dev); + + for (i = 0; i < vp_dev->msix_vectors; i++) + if (vp_dev->msix_affinity_masks[i]) + free_cpumask_var(vp_dev->msix_affinity_masks[i]); + + if (vp_dev->msix_enabled) { + /* Disable the vector used for configuration */ + vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR); + + pci_free_irq_vectors(vp_dev->pci_dev); + vp_dev->msix_enabled = 0; + } + + vp_dev->msix_vectors = 0; + vp_dev->msix_used_vectors = 0; + kfree(vp_dev->msix_names); + vp_dev->msix_names = NULL; + kfree(vp_dev->msix_affinity_masks); + vp_dev->msix_affinity_masks = NULL; kfree(vp_dev->vqs); vp_dev->vqs = NULL; } -- cgit v1.2.3 From a3cbec69727c8c149ee2b3652e184818cc269fe6 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Thu, 17 Nov 2016 11:43:16 +0100 Subject: virtio_pci: split vp_try_to_find_vqs into INTx and MSI-X variants There is basically no shared logic between the INTx and MSI-X case in vp_try_to_find_vqs, so split the function into two and clean them up a little bit. Also remove the fairly pointless vp_request_intx wrapper while we're at it. Signed-off-by: Christoph Hellwig Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_pci_common.c | 97 ++++++++++++++++++++++---------------- 1 file changed, 57 insertions(+), 40 deletions(-) (limited to 'drivers/virtio/virtio_pci_common.c') diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index c93739eaeea0..186cbab327b8 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -167,18 +167,6 @@ error: return err; } -static int vp_request_intx(struct virtio_device *vdev) -{ - int err; - struct virtio_pci_device *vp_dev = to_vp_device(vdev); - - err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, - IRQF_SHARED, dev_name(&vdev->dev), vp_dev); - if (!err) - vp_dev->intx_enabled = 1; - return err; -} - static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned index, void (*callback)(struct virtqueue *vq), const char *name, @@ -277,50 +265,44 @@ void vp_del_vqs(struct virtio_device *vdev) vp_dev->vqs = NULL; } -static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs, +static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs, struct virtqueue *vqs[], vq_callback_t *callbacks[], const char * const names[], - bool use_msix, bool per_vq_vectors) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); u16 msix_vec; int i, err, nvectors, allocated_vectors; - vp_dev->vqs = kmalloc(nvqs * sizeof *vp_dev->vqs, GFP_KERNEL); + vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); if (!vp_dev->vqs) return -ENOMEM; - if (!use_msix) { - /* Old style: one normal interrupt for change and all vqs. */ - err = vp_request_intx(vdev); - if (err) - goto error_find; + if (per_vq_vectors) { + /* Best option: one for change interrupt, one per vq. */ + nvectors = 1; + for (i = 0; i < nvqs; ++i) + if (callbacks[i]) + ++nvectors; } else { - if (per_vq_vectors) { - /* Best option: one for change interrupt, one per vq. */ - nvectors = 1; - for (i = 0; i < nvqs; ++i) - if (callbacks[i]) - ++nvectors; - } else { - /* Second best: one for change, shared for all vqs. */ - nvectors = 2; - } - - err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors); - if (err) - goto error_find; + /* Second best: one for change, shared for all vqs. */ + nvectors = 2; } + err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors); + if (err) + goto error_find; + vp_dev->per_vq_vectors = per_vq_vectors; allocated_vectors = vp_dev->msix_used_vectors; for (i = 0; i < nvqs; ++i) { if (!names[i]) { vqs[i] = NULL; continue; - } else if (!callbacks[i] || !vp_dev->msix_enabled) + } + + if (!callbacks[i]) msix_vec = VIRTIO_MSI_NO_VECTOR; else if (vp_dev->per_vq_vectors) msix_vec = allocated_vectors++; @@ -354,6 +336,43 @@ error_find: return err; } +static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs, + struct virtqueue *vqs[], vq_callback_t *callbacks[], + const char * const names[]) +{ + struct virtio_pci_device *vp_dev = to_vp_device(vdev); + int i, err; + + vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); + if (!vp_dev->vqs) + return -ENOMEM; + + err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, IRQF_SHARED, + dev_name(&vdev->dev), vp_dev); + if (err) + goto out_del_vqs; + + vp_dev->intx_enabled = 1; + vp_dev->per_vq_vectors = false; + for (i = 0; i < nvqs; ++i) { + if (!names[i]) { + vqs[i] = NULL; + continue; + } + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], + VIRTIO_MSI_NO_VECTOR); + if (IS_ERR(vqs[i])) { + err = PTR_ERR(vqs[i]); + goto out_del_vqs; + } + } + + return 0; +out_del_vqs: + vp_del_vqs(vdev); + return err; +} + /* the config->find_vqs() implementation */ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs, struct virtqueue *vqs[], @@ -363,17 +382,15 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs, int err; /* Try MSI-X with one vector per queue. */ - err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names, true, true); + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true); if (!err) return 0; /* Fallback: MSI-X with one vector for config, one shared for queues. */ - err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names, - true, false); + err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false); if (!err) return 0; /* Finally fall back to regular interrupts. */ - return vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names, - false, false); + return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names); } const char *vp_bus_name(struct virtio_device *vdev) -- cgit v1.2.3