* [PATCH 1/7] virtio_pci: use pci_alloc_irq_vectors
2016-11-06 23:09 virtio_pci irq handling cleanups Christoph Hellwig
@ 2016-11-06 23:09 ` Christoph Hellwig
2016-11-06 23:09 ` [PATCH 2/7] virtio_pci: remove the call to vp_free_vectors in vp_request_msix_vectors Christoph Hellwig
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-11-06 23:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel
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 <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 42 +++++++++++++++-----------------------
drivers/virtio/virtio_pci_common.h | 1 -
2 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index d9a9058..f5e2751 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]);
@@ -419,7 +411,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 2826320..b2f6662 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. */
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/7] virtio_pci: remove the call to vp_free_vectors in vp_request_msix_vectors
2016-11-06 23:09 virtio_pci irq handling cleanups Christoph Hellwig
2016-11-06 23:09 ` [PATCH 1/7] virtio_pci: use pci_alloc_irq_vectors Christoph Hellwig
@ 2016-11-06 23:09 ` Christoph Hellwig
2016-11-06 23:09 ` [PATCH 3/7] virtio_pci: merge vp_free_vectors into vp_del_vqs Christoph Hellwig
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-11-06 23:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel
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 <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index f5e2751..93700c5 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;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/7] virtio_pci: merge vp_free_vectors into vp_del_vqs
2016-11-06 23:09 virtio_pci irq handling cleanups Christoph Hellwig
2016-11-06 23:09 ` [PATCH 1/7] virtio_pci: use pci_alloc_irq_vectors Christoph Hellwig
2016-11-06 23:09 ` [PATCH 2/7] virtio_pci: remove the call to vp_free_vectors in vp_request_msix_vectors Christoph Hellwig
@ 2016-11-06 23:09 ` Christoph Hellwig
2016-11-06 23:09 ` [PATCH 4/7] virtio_pci: split the INTx case out of vp_try_to_find_vqs Christoph Hellwig
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-11-06 23:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 61 +++++++++++++++++---------------------
1 file changed, 27 insertions(+), 34 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 93700c5..f6c5499 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;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/7] virtio_pci: split the INTx case out of vp_try_to_find_vqs
2016-11-06 23:09 virtio_pci irq handling cleanups Christoph Hellwig
` (2 preceding siblings ...)
2016-11-06 23:09 ` [PATCH 3/7] virtio_pci: merge vp_free_vectors into vp_del_vqs Christoph Hellwig
@ 2016-11-06 23:09 ` Christoph Hellwig
2016-11-06 23:09 ` [PATCH 5/7] virtio_pci: add a helper to request MSI-X interrupts Christoph Hellwig
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-11-06 23:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel
There is basically no shared logic with the two MSI-X variants, so move
it into a separate function. Also remove the fairly pointless
vp_request_intx wrapper while we're at it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 89 ++++++++++++++++++++++----------------
1 file changed, 52 insertions(+), 37 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index f6c5499..644bd80 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -167,18 +167,6 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
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,
@@ -281,7 +269,6 @@ static int vp_try_to_find_vqs(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);
@@ -292,28 +279,21 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
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) {
@@ -356,6 +336,43 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
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[],
@@ -365,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_try_to_find_vqs(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_try_to_find_vqs(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)
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/7] virtio_pci: add a helper to request MSI-X interrupts
2016-11-06 23:09 virtio_pci irq handling cleanups Christoph Hellwig
` (3 preceding siblings ...)
2016-11-06 23:09 ` [PATCH 4/7] virtio_pci: split the INTx case out of vp_try_to_find_vqs Christoph Hellwig
@ 2016-11-06 23:09 ` Christoph Hellwig
2016-11-06 23:09 ` [PATCH 6/7] virtio_pci: split the shared and per-VQ MSI-X setup routines Christoph Hellwig
2016-11-06 23:09 ` [PATCH 7/7] virtio_pci: clean up interrupt state in struct virtio_pci_device Christoph Hellwig
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-11-06 23:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel
Factor out some duplicated code to format that MSI-X vector name and
request the interrupt for it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 38 +++++++++++++++-----------------------
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 644bd80..7c44891 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -102,11 +102,19 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
return vp_vring_interrupt(irq, opaque);
}
+static int vp_request_msix_vector(struct virtio_pci_device *vp_dev, int vec,
+ irq_handler_t handler, const char *name, void *dev_id)
+{
+ snprintf(vp_dev->msix_names[vec], sizeof(vp_dev->msix_names[vec]),
+ "%s-%s", dev_name(&vp_dev->vdev.dev), name);
+ return request_irq(pci_irq_vector(vp_dev->pci_dev, vec), handler, 0,
+ vp_dev->msix_names[vec], dev_id);
+}
+
static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
bool per_vq_vectors)
{
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;
@@ -133,15 +141,10 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int 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(pci_irq_vector(vp_dev->pci_dev, v),
- vp_config_changed, 0, vp_dev->msix_names[v],
- vp_dev);
+ err = vp_request_msix_vector(vp_dev, vp_dev->msix_used_vectors++,
+ vp_config_changed, "config", vp_dev);
if (err)
goto error;
- ++vp_dev->msix_used_vectors;
v = vp_dev->config_vector(vp_dev, v);
/* Verify we had enough resources to assign the vector */
@@ -152,15 +155,10 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
if (!per_vq_vectors) {
/* Shared vector for all VQs */
- v = vp_dev->msix_used_vectors;
- snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
- "%s-virtqueues", name);
- err = request_irq(pci_irq_vector(vp_dev->pci_dev, v),
- vp_vring_interrupt, 0, vp_dev->msix_names[v],
- vp_dev);
+ err = vp_request_msix_vector(vp_dev, vp_dev->msix_used_vectors++,
+ vp_vring_interrupt, "virtqueues", vp_dev);
if (err)
goto error;
- ++vp_dev->msix_used_vectors;
}
return 0;
error:
@@ -316,14 +314,8 @@ static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
continue;
/* allocate per-vq irq if available and necessary */
- snprintf(vp_dev->msix_names[msix_vec],
- sizeof *vp_dev->msix_names,
- "%s-%s",
- dev_name(&vp_dev->vdev.dev), names[i]);
- err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
- vring_interrupt, 0,
- vp_dev->msix_names[msix_vec],
- vqs[i]);
+ err = vp_request_msix_vector(vp_dev, msix_vec, vring_interrupt,
+ names[i], vqs[i]);
if (err) {
vp_del_vq(vqs[i]);
goto error_find;
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 6/7] virtio_pci: split the shared and per-VQ MSI-X setup routines
2016-11-06 23:09 virtio_pci irq handling cleanups Christoph Hellwig
` (4 preceding siblings ...)
2016-11-06 23:09 ` [PATCH 5/7] virtio_pci: add a helper to request MSI-X interrupts Christoph Hellwig
@ 2016-11-06 23:09 ` Christoph Hellwig
2016-11-06 23:09 ` [PATCH 7/7] virtio_pci: clean up interrupt state in struct virtio_pci_device Christoph Hellwig
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-11-06 23:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel
While this is a tiny bit more code in terms of LOC it clearly separates
the purposes of both routines and makes them a lot easier to read, and
more importantly to modify for PCI-level affinity assignment which will
only go into the per-VQ version.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 103 +++++++++++++++++++++----------------
1 file changed, 59 insertions(+), 44 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 7c44891..3f166bc 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -111,8 +111,7 @@ static int vp_request_msix_vector(struct virtio_pci_device *vp_dev, int vec,
vp_dev->msix_names[vec], dev_id);
}
-static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
- bool per_vq_vectors)
+static int vp_setup_msix_vectors(struct virtio_device *vdev, int nvectors)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
unsigned i, v;
@@ -153,13 +152,6 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
goto error;
}
- if (!per_vq_vectors) {
- /* Shared vector for all VQs */
- err = vp_request_msix_vector(vp_dev, vp_dev->msix_used_vectors++,
- vp_vring_interrupt, "virtqueues", vp_dev);
- if (err)
- goto error;
- }
return 0;
error:
return err;
@@ -263,67 +255,90 @@ 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,
- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[],
- bool per_vq_vectors)
+static int vp_find_vqs_msix(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, nvectors = 1, allocated_vectors;
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 (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;
+ for (i = 0; i < nvqs; ++i) {
+ if (callbacks[i])
+ nvectors++;
}
-
- err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors);
+ err = vp_setup_msix_vectors(vdev, nvectors);
if (err)
- goto error_find;
+ goto out_del_vqs;
- vp_dev->per_vq_vectors = per_vq_vectors;
+ vp_dev->per_vq_vectors = true;
allocated_vectors = vp_dev->msix_used_vectors;
for (i = 0; i < nvqs; ++i) {
- if (!names[i]) {
- vqs[i] = NULL;
+ if (!names[i])
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++;
else
- msix_vec = VP_MSIX_VQ_VECTOR;
+ msix_vec = allocated_vectors++;
+
vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
if (IS_ERR(vqs[i])) {
err = PTR_ERR(vqs[i]);
- goto error_find;
+ goto out_del_vqs;
}
-
- if (!vp_dev->per_vq_vectors || msix_vec == VIRTIO_MSI_NO_VECTOR)
+ if (msix_vec == VIRTIO_MSI_NO_VECTOR)
continue;
-
- /* allocate per-vq irq if available and necessary */
err = vp_request_msix_vector(vp_dev, msix_vec, vring_interrupt,
names[i], vqs[i]);
if (err) {
vp_del_vq(vqs[i]);
- goto error_find;
+ goto out_del_vqs;
}
}
+
return 0;
+out_del_vqs:
+ vp_del_vqs(vdev);
+ return err;
+}
+
+static int vp_find_vqs_msix_shared(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 = vp_setup_msix_vectors(vdev, 2);
+ if (err)
+ goto out_del_vqs;
+ err = vp_request_msix_vector(vp_dev, vp_dev->msix_used_vectors++,
+ vp_vring_interrupt, "virtqueues", vp_dev);
+ if (err)
+ goto out_del_vqs;
-error_find:
+ vp_dev->per_vq_vectors = false;
+ for (i = 0; i < nvqs; ++i) {
+ if (!names[i])
+ continue;
+ vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i],
+ callbacks[i] ? VP_MSIX_VQ_VECTOR :
+ 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;
}
@@ -374,11 +389,11 @@ 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);
+ err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names);
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, false);
+ err = vp_find_vqs_msix_shared(vdev, nvqs, vqs, callbacks, names);
if (!err)
return 0;
/* Finally fall back to regular interrupts. */
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 7/7] virtio_pci: clean up interrupt state in struct virtio_pci_device
2016-11-06 23:09 virtio_pci irq handling cleanups Christoph Hellwig
` (5 preceding siblings ...)
2016-11-06 23:09 ` [PATCH 6/7] virtio_pci: split the shared and per-VQ MSI-X setup routines Christoph Hellwig
@ 2016-11-06 23:09 ` Christoph Hellwig
6 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2016-11-06 23:09 UTC (permalink / raw)
To: mst; +Cc: virtualization, linux-kernel
Currently there is a lot of interrupt-related state in struct
virtio_pci_device, must of which is not needed:
- msix_enabled is stored in struct pci_device and can be used there
- msix_enabled isn't needed as we can call pci_free_irq_vectors even
for the INTx case and it will do the right thing, as will a call
to pci_irq_vector for vector 0
- used_msix_vectors is not needed because we can just subtract the number
of per-VQ vectors from the total number and get it
- per_vq_vectors is not needed as we can just check the msix_vec field
in the VQ for a valid vector
This just laves us with the old msix_vectors field that has been renamed
to irq_vectors and will be maintained for the INTx case as well to make
our life easier.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/virtio/virtio_pci_common.c | 64 ++++++++++++++------------------------
drivers/virtio/virtio_pci_common.h | 12 ++-----
drivers/virtio/virtio_pci_legacy.c | 2 +-
drivers/virtio/virtio_pci_modern.c | 2 +-
include/uapi/linux/virtio_pci.h | 2 +-
5 files changed, 29 insertions(+), 53 deletions(-)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 3f166bc..4b2af83 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -33,10 +33,7 @@ void vp_synchronize_vectors(struct virtio_device *vdev)
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
int i;
- if (vp_dev->intx_enabled)
- synchronize_irq(vp_dev->pci_dev->irq);
-
- for (i = 0; i < vp_dev->msix_vectors; ++i)
+ for (i = 0; i < vp_dev->irq_vectors; ++i)
synchronize_irq(pci_irq_vector(vp_dev->pci_dev, i));
}
@@ -117,8 +114,6 @@ static int vp_setup_msix_vectors(struct virtio_device *vdev, int nvectors)
unsigned i, v;
int err = -ENOMEM;
- vp_dev->msix_vectors = nvectors;
-
vp_dev->msix_names = kmalloc(nvectors * sizeof *vp_dev->msix_names,
GFP_KERNEL);
if (!vp_dev->msix_names)
@@ -137,10 +132,9 @@ static int vp_setup_msix_vectors(struct virtio_device *vdev, int nvectors)
PCI_IRQ_MSIX);
if (err < 0)
goto error;
- vp_dev->msix_enabled = 1;
/* Set the vector used for configuration */
- err = vp_request_msix_vector(vp_dev, vp_dev->msix_used_vectors++,
+ err = vp_request_msix_vector(vp_dev, vp_dev->irq_vectors++,
vp_config_changed, "config", vp_dev);
if (err)
goto error;
@@ -211,46 +205,40 @@ void vp_del_vqs(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
struct virtqueue *vq, *n;
+ int irq_vectors = vp_dev->irq_vectors;
int i;
list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
- if (vp_dev->per_vq_vectors) {
- int v = vp_dev->vqs[vq->index]->msix_vector;
+ int vec = vp_dev->vqs[vq->index]->msix_vector;
- if (v != VIRTIO_MSI_NO_VECTOR)
- free_irq(pci_irq_vector(vp_dev->pci_dev, v),
- vq);
+ if (vec != VIRTIO_MSI_NO_VECTOR && vec != VP_MSIX_VQ_VECTOR) {
+ free_irq(pci_irq_vector(vp_dev->pci_dev, vec), vq);
+ irq_vectors--;
}
- vp_del_vq(vq);
- }
- vp_dev->per_vq_vectors = false;
- if (vp_dev->intx_enabled) {
- free_irq(vp_dev->pci_dev->irq, vp_dev);
- vp_dev->intx_enabled = 0;
+ vp_del_vq(vq);
}
- for (i = 0; i < vp_dev->msix_used_vectors; ++i)
+ for (i = irq_vectors - 1; i >= 0; 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])
+ if (vp_dev->msix_affinity_masks) {
+ for (i = 0; i < vp_dev->irq_vectors; i++)
free_cpumask_var(vp_dev->msix_affinity_masks[i]);
+ kfree(vp_dev->msix_affinity_masks);
+ vp_dev->msix_affinity_masks = NULL;
+ }
- if (vp_dev->msix_enabled) {
- /* Disable the vector used for configuration */
+ /* Disable the vector used for configuration */
+ if (vp_dev->pci_dev->msix_enabled)
vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
- pci_free_irq_vectors(vp_dev->pci_dev);
- vp_dev->msix_enabled = 0;
- }
+ pci_free_irq_vectors(vp_dev->pci_dev);
+ vp_dev->irq_vectors = 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;
}
@@ -260,7 +248,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
const char * const names[])
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
- int i, err, nvectors = 1, allocated_vectors;
+ int i, err, nvectors = 1;
u16 msix_vec;
vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
@@ -275,15 +263,13 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
if (err)
goto out_del_vqs;
- vp_dev->per_vq_vectors = true;
- allocated_vectors = vp_dev->msix_used_vectors;
for (i = 0; i < nvqs; ++i) {
if (!names[i])
continue;
if (!callbacks[i])
msix_vec = VIRTIO_MSI_NO_VECTOR;
else
- msix_vec = allocated_vectors++;
+ msix_vec = vp_dev->irq_vectors++;
vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], msix_vec);
if (IS_ERR(vqs[i])) {
@@ -319,12 +305,11 @@ static int vp_find_vqs_msix_shared(struct virtio_device *vdev, unsigned nvqs,
err = vp_setup_msix_vectors(vdev, 2);
if (err)
goto out_del_vqs;
- err = vp_request_msix_vector(vp_dev, vp_dev->msix_used_vectors++,
+ err = vp_request_msix_vector(vp_dev, vp_dev->irq_vectors++,
vp_vring_interrupt, "virtqueues", vp_dev);
if (err)
goto out_del_vqs;
- vp_dev->per_vq_vectors = false;
for (i = 0; i < nvqs; ++i) {
if (!names[i])
continue;
@@ -358,9 +343,8 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned nvqs,
dev_name(&vdev->dev), vp_dev);
if (err)
goto out_del_vqs;
+ vp_dev->irq_vectors = 1;
- vp_dev->intx_enabled = 1;
- vp_dev->per_vq_vectors = false;
for (i = 0; i < nvqs; ++i) {
if (!names[i]) {
vqs[i] = NULL;
@@ -423,7 +407,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
if (!vq->callback)
return -EINVAL;
- if (vp_dev->msix_enabled) {
+ if (vp_dev->pci_dev->msix_enabled) {
mask = vp_dev->msix_affinity_masks[info->msix_vector];
irq = pci_irq_vector(vp_dev->pci_dev, info->msix_vector);
if (cpu == -1)
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index b2f6662..d3e3a1b 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -82,20 +82,12 @@ struct virtio_pci_device {
/* array of all queues for house-keeping */
struct virtio_pci_vq_info **vqs;
- /* MSI-X support */
- int msix_enabled;
- int intx_enabled;
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. */
char (*msix_names)[256];
- /* Number of available vectors */
- unsigned msix_vectors;
- /* Vectors allocated, excluding per-vq vectors if any */
- unsigned msix_used_vectors;
-
- /* Whether we have vector per vq */
- bool per_vq_vectors;
+ /* Total number of interrupt vectors (INTx or MSI-X) */
+ unsigned irq_vectors;
struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev,
struct virtio_pci_vq_info *info,
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index 8c4e617..25f90f0 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -169,7 +169,7 @@ static void del_vq(struct virtio_pci_vq_info *info)
iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
- if (vp_dev->msix_enabled) {
+ if (vp_dev->pci_dev->msix_enabled) {
iowrite16(VIRTIO_MSI_NO_VECTOR,
vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
/* Flush the write out to device */
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index e76bd91..e18d0b0 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -416,7 +416,7 @@ static void del_vq(struct virtio_pci_vq_info *info)
vp_iowrite16(vq->index, &vp_dev->common->queue_select);
- if (vp_dev->msix_enabled) {
+ if (vp_dev->pci_dev->msix_enabled) {
vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
&vp_dev->common->queue_msix_vector);
/* Flush the write out to device */
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 90007a1..15b4385 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -79,7 +79,7 @@
* configuration space */
#define VIRTIO_PCI_CONFIG_OFF(msix_enabled) ((msix_enabled) ? 24 : 20)
/* Deprecated: please use VIRTIO_PCI_CONFIG_OFF instead */
-#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->msix_enabled)
+#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->pci_dev->msix_enabled)
/* Virtio ABI version, this must match exactly */
#define VIRTIO_PCI_ABI_VERSION 0
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread