virtio ring entries have exactly the acquire/release semantics: - reading used index acquires a ring entry from host - updating the available index releases it to host Thus when using weak barriers (as most people do), __smp_load_acquire and __smp_store_release will do exactly the right thing to synchronize with the host. In fact, QEMU already uses __atomic_thread_fence(__ATOMIC_ACQUIRE) and __atomic_thread_fence(__ATOMIC_RELEASE); Documentation/circular-buffers.txt suggests smp_load_acquire and smp_store_release for head and tail updates. Since we have to worry about strong barrier users, let's add APIs to wrap these, and use in virtio_ring.c: in case of the string barriers, we fall back on rmb/wmb as previously. It is tempting to use this in vhost as well, this needs a bit more work to make acquire/release macros work on __user pointers. Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx> --- include/linux/virtio_ring.h | 26 ++++++++++++++++++++++++++ drivers/virtio/virtio_ring.c | 20 ++++++++++---------- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 71176be..528cf81 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -45,6 +45,32 @@ static inline void virtio_wmb(bool weak_barriers) wmb(); } +/* a load + acquire barrier, but only guaranteed to order reads */ +static inline __virtio16 virtio_load_acquire_rmb(bool weak_barriers, + __virtio16 *p) +{ + if (weak_barriers) + return __smp_load_acquire(p); + else { + __virtio16 v = READ_ONCE(*p); + rmb(); + return v; + } +} + +/* a release barrier + store, but only guaranteed to order writes */ +static inline void virtio_store_release_wmb(bool weak_barriers, + __virtio16 *p, __virtio16 v) +{ + if (weak_barriers) + __smp_store_release(p, v); + else { + wmb(); + WRITE_ONCE(*p, v); + return; + } +} + struct virtio_device; struct virtqueue; diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index ee663c4..edc01d0 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -244,11 +244,11 @@ static inline int virtqueue_add(struct virtqueue *_vq, avail = vq->avail_idx_shadow & (vq->vring.num - 1); vq->vring.avail->ring[avail] = cpu_to_virtio16(_vq->vdev, head); + vq->avail_idx_shadow++; /* Descriptors and available array need to be set before we expose the * new available array entries. */ - virtio_wmb(vq->weak_barriers); - vq->avail_idx_shadow++; - vq->vring.avail->idx = cpu_to_virtio16(_vq->vdev, vq->avail_idx_shadow); + virtio_store_release_wmb(vq->weak_barriers, &vq->vring.avail->idx, + cpu_to_virtio16(_vq->vdev, vq->avail_idx_shadow)); vq->num_added++; pr_debug("Added buffer head %i to %p\n", head, vq); @@ -453,9 +453,10 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) vq->vq.num_free++; } -static inline bool more_used(const struct vring_virtqueue *vq) +static inline +bool more_used(const struct vring_virtqueue *vq, __virtio16 used_idx) { - return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, vq->vring.used->idx); + return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, used_idx); } /** @@ -488,15 +489,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) return NULL; } - if (!more_used(vq)) { + /* Only get used array entries after they have been exposed by host. */ + if (!more_used(vq, virtio_load_acquire_rmb(vq->weak_barriers, + &vq->vring.used->idx))) { pr_debug("No more buffers in queue\n"); END_USE(vq); return NULL; } - /* Only get used array entries after they have been exposed by host. */ - virtio_rmb(vq->weak_barriers); - last_used = (vq->last_used_idx & (vq->vring.num - 1)); i = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].id); *len = virtio32_to_cpu(_vq->vdev, vq->vring.used->ring[last_used].len); @@ -704,7 +704,7 @@ irqreturn_t vring_interrupt(int irq, void *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); - if (!more_used(vq)) { + if (!more_used(vq, vq->vring.used->idx)) { pr_debug("virtqueue interrupt with no work for %p\n", vq); return IRQ_NONE; } -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization