[PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Michael S. Tsirkin]

This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
and updates all drivers. This is needed for MSI support, because MSI
needs to know the total number of vectors upfront.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/block/virtio_blk.c          |   11 +++---
 drivers/char/hw_random/virtio-rng.c |   11 +++---
 drivers/char/virtio_console.c       |   27 ++++++--------
 drivers/lguest/lguest_device.c      |   49 +++++++++++++++++++++++++-
 drivers/net/virtio_net.c            |   48 +++++++++++---------------
 drivers/s390/kvm/kvm_virtio.c       |   64 +++++++++++++++++++++++++++++++++-
 drivers/virtio/virtio_balloon.c     |   29 +++++++---------
 drivers/virtio/virtio_pci.c         |   43 +++++++++++++++++++----
 include/linux/virtio_config.h       |   29 +++++++++++-----
 net/9p/trans_virtio.c               |    7 ++--
 10 files changed, 225 insertions(+), 93 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5d34764..7b7435d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -197,6 +197,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 	u64 cap;
 	u32 v;
 	u32 blk_size, sg_elems;
+	virtqueue_callback *callback[] = { blk_done };
 
 	if (index_to_minor(index) >= 1 << MINORBITS)
 		return -ENOSPC;
@@ -224,11 +225,9 @@ static int virtblk_probe(struct virtio_device *vdev)
 	sg_init_table(vblk->sg, vblk->sg_elems);
 
 	/* We expect one virtqueue, for output. */
-	vblk->vq = vdev->config->find_vq(vdev, 0, blk_done);
-	if (IS_ERR(vblk->vq)) {
-		err = PTR_ERR(vblk->vq);
+	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, callback);
+	if (err)
 		goto out_free_vblk;
-	}
 
 	vblk->pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
 	if (!vblk->pool) {
@@ -323,7 +322,7 @@ out_put_disk:
 out_mempool:
 	mempool_destroy(vblk->pool);
 out_free_vq:
-	vdev->config->del_vq(vblk->vq);
+	vdev->config->del_vqs(vdev);
 out_free_vblk:
 	kfree(vblk);
 out:
@@ -344,7 +343,7 @@ static void virtblk_remove(struct virtio_device *vdev)
 	blk_cleanup_queue(vblk->disk->queue);
 	put_disk(vblk->disk);
 	mempool_destroy(vblk->pool);
-	vdev->config->del_vq(vblk->vq);
+	vdev->config->del_vqs(vdev);
 	kfree(vblk);
 }
 
diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 86e83f8..18eabe4 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -91,16 +91,17 @@ static struct hwrng virtio_hwrng = {
 
 static int virtrng_probe(struct virtio_device *vdev)
 {
+	virtqueue_callback *callback[] = { random_recv_done };
 	int err;
 
 	/* We expect a single virtqueue. */
-	vq = vdev->config->find_vq(vdev, 0, random_recv_done);
-	if (IS_ERR(vq))
-		return PTR_ERR(vq);
+	err = vdev->config->find_vqs(vdev, 1, &vq, callback);
+	if (err)
+		return err;
 
 	err = hwrng_register(&virtio_hwrng);
 	if (err) {
-		vdev->config->del_vq(vq);
+		vdev->config->del_vqs(vdev);
 		return err;
 	}
 
@@ -112,7 +113,7 @@ static void virtrng_remove(struct virtio_device *vdev)
 {
 	vdev->config->reset(vdev);
 	hwrng_unregister(&virtio_hwrng);
-	vdev->config->del_vq(vq);
+	vdev->config->del_vqs(vdev);
 }
 
 static struct virtio_device_id id_table[] = {
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index ff6f5a4..1fd5376 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -188,6 +188,8 @@ static void hvc_handle_input(struct virtqueue *vq)
  * Finally we put our input buffer in the input queue, ready to receive. */
 static int __devinit virtcons_probe(struct virtio_device *dev)
 {
+	struct virtqueue *vqs[2];
+	virtqueue_callback *callbacks[2];
 	int err;
 
 	vdev = dev;
@@ -199,20 +201,17 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 		goto fail;
 	}
 
-	/* Find the input queue. */
+	/* Find the queues. */
 	/* FIXME: This is why we want to wean off hvc: we do nothing
 	 * when input comes in. */
-	in_vq = vdev->config->find_vq(vdev, 0, hvc_handle_input);
-	if (IS_ERR(in_vq)) {
-		err = PTR_ERR(in_vq);
+	callbacks[0] = hvc_handle_input;
+	callbacks[1] = NULL;
+	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks);
+	if (err)
 		goto free;
-	}
 
-	out_vq = vdev->config->find_vq(vdev, 1, NULL);
-	if (IS_ERR(out_vq)) {
-		err = PTR_ERR(out_vq);
-		goto free_in_vq;
-	}
+	in_vq = vqs[0];
+	out_vq = vqs[1];
 
 	/* Start using the new console output. */
 	virtio_cons.get_chars = get_chars;
@@ -233,17 +232,15 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	hvc = hvc_alloc(0, 0, &virtio_cons, PAGE_SIZE);
 	if (IS_ERR(hvc)) {
 		err = PTR_ERR(hvc);
-		goto free_out_vq;
+		goto free_vqs;
 	}
 
 	/* Register the input buffer the first time. */
 	add_inbuf();
 	return 0;
 
-free_out_vq:
-	vdev->config->del_vq(out_vq);
-free_in_vq:
-	vdev->config->del_vq(in_vq);
+free_vqs:
+	vdev->config->del_vqs(vdev);
 free:
 	kfree(inbuf);
 fail:
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index df44d96..53699aa 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -37,6 +37,10 @@ static inline void lguest_unmap(void *addr)
 struct lguest_device {
 	struct virtio_device vdev;
 
+	/* Array of virtqueues */
+	struct virtqueue **vqs;
+	int nvqs;
+
 	/* The entry in the lguest_devices page for this device. */
 	struct lguest_device_desc *desc;
 };
@@ -312,6 +316,47 @@ static void lg_del_vq(struct virtqueue *vq)
 	kfree(lvq);
 }
 
+static void lg_del_vqs(struct virtio_device *vdev)
+{
+	struct lguest_device *ldev = to_lgdev(vdev);
+	int i;
+	if (!ldev->vqs)
+		return;
+	for (i = 0; i < ldev->nvqs; ++i)
+		lg_del_vq(ldev->vqs[i]);
+	kfree(ldev->vqs);
+	ldev->vqs = NULL;
+	ldev->nvqs = 0;
+}
+
+static int lg_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+		       struct virtqueue *vqs[]
+		       void (*callbacks)[](struct virtqueue *))
+{
+	struct lguest_device *ldev = to_lgdev(vdev);
+	int i;
+
+	/* We must have this many virtqueues. */
+	if (nvqs > ldev->desc->num_vq)
+		return ERR_PTR(-ENOENT);
+
+	ldev->vqs = kmalloc(GFP_KERNEL, nvqs * sizeof *ldev->vqs);
+	if (!ldev->vqs)
+		return -ENOMEM;
+
+	for (i = 0; i < nvqs; ++i) {
+		ldev->vqs[i] = vqs[i] = lg_find_vq(vdev, i, callbacks[i]);
+		if (IS_ERR(vqs[i]))
+			goto error;
+		ldev->nvqs = i;
+	}
+	return 0;
+
+error:
+	vp_del_vqs(vdev);
+	return PTR_ERR(vqs[i]);
+}
+
 /* The ops structure which hooks everything together. */
 static struct virtio_config_ops lguest_config_ops = {
 	.get_features = lg_get_features,
@@ -321,8 +366,8 @@ static struct virtio_config_ops lguest_config_ops = {
 	.get_status = lg_get_status,
 	.set_status = lg_set_status,
 	.reset = lg_reset,
-	.find_vq = lg_find_vq,
-	.del_vq = lg_del_vq,
+	.find_vqs = lg_find_vqs,
+	.del_vqs = lg_del_vqs,
 };
 
 /* The root device for the lguest virtio devices.  This makes them appear as
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4d1d479..862a314 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -845,6 +845,9 @@ static int virtnet_probe(struct virtio_device *vdev)
 	int err;
 	struct net_device *dev;
 	struct virtnet_info *vi;
+	struct virtqueue *vqs[3];
+	virtqueue_callback *callbacks[3];
+	int nvqs;
 
 	/* Allocate ourselves a network device with room for our info */
 	dev = alloc_etherdev(sizeof(struct virtnet_info));
@@ -905,25 +908,23 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
 		vi->mergeable_rx_bufs = true;
 
-	/* We expect two virtqueues, receive then send. */
-	vi->rvq = vdev->config->find_vq(vdev, 0, skb_recv_done);
-	if (IS_ERR(vi->rvq)) {
-		err = PTR_ERR(vi->rvq);
+	/* We expect two virtqueues, receive then send,
+	 * and optionally control. */
+	callbacks[0] = skb_recv_done;
+	callbacks[1] = skb_xmit_done;
+	callbacks[2] = NULL;
+
+	nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
+
+	err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks);
+	if (err)
 		goto free;
-	}
 
-	vi->svq = vdev->config->find_vq(vdev, 1, skb_xmit_done);
-	if (IS_ERR(vi->svq)) {
-		err = PTR_ERR(vi->svq);
-		goto free_recv;
-	}
+	vi->rvq = vqs[0];
+	vi->svq = vqs[1];
 
 	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
-		vi->cvq = vdev->config->find_vq(vdev, 2, NULL);
-		if (IS_ERR(vi->cvq)) {
-			err = PTR_ERR(vi->svq);
-			goto free_send;
-		}
+		vi->cvq = vqs[2];
 
 		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
 			dev->features |= NETIF_F_HW_VLAN_FILTER;
@@ -941,7 +942,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 	err = register_netdev(dev);
 	if (err) {
 		pr_debug("virtio_net: registering device failed\n");
-		goto free_ctrl;
+		goto free_vqs;
 	}
 
 	/* Last of all, set up some receive buffers. */
@@ -962,13 +963,8 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 unregister:
 	unregister_netdev(dev);
-free_ctrl:
-	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ))
-		vdev->config->del_vq(vi->cvq);
-free_send:
-	vdev->config->del_vq(vi->svq);
-free_recv:
-	vdev->config->del_vq(vi->rvq);
+free_vqs:
+	vdev->config->del_vqs(vdev);
 free:
 	free_netdev(dev);
 	return err;
@@ -994,12 +990,10 @@ static void virtnet_remove(struct virtio_device *vdev)
 
 	BUG_ON(vi->num != 0);
 
-	vdev->config->del_vq(vi->svq);
-	vdev->config->del_vq(vi->rvq);
-	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ))
-		vdev->config->del_vq(vi->cvq);
 	unregister_netdev(vi->dev);
 
+	vdev->config->del_vqs(vi->vdev);
+
 	while (vi->pages)
 		__free_pages(get_a_page(vi, GFP_KERNEL), 0);
 
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index cbc8566..6787426 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -34,6 +34,11 @@ static void *kvm_devices;
 
 struct kvm_device {
 	struct virtio_device vdev;
+
+	/* Array of virtqueues */
+	struct virtqueue **vqs;
+	int nvqs;
+
 	struct kvm_device_desc *desc;
 };
 
@@ -226,6 +231,61 @@ static void kvm_del_vq(struct virtqueue *vq)
 				       KVM_S390_VIRTIO_RING_ALIGN));
 }
 
