From f59281dafb832b161133743fcf3dc29051e6fdb8 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Tue, 21 Jun 2011 18:04:27 +0800 Subject: [PATCH 1/4] vhost: init used ring after backend was set Move the used ring initialization after backend was set. This makes it possible to disable the backend and tweak the used ring, then restart. This will also make it possible to log the used ring write correctly. Signed-off-by: Jason Wang Signed-off-by: Michael S. Tsirkin --- drivers/vhost/net.c | 4 ++++ drivers/vhost/test.c | 5 +++++ drivers/vhost/vhost.c | 14 ++++++-------- drivers/vhost/vhost.h | 1 + 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index f0fd52cdfadc..70ac60437d17 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -703,6 +703,10 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd) vhost_net_disable_vq(n, vq); rcu_assign_pointer(vq->private_data, sock); vhost_net_enable_vq(n, vq); + + r = vhost_init_used(vq); + if (r) + goto err_vq; } mutex_unlock(&vq->mutex); diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c index 734e1d74ad80..fc9a1d75281f 100644 --- a/drivers/vhost/test.c +++ b/drivers/vhost/test.c @@ -195,8 +195,13 @@ static long vhost_test_run(struct vhost_test *n, int test) lockdep_is_held(&vq->mutex)); rcu_assign_pointer(vq->private_data, priv); + r = vhost_init_used(&n->vqs[index]); + mutex_unlock(&vq->mutex); + if (r) + goto err; + if (oldpriv) { vhost_test_flush_vq(n, index); } diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 5ef2f62becf4..9a108038fe52 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -629,15 +629,17 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) return 0; } -static int init_used(struct vhost_virtqueue *vq, - struct vring_used __user *used) +int vhost_init_used(struct vhost_virtqueue *vq) { - int r = put_user(vq->used_flags, &used->flags); + int r; + if (!vq->private_data) + return 0; + r = put_user(vq->used_flags, &vq->used->flags); if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &used->idx); + return get_user(vq->last_used_idx, &vq->used->idx); } static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp) @@ -752,10 +754,6 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp) } } - r = init_used(vq, (struct vring_used __user *)(unsigned long) - a.used_user_addr); - if (r) - break; vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG)); vq->desc = (void __user *)(unsigned long)a.desc_user_addr; vq->avail = (void __user *)(unsigned long)a.avail_user_addr; diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 1544b782529b..14c9abf0d800 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -174,6 +174,7 @@ int vhost_get_vq_desc(struct vhost_dev *, struct vhost_virtqueue *, struct vhost_log *log, unsigned int *log_num); void vhost_discard_vq_desc(struct vhost_virtqueue *, int n); +int vhost_init_used(struct vhost_virtqueue *); int vhost_add_used(struct vhost_virtqueue *, unsigned int head, int len); int vhost_add_used_n(struct vhost_virtqueue *, struct vring_used_elem *heads, unsigned count); From 2723feaa8ec64f677f644c9189ed87d83f5559c1 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Tue, 21 Jun 2011 18:04:38 +0800 Subject: [PATCH 2/4] vhost: set log when updating used flags or avail event We need to log writes when updating used flags and avail event fields. Otherwise the guest may see a stale value after migration and miss notifying the host. Signed-off-by: Jason Wang Signed-off-by: Michael S. Tsirkin --- drivers/vhost/vhost.c | 84 +++++++++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 9a108038fe52..46822c0d9f7f 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -629,19 +629,6 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m) return 0; } -int vhost_init_used(struct vhost_virtqueue *vq) -{ - int r; - if (!vq->private_data) - return 0; - - r = put_user(vq->used_flags, &vq->used->flags); - if (r) - return r; - vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); -} - static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp) { struct file *eventfp, *filep = NULL, @@ -1008,6 +995,57 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, return 0; } +static int vhost_update_used_flags(struct vhost_virtqueue *vq) +{ + void __user *used; + if (put_user(vq->used_flags, &vq->used->flags) < 0) + return -EFAULT; + if (unlikely(vq->log_used)) { + /* Make sure the flag is seen before log. */ + smp_wmb(); + /* Log used flag write. */ + used = &vq->used->flags; + log_write(vq->log_base, vq->log_addr + + (used - (void __user *)vq->used), + sizeof vq->used->flags); + if (vq->log_ctx) + eventfd_signal(vq->log_ctx, 1); + } + return 0; +} + +static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event) +{ + if (put_user(vq->avail_idx, vhost_avail_event(vq))) + return -EFAULT; + if (unlikely(vq->log_used)) { + void __user *used; + /* Make sure the event is seen before log. */ + smp_wmb(); + /* Log avail event write */ + used = vhost_avail_event(vq); + log_write(vq->log_base, vq->log_addr + + (used - (void __user *)vq->used), + sizeof *vhost_avail_event(vq)); + if (vq->log_ctx) + eventfd_signal(vq->log_ctx, 1); + } + return 0; +} + +int vhost_init_used(struct vhost_virtqueue *vq) +{ + int r; + if (!vq->private_data) + return 0; + + r = vhost_update_used_flags(vq); + if (r) + return r; + vq->signalled_used_valid = false; + return get_user(vq->last_used_idx, &vq->used->idx); +} + static int translate_desc(struct vhost_dev *dev, u64 addr, u32 len, struct iovec iov[], int iov_size) { @@ -1479,34 +1517,20 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq) return false; vq->used_flags &= ~VRING_USED_F_NO_NOTIFY; if (!vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) { - r = put_user(vq->used_flags, &vq->used->flags); + r = vhost_update_used_flags(vq); if (r) { vq_err(vq, "Failed to enable notification at %p: %d\n", &vq->used->flags, r); return false; } } else { - r = put_user(vq->avail_idx, vhost_avail_event(vq)); + r = vhost_update_avail_event(vq, vq->avail_idx); if (r) { vq_err(vq, "Failed to update avail event index at %p: %d\n", vhost_avail_event(vq), r); return false; } } - if (unlikely(vq->log_used)) { - void __user *used; - /* Make sure data is seen before log. */ - smp_wmb(); - used = vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX) ? - &vq->used->flags : vhost_avail_event(vq); - /* Log used flags or event index entry write. Both are 16 bit - * fields. */ - log_write(vq->log_base, vq->log_addr + - (used - (void __user *)vq->used), - sizeof(u16)); - if (vq->log_ctx) - eventfd_signal(vq->log_ctx, 1); - } /* They could have slipped one in as we were doing that: make * sure it's written, then check again. */ smp_mb(); @@ -1529,7 +1553,7 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq) return; vq->used_flags |= VRING_USED_F_NO_NOTIFY; if (!vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) { - r = put_user(vq->used_flags, &vq->used->flags); + r = vhost_update_used_flags(vq); if (r) vq_err(vq, "Failed to enable notification at %p: %d\n", &vq->used->flags, r); From 75fd9edc1054a1fa1d1411adec368dadf68e764e Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 19 Jul 2011 13:19:18 +0300 Subject: [PATCH 3/4] vhost: fix zcopy reference counting Fix get/put refcount imbalance with zero copy, which caused qemu to hang forever on guest driver unload. Signed-off-by: Michael S. Tsirkin --- drivers/vhost/vhost.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 46822c0d9f7f..c16d22545868 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -1578,7 +1578,6 @@ struct vhost_ubuf_ref *vhost_ubuf_alloc(struct vhost_virtqueue *vq, if (!ubufs) return ERR_PTR(-ENOMEM); kref_init(&ubufs->kref); - kref_get(&ubufs->kref); init_waitqueue_head(&ubufs->wait); ubufs->vq = vq; return ubufs; From b834226b04d6fb51178a64e98872856986c71474 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 19 Jul 2011 17:15:43 +0300 Subject: [PATCH 4/4] vhost: optimize interrupt enable/disable As we now only update used ring after enabling the backend, we can write flags with __put_user: as that's done on data path, it matters. Signed-off-by: Michael S. Tsirkin --- drivers/vhost/vhost.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index c16d22545868..c14c42b95ab8 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -998,7 +998,7 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, static int vhost_update_used_flags(struct vhost_virtqueue *vq) { void __user *used; - if (put_user(vq->used_flags, &vq->used->flags) < 0) + if (__put_user(vq->used_flags, &vq->used->flags) < 0) return -EFAULT; if (unlikely(vq->log_used)) { /* Make sure the flag is seen before log. */ @@ -1016,7 +1016,7 @@ static int vhost_update_used_flags(struct vhost_virtqueue *vq) static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event) { - if (put_user(vq->avail_idx, vhost_avail_event(vq))) + if (__put_user(vq->avail_idx, vhost_avail_event(vq))) return -EFAULT; if (unlikely(vq->log_used)) { void __user *used;