From 18445c4d501b9ab4336f66ef46b092661ddaf336 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Feb 2008 23:49:57 -0500 Subject: virtio: explicit enable_cb/disable_cb rather than callback return. It seems that virtio_net wants to disable callbacks (interrupts) before calling netif_rx_schedule(), so we can't use the return value to do so. Rename "restart" to "cb_enable" and introduce "cb_disable" hook: callback now returns void, rather than a boolean. Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'drivers/virtio/virtio_ring.c') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 1dc04b6684e6..342bb0363fbe 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -220,7 +220,17 @@ static void *vring_get_buf(struct virtqueue *_vq, unsigned int *len) return ret; } -static bool vring_restart(struct virtqueue *_vq) +static void vring_disable_cb(struct virtqueue *_vq) +{ + struct vring_virtqueue *vq = to_vvq(_vq); + + START_USE(vq); + BUG_ON(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); + vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + END_USE(vq); +} + +static bool vring_enable_cb(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); @@ -254,8 +264,8 @@ irqreturn_t vring_interrupt(int irq, void *_vq) return IRQ_HANDLED; pr_debug("virtqueue callback for %p (%p)\n", vq, vq->vq.callback); - if (vq->vq.callback && !vq->vq.callback(&vq->vq)) - vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + if (vq->vq.callback) + vq->vq.callback(&vq->vq); return IRQ_HANDLED; } @@ -264,7 +274,8 @@ static struct virtqueue_ops vring_vq_ops = { .add_buf = vring_add_buf, .get_buf = vring_get_buf, .kick = vring_kick, - .restart = vring_restart, + .disable_cb = vring_disable_cb, + .enable_cb = vring_enable_cb, .shutdown = vring_shutdown, }; @@ -272,7 +283,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int num, struct virtio_device *vdev, void *pages, void (*notify)(struct virtqueue *), - bool (*callback)(struct virtqueue *)) + void (*callback)(struct virtqueue *)) { struct vring_virtqueue *vq; unsigned int i; -- cgit v1.2.2 From 426e3e0af5d2473e67d4256fc1340b7faebd1cc7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Feb 2008 23:49:59 -0500 Subject: virtio: clarify NO_NOTIFY flag usage The other side (host) can set the NO_NOTIFY flag as an optimization, to say "no need to kick me when you add things". Make it clear that this is advisory only; especially that we should always notify when the ring is full. Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/virtio/virtio_ring.c') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 342bb0363fbe..dbe1d35db32a 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -87,6 +87,8 @@ static int vring_add_buf(struct virtqueue *_vq, if (vq->num_free < out + in) { pr_debug("Can't add buf len %i - avail = %i\n", out + in, vq->num_free); + /* We notify *even if* VRING_USED_F_NO_NOTIFY is set here. */ + vq->notify(&vq->vq); END_USE(vq); return -ENOSPC; } -- cgit v1.2.2 From 6e5aa7efb27aec7e55b6463fa2c8db594c4226fa Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Feb 2008 23:50:03 -0500 Subject: virtio: reset function A reset function solves three problems: 1) It allows us to renegotiate features, eg. if we want to upgrade a guest driver without rebooting the guest. 2) It gives us a clean way of shutting down virtqueues: after a reset, we know that the buffers won't be used by the host, and 3) It helps the guest recover from messed-up drivers. So we remove the ->shutdown hook, and the only way we now remove feature bits is via reset. We leave it to the driver to do the reset before it deletes queues: the balloon driver, for example, needs to chat to the host in its remove function. Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'drivers/virtio/virtio_ring.c') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index dbe1d35db32a..9849babd6b37 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -173,16 +173,6 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) vq->num_free++; } -/* FIXME: We need to tell other side about removal, to synchronize. */ -static void vring_shutdown(struct virtqueue *_vq) -{ - struct vring_virtqueue *vq = to_vvq(_vq); - unsigned int i; - - for (i = 0; i < vq->vring.num; i++) - detach_buf(vq, i); -} - static inline bool more_used(const struct vring_virtqueue *vq) { return vq->last_used_idx != vq->vring.used->idx; @@ -278,7 +268,6 @@ static struct virtqueue_ops vring_vq_ops = { .kick = vring_kick, .disable_cb = vring_disable_cb, .enable_cb = vring_enable_cb, - .shutdown = vring_shutdown, }; struct virtqueue *vring_new_virtqueue(unsigned int num, -- cgit v1.2.2 From 81a8deab1ce3816c6a89e3429e234e7d3686da94 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Feb 2008 23:50:04 -0500 Subject: virtio: handle interrupts after callbacks turned off Anthony Liguori found double interrupt suppression in the virtio_net driver, triggered by two skb_recv_done's in a row. This is because virtio_ring's interrupt suppression is a best-effort optimization: it contains no synchronization so the host can miss it and still send interrupts. But it's certainly nicer for virtio users if calling disable_cb actually disables callbacks, so we check for the race in the interrupt routine. Note: SMP guests might require syncronization here, but since disable_cb is actually called from interrupt context, there has to be some form of synchronization before the next same interrupt handler is called (Linux guarantees that the same device's irq handler will never run simultanously on multiple CPUs). Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/virtio/virtio_ring.c') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 9849babd6b37..9859213aa658 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -255,6 +255,13 @@ irqreturn_t vring_interrupt(int irq, void *_vq) if (unlikely(vq->broken)) return IRQ_HANDLED; + /* Other side may have missed us turning off the interrupt, + * but we should preserve disable semantic for virtio users. */ + if (unlikely(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) { + pr_debug("virtqueue interrupt after disable for %p\n", vq); + return IRQ_HANDLED; + } + pr_debug("virtqueue callback for %p (%p)\n", vq, vq->vq.callback); if (vq->vq.callback) vq->vq.callback(&vq->vq); -- cgit v1.2.2 From 15f9c8903cbdb02aee0f1bcf86a97c2e238b9a3d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Feb 2008 23:50:05 -0500 Subject: virtio: Use the sg_phys convenience function. Simple cleanup. Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/virtio/virtio_ring.c') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 9859213aa658..74c245092b5c 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -99,16 +99,14 @@ static int vring_add_buf(struct virtqueue *_vq, head = vq->free_head; for (i = vq->free_head; out; i = vq->vring.desc[i].next, out--) { vq->vring.desc[i].flags = VRING_DESC_F_NEXT; - vq->vring.desc[i].addr = (page_to_pfn(sg_page(sg))<offset; + vq->vring.desc[i].addr = sg_phys(sg); vq->vring.desc[i].len = sg->length; prev = i; sg++; } for (; in; i = vq->vring.desc[i].next, in--) { vq->vring.desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE; - vq->vring.desc[i].addr = (page_to_pfn(sg_page(sg))<offset; + vq->vring.desc[i].addr = sg_phys(sg); vq->vring.desc[i].len = sg->length; prev = i; sg++; -- cgit v1.2.2 From c6fd47011b4bdebad3f1513bac75fe4895e332ee Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Feb 2008 23:50:05 -0500 Subject: virtio: Allow virtio to be modular and used by modules This is needed for the virtio PCI device to be compiled as a module. Signed-off-by: Anthony Liguori Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/virtio/virtio_ring.c') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 74c245092b5c..3a28c1382131 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -266,6 +266,7 @@ irqreturn_t vring_interrupt(int irq, void *_vq) return IRQ_HANDLED; } +EXPORT_SYMBOL_GPL(vring_interrupt); static struct virtqueue_ops vring_vq_ops = { .add_buf = vring_add_buf, @@ -318,9 +319,12 @@ struct virtqueue *vring_new_virtqueue(unsigned int num, return &vq->vq; } +EXPORT_SYMBOL_GPL(vring_new_virtqueue); void vring_del_virtqueue(struct virtqueue *vq) { kfree(to_vvq(vq)); } +EXPORT_SYMBOL_GPL(vring_del_virtqueue); +MODULE_LICENSE("GPL"); -- cgit v1.2.2