+static void kvm_del_vqs(struct virtio_device *vdev)
+{
+	struct kvm_device *kdev = to_kvmdev(vdev);
+	int i;
+	if (!kdev->vqs)
+		return;
+	for (i = 0; i < ldev->nvqs; ++i)
+		kvm_del_vq(kdev->vqs[i]);
+	kfree(ldev->vqs);
+	ldev->vqs = NULL;
+	ldev->nvqs = 0;
+}
+
+static int kvm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+			struct virtqueue *vqs[]
+			void (*callbacks)[](struct virtqueue *))
+{
+	struct kvm_device *kdev = to_kvmdev(vdev);
+	int i;
+
+	/* We must have this many virtqueues. */
+	if (nvqs > kdev->desc->num_vq)
+		return ERR_PTR(-ENOENT);
+
+	kdev->vqs = kmalloc(GFP_KERNEL, nvqs * sizeof *kdev->vqs);
+	if (!kdev->vqs)
+		return -ENOMEM;
+
+	for (i = 0; i < nvqs; ++i) {
+		kdev->vqs[i] = vqs[i] = kvm_find_vq(vdev, i, callbacks[i]);
+		if (IS_ERR(vqs[i]))
+			goto error;
+		kdev->nvqs = i;
+	}
+	return 0;
+
+error:
+	kvm_del_vqs(vdev);
+	return PTR_ERR(vqs[i]);
+}
+
+static void kvm_del_vqs(struct virtio_device *vdev)
+{
+	struct lguest_device *ldev = to_lgdev(vdev);
+	int i;
+
+	if (!ldev->vqs)
+		return;
+	for (i = 0; i < ldev->nvqs; ++i)
+		lg_del_vq(ldev->vqs[i]);
+	kfree(ldev->vqs);
+	ldev->vqs = NULL;
+	ldev->nvqs = 0;
+}
+
 /*
  * The config ops structure as defined by virtio config
  */
@@ -237,8 +297,8 @@ static struct virtio_config_ops kvm_vq_configspace_ops = {
 	.get_status = kvm_get_status,
 	.set_status = kvm_set_status,
 	.reset = kvm_reset,
-	.find_vq = kvm_find_vq,
-	.del_vq = kvm_del_vq,
+	.find_vqs = kvm_find_vqs,
+	.del_vqs = kvm_del_vqs,
 };
 
 /*
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 9c76a06..ef7c481 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -204,6 +204,8 @@ static int balloon(void *_vballoon)
 static int virtballoon_probe(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb;
+	struct virtqueue *vqs[2];
+	virtqueue_callback *callbacks[2];
 	int err;
 
 	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
@@ -218,22 +220,20 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	vb->vdev = vdev;
 
 	/* We expect two virtqueues. */
-	vb->inflate_vq = vdev->config->find_vq(vdev, 0, balloon_ack);
-	if (IS_ERR(vb->inflate_vq)) {
-		err = PTR_ERR(vb->inflate_vq);
+	callbacks[0] = balloon_ack;
+	callbacks[1] = balloon_ack;
+
+	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks);
+	if (err)
 		goto out_free_vb;
-	}
 
-	vb->deflate_vq = vdev->config->find_vq(vdev, 1, balloon_ack);
-	if (IS_ERR(vb->deflate_vq)) {
-		err = PTR_ERR(vb->deflate_vq);
-		goto out_del_inflate_vq;
-	}
+	vb->inflate_vq = vqs[0];
+	vb->deflate_vq = vqs[1];
 
 	vb->thread = kthread_run(balloon, vb, "vballoon");
 	if (IS_ERR(vb->thread)) {
 		err = PTR_ERR(vb->thread);
-		goto out_del_deflate_vq;
+		goto out_del_vqs;
 	}
 
 	vb->tell_host_first
@@ -241,10 +241,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
 
 	return 0;
 
-out_del_deflate_vq:
-	vdev->config->del_vq(vb->deflate_vq);
-out_del_inflate_vq:
-	vdev->config->del_vq(vb->inflate_vq);
+out_del_vqs:
+	vdev->config->del_vqs(vdev);
 out_free_vb:
 	kfree(vb);
 out:
@@ -264,8 +262,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	/* Now we reset the device so we can clean up the queues. */
 	vdev->config->reset(vdev);
 
-	vdev->config->del_vq(vb->deflate_vq);
-	vdev->config->del_vq(vb->inflate_vq);
+	vdev->config->del_vqs(vdev);
 	kfree(vb);
 }
 
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 330aacb..d5e030f 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -275,11 +275,7 @@ static void vp_del_vq(struct virtqueue *vq)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
 	struct virtio_pci_vq_info *info = vq->priv;
-	unsigned long flags, size;
-
-	spin_lock_irqsave(&vp_dev->lock, flags);
-	list_del(&info->node);
-	spin_unlock_irqrestore(&vp_dev->lock, flags);
+	unsigned long size;
 
 	vring_del_virtqueue(vq);
 
@@ -292,14 +288,47 @@ static void vp_del_vq(struct virtqueue *vq)
 	kfree(info);
 }
 
+static void vp_del_vqs(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtio_pci_vq_info *info;
+	struct list_head virtqueues;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp_dev->lock, flags);
+	list_replace_init(&vp_dev->virtqueues, &virtqueues);
+	spin_unlock_irqrestore(&vp_dev->lock, flags);
+
+	list_for_each_entry(info, &virtqueues, node)
+		vp_del_vq(info->vq);
+}
+
+static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+		       struct virtqueue *vqs[],
+		       virtqueue_callback *callbacks[])
+{
+	int i;
+
+	for (i = 0; i < nvqs; ++i) {
+		vqs[i] = vp_find_vq(vdev, i, callbacks[i]);
+		if (IS_ERR(vqs[i]))
+			goto error;
+	}
+	return 0;
+
+error:
+	vp_del_vqs(vdev);
+	return PTR_ERR(vqs[i]);
+}
+
 static struct virtio_config_ops virtio_pci_config_ops = {
 	.get		= vp_get,
 	.set		= vp_set,
 	.get_status	= vp_get_status,
 	.set_status	= vp_set_status,
 	.reset		= vp_reset,
-	.find_vq	= vp_find_vq,
-	.del_vq		= vp_del_vq,
+	.find_vqs	= vp_find_vqs,
+	.del_vqs	= vp_del_vqs,
 	.get_features	= vp_get_features,
 	.finalize_features = vp_finalize_features,
 };
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index bf8ec28..32f6ce8 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -49,15 +49,25 @@
  * @set_status: write the status byte
  *	vdev: the virtio_device
  *	status: the new status byte
+ * @request_vqs: request the specified number of virtqueues
+ *	vdev: the virtio_device
+ *	max_vqs: the max number of virtqueues we want
+ *      If supplied, must call before any virtqueues are instantiated.
+ *      To modify the max number of virtqueues after request_vqs has been
+ *      called, call free_vqs and then request_vqs with a new value.
+ * @free_vqs: cleanup resources allocated by request_vqs
+ *	vdev: the virtio_device
+ *      If supplied, must call after all virtqueues have been deleted.
  * @reset: reset the device
  *	vdev: the virtio device
  *	After this, status and feature negotiation must be done again
- * @find_vq: find a virtqueue and instantiate it.
+ * @find_vqs: find virtqueues and instantiate them.
  *	vdev: the virtio_device
- *	index: the 0-based virtqueue number in case there's more than one.
- *	callback: the virqtueue callback
- *	Returns the new virtqueue or ERR_PTR() (eg. -ENOENT).
- * @del_vq: free a virtqueue found by find_vq().
+ *	nvqs: the number of virtqueues to find
+ *	vqs: on success, includes new virtqueues
+ *	callbacks: array of callbacks, for each virtqueue
+ *	Returns 0 on success or error status
+ * @del_vqs: free virtqueues found by find_vqs().
  * @get_features: get the array of feature bits for this device.
  *	vdev: the virtio_device
  *	Returns the first 32 feature bits (all we currently need).
@@ -66,6 +76,7 @@
  *	This gives the final feature bits for the device: it can change
  *	the dev->feature bits if it wants.
  */
+typedef void virtqueue_callback(struct virtqueue *);
 struct virtio_config_ops
 {
 	void (*get)(struct virtio_device *vdev, unsigned offset,
@@ -75,10 +86,10 @@ struct virtio_config_ops
 	u8 (*get_status)(struct virtio_device *vdev);
 	void (*set_status)(struct virtio_device *vdev, u8 status);
 	void (*reset)(struct virtio_device *vdev);
-	struct virtqueue *(*find_vq)(struct virtio_device *vdev,
-				     unsigned index,
-				     void (*callback)(struct virtqueue *));
-	void (*del_vq)(struct virtqueue *vq);
+	int (*find_vqs)(struct virtio_device *, unsigned nvqs,
+			struct virtqueue *vqs[],
+			virtqueue_callback *callbacks[]);
+	void (*del_vqs)(struct virtio_device *);
 	u32 (*get_features)(struct virtio_device *vdev);
 	void (*finalize_features)(struct virtio_device *vdev);
 };
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index bb8579a..fd113a1 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -230,6 +230,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	int err;
 	struct virtio_chan *chan;
 	int index;
+	virtqueue_callback *callbacks[] = { req_done };
 
 	mutex_lock(&virtio_9p_lock);
 	index = chan_index++;
@@ -246,11 +247,9 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	chan->vdev = vdev;
 
 	/* We expect one virtqueue, for requests. */
-	chan->vq = vdev->config->find_vq(vdev, 0, req_done);
-	if (IS_ERR(chan->vq)) {
-		err = PTR_ERR(chan->vq);
+	err = vdev->config->find_vqs(vdev, 1, &chan->vq, callbacks);
+	if (err)
 		goto out_free_vq;
-	}
 	chan->vq->vdev->priv = chan;
 	spin_lock_init(&chan->lock);
 
-- 
1.6.3.rc3.1.g830204

[PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Michael S. Tsirkin]

This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
and updates all drivers. This is needed for MSI support, because MSI
needs to know the total number of vectors upfront.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/block/virtio_blk.c          |   11 +++---
 drivers/char/hw_random/virtio-rng.c |   11 +++---
 drivers/char/virtio_console.c       |   27 ++++++--------
 drivers/lguest/lguest_device.c      |   49 +++++++++++++++++++++++++-
 drivers/net/virtio_net.c            |   48 +++++++++++---------------
 drivers/s390/kvm/kvm_virtio.c       |   64 +++++++++++++++++++++++++++++++++-
 drivers/virtio/virtio_balloon.c     |   29 +++++++---------
 drivers/virtio/virtio_pci.c         |   43 +++++++++++++++++++----
 include/linux/virtio_config.h       |   29 +++++++++++-----
 net/9p/trans_virtio.c               |    7 ++--
 10 files changed, 225 insertions(+), 93 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5d34764..7b7435d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -197,6 +197,7 @@ static int virtblk_probe(struct virtio_device *vdev)
 	u64 cap;
 	u32 v;
 	u32 blk_size, sg_elems;
+	virtqueue_callback *callback[] = { blk_done };
 
 	if (index_to_minor(index) >= 1 << MINORBITS)
 		return -ENOSPC;
@@ -224,11 +225,9 @@ static int virtblk_probe(struct virtio_device *vdev)
 	sg_init_table(vblk->sg, vblk->sg_elems);
 
 	/* We expect one virtqueue, for output. */
-	vblk->vq = vdev->config->find_vq(vdev, 0, blk_done);
-	if (IS_ERR(vblk->vq)) {
-		err = PTR_ERR(vblk->vq);
+	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, callback);
+	if (err)
 		goto out_free_vblk;
-	}
 
 	vblk->pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
 	if (!vblk->pool) {
@@ -323,7 +322,7 @@ out_put_disk:
 out_mempool:
 	mempool_destroy(vblk->pool);
 out_free_vq:
-	vdev->config->del_vq(vblk->vq);
+	vdev->config->del_vqs(vdev);
 out_free_vblk:
 	kfree(vblk);
 out:
@@ -344,7 +343,7 @@ static void virtblk_remove(struct virtio_device *vdev)
 	blk_cleanup_queue(vblk->disk->queue);
 	put_disk(vblk->disk);
 	mempool_destroy(vblk->pool);
-	vdev->config->del_vq(vblk->vq);
+	vdev->config->del_vqs(vdev);
 	kfree(vblk);
 }
 
diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 86e83f8..18eabe4 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -91,16 +91,17 @@ static struct hwrng virtio_hwrng = {
 
 static int virtrng_probe(struct virtio_device *vdev)
 {
+	virtqueue_callback *callback[] = { random_recv_done };
 	int err;
 
 	/* We expect a single virtqueue. */
-	vq = vdev->config->find_vq(vdev, 0, random_recv_done);
-	if (IS_ERR(vq))
-		return PTR_ERR(vq);
+	err = vdev->config->find_vqs(vdev, 1, &vq, callback);
+	if (err)
+		return err;
 
 	err = hwrng_register(&virtio_hwrng);
 	if (err) {
-		vdev->config->del_vq(vq);
+		vdev->config->del_vqs(vdev);
 		return err;
 	}
 
@@ -112,7 +113,7 @@ static void virtrng_remove(struct virtio_device *vdev)
 {
 	vdev->config->reset(vdev);
 	hwrng_unregister(&virtio_hwrng);
-	vdev->config->del_vq(vq);
+	vdev->config->del_vqs(vdev);
 }
 
 static struct virtio_device_id id_table[] = {
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index ff6f5a4..1fd5376 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -188,6 +188,8 @@ static void hvc_handle_input(struct virtqueue *vq)
  * Finally we put our input buffer in the input queue, ready to receive. */
 static int __devinit virtcons_probe(struct virtio_device *dev)
 {
+	struct virtqueue *vqs[2];
+	virtqueue_callback *callbacks[2];
 	int err;
 
 	vdev = dev;
@@ -199,20 +201,17 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 		goto fail;
 	}
 
-	/* Find the input queue. */
+	/* Find the queues. */
 	/* FIXME: This is why we want to wean off hvc: we do nothing
 	 * when input comes in. */
-	in_vq = vdev->config->find_vq(vdev, 0, hvc_handle_input);
-	if (IS_ERR(in_vq)) {
-		err = PTR_ERR(in_vq);
+	callbacks[0] = hvc_handle_input;
+	callbacks[1] = NULL;
+	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks);
+	if (err)
 		goto free;
-	}
 
-	out_vq = vdev->config->find_vq(vdev, 1, NULL);
-	if (IS_ERR(out_vq)) {
-		err = PTR_ERR(out_vq);
-		goto free_in_vq;
-	}
+	in_vq = vqs[0];
+	out_vq = vqs[1];
 
 	/* Start using the new console output. */
 	virtio_cons.get_chars = get_chars;
@@ -233,17 +232,15 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	hvc = hvc_alloc(0, 0, &virtio_cons, PAGE_SIZE);
 	if (IS_ERR(hvc)) {
 		err = PTR_ERR(hvc);
-		goto free_out_vq;
+		goto free_vqs;
 	}
 
 	/* Register the input buffer the first time. */
 	add_inbuf();
 	return 0;
 
-free_out_vq:
-	vdev->config->del_vq(out_vq);
-free_in_vq:
-	vdev->config->del_vq(in_vq);
+free_vqs:
+	vdev->config->del_vqs(vdev);
 free:
 	kfree(inbuf);
 fail:
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index df44d96..53699aa 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -37,6 +37,10 @@ static inline void lguest_unmap(void *addr)
 struct lguest_device {
 	struct virtio_device vdev;
 
+	/* Array of virtqueues */
+	struct virtqueue **vqs;
+	int nvqs;
+
 	/* The entry in the lguest_devices page for this device. */
 	struct lguest_device_desc *desc;
 };
@@ -312,6 +316,47 @@ static void lg_del_vq(struct virtqueue *vq)
 	kfree(lvq);
 }
 
+static void lg_del_vqs(struct virtio_device *vdev)
+{
+	struct lguest_device *ldev = to_lgdev(vdev);
+	int i;
+	if (!ldev->vqs)
+		return;
+	for (i = 0; i < ldev->nvqs; ++i)
+		lg_del_vq(ldev->vqs[i]);
+	kfree(ldev->vqs);
+	ldev->vqs = NULL;
+	ldev->nvqs = 0;
+}
+
+static int lg_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+		       struct virtqueue *vqs[]
+		       void (*callbacks)[](struct virtqueue *))
+{
+	struct lguest_device *ldev = to_lgdev(vdev);
+	int i;
+
+	/* We must have this many virtqueues. */
+	if (nvqs > ldev->desc->num_vq)
+		return ERR_PTR(-ENOENT);
+
+	ldev->vqs = kmalloc(GFP_KERNEL, nvqs * sizeof *ldev->vqs);
+	if (!ldev->vqs)
+		return -ENOMEM;
+
+	for (i = 0; i < nvqs; ++i) {
+		ldev->vqs[i] = vqs[i] = lg_find_vq(vdev, i, callbacks[i]);
+		if (IS_ERR(vqs[i]))
+			goto error;
+		ldev->nvqs = i;
+	}
+	return 0;
+
+error:
+	vp_del_vqs(vdev);
+	return PTR_ERR(vqs[i]);
+}
+
 /* The ops structure which hooks everything together. */
 static struct virtio_config_ops lguest_config_ops = {
 	.get_features = lg_get_features,
@@ -321,8 +366,8 @@ static struct virtio_config_ops lguest_config_ops = {
 	.get_status = lg_get_status,
 	.set_status = lg_set_status,
 	.reset = lg_reset,
-	.find_vq = lg_find_vq,
-	.del_vq = lg_del_vq,
+	.find_vqs = lg_find_vqs,
+	.del_vqs = lg_del_vqs,
 };
 
 /* The root device for the lguest virtio devices.  This makes them appear as
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4d1d479..862a314 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -845,6 +845,9 @@ static int virtnet_probe(struct virtio_device *vdev)
 	int err;
 	struct net_device *dev;
 	struct virtnet_info *vi;
+	struct virtqueue *vqs[3];
+	virtqueue_callback *callbacks[3];
+	int nvqs;
 
 	/* Allocate ourselves a network device with room for our info */
 	dev = alloc_etherdev(sizeof(struct virtnet_info));
@@ -905,25 +908,23 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
 		vi->mergeable_rx_bufs = true;
 
-	/* We expect two virtqueues, receive then send. */
-	vi->rvq = vdev->config->find_vq(vdev, 0, skb_recv_done);
-	if (IS_ERR(vi->rvq)) {
-		err = PTR_ERR(vi->rvq);
+	/* We expect two virtqueues, receive then send,
+	 * and optionally control. */
+	callbacks[0] = skb_recv_done;
+	callbacks[1] = skb_xmit_done;
+	callbacks[2] = NULL;
+
+	nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
+
+	err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks);
+	if (err)
 		goto free;
-	}
 
-	vi->svq = vdev->config->find_vq(vdev, 1, skb_xmit_done);
-	if (IS_ERR(vi->svq)) {
-		err = PTR_ERR(vi->svq);
-		goto free_recv;
-	}
+	vi->rvq = vqs[0];
+	vi->svq = vqs[1];
 
 	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
-		vi->cvq = vdev->config->find_vq(vdev, 2, NULL);
-		if (IS_ERR(vi->cvq)) {
-			err = PTR_ERR(vi->svq);
-			goto free_send;
-		}
+		vi->cvq = vqs[2];
 
 		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
 			dev->features |= NETIF_F_HW_VLAN_FILTER;
@@ -941,7 +942,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 	err = register_netdev(dev);
 	if (err) {
 		pr_debug("virtio_net: registering device failed\n");
-		goto free_ctrl;
+		goto free_vqs;
 	}
 
 	/* Last of all, set up some receive buffers. */
@@ -962,13 +963,8 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 unregister:
 	unregister_netdev(dev);
-free_ctrl:
-	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ))
-		vdev->config->del_vq(vi->cvq);
-free_send:
-	vdev->config->del_vq(vi->svq);
-free_recv:
-	vdev->config->del_vq(vi->rvq);
+free_vqs:
+	vdev->config->del_vqs(vdev);
 free:
 	free_netdev(dev);
 	return err;
@@ -994,12 +990,10 @@ static void virtnet_remove(struct virtio_device *vdev)
 
 	BUG_ON(vi->num != 0);
 
-	vdev->config->del_vq(vi->svq);
-	vdev->config->del_vq(vi->rvq);
-	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ))
-		vdev->config->del_vq(vi->cvq);
 	unregister_netdev(vi->dev);
 
+	vdev->config->del_vqs(vi->vdev);
+
 	while (vi->pages)
 		__free_pages(get_a_page(vi, GFP_KERNEL), 0);
 
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index cbc8566..6787426 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -34,6 +34,11 @@ static void *kvm_devices;
 
 struct kvm_device {
 	struct virtio_device vdev;
+
+	/* Array of virtqueues */
+	struct virtqueue **vqs;
+	int nvqs;
+
 	struct kvm_device_desc *desc;
 };
 
@@ -226,6 +231,61 @@ static void kvm_del_vq(struct virtqueue *vq)
 				       KVM_S390_VIRTIO_RING_ALIGN));
 }
 
+static void kvm_del_vqs(struct virtio_device *vdev)
+{
+	struct kvm_device *kdev = to_kvmdev(vdev);
+	int i;
+	if (!kdev->vqs)
+		return;
+	for (i = 0; i < ldev->nvqs; ++i)
+		kvm_del_vq(kdev->vqs[i]);
+	kfree(ldev->vqs);
+	ldev->vqs = NULL;
+	ldev->nvqs = 0;
+}
+
+static int kvm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+			struct virtqueue *vqs[]
+			void (*callbacks)[](struct virtqueue *))
+{
+	struct kvm_device *kdev = to_kvmdev(vdev);
+	int i;
+
+	/* We must have this many virtqueues. */
+	if (nvqs > kdev->desc->num_vq)
+		return ERR_PTR(-ENOENT);
+
+	kdev->vqs = kmalloc(GFP_KERNEL, nvqs * sizeof *kdev->vqs);
+	if (!kdev->vqs)
+		return -ENOMEM;
+
+	for (i = 0; i < nvqs; ++i) {
+		kdev->vqs[i] = vqs[i] = kvm_find_vq(vdev, i, callbacks[i]);
+		if (IS_ERR(vqs[i]))
+			goto error;
+		kdev->nvqs = i;
+	}
+	return 0;
+
+error:
+	kvm_del_vqs(vdev);
+	return PTR_ERR(vqs[i]);
+}
+
+static void kvm_del_vqs(struct virtio_device *vdev)
+{
+	struct lguest_device *ldev = to_lgdev(vdev);
+	int i;
+
+	if (!ldev->vqs)
+		return;
+	for (i = 0; i < ldev->nvqs; ++i)
+		lg_del_vq(ldev->vqs[i]);
+	kfree(ldev->vqs);
+	ldev->vqs = NULL;
+	ldev->nvqs = 0;
+}
+
 /*
  * The config ops structure as defined by virtio config
  */
@@ -237,8 +297,8 @@ static struct virtio_config_ops kvm_vq_configspace_ops = {
 	.get_status = kvm_get_status,
 	.set_status = kvm_set_status,
 	.reset = kvm_reset,
-	.find_vq = kvm_find_vq,
-	.del_vq = kvm_del_vq,
+	.find_vqs = kvm_find_vqs,
+	.del_vqs = kvm_del_vqs,
 };
 
 /*
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 9c76a06..ef7c481 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -204,6 +204,8 @@ static int balloon(void *_vballoon)
 static int virtballoon_probe(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb;
+	struct virtqueue *vqs[2];
+	virtqueue_callback *callbacks[2];
 	int err;
 
 	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
@@ -218,22 +220,20 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	vb->vdev = vdev;
 
 	/* We expect two virtqueues. */
-	vb->inflate_vq = vdev->config->find_vq(vdev, 0, balloon_ack);
-	if (IS_ERR(vb->inflate_vq)) {
-		err = PTR_ERR(vb->inflate_vq);
+	callbacks[0] = balloon_ack;
+	callbacks[1] = balloon_ack;
+
+	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks);
+	if (err)
 		goto out_free_vb;
