Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e969fed5 authored by Michael S. Tsirkin's avatar Michael S. Tsirkin Committed by Rusty Russell
Browse files

virtio: refactor find_vqs 



This refactors find_vqs, making it more readable and robust, and fixing
two regressions from 2.6.30:
- double free_irq causing BUG_ON on device removal
- probe failure when vq can't be assigned to msi-x vector
  (reported on old host kernels)

Tested-by: default avatarAmit Shah <amit.shah@redhat.com>
Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
parent f6c82507

drivers/virtio/virtio_pci.c

+119 −93
Original line number Diff line number Diff line
@@ -52,8 +52,10 @@ struct virtio_pci_device 
	char (*msix_names)[256];

	/* Number of available vectors */

	unsigned msix_vectors;

	/* Vectors allocated */

	/* Vectors allocated, excluding per-vq vectors if any */

	unsigned msix_used_vectors;

	/* Whether we have vector per vq */

	bool per_vq_vectors;

};



/* Constants for MSI-X */
@@ -278,27 +280,24 @@ static void vp_free_vectors(struct virtio_device *vdev) 
	vp_dev->msix_entries = NULL;

}



static int vp_enable_msix(struct pci_dev *dev, struct msix_entry *entries,

			  int *options, int noptions)

{

	int i;

	for (i = 0; i < noptions; ++i)

		if (!pci_enable_msix(dev, entries, options[i]))

			return options[i];

	return -EBUSY;

}



static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs)

static int vp_request_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;

	/* 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]);



	if (!nvectors) {

		/* Can't allocate 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)

			return err;

		vp_dev->intx_enabled = 1;

		return 0;

	}



	vp_dev->msix_entries = kmalloc(nvectors * sizeof *vp_dev->msix_entries,

				       GFP_KERNEL);
@@ -312,18 +311,12 @@ static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs) 
	for (i = 0; i < nvectors; ++i)

		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);

	err = pci_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries, nvectors);

	if (err > 0)

		err = -ENOSPC;

	if (err)

		goto error;

		vp_dev->intx_enabled = 1;

	} else {

		vp_dev->msix_vectors = err;

	vp_dev->msix_vectors = nvectors;

	vp_dev->msix_enabled = 1;



	/* Set the vector used for configuration */
@@ -344,9 +337,8 @@ static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs) 
		err = -EBUSY;

		goto error;

	}

	}



	if (vp_dev->msix_vectors && vp_dev->msix_vectors != max_vqs + 1) {

	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,
@@ -366,13 +358,14 @@ static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs) 


static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,

				    void (*callback)(struct virtqueue *vq),

				    const char *name)

				    const char *name,

				    u16 vector)

