OS-7613 bhyve virtio needs barriers

OS-7614 viona should use proper barriers
Reviewed by: John Levon <john.levon@joyent.com>
Reviewed by: Hans Rosenfeld <hans.rosenfeld@joyent.com>
Approved by: Dan McDonald <danmcd@joyent.com>

3 files changed, 31 insertions, 5 deletions

diff --git a/usr/src/cmd/bhyve/virtio.c b/usr/src/cmd/bhyve/virtio.c
index 4c85000796..d3ff5e3951 100644
--- a/usr/src/cmd/bhyve/virtio.c
+++ b/usr/src/cmd/bhyve/virtio.c
@@ -422,6 +422,13 @@ vq_relchain(struct vqueue_info *vq, uint16_t idx, uint32_t iolen)
 	vue = &vuh->vu_ring[uidx++ & mask];
 	vue->vu_idx = idx;
 	vue->vu_tlen = iolen;
+#ifndef __FreeBSD__
+	/*
+	 * Ensure the used descriptor is visible before updating the index.
+	 * This is necessary on ISAs with memory ordering less strict than x86.
+	 */
+	wmb();
+#endif
 	vuh->vu_idx = uidx;
 }
 
@@ -459,6 +466,14 @@ vq_endchains(struct vqueue_info *vq, int used_all_avail)
 	vs = vq->vq_vs;
 	old_idx = vq->vq_save_used;
 	vq->vq_save_used = new_idx = vq->vq_used->vu_idx;
+#ifndef __FreeBSD__
+	/*
+	 * Use full memory barrier between vu_idx store from preceding
+	 * vq_relchain() call and the loads from VQ_USED_EVENT_IDX() or
+	 * va_flags below.
+	 */
+	mb();
+#endif
 	if (used_all_avail &&
 	    (vs->vs_negotiated_caps & VIRTIO_F_NOTIFY_ON_EMPTY))
 		intr = 1;
diff --git a/usr/src/compat/freebsd/amd64/machine/atomic.h b/usr/src/compat/freebsd/amd64/machine/atomic.h
index 6d8235d37c..d8e8131840 100644
--- a/usr/src/compat/freebsd/amd64/machine/atomic.h
+++ b/usr/src/compat/freebsd/amd64/machine/atomic.h
@@ -11,7 +11,7 @@
 
 /*
  * Copyright 2014 Pluribus Networks Inc.
- * Copyright 2017 Joyent, Inc.
+ * Copyright 2019 Joyent, Inc.
  */
 
 #ifndef _COMPAT_FREEBSD_AMD64_MACHINE_ATOMIC_H_
@@ -238,6 +238,11 @@ atomic_swap_long(volatile u_long *p, u_long v)
 /* Operations on pointers. */
 #define	atomic_cmpset_ptr	atomic_cmpset_long
 
-#define      mb()    __asm __volatile("mfence;" : : : "memory")
+/* Needed for the membar functions */
+#include_next <sys/atomic.h>
+
+#define	mb()			membar_enter()
+#define	rmb()			membar_consumer()
+#define	wmb()			membar_producer()
 
 #endif	/* _COMPAT_FREEBSD_AMD64_MACHINE_ATOMIC_H_ */
diff --git a/usr/src/uts/i86pc/io/viona/viona.c b/usr/src/uts/i86pc/io/viona/viona.c
index 74fd237fc9..6a62ae7ccf 100644
--- a/usr/src/uts/i86pc/io/viona/viona.c
+++ b/usr/src/uts/i86pc/io/viona/viona.c
@@ -1537,7 +1537,7 @@ viona_worker_rx(viona_vring_t *ring, viona_link_t *link)
 	ASSERT(MUTEX_HELD(&ring->vr_lock));
 	ASSERT3U(ring->vr_state, ==, VRS_RUN);
 
-	atomic_or_16(ring->vr_used_flags, VRING_USED_F_NO_NOTIFY);
+	*ring->vr_used_flags |= VRING_USED_F_NO_NOTIFY;
 	mac_rx_set(link->l_mch, viona_rx, link);
 
 	do {
@@ -1578,7 +1578,7 @@ viona_worker_tx(viona_vring_t *ring, viona_link_t *link)
 		boolean_t bail = B_FALSE;
 		uint_t ntx = 0;
 
-		atomic_or_16(ring->vr_used_flags, VRING_USED_F_NO_NOTIFY);
+		*ring->vr_used_flags |= VRING_USED_F_NO_NOTIFY;
 		while (viona_vr_num_avail(ring)) {
 			viona_tx(link, ring);
 
@@ -1590,14 +1590,18 @@ viona_worker_tx(viona_vring_t *ring, viona_link_t *link)
 			if (ntx++ >= ring->vr_size)
 				break;
 		}
-		atomic_and_16(ring->vr_used_flags, ~VRING_USED_F_NO_NOTIFY);
+		*ring->vr_used_flags &= ~VRING_USED_F_NO_NOTIFY;
 
 		VIONA_PROBE2(tx, viona_link_t *, link, uint_t, ntx);
 
 		/*
 		 * Check for available descriptors on the ring once more in
 		 * case a late addition raced with the NO_NOTIFY flag toggle.
+		 *
+		 * The barrier ensures that visibility of the vr_used_flags
+		 * store does not cross the viona_vr_num_avail() check below.
 		 */
+		membar_enter();
 		bail = VRING_NEED_BAIL(ring, p);
 		if (!bail && viona_vr_num_avail(ring)) {
 			continue;
@@ -2457,6 +2461,7 @@ pad_drop:
 		mp = next;
 	}
 
+	membar_enter();
 	if ((*ring->vr_avail_flags & VRING_AVAIL_F_NO_INTERRUPT) == 0) {
 		viona_intr_ring(ring);
 	}
@@ -2484,6 +2489,7 @@ viona_tx_done(viona_vring_t *ring, uint32_t len, uint16_t cookie)
 {
 	vq_pushchain(ring, len, cookie);
 
+	membar_enter();
 	if ((*ring->vr_avail_flags & VRING_AVAIL_F_NO_INTERRUPT) == 0) {
 		viona_intr_ring(ring);
 	}