-	}
 
-	vb->deflate_vq = vdev->config->find_vq(vdev, 1, balloon_ack);
-	if (IS_ERR(vb->deflate_vq)) {
-		err = PTR_ERR(vb->deflate_vq);
-		goto out_del_inflate_vq;
-	}
+	vb->inflate_vq = vqs[0];
+	vb->deflate_vq = vqs[1];
 
 	vb->thread = kthread_run(balloon, vb, "vballoon");
 	if (IS_ERR(vb->thread)) {
 		err = PTR_ERR(vb->thread);
-		goto out_del_deflate_vq;
+		goto out_del_vqs;
 	}
 
 	vb->tell_host_first
@@ -241,10 +241,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
 
 	return 0;
 
-out_del_deflate_vq:
-	vdev->config->del_vq(vb->deflate_vq);
-out_del_inflate_vq:
-	vdev->config->del_vq(vb->inflate_vq);
+out_del_vqs:
+	vdev->config->del_vqs(vdev);
 out_free_vb:
 	kfree(vb);
 out:
@@ -264,8 +262,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	/* Now we reset the device so we can clean up the queues. */
 	vdev->config->reset(vdev);
 
-	vdev->config->del_vq(vb->deflate_vq);
-	vdev->config->del_vq(vb->inflate_vq);
+	vdev->config->del_vqs(vdev);
 	kfree(vb);
 }
 
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 330aacb..d5e030f 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -275,11 +275,7 @@ static void vp_del_vq(struct virtqueue *vq)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
 	struct virtio_pci_vq_info *info = vq->priv;
-	unsigned long flags, size;
-
-	spin_lock_irqsave(&vp_dev->lock, flags);
-	list_del(&info->node);
-	spin_unlock_irqrestore(&vp_dev->lock, flags);
+	unsigned long size;
 
 	vring_del_virtqueue(vq);
 
@@ -292,14 +288,47 @@ static void vp_del_vq(struct virtqueue *vq)
 	kfree(info);
 }
 
+static void vp_del_vqs(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtio_pci_vq_info *info;
+	struct list_head virtqueues;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp_dev->lock, flags);
+	list_replace_init(&vp_dev->virtqueues, &virtqueues);
+	spin_unlock_irqrestore(&vp_dev->lock, flags);
+
+	list_for_each_entry(info, &virtqueues, node)
+		vp_del_vq(info->vq);
+}
+
+static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+		       struct virtqueue *vqs[],
+		       virtqueue_callback *callbacks[])
+{
+	int i;
+
+	for (i = 0; i < nvqs; ++i) {
+		vqs[i] = vp_find_vq(vdev, i, callbacks[i]);
+		if (IS_ERR(vqs[i]))
+			goto error;
+	}
+	return 0;
+
+error:
+	vp_del_vqs(vdev);
+	return PTR_ERR(vqs[i]);
+}
+
 static struct virtio_config_ops virtio_pci_config_ops = {
 	.get		= vp_get,
 	.set		= vp_set,
 	.get_status	= vp_get_status,
 	.set_status	= vp_set_status,
 	.reset		= vp_reset,
-	.find_vq	= vp_find_vq,
-	.del_vq		= vp_del_vq,
+	.find_vqs	= vp_find_vqs,
+	.del_vqs	= vp_del_vqs,
 	.get_features	= vp_get_features,
 	.finalize_features = vp_finalize_features,
 };
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index bf8ec28..32f6ce8 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -49,15 +49,25 @@
  * @set_status: write the status byte
  *	vdev: the virtio_device
  *	status: the new status byte
+ * @request_vqs: request the specified number of virtqueues
+ *	vdev: the virtio_device
+ *	max_vqs: the max number of virtqueues we want
+ *      If supplied, must call before any virtqueues are instantiated.
+ *      To modify the max number of virtqueues after request_vqs has been
+ *      called, call free_vqs and then request_vqs with a new value.
+ * @free_vqs: cleanup resources allocated by request_vqs
+ *	vdev: the virtio_device
+ *      If supplied, must call after all virtqueues have been deleted.
  * @reset: reset the device
  *	vdev: the virtio device
  *	After this, status and feature negotiation must be done again
- * @find_vq: find a virtqueue and instantiate it.
+ * @find_vqs: find virtqueues and instantiate them.
  *	vdev: the virtio_device
- *	index: the 0-based virtqueue number in case there's more than one.
- *	callback: the virqtueue callback
- *	Returns the new virtqueue or ERR_PTR() (eg. -ENOENT).
- * @del_vq: free a virtqueue found by find_vq().
+ *	nvqs: the number of virtqueues to find
+ *	vqs: on success, includes new virtqueues
+ *	callbacks: array of callbacks, for each virtqueue
+ *	Returns 0 on success or error status
+ * @del_vqs: free virtqueues found by find_vqs().
  * @get_features: get the array of feature bits for this device.
  *	vdev: the virtio_device
  *	Returns the first 32 feature bits (all we currently need).
@@ -66,6 +76,7 @@
  *	This gives the final feature bits for the device: it can change
  *	the dev->feature bits if it wants.
  */
+typedef void virtqueue_callback(struct virtqueue *);
 struct virtio_config_ops
 {
 	void (*get)(struct virtio_device *vdev, unsigned offset,
@@ -75,10 +86,10 @@ struct virtio_config_ops
 	u8 (*get_status)(struct virtio_device *vdev);
 	void (*set_status)(struct virtio_device *vdev, u8 status);
 	void (*reset)(struct virtio_device *vdev);
-	struct virtqueue *(*find_vq)(struct virtio_device *vdev,
-				     unsigned index,
-				     void (*callback)(struct virtqueue *));
-	void (*del_vq)(struct virtqueue *vq);
+	int (*find_vqs)(struct virtio_device *, unsigned nvqs,
+			struct virtqueue *vqs[],
+			virtqueue_callback *callbacks[]);
+	void (*del_vqs)(struct virtio_device *);
 	u32 (*get_features)(struct virtio_device *vdev);
 	void (*finalize_features)(struct virtio_device *vdev);
 };
diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index bb8579a..fd113a1 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -230,6 +230,7 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	int err;
 	struct virtio_chan *chan;
 	int index;
+	virtqueue_callback *callbacks[] = { req_done };
 
 	mutex_lock(&virtio_9p_lock);
 	index = chan_index++;
@@ -246,11 +247,9 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	chan->vdev = vdev;
 
 	/* We expect one virtqueue, for requests. */
-	chan->vq = vdev->config->find_vq(vdev, 0, req_done);
-	if (IS_ERR(chan->vq)) {
-		err = PTR_ERR(chan->vq);
+	err = vdev->config->find_vqs(vdev, 1, &chan->vq, callbacks);
+	if (err)
 		goto out_free_vq;
-	}
 	chan->vq->vdev->priv = chan;
 	spin_lock_init(&chan->lock);
 
-- 
1.6.3.rc3.1.g830204

[PATCH 2/3] virtio_pci: split up vp_interrupt

[Michael S. Tsirkin]

This reorganizes virtio-pci code in vp_interrupt slightly, so that
it's easier to add per-vq MSI support on top.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c |   53 +++++++++++++++++++++++++++---------------
 1 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index d5e030f..a6bebe2 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -164,6 +164,37 @@ static void vp_notify(struct virtqueue *vq)
 	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY);
 }
 
+/* Handle a configuration change: Tell driver if it wants to know. */
+static irqreturn_t vp_config_changed(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_driver *drv;
+	drv = container_of(vp_dev->vdev.dev.driver,
+			   struct virtio_driver, driver);
+
+	if (drv && drv->config_changed)
+		drv->config_changed(&vp_dev->vdev);
+	return IRQ_HANDLED;
+}
+
+/* Notify all virtqueues on an interrupt. */
+static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_pci_vq_info *info;
+	irqreturn_t ret = IRQ_NONE;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp_dev->lock, flags);
+	list_for_each_entry(info, &vp_dev->virtqueues, node) {
+		if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+			ret = IRQ_HANDLED;
+	}
+	spin_unlock_irqrestore(&vp_dev->lock, flags);
+
+	return ret;
+}
+
 /* A small wrapper to also acknowledge the interrupt when it's handled.
  * I really need an EIO hook for the vring so I can ack the interrupt once we
  * know that we'll be handling the IRQ but before we invoke the callback since
@@ -173,9 +204,6 @@ static void vp_notify(struct virtqueue *vq)
 static irqreturn_t vp_interrupt(int irq, void *opaque)
 {
 	struct virtio_pci_device *vp_dev = opaque;
-	struct virtio_pci_vq_info *info;
-	irqreturn_t ret = IRQ_NONE;
-	unsigned long flags;
 	u8 isr;
 
 	/* reading the ISR has the effect of also clearing it so it's very
@@ -187,23 +215,10 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
 		return IRQ_NONE;
 
 	/* Configuration change?  Tell driver if it wants to know. */
-	if (isr & VIRTIO_PCI_ISR_CONFIG) {
-		struct virtio_driver *drv;
-		drv = container_of(vp_dev->vdev.dev.driver,
-				   struct virtio_driver, driver);
-
-		if (drv && drv->config_changed)
-			drv->config_changed(&vp_dev->vdev);
-	}
+	if (isr & VIRTIO_PCI_ISR_CONFIG)
+		vp_config_changed(irq, opaque);
 
-	spin_lock_irqsave(&vp_dev->lock, flags);
-	list_for_each_entry(info, &vp_dev->virtqueues, node) {
-		if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
-			ret = IRQ_HANDLED;
-	}
-	spin_unlock_irqrestore(&vp_dev->lock, flags);
-
-	return ret;
+	return vp_vring_interrupt(irq, opaque);
 }
 
 /* the config->find_vq() implementation */
-- 
1.6.3.rc3.1.g830204

[PATCH 2/3] virtio_pci: split up vp_interrupt

[Michael S. Tsirkin]

This reorganizes virtio-pci code in vp_interrupt slightly, so that
it's easier to add per-vq MSI support on top.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c |   53 +++++++++++++++++++++++++++---------------
 1 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index d5e030f..a6bebe2 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -164,6 +164,37 @@ static void vp_notify(struct virtqueue *vq)
 	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY);
 }
 
+/* Handle a configuration change: Tell driver if it wants to know. */
+static irqreturn_t vp_config_changed(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_driver *drv;
+	drv = container_of(vp_dev->vdev.dev.driver,
+			   struct virtio_driver, driver);
+
+	if (drv && drv->config_changed)
+		drv->config_changed(&vp_dev->vdev);
+	return IRQ_HANDLED;
+}
+
+/* Notify all virtqueues on an interrupt. */
+static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_pci_vq_info *info;
+	irqreturn_t ret = IRQ_NONE;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp_dev->lock, flags);
+	list_for_each_entry(info, &vp_dev->virtqueues, node) {
+		if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+			ret = IRQ_HANDLED;
+	}
+	spin_unlock_irqrestore(&vp_dev->lock, flags);
+
+	return ret;
+}
+
 /* A small wrapper to also acknowledge the interrupt when it's handled.
  * I really need an EIO hook for the vring so I can ack the interrupt once we
  * know that we'll be handling the IRQ but before we invoke the callback since
@@ -173,9 +204,6 @@ static void vp_notify(struct virtqueue *vq)
 static irqreturn_t vp_interrupt(int irq, void *opaque)
 {
 	struct virtio_pci_device *vp_dev = opaque;
-	struct virtio_pci_vq_info *info;
-	irqreturn_t ret = IRQ_NONE;
-	unsigned long flags;
 	u8 isr;
 
 	/* reading the ISR has the effect of also clearing it so it's very
@@ -187,23 +215,10 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
 		return IRQ_NONE;
 
 	/* Configuration change?  Tell driver if it wants to know. */