{

	struct virtio_pci_device *vp_dev = to_vp_device(vdev);

	struct virtio_pci_vq_info *info;

	struct virtqueue *vq;

	unsigned long flags, size;

	u16 num, vector;

	u16 num;

	int err;



	/* Select the queue we're interested in */
@@ -391,7 +384,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index, 


	info->queue_index = index;

	info->num = num;

	info->vector = VIRTIO_MSI_NO_VECTOR;

	info->vector = vector;



	size = PAGE_ALIGN(vring_size(num, VIRTIO_PCI_VRING_ALIGN));

	info->queue = alloc_pages_exact(size, GFP_KERNEL|__GFP_ZERO);
@@ -415,22 +408,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index, 
	vq->priv = info;

	info->vq = vq;



	/* allocate per-vq vector if available and necessary */

	if (callback && vp_dev->msix_used_vectors < vp_dev->msix_vectors) {

		vector = vp_dev->msix_used_vectors;

		snprintf(vp_dev->msix_names[vector], sizeof *vp_dev->msix_names,

			 "%s-%s", dev_name(&vp_dev->vdev.dev), name);

		err = request_irq(vp_dev->msix_entries[vector].vector,

				  vring_interrupt, 0,

				  vp_dev->msix_names[vector], vq);

		if (err)

			goto out_request_irq;

		info->vector = vector;

		++vp_dev->msix_used_vectors;

	} else

		vector = VP_MSIX_VQ_VECTOR;



	 if (callback && vp_dev->msix_enabled) {

	 if (vector != VIRTIO_MSI_NO_VECTOR) {

		iowrite16(vector, vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);

		vector = ioread16(vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);

		if (vector == VIRTIO_MSI_NO_VECTOR) {
@@ -446,11 +424,6 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index, 
	return vq;



out_assign:

	if (info->vector != VIRTIO_MSI_NO_VECTOR) {

		free_irq(vp_dev->msix_entries[info->vector].vector, vq);

		--vp_dev->msix_used_vectors;

	}

out_request_irq:

	vring_del_virtqueue(vq);

out_activate_queue:

	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
@@ -472,9 +445,6 @@ static void vp_del_vq(struct virtqueue *vq) 


	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);



	if (info->vector != VIRTIO_MSI_NO_VECTOR)

		free_irq(vp_dev->msix_entries[info->vector].vector, vq);



	if (vp_dev->msix_enabled) {

		iowrite16(VIRTIO_MSI_NO_VECTOR,

			  vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
@@ -495,44 +465,100 @@ static void vp_del_vq(struct virtqueue *vq) 
/* the config->del_vqs() implementation */

static 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)

	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {

		info = vq->priv;

		if (vp_dev->per_vq_vectors)

			free_irq(vp_dev->msix_entries[info->vector].vector, vq);

		vp_del_vq(vq);

	}

	vp_dev->per_vq_vectors = false;



	vp_free_vectors(vdev);

}



/* the config->find_vqs() implementation */

static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,

static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,

			      struct virtqueue *vqs[],

			      vq_callback_t *callbacks[],

		       const char *names[])

			      const char *names[],

			      int nvectors,

			      bool per_vq_vectors)

{

	int vectors = 0;

	int i, err;



	/* How many vectors would we like? */

	for (i = 0; i < nvqs; ++i)

		if (callbacks[i])

			++vectors;

	struct virtio_pci_device *vp_dev = to_vp_device(vdev);

	u16 vector;

	int i, err, allocated_vectors;



	err = vp_request_vectors(vdev, vectors);

	err = vp_request_vectors(vdev, nvectors, per_vq_vectors);

	if (err)

		goto error_request;



	vp_dev->per_vq_vectors = per_vq_vectors;

	allocated_vectors = vp_dev->msix_used_vectors;

	for (i = 0; i < nvqs; ++i) {

		vqs[i] = vp_find_vq(vdev, i, callbacks[i], names[i]);

		if (IS_ERR(vqs[i]))

		if (!callbacks[i] || !vp_dev->msix_enabled)

			vector = VIRTIO_MSI_NO_VECTOR;

		else if (vp_dev->per_vq_vectors)

			vector = allocated_vectors++;

		else

			vector = VP_MSIX_VQ_VECTOR;

		vqs[i] = vp_find_vq(vdev, i, callbacks[i], names[i], vector);

		if (IS_ERR(vqs[i])) {

			err = PTR_ERR(vqs[i]);

			goto error_find;

		}

		/* allocate per-vq irq if available and necessary */

		if (vp_dev->per_vq_vectors && vector != VIRTIO_MSI_NO_VECTOR) {

			snprintf(vp_dev->msix_names[vector], sizeof *vp_dev->msix_names,

				 "%s-%s", dev_name(&vp_dev->vdev.dev), names[i]);

			err = request_irq(vp_dev->msix_entries[vector].vector,

					  vring_interrupt, 0,

					  vp_dev->msix_names[vector], vqs[i]);

			if (err) {

				vp_del_vq(vqs[i]);

				goto error_find;

			}

		}

	}

	return 0;



error_find:

	vp_del_vqs(vdev);



error_request:

	return PTR_ERR(vqs[i]);

	return err;

}



/* 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[])

{

	int vectors = 0;

	int i, uninitialized_var(err);



	/* How many vectors would we like? */

	for (i = 0; i < nvqs; ++i)

		if (callbacks[i])

			++vectors;



	/* We want at most one vector per queue and one for config changes. */

	err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names,

				 vectors + 1, true);

	if (!err)

		return 0;

	/* Fallback to separate vectors for config and a shared for queues. */

	err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names,

				 2, false);

	if (!err)

		return 0;

	/* Finally fall back to regular interrupts. */

	err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names,

				 0, false);

	return err;

}



static struct virtio_config_ops virtio_pci_config_ops = {