cleanup memory barrier usage bringing it in sync with what linux guest does. The rules are simple: - read barrier after index read - write barrier before index write Also, call macros smp_rmb/smp_wmb to stress we are not syncing with a real io device here.
While I don't think compiler is crazy/powerful enough to reorder these, anyway, the bogus barriers we currently have in code will confuse anyone who tries to copy/reuse it.
Signed-off-by: Michael S. Tsirkin mst@redhat.com Cc: Gleb Natapov gleb@redhat.com ---
This depends on the NO_NOTIFY patch I sent earlier. Note: compile-tested only Gleb, could you try it out pls?
src/virtio-ring.c | 13 ++++++------- src/virtio-ring.h | 5 +++-- 2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/virtio-ring.c b/src/virtio-ring.c index 493d8b5..97a3ffe 100644 --- a/src/virtio-ring.c +++ b/src/virtio-ring.c @@ -38,8 +38,10 @@ int vring_more_used(struct vring_virtqueue *vq) { struct vring_used *used = GET_FLATPTR(vq->vring.used); - wmb(); - return GET_FLATPTR(vq->last_used_idx) != GET_FLATPTR(used->idx); + int more = GET_FLATPTR(vq->last_used_idx) != GET_FLATPTR(used->idx); + /* Make sure ring reads are done after idx read above. */ + smp_rmb(); + return more; }
/* @@ -63,7 +65,6 @@ void vring_detach(struct vring_virtqueue *vq, unsigned int head) /* link it with free list and point to it */
SET_FLATPTR(desc[i].next, GET_FLATPTR(vq->free_head)); - wmb(); SET_FLATPTR(vq->free_head, head); }
@@ -85,7 +86,6 @@ int vring_get_buf(struct vring_virtqueue *vq, unsigned int *len) // BUG_ON(!vring_more_used(vq));
elem = &used->ring[GET_FLATPTR(vq->last_used_idx) % GET_FLATPTR(vr->num)]; - wmb(); id = GET_FLATPTR(elem->id); if (len != NULL) *len = GET_FLATPTR(elem->len); @@ -136,7 +136,6 @@ void vring_add_buf(struct vring_virtqueue *vq,
av = (GET_FLATPTR(avail->idx) + num_added) % GET_FLATPTR(vr->num); SET_FLATPTR(avail->ring[av], head); - wmb(); }
void vring_kick(unsigned int ioaddr, struct vring_virtqueue *vq, int num_added) @@ -144,9 +143,9 @@ void vring_kick(unsigned int ioaddr, struct vring_virtqueue *vq, int num_added) struct vring *vr = &vq->vring; struct vring_avail *avail = GET_FLATPTR(vr->avail);
- wmb(); + /* Make sure idx update is done after ring write. */ + smp_wmb(); SET_FLATPTR(avail->idx, GET_FLATPTR(avail->idx) + num_added);
- mb(); vp_notify(ioaddr, GET_FLATPTR(vq->queue_index)); } diff --git a/src/virtio-ring.h b/src/virtio-ring.h index 3fb86fe..8b546f4 100644 --- a/src/virtio-ring.h +++ b/src/virtio-ring.h @@ -9,8 +9,9 @@
#define virt_to_phys(v) (unsigned long)(v) #define phys_to_virt(p) (void*)(p) -#define wmb() barrier() -#define mb() barrier() +/* Compiler barrier is enough as an x86 CPU does not reorder reads or writes */ +#define smp_rmb() barrier() +#define smp_wmb() barrier()
/* Status byte for guest to report progress, and synchronize features. */ /* We have seen device and processed generic fields (VIRTIO_CONFIG_F_VIRTIO) */