-	if (isr & VIRTIO_PCI_ISR_CONFIG) {
-		struct virtio_driver *drv;
-		drv = container_of(vp_dev->vdev.dev.driver,
-				   struct virtio_driver, driver);
-
-		if (drv && drv->config_changed)
-			drv->config_changed(&vp_dev->vdev);
-	}
+	if (isr & VIRTIO_PCI_ISR_CONFIG)
+		vp_config_changed(irq, opaque);
 
-	spin_lock_irqsave(&vp_dev->lock, flags);
-	list_for_each_entry(info, &vp_dev->virtqueues, node) {
-		if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
-			ret = IRQ_HANDLED;
-	}
-	spin_unlock_irqrestore(&vp_dev->lock, flags);
-
-	return ret;
+	return vp_vring_interrupt(irq, opaque);
 }
 
 /* the config->find_vq() implementation */
-- 
1.6.3.rc3.1.g830204

[PATCH 3/3] virtio_pci: optional MSI-X support

[Michael S. Tsirkin]

This implements optional MSI-X support in virtio_pci.
MSI-X is used whenever the host supports at least 2 MSI-X
vectors: 1 for configuration changes and 1 for virtqueues.
Per-virtqueue vectors are allocated if enough vectors
available.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c |  211 +++++++++++++++++++++++++++++++++++++++----
 include/linux/virtio_pci.h  |    8 ++-
 2 files changed, 199 insertions(+), 20 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index a6bebe2..d21e2e6 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -42,6 +42,29 @@ struct virtio_pci_device
 	/* a list of queues so we can dispatch IRQs */
 	spinlock_t lock;
 	struct list_head virtqueues;
+
+	/* MSI-X support */
+	int msix_enabled;
+	int intx_enabled;
+	struct msix_entry *msix_entries;
+	/* 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 vectors configured at startup (excludes per-virtqueue
+	 * vectors if any) */
+	unsigned msix_preset_vectors;
+	/* Number of per-virtqueue vectors if any. */
+	unsigned msix_per_vq_vectors;
+};
+
+/* Constants for MSI-X */
+/* Use first vector for configuration changes, second and the rest for
+ * virtqueues Thus, we need at least 2 vectors for MSI. */
+enum {
+	VP_MSIX_CONFIG_VECTOR = 0,
+	VP_MSIX_VQ_VECTOR = 1,
+	VP_MSIX_MIN_VECTORS = 2,
+	VP_MSIX_NO_VECTOR = 0xffff,
 };
 
 struct virtio_pci_vq_info
@@ -60,6 +83,9 @@ struct virtio_pci_vq_info
 
 	/* the list node for the virtqueues list */
 	struct list_head node;
+
+	/* MSI-X vector (or none) */
+	unsigned vector;
 };
 
 /* Qumranet donated their vendor ID for devices 0x1000 thru 0x10FF. */
@@ -109,7 +135,8 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
 		   void *buf, unsigned len)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
-	void __iomem *ioaddr = vp_dev->ioaddr + VIRTIO_PCI_CONFIG + offset;
+	void __iomem *ioaddr = vp_dev->ioaddr +
+                               VIRTIO_PCI_CONFIG(vp_dev) + offset;
 	u8 *ptr = buf;
 	int i;
 
@@ -123,7 +150,8 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
 		   const void *buf, unsigned len)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
-	void __iomem *ioaddr = vp_dev->ioaddr + VIRTIO_PCI_CONFIG + offset;
+	void __iomem *ioaddr = vp_dev->ioaddr +
+		               VIRTIO_PCI_CONFIG(vp_dev) + offset;
 	const u8 *ptr = buf;
 	int i;
 
@@ -221,7 +249,116 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
 	return vp_vring_interrupt(irq, opaque);
 }
 
-/* the config->find_vq() implementation */
+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_preset_vectors; ++i)
+		free_irq(vp_dev->msix_entries[i].vector, vp_dev);
+	vp_dev->msix_preset_vectors = 0;
+
+	if (vp_dev->msix_enabled) {
+		/* Disable the vector used for configuration */
+		iowrite16(VP_MSIX_NO_VECTOR,
+			  vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
+
+		vp_dev->msix_enabled = 0;
+		pci_disable_msix(vp_dev->pci_dev);
+	}
+}
+
+static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	const char *name = dev_name(&vp_dev->vdev.dev);
+	unsigned i, vectors;
+	int err = -ENOMEM;
+
+	/* We need at most one vector per queue and one for config changes */
+	vectors = VP_MSIX_VQ_VECTOR + max_vqs;
+	vp_dev->msix_entries = kmalloc(vectors * sizeof *vp_dev->msix_entries,
+				       GFP_KERNEL);
+	if (!vp_dev->msix_entries)
+		goto error_entries;
+	vp_dev->msix_names = kmalloc(vectors * sizeof *vp_dev->msix_names,
+				     GFP_KERNEL);
+	if (!vp_dev->msix_names)
+		goto error_names;
+
+	snprintf(vp_dev->msix_names[VP_MSIX_CONFIG_VECTOR],
+		 sizeof *vp_dev->msix_names, "%s-config", name);
+	for (i = 0; i < max_vqs; ++i)
+		snprintf(vp_dev->msix_names[i + VP_MSIX_VQ_VECTOR],
+			 sizeof *vp_dev->msix_names, "%s-vq-%d", name, i);
+	for (i = 0; i < vectors; ++i)
+		vp_dev->msix_entries[i].entry = i;
+
+	vp_dev->msix_preset_vectors = 1;
+	vp_dev->msix_per_vq_vectors = max_vqs;
+	for (;;) {
+		err = pci_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries,
+				      vectors);
+		/* Error out if not enough vectors */
+		if (err > 0 && err < VP_MSIX_MIN_VECTORS)
+			err = -EBUSY;
+		if (err <= 0)
+			break;
+		/* Not enough vectors for all queues. Retry, disabling
+		 * per-queue interrupts */
+		vectors = VP_MSIX_MIN_VECTORS;
+		vp_dev->msix_preset_vectors = VP_MSIX_MIN_VECTORS;
+		vp_dev->msix_per_vq_vectors = 0;
+		snprintf(vp_dev->msix_names[VP_MSIX_VQ_VECTOR],
+			 sizeof *vp_dev->msix_names, "%s-vq", name);
+	}
+
+	if (!err)
+		vp_dev->msix_enabled = 1;
+	if (err) {
+		/* Can't allocate enough MSI-X vectors, use regular interrupt */
+		vp_dev->msix_enabled = 0;
+		vp_dev->msix_preset_vectors = 0;
+		vp_dev->msix_per_vq_vectors = 0;
+		/* Register a handler for the queue with the PCI device's
+		 * interrupt */
+		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;
+	}
+	for (i = 0; i < vp_dev->msix_preset_vectors; ++i) {
+		err = request_irq(vp_dev->msix_entries[i].vector,
+				  i == VP_MSIX_CONFIG_VECTOR ?
+				  vp_config_changed : vp_vring_interrupt,
+				  0, vp_dev->msix_names[i], vp_dev);
+		if (err) {
+			/* Set msix_preset_vectors so that only vectors we
+			 * already allocated will be freed by vp_free_vqs. */
+			vp_dev->msix_preset_vectors = i;
+			goto error_irq;
+		}
+
+		/* Set the vector used for configuration */
+		if (i == VP_MSIX_CONFIG_VECTOR)
+			iowrite16(VP_MSIX_CONFIG_VECTOR,
+				  vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
+	}
+	return 0;
+error_irq:
+	vp_free_vectors(vdev);
+	kfree(vp_dev->msix_names);
+error_names:
+	kfree(vp_dev->msix_entries);
+error_entries:
+	return err;
+}
+
 static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 				    void (*callback)(struct virtqueue *vq))
 {
@@ -229,7 +366,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 	struct virtio_pci_vq_info *info;
 	struct virtqueue *vq;
 	unsigned long flags, size;
-	u16 num;
+	u16 num, vector;
 	int err;
 
 	/* Select the queue we're interested in */
@@ -248,6 +385,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 
 	info->queue_index = index;
 	info->num = num;
+	info->vector = VP_MSIX_NO_VECTOR;
 
 	size = PAGE_ALIGN(vring_size(num, VIRTIO_PCI_VRING_ALIGN));
 	info->queue = alloc_pages_exact(size, GFP_KERNEL|__GFP_ZERO);
@@ -271,12 +409,29 @@ 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 (vp_dev->msix_per_vq_vectors && callback) {
+		vector = VP_MSIX_VQ_VECTOR + vp_dev->msix_per_vq_vectors - 1;
+		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_per_vq_vectors--;
+		iowrite16(vector, vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
+	} else if (vp_dev->msix_enabled)
+		iowrite16(VP_MSIX_VQ_VECTOR,
+			  vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
+
 	spin_lock_irqsave(&vp_dev->lock, flags);
 	list_add(&info->node, &vp_dev->virtqueues);
 	spin_unlock_irqrestore(&vp_dev->lock, flags);
 
 	return vq;
 
+out_request_irq:
+	vring_del_virtqueue(vq);
 out_activate_queue:
 	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
 	free_pages_exact(info->queue, size);
@@ -285,17 +440,27 @@ out_info:
 	return ERR_PTR(err);
 }
 
-/* the config->del_vq() implementation */
 static void vp_del_vq(struct virtqueue *vq)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
 	struct virtio_pci_vq_info *info = vq->priv;
 	unsigned long size;
 
+	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
+
+	if (info->vector != VP_MSIX_NO_VECTOR)
+		free_irq(vp_dev->msix_entries[info->vector].vector, vq);
+
+	if (vp_dev->msix_enabled) {
+		iowrite16(VP_MSIX_NO_VECTOR,
+			  vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
+		/* Flush the write out to device */
+		ioread8(vp_dev->ioaddr + VIRTIO_PCI_ISR);
+	}
+
 	vring_del_virtqueue(vq);
 
 	/* Select and deactivate the queue */
-	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
 	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
 
 	size = PAGE_ALIGN(vring_size(info->num, VIRTIO_PCI_VRING_ALIGN));
@@ -303,6 +468,7 @@ static void vp_del_vq(struct virtqueue *vq)
 	kfree(info);
 }
 
+/* the config->del_vqs() implementation */
 static void vp_del_vqs(struct virtio_device *vdev)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -316,23 +482,38 @@ static void vp_del_vqs(struct virtio_device *vdev)
 
 	list_for_each_entry(info, &virtqueues, node)
 		vp_del_vq(info->vq);
+
+	vp_free_vectors(vdev);
 }
 
+/* the config->find_vqs() implementation */
 static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 		       struct virtqueue *vqs[],
 		       virtqueue_callback *callbacks[])
 {
-	int i;
+	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 (err)
+		goto error_request;
 
 	for (i = 0; i < nvqs; ++i) {
 		vqs[i] = vp_find_vq(vdev, i, callbacks[i]);
 		if (IS_ERR(vqs[i]))
-			goto error;
+			goto error_find;
 	}
 	return 0;
 
-error:
+error_find:
 	vp_del_vqs(vdev);
+
+error_request:
 	return PTR_ERR(vqs[i]);
 }
 
@@ -354,7 +535,7 @@ static void virtio_pci_release_dev(struct device *_d)
 	struct virtio_pci_device *vp_dev = to_vp_device(dev);
 	struct pci_dev *pci_dev = vp_dev->pci_dev;
 
-	free_irq(pci_dev->irq, vp_dev);
+	vp_del_vqs(dev);
 	pci_set_drvdata(pci_dev, NULL);
 	pci_iounmap(pci_dev, vp_dev->ioaddr);
 	pci_release_regions(pci_dev);
@@ -413,21 +594,13 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	vp_dev->vdev.id.vendor = pci_dev->subsystem_vendor;
 	vp_dev->vdev.id.device = pci_dev->subsystem_device;
 
-	/* register a handler for the queue with the PCI device's interrupt */
-	err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, IRQF_SHARED,
-			  dev_name(&vp_dev->vdev.dev), vp_dev);
-	if (err)
-		goto out_set_drvdata;
-
 	/* finally register the virtio device */
 	err = register_virtio_device(&vp_dev->vdev);
 	if (err)
-		goto out_req_irq;
+		goto out_set_drvdata;
 
 	return 0;
 
-out_req_irq:
-	free_irq(pci_dev->irq, vp_dev);
 out_set_drvdata:
 	pci_set_drvdata(pci_dev, NULL);
 	pci_iounmap(pci_dev, vp_dev->ioaddr);
diff --git a/include/linux/virtio_pci.h b/include/linux/virtio_pci.h
index cd0fd5d..4a0275b 100644
--- a/include/linux/virtio_pci.h
+++ b/include/linux/virtio_pci.h
@@ -47,9 +47,15 @@
 /* The bit of the ISR which indicates a device configuration change. */
 #define VIRTIO_PCI_ISR_CONFIG		0x2
 
+/* MSI-X registers: only enabled if MSI-X is enabled. */
+/* A 16-bit vector for configuration changes. */
+#define VIRTIO_MSI_CONFIG_VECTOR        20
+/* A 16-bit vector for selected queue notifications. */
+#define VIRTIO_MSI_QUEUE_VECTOR         22
+
 /* The remaining space is defined by each driver as the per-driver
  * configuration space */
-#define VIRTIO_PCI_CONFIG		20
+#define VIRTIO_PCI_CONFIG(dev)		((dev)->msix_enabled ? 24 : 20)
 
 /* Virtio ABI version, this must match exactly */
 #define VIRTIO_PCI_ABI_VERSION		0
-- 
1.6.3.rc3.1.g830204

[PATCH 3/3] virtio_pci: optional MSI-X support

[Michael S. Tsirkin]

This implements optional MSI-X support in virtio_pci.
MSI-X is used whenever the host supports at least 2 MSI-X
vectors: 1 for configuration changes and 1 for virtqueues.
Per-virtqueue vectors are allocated if enough vectors
available.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c |  211 +++++++++++++++++++++++++++++++++++++++----
 include/linux/virtio_pci.h  |    8 ++-
 2 files changed, 199 insertions(+), 20 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index a6bebe2..d21e2e6 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -42,6 +42,29 @@ struct virtio_pci_device
 	/* a list of queues so we can dispatch IRQs */
 	spinlock_t lock;
 	struct list_head virtqueues;
+
+	/* MSI-X support */
+	int msix_enabled;
+	int intx_enabled;
+	struct msix_entry *msix_entries;
+	/* 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 vectors configured at startup (excludes per-virtqueue
+	 * vectors if any) */
+	unsigned msix_preset_vectors;
+	/* Number of per-virtqueue vectors if any. */
+	unsigned msix_per_vq_vectors;
+};
+
+/* Constants for MSI-X */
+/* Use first vector for configuration changes, second and the rest for
+ * virtqueues Thus, we need at least 2 vectors for MSI. */
+enum {
+	VP_MSIX_CONFIG_VECTOR = 0,
+	VP_MSIX_VQ_VECTOR = 1,
+	VP_MSIX_MIN_VECTORS = 2,
+	VP_MSIX_NO_VECTOR = 0xffff,
 };
 
 struct virtio_pci_vq_info
@@ -60,6 +83,9 @@ struct virtio_pci_vq_info
 
 	/* the list node for the virtqueues list */
 	struct list_head node;
+
+	/* MSI-X vector (or none) */
+	unsigned vector;
 };
 
 /* Qumranet donated their vendor ID for devices 0x1000 thru 0x10FF. */
@@ -109,7 +135,8 @@ static void vp_get(struct virtio_device *vdev, unsigned offset,
 		   void *buf, unsigned len)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
-	void __iomem *ioaddr = vp_dev->ioaddr + VIRTIO_PCI_CONFIG + offset;
+	void __iomem *ioaddr = vp_dev->ioaddr +
+                               VIRTIO_PCI_CONFIG(vp_dev) + offset;
 	u8 *ptr = buf;
 	int i;
 
@@ -123,7 +150,8 @@ static void vp_set(struct virtio_device *vdev, unsigned offset,
 		   const void *buf, unsigned len)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
-	void __iomem *ioaddr = vp_dev->ioaddr + VIRTIO_PCI_CONFIG + offset;
+	void __iomem *ioaddr = vp_dev->ioaddr +
+		               VIRTIO_PCI_CONFIG(vp_dev) + offset;
 	const u8 *ptr = buf;
 	int i;
 
@@ -221,7 +249,116 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
 	return vp_vring_interrupt(irq, opaque);
 }
 
-/* the config->find_vq() implementation */
+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_preset_vectors; ++i)
+		free_irq(vp_dev->msix_entries[i].vector, vp_dev);
+	vp_dev->msix_preset_vectors = 0;
+
+	if (vp_dev->msix_enabled) {
+		/* Disable the vector used for configuration */
+		iowrite16(VP_MSIX_NO_VECTOR,
+			  vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
+
+		vp_dev->msix_enabled = 0;
+		pci_disable_msix(vp_dev->pci_dev);
+	}
+}
+
+static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	const char *name = dev_name(&vp_dev->vdev.dev);
+	unsigned i, vectors;
+	int err = -ENOMEM;
+
+	/* We need at most one vector per queue and one for config changes */
+	vectors = VP_MSIX_VQ_VECTOR + max_vqs;
+	vp_dev->msix_entries = kmalloc(vectors * sizeof *vp_dev->msix_entries,
+				       GFP_KERNEL);
+	if (!vp_dev->msix_entries)
+		goto error_entries;
+	vp_dev->msix_names = kmalloc(vectors * sizeof *vp_dev->msix_names,
+				     GFP_KERNEL);
+	if (!vp_dev->msix_names)
+		goto error_names;
+
+	snprintf(vp_dev->msix_names[VP_MSIX_CONFIG_VECTOR],
+		 sizeof *vp_dev->msix_names, "%s-config", name);
+	for (i = 0; i < max_vqs; ++i)
+		snprintf(vp_dev->msix_names[i + VP_MSIX_VQ_VECTOR],
+			 sizeof *vp_dev->msix_names, "%s-vq-%d", name, i);
+	for (i = 0; i < vectors; ++i)
+		vp_dev->msix_entries[i].entry = i;
+
+	vp_dev->msix_preset_vectors = 1;
+	vp_dev->msix_per_vq_vectors = max_vqs;
+	for (;;) {
+		err = pci_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries,
+				      vectors);
+		/* Error out if not enough vectors */
+		if (err > 0 && err < VP_MSIX_MIN_VECTORS)
+			err = -EBUSY;
+		if (err <= 0)
+			break;
+		/* Not enough vectors for all queues. Retry, disabling
+		 * per-queue interrupts */
+		vectors = VP_MSIX_MIN_VECTORS;
+		vp_dev->msix_preset_vectors = VP_MSIX_MIN_VECTORS;
+		vp_dev->msix_per_vq_vectors = 0;
+		snprintf(vp_dev->msix_names[VP_MSIX_VQ_VECTOR],
+			 sizeof *vp_dev->msix_names, "%s-vq", name);
+	}
+
+	if (!err)
+		vp_dev->msix_enabled = 1;
+	if (err) {
+		/* Can't allocate enough MSI-X vectors, use regular interrupt */
+		vp_dev->msix_enabled = 0;
+		vp_dev->msix_preset_vectors = 0;
+		vp_dev->msix_per_vq_vectors = 0;
+		/* Register a handler for the queue with the PCI device's
+		 * interrupt */
+		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;
+	}
+	for (i = 0; i < vp_dev->msix_preset_vectors; ++i) {
+		err = request_irq(vp_dev->msix_entries[i].vector,
+				  i == VP_MSIX_CONFIG_VECTOR ?
+				  vp_config_changed : vp_vring_interrupt,
+				  0, vp_dev->msix_names[i], vp_dev);
+		if (err) {
+			/* Set msix_preset_vectors so that only vectors we
+			 * already allocated will be freed by vp_free_vqs. */
+			vp_dev->msix_preset_vectors = i;
+			goto error_irq;
+		}
+
+		/* Set the vector used for configuration */
+		if (i == VP_MSIX_CONFIG_VECTOR)
+			iowrite16(VP_MSIX_CONFIG_VECTOR,
+				  vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
+	}
+	return 0;
+error_irq:
+	vp_free_vectors(vdev);
+	kfree(vp_dev->msix_names);
+error_names:
+	kfree(vp_dev->msix_entries);
+error_entries:
+	return err;
+}
+
 static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 				    void (*callback)(struct virtqueue *vq))
 {
@@ -229,7 +366,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 	struct virtio_pci_vq_info *info;
 	struct virtqueue *vq;
 	unsigned long flags, size;
-	u16 num;
+	u16 num, vector;
 	int err;
 
 	/* Select the queue we're interested in */
@@ -248,6 +385,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 
 	info->queue_index = index;
 	info->num = num;
+	info->vector = VP_MSIX_NO_VECTOR;
 
 	size = PAGE_ALIGN(vring_size(num, VIRTIO_PCI_VRING_ALIGN));
 	info->queue = alloc_pages_exact(size, GFP_KERNEL|__GFP_ZERO);
@@ -271,12 +409,29 @@ 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 (vp_dev->msix_per_vq_vectors && callback) {
+		vector = VP_MSIX_VQ_VECTOR + vp_dev->msix_per_vq_vectors - 1;
+		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_per_vq_vectors--;
+		iowrite16(vector, vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
+	} else if (vp_dev->msix_enabled)
+		iowrite16(VP_MSIX_VQ_VECTOR,
+			  vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
+
 	spin_lock_irqsave(&vp_dev->lock, flags);
 	list_add(&info->node, &vp_dev->virtqueues);
 	spin_unlock_irqrestore(&vp_dev->lock, flags);
 
 	return vq;
 
+out_request_irq:
+	vring_del_virtqueue(vq);
 out_activate_queue:
 	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
 	free_pages_exact(info->queue, size);
@@ -285,17 +440,27 @@ out_info:
 	return ERR_PTR(err);
 }
 
-/* the config->del_vq() implementation */
 static void vp_del_vq(struct virtqueue *vq)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
 	struct virtio_pci_vq_info *info = vq->priv;
 	unsigned long size;
 
+	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
+
+	if (info->vector != VP_MSIX_NO_VECTOR)
+		free_irq(vp_dev->msix_entries[info->vector].vector, vq);
+
+	if (vp_dev->msix_enabled) {
+		iowrite16(VP_MSIX_NO_VECTOR,
+			  vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
+		/* Flush the write out to device */
+		ioread8(vp_dev->ioaddr + VIRTIO_PCI_ISR);
+	}
+
 	vring_del_virtqueue(vq);
 
 	/* Select and deactivate the queue */
-	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
 	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
 
 	size = PAGE_ALIGN(vring_size(info->num, VIRTIO_PCI_VRING_ALIGN));
@@ -303,6 +468,7 @@ static void vp_del_vq(struct virtqueue *vq)
 	kfree(info);
 }
 
+/* the config->del_vqs() implementation */
 static void vp_del_vqs(struct virtio_device *vdev)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -316,23 +482,38 @@ static void vp_del_vqs(struct virtio_device *vdev)
 
 	list_for_each_entry(info, &virtqueues, node)
 		vp_del_vq(info->vq);
+
+	vp_free_vectors(vdev);
 }
 
+/* the config->find_vqs() implementation */
 static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 		       struct virtqueue *vqs[],
 		       virtqueue_callback *callbacks[])
 {
-	int i;
+	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 (err)
+		goto error_request;
 
 	for (i = 0; i < nvqs; ++i) {
 		vqs[i] = vp_find_vq(vdev, i, callbacks[i]);
 		if (IS_ERR(vqs[i]))
-			goto error;
+			goto error_find;
 	}
 	return 0;
 
-error:
+error_find:
 	vp_del_vqs(vdev);
+
+error_request:
 	return PTR_ERR(vqs[i]);
 }
 
@@ -354,7 +535,7 @@ static void virtio_pci_release_dev(struct device *_d)
 	struct virtio_pci_device *vp_dev = to_vp_device(dev);
 	struct pci_dev *pci_dev = vp_dev->pci_dev;
 
-	free_irq(pci_dev->irq, vp_dev);
+	vp_del_vqs(dev);
 	pci_set_drvdata(pci_dev, NULL);
 	pci_iounmap(pci_dev, vp_dev->ioaddr);
 	pci_release_regions(pci_dev);
@@ -413,21 +594,13 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	vp_dev->vdev.id.vendor = pci_dev->subsystem_vendor;
 	vp_dev->vdev.id.device = pci_dev->subsystem_device;
 
-	/* register a handler for the queue with the PCI device's interrupt */
-	err = request_irq(vp_dev->pci_dev->irq, vp_interrupt, IRQF_SHARED,
-			  dev_name(&vp_dev->vdev.dev), vp_dev);
-	if (err)
-		goto out_set_drvdata;
-
 	/* finally register the virtio device */
 	err = register_virtio_device(&vp_dev->vdev);
 	if (err)
-		goto out_req_irq;
+		goto out_set_drvdata;
 
 	return 0;
 
-out_req_irq:
-	free_irq(pci_dev->irq, vp_dev);
 out_set_drvdata:
 	pci_set_drvdata(pci_dev, NULL);
 	pci_iounmap(pci_dev, vp_dev->ioaddr);
diff --git a/include/linux/virtio_pci.h b/include/linux/virtio_pci.h
index cd0fd5d..4a0275b 100644
--- a/include/linux/virtio_pci.h
+++ b/include/linux/virtio_pci.h
@@ -47,9 +47,15 @@
 /* The bit of the ISR which indicates a device configuration change. */
 #define VIRTIO_PCI_ISR_CONFIG		0x2
 
+/* MSI-X registers: only enabled if MSI-X is enabled. */
+/* A 16-bit vector for configuration changes. */
+#define VIRTIO_MSI_CONFIG_VECTOR        20
+/* A 16-bit vector for selected queue notifications. */
+#define VIRTIO_MSI_QUEUE_VECTOR         22
+
 /* The remaining space is defined by each driver as the per-driver
  * configuration space */
-#define VIRTIO_PCI_CONFIG		20
+#define VIRTIO_PCI_CONFIG(dev)		((dev)->msix_enabled ? 24 : 20)
 
 /* Virtio ABI version, this must match exactly */
 #define VIRTIO_PCI_ABI_VERSION		0
-- 
1.6.3.rc3.1.g830204

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Christian Borntraeger]

Am Tuesday 12 May 2009 00:19:32 schrieb Michael S. Tsirkin:
> This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
> and updates all drivers. This is needed for MSI support, because MSI
> needs to know the total number of vectors upfront.
[...]
> diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c

This file contains several copy/paste breakages and needs at least the
patch below to compile.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

---
 drivers/s390/kvm/kvm_virtio.c |   28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

Index: kvm/drivers/s390/kvm/kvm_virtio.c
===================================================================
--- kvm.orig/drivers/s390/kvm/kvm_virtio.c
+++ kvm/drivers/s390/kvm/kvm_virtio.c
@@ -237,23 +237,23 @@ static void kvm_del_vqs(struct virtio_de
 	int i;
 	if (!kdev->vqs)
 		return;
-	for (i = 0; i < ldev->nvqs; ++i)
+	for (i = 0; i < kdev->nvqs; ++i)
 		kvm_del_vq(kdev->vqs[i]);
-	kfree(ldev->vqs);
-	ldev->vqs = NULL;
-	ldev->nvqs = 0;
+	kfree(kdev->vqs);
+	kdev->vqs = NULL;
+	kdev->nvqs = 0;
 }
 
 static int kvm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
-			struct virtqueue *vqs[]
-			void (*callbacks)[](struct virtqueue *))
+			struct virtqueue *vqs[],
+			virtqueue_callback *callbacks[])
 {
 	struct kvm_device *kdev = to_kvmdev(vdev);
 	int i;
 
 	/* We must have this many virtqueues. */
 	if (nvqs > kdev->desc->num_vq)
-		return ERR_PTR(-ENOENT);
+		return -ENOENT;
 
 	kdev->vqs = kmalloc(GFP_KERNEL, nvqs * sizeof *kdev->vqs);
 	if (!kdev->vqs)
@@ -272,20 +272,6 @@ error:
 	return PTR_ERR(vqs[i]);
 }
 
-static void kvm_del_vqs(struct virtio_device *vdev)
-{
-	struct lguest_device *ldev = to_lgdev(vdev);
-	int i;
-
-	if (!ldev->vqs)
-		return;
-	for (i = 0; i < ldev->nvqs; ++i)
-		lg_del_vq(ldev->vqs[i]);
-	kfree(ldev->vqs);
-	ldev->vqs = NULL;
-	ldev->nvqs = 0;
-}
-
 /*
  * The config ops structure as defined by virtio config
  */

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Christian Borntraeger]

Am Tuesday 12 May 2009 00:19:32 schrieb Michael S. Tsirkin:
> This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
> and updates all drivers. This is needed for MSI support, because MSI
> needs to know the total number of vectors upfront.
[...]
> diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c

This file contains several copy/paste breakages and needs at least the
patch below to compile.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

---
 drivers/s390/kvm/kvm_virtio.c |   28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

Index: kvm/drivers/s390/kvm/kvm_virtio.c
===================================================================
--- kvm.orig/drivers/s390/kvm/kvm_virtio.c
+++ kvm/drivers/s390/kvm/kvm_virtio.c
@@ -237,23 +237,23 @@ static void kvm_del_vqs(struct virtio_de
 	int i;
 	if (!kdev->vqs)
 		return;
-	for (i = 0; i < ldev->nvqs; ++i)
+	for (i = 0; i < kdev->nvqs; ++i)
 		kvm_del_vq(kdev->vqs[i]);
-	kfree(ldev->vqs);
-	ldev->vqs = NULL;
-	ldev->nvqs = 0;
+	kfree(kdev->vqs);
+	kdev->vqs = NULL;
+	kdev->nvqs = 0;
 }
 
 static int kvm_find_vqs(struct virtio_device *vdev, unsigned nvqs,
-			struct virtqueue *vqs[]
-			void (*callbacks)[](struct virtqueue *))
+			struct virtqueue *vqs[],
+			virtqueue_callback *callbacks[])
 {
 	struct kvm_device *kdev = to_kvmdev(vdev);
 	int i;
 
 	/* We must have this many virtqueues. */
 	if (nvqs > kdev->desc->num_vq)
-		return ERR_PTR(-ENOENT);
+		return -ENOENT;
 
 	kdev->vqs = kmalloc(GFP_KERNEL, nvqs * sizeof *kdev->vqs);
 	if (!kdev->vqs)
@@ -272,20 +272,6 @@ error:
 	return PTR_ERR(vqs[i]);
 }
 
-static void kvm_del_vqs(struct virtio_device *vdev)
-{
-	struct lguest_device *ldev = to_lgdev(vdev);
-	int i;
-
-	if (!ldev->vqs)
-		return;
-	for (i = 0; i < ldev->nvqs; ++i)
-		lg_del_vq(ldev->vqs[i]);
-	kfree(ldev->vqs);
-	ldev->vqs = NULL;
-	ldev->nvqs = 0;
-}
-
 /*
  * The config ops structure as defined by virtio config
  */

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Rusty Russell]

On Tue, 12 May 2009 07:49:32 am Michael S. Tsirkin wrote:
> This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
> and updates all drivers. This is needed for MSI support, because MSI
> needs to know the total number of vectors upfront.

Sorry, is this not on top of my virtio_device vq linked list patch?

Two other things: prefer vq_callback_t as the type name, and perhaps consider 
varargs for the callbacks (or would that be too horrible at the implementation 
end?)

Thanks,
Rusty.

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Rusty Russell]

On Tue, 12 May 2009 07:49:32 am Michael S. Tsirkin wrote:
> This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
> and updates all drivers. This is needed for MSI support, because MSI
> needs to know the total number of vectors upfront.

Sorry, is this not on top of my virtio_device vq linked list patch?

Two other things: prefer vq_callback_t as the type name, and perhaps consider 
varargs for the callbacks (or would that be too horrible at the implementation 
end?)

Thanks,
Rusty.

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Michael S. Tsirkin]

On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote:
> On Tue, 12 May 2009 07:49:32 am Michael S. Tsirkin wrote:
> > This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
> > and updates all drivers. This is needed for MSI support, because MSI
> > needs to know the total number of vectors upfront.
> 
> Sorry, is this not on top of my virtio_device vq linked list patch?

Not yet, working on that.

> Two other things: prefer vq_callback_t as the type name,

ok

> and perhaps consider 
> varargs for the callbacks (or would that be too horrible at the implementation 
> end?)
> 
> Thanks,
> Rusty.

Ugh ... I think it will be. And AFAIK gcc generates a lot of code
for varargs - not something we want to do in each interrupt handler.

-- 
MST

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Michael S. Tsirkin]

On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote:
> On Tue, 12 May 2009 07:49:32 am Michael S. Tsirkin wrote:
> > This replaces find_vq/del_vq with find_vqs/del_vqs virtio operations,
> > and updates all drivers. This is needed for MSI support, because MSI
> > needs to know the total number of vectors upfront.
> 
> Sorry, is this not on top of my virtio_device vq linked list patch?

Not yet, working on that.

> Two other things: prefer vq_callback_t as the type name,

ok

> and perhaps consider 
> varargs for the callbacks (or would that be too horrible at the implementation 
> end?)
> 
> Thanks,
> Rusty.

Ugh ... I think it will be. And AFAIK gcc generates a lot of code
for varargs - not something we want to do in each interrupt handler.

-- 
MST

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Rusty Russell]

On Wed, 13 May 2009 01:03:30 am Michael S. Tsirkin wrote:
> On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote
> > and perhaps consider
> > varargs for the callbacks (or would that be too horrible at the
> > implementation end?)
> >
> > Thanks,
> > Rusty.
>
> Ugh ... I think it will be. And AFAIK gcc generates a lot of code
> for varargs - not something we want to do in each interrupt handler.

Err, no I mean for find_vqs:  eg.
	(block device)
	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, blk_done);

	(net device)
	err = vdev->config->find_vqs(vdev, 3, vqs, skb_recv_done, skb_xmit_done, NULL);

A bit neater for for the single-queue case.

Cheers,
Rusty.

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Rusty Russell]

On Wed, 13 May 2009 01:03:30 am Michael S. Tsirkin wrote:
> On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote
> > and perhaps consider
> > varargs for the callbacks (or would that be too horrible at the
> > implementation end?)
> >
> > Thanks,
> > Rusty.
>
> Ugh ... I think it will be. And AFAIK gcc generates a lot of code
> for varargs - not something we want to do in each interrupt handler.

Err, no I mean for find_vqs:  eg.
	(block device)
	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, blk_done);

	(net device)
	err = vdev->config->find_vqs(vdev, 3, vqs, skb_recv_done, skb_xmit_done, NULL);

A bit neater for for the single-queue case.

Cheers,
Rusty.

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Michael S. Tsirkin]

On Wed, May 13, 2009 at 10:47:08AM +0930, Rusty Russell wrote:
> On Wed, 13 May 2009 01:03:30 am Michael S. Tsirkin wrote:
> > On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote
> > > and perhaps consider
> > > varargs for the callbacks (or would that be too horrible at the
> > > implementation end?)
> > >
> > > Thanks,
> > > Rusty.
> >
> > Ugh ... I think it will be. And AFAIK gcc generates a lot of code
> > for varargs - not something we want to do in each interrupt handler.
> 
> Err, no I mean for find_vqs:  eg.
> 	(block device)
> 	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, blk_done);
> 
> 	(net device)
> 	err = vdev->config->find_vqs(vdev, 3, vqs, skb_recv_done, skb_xmit_done, NULL);
> 
> A bit neater for for the single-queue case.
> 
> Cheers,
> Rusty.

Oh. I see. But it becomes messy now that we also need to pass in the
names, and we lose type safety.
Let's just add a helper function for the single vq case?

static inline struct virtqueue *virtio_find_vq(struct virtio_devide *vdev,
					       vq_callback_t *c, const char *n)
{
	vq_callback_t *callbacks[] = { c };
	const char *names[] = { n };
	struct virtqueue *vq;
	int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names);
	if (err < 0)
		return ERR_PTR(err);
	return vq;
}


-- 
MST

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Michael S. Tsirkin]

On Wed, May 13, 2009 at 10:47:08AM +0930, Rusty Russell wrote:
> On Wed, 13 May 2009 01:03:30 am Michael S. Tsirkin wrote:
> > On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote
> > > and perhaps consider
> > > varargs for the callbacks (or would that be too horrible at the
> > > implementation end?)
> > >
> > > Thanks,
> > > Rusty.
> >
> > Ugh ... I think it will be. And AFAIK gcc generates a lot of code
> > for varargs - not something we want to do in each interrupt handler.
> 
> Err, no I mean for find_vqs:  eg.
> 	(block device)
> 	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, blk_done);
> 
> 	(net device)
> 	err = vdev->config->find_vqs(vdev, 3, vqs, skb_recv_done, skb_xmit_done, NULL);
> 
> A bit neater for for the single-queue case.
> 
> Cheers,
> Rusty.

Oh. I see. But it becomes messy now that we also need to pass in the
names, and we lose type safety.
Let's just add a helper function for the single vq case?

static inline struct virtqueue *virtio_find_vq(struct virtio_devide *vdev,
					       vq_callback_t *c, const char *n)
{
	vq_callback_t *callbacks[] = { c };
	const char *names[] = { n };
	struct virtqueue *vq;
	int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names);
	if (err < 0)
		return ERR_PTR(err);
	return vq;
}


-- 
MST

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Avi Kivity]

Michael S. Tsirkin wrote:
> On Wed, May 13, 2009 at 10:47:08AM +0930, Rusty Russell wrote:
>   
>> On Wed, 13 May 2009 01:03:30 am Michael S. Tsirkin wrote:
>>     
>>> On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote
>>>       
>>>> and perhaps consider
>>>> varargs for the callbacks (or would that be too horrible at the
>>>> implementation end?)
>>>>
>>>> Thanks,
>>>> Rusty.
>>>>         
>>> Ugh ... I think it will be. And AFAIK gcc generates a lot of code
>>> for varargs - not something we want to do in each interrupt handler.
>>>       
>> Err, no I mean for find_vqs:  eg.
>> 	(block device)
>> 	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, blk_done);
>>
>> 	(net device)
>> 	err = vdev->config->find_vqs(vdev, 3, vqs, skb_recv_done, skb_xmit_done, NULL);
>>
>> A bit neater for for the single-queue case.
>>
>> Cheers,
>> Rusty.
>>     
>
> Oh. I see. But it becomes messy now that we also need to pass in the
> names, and we lose type safety.
> Let's just add a helper function for the single vq case?
>
> static inline struct virtqueue *virtio_find_vq(struct virtio_devide *vdev,
> 					       vq_callback_t *c, const char *n)
> {
> 	vq_callback_t *callbacks[] = { c };
> 	const char *names[] = { n };
> 	struct virtqueue *vq;
> 	int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names);
> 	if (err < 0)
> 		return ERR_PTR(err);
> 	return vq;
> }
>   

Much saner.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Avi Kivity]

Michael S. Tsirkin wrote:
> On Wed, May 13, 2009 at 10:47:08AM +0930, Rusty Russell wrote:
>   
>> On Wed, 13 May 2009 01:03:30 am Michael S. Tsirkin wrote:
>>     
>>> On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote
>>>       
>>>> and perhaps consider
>>>> varargs for the callbacks (or would that be too horrible at the
>>>> implementation end?)
>>>>
>>>> Thanks,
>>>> Rusty.
>>>>         
>>> Ugh ... I think it will be. And AFAIK gcc generates a lot of code
>>> for varargs - not something we want to do in each interrupt handler.
>>>       
>> Err, no I mean for find_vqs:  eg.
>> 	(block device)
>> 	err = vdev->config->find_vqs(vdev, 1, &vblk->vq, blk_done);
>>
>> 	(net device)
>> 	err = vdev->config->find_vqs(vdev, 3, vqs, skb_recv_done, skb_xmit_done, NULL);
>>
>> A bit neater for for the single-queue case.
>>
>> Cheers,
>> Rusty.
>>     
>
> Oh. I see. But it becomes messy now that we also need to pass in the
> names, and we lose type safety.
> Let's just add a helper function for the single vq case?
>
> static inline struct virtqueue *virtio_find_vq(struct virtio_devide *vdev,
> 					       vq_callback_t *c, const char *n)
> {
> 	vq_callback_t *callbacks[] = { c };
> 	const char *names[] = { n };
> 	struct virtqueue *vq;
> 	int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names);
> 	if (err < 0)
> 		return ERR_PTR(err);
> 	return vq;
> }
>   

Much saner.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Rusty Russell]

On Wed, 13 May 2009 04:48:34 pm Michael S. Tsirkin wrote:
> Let's just add a helper function for the single vq case?
>
> static inline struct virtqueue *virtio_find_vq(struct virtio_devide *vdev,
> 					       vq_callback_t *c, const char *n)

virtio_find_single_vq() to emphasize the singular nature, and it looks good.

Thanks!
Rusty.

Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations

[Rusty Russell]

On Wed, 13 May 2009 04:48:34 pm Michael S. Tsirkin wrote:
> Let's just add a helper function for the single vq case?
>
> static inline struct virtqueue *virtio_find_vq(struct virtio_devide *vdev,
> 					       vq_callback_t *c, const char *n)

virtio_find_single_vq() to emphasize the singular nature, and it looks good.

Thanks!
Rusty.

[PATCH 2/3] virtio_pci: split up vp_interrupt

[Michael S. Tsirkin]

This reorganizes virtio-pci code in vp_interrupt slightly, so that
it's easier to add per-vq MSI support on top.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c |   45 +++++++++++++++++++++++++++++++++---------
 1 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3671c42..f7b79a2 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -164,6 +164,37 @@ static void vp_notify(struct virtqueue *vq)
 	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY);
 }
 
+/* Handle a configuration change: Tell driver if it wants to know. */
+static irqreturn_t vp_config_changed(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_driver *drv;
+	drv = container_of(vp_dev->vdev.dev.driver,
+			   struct virtio_driver, driver);
+
+	if (drv && drv->config_changed)
+		drv->config_changed(&vp_dev->vdev);
+	return IRQ_HANDLED;
+}
+
+/* Notify all virtqueues on an interrupt. */
+static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_pci_vq_info *info;
+	irqreturn_t ret = IRQ_NONE;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp_dev->lock, flags);
+	list_for_each_entry(info, &vp_dev->virtqueues, node) {
+		if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+			ret = IRQ_HANDLED;
+	}
+	spin_unlock_irqrestore(&vp_dev->lock, flags);
+
+	return ret;
+}
+
 /* A small wrapper to also acknowledge the interrupt when it's handled.
  * I really need an EIO hook for the vring so I can ack the interrupt once we
  * know that we'll be handling the IRQ but before we invoke the callback since
@@ -173,9 +204,6 @@ static void vp_notify(struct virtqueue *vq)
 static irqreturn_t vp_interrupt(int irq, void *opaque)
 {
 	struct virtio_pci_device *vp_dev = opaque;
-	struct virtio_pci_vq_info *info;
-	irqreturn_t ret = IRQ_NONE;
-	unsigned long flags;
 	u8 isr;
 
 	/* reading the ISR has the effect of also clearing it so it's very
@@ -187,14 +215,11 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
 		return IRQ_NONE;
 
 	/* Configuration change?  Tell driver if it wants to know. */
-	if (isr & VIRTIO_PCI_ISR_CONFIG) {
-		struct virtio_driver *drv;
-		drv = container_of(vp_dev->vdev.dev.driver,
-				   struct virtio_driver, driver);
+	if (isr & VIRTIO_PCI_ISR_CONFIG)
+		vp_config_changed(irq, opaque);
 
-		if (drv && drv->config_changed)
-			drv->config_changed(&vp_dev->vdev);
-	}
+	return vp_vring_interrupt(irq, opaque);
+}
 
 	spin_lock_irqsave(&vp_dev->lock, flags);
 	list_for_each_entry(info, &vp_dev->virtqueues, node) {
-- 
1.6.3.rc3.1.g830204

[PATCH 2/3] virtio_pci: split up vp_interrupt

[Michael S. Tsirkin]

This reorganizes virtio-pci code in vp_interrupt slightly, so that
it's easier to add per-vq MSI support on top.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci.c |   45 +++++++++++++++++++++++++++++++++---------
 1 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3671c42..f7b79a2 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -164,6 +164,37 @@ static void vp_notify(struct virtqueue *vq)
 	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY);
 }
 
+/* Handle a configuration change: Tell driver if it wants to know. */
+static irqreturn_t vp_config_changed(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_driver *drv;
+	drv = container_of(vp_dev->vdev.dev.driver,
+			   struct virtio_driver, driver);
+
+	if (drv && drv->config_changed)
+		drv->config_changed(&vp_dev->vdev);
+	return IRQ_HANDLED;
+}
+
+/* Notify all virtqueues on an interrupt. */
+static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
+{
+	struct virtio_pci_device *vp_dev = opaque;
+	struct virtio_pci_vq_info *info;
+	irqreturn_t ret = IRQ_NONE;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp_dev->lock, flags);
+	list_for_each_entry(info, &vp_dev->virtqueues, node) {
+		if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+			ret = IRQ_HANDLED;
+	}
+	spin_unlock_irqrestore(&vp_dev->lock, flags);
+
+	return ret;
+}
+
 /* A small wrapper to also acknowledge the interrupt when it's handled.
  * I really need an EIO hook for the vring so I can ack the interrupt once we
  * know that we'll be handling the IRQ but before we invoke the callback since
@@ -173,9 +204,6 @@ static void vp_notify(struct virtqueue *vq)
 static irqreturn_t vp_interrupt(int irq, void *opaque)
 {
 	struct virtio_pci_device *vp_dev = opaque;
-	struct virtio_pci_vq_info *info;
-	irqreturn_t ret = IRQ_NONE;
-	unsigned long flags;
 	u8 isr;
 
 	/* reading the ISR has the effect of also clearing it so it's very
@@ -187,14 +215,11 @@ static irqreturn_t vp_interrupt(int irq, void *opaque)
 		return IRQ_NONE;
 
 	/* Configuration change?  Tell driver if it wants to know. */
-	if (isr & VIRTIO_PCI_ISR_CONFIG) {
-		struct virtio_driver *drv;
-		drv = container_of(vp_dev->vdev.dev.driver,
-				   struct virtio_driver, driver);
+	if (isr & VIRTIO_PCI_ISR_CONFIG)
+		vp_config_changed(irq, opaque);
 
-		if (drv && drv->config_changed)
-			drv->config_changed(&vp_dev->vdev);
-	}
+	return vp_vring_interrupt(irq, opaque);
+}
 
 	spin_lock_irqsave(&vp_dev->lock, flags);
 	list_for_each_entry(info, &vp_dev->virtqueues, node) {
-- 
1.6.3.rc3.1.g830204