Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 11 Jun 2019 15:52:41 +0000 (UTC)
From:      Vincenzo Maffione <vmaffione@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r348929 - head/usr.sbin/bhyve
Message-ID:  <201906111552.x5BFqf3l062530@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: vmaffione
Date: Tue Jun 11 15:52:41 2019
New Revision: 348929
URL: https://svnweb.freebsd.org/changeset/base/348929

Log:
  bhyve: virtio: introduce vq_kick_enable() and vq_kick_disable()
  
  The VirtIO standard supports two schemes for notification suppression:
  a notification enable bit and a more sophisticated one (event_idx) that
  also supports delayed notifications. Currently bhyve fully supports
  only the first scheme. This patch hides the notification suppression
  internals by means of two inline routines, vq_kick_enable() and
  vq_kick_disable(), and makes the code more readable.
  Moreover, further improve readability by replacing the call to mb()
  with a call to atomic_thread_fence_seq_cst(), which is already used
  in virtio.c
  
  Reviewed by:	pmooney_pfmooney.com, bryanv
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D20581

Modified:
  head/usr.sbin/bhyve/pci_virtio_console.c
  head/usr.sbin/bhyve/pci_virtio_net.c
  head/usr.sbin/bhyve/pci_virtio_scsi.c
  head/usr.sbin/bhyve/virtio.c
  head/usr.sbin/bhyve/virtio.h

Modified: head/usr.sbin/bhyve/pci_virtio_console.c
==============================================================================
--- head/usr.sbin/bhyve/pci_virtio_console.c	Tue Jun 11 15:36:36 2019	(r348928)
+++ head/usr.sbin/bhyve/pci_virtio_console.c	Tue Jun 11 15:52:41 2019	(r348929)
@@ -607,7 +607,7 @@ pci_vtcon_notify_rx(void *vsc, struct vqueue_info *vq)
 
 	if (!port->vsp_rx_ready) {
 		port->vsp_rx_ready = 1;
-		vq->vq_used->vu_flags |= VRING_USED_F_NO_NOTIFY;
+		vq_kick_disable(vq);
 	}
 }
 

Modified: head/usr.sbin/bhyve/pci_virtio_net.c
==============================================================================
--- head/usr.sbin/bhyve/pci_virtio_net.c	Tue Jun 11 15:36:36 2019	(r348928)
+++ head/usr.sbin/bhyve/pci_virtio_net.c	Tue Jun 11 15:52:41 2019	(r348929)
@@ -39,7 +39,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/select.h>
 #include <sys/uio.h>
 #include <sys/ioctl.h>
-#include <machine/atomic.h>
 #include <net/ethernet.h>
 #ifndef NETMAP_WITH_LIBS
 #define NETMAP_WITH_LIBS
@@ -585,7 +584,7 @@ pci_vtnet_ping_rxq(void *vsc, struct vqueue_info *vq)
 	 */
 	if (sc->vsc_rx_ready == 0) {
 		sc->vsc_rx_ready = 1;
-		vq->vq_used->vu_flags |= VRING_USED_F_NO_NOTIFY;
+		vq_kick_disable(vq);
 	}
 }
 
@@ -631,7 +630,7 @@ pci_vtnet_ping_txq(void *vsc, struct vqueue_info *vq)
 
 	/* Signal the tx thread for processing */
 	pthread_mutex_lock(&sc->tx_mtx);
-	vq->vq_used->vu_flags |= VRING_USED_F_NO_NOTIFY;
+	vq_kick_disable(vq);
 	if (sc->tx_in_progress == 0)
 		pthread_cond_signal(&sc->tx_cond);
 	pthread_mutex_unlock(&sc->tx_mtx);
@@ -660,8 +659,7 @@ pci_vtnet_tx_thread(void *param)
 	for (;;) {
 		/* note - tx mutex is locked here */
 		while (sc->resetting || !vq_has_descs(vq)) {
-			vq->vq_used->vu_flags &= ~VRING_USED_F_NO_NOTIFY;
-			mb();
+			vq_kick_enable(vq);
 			if (!sc->resetting && vq_has_descs(vq))
 				break;
 
@@ -669,7 +667,7 @@ pci_vtnet_tx_thread(void *param)
 			error = pthread_cond_wait(&sc->tx_cond, &sc->tx_mtx);
 			assert(error == 0);
 		}
-		vq->vq_used->vu_flags |= VRING_USED_F_NO_NOTIFY;
+		vq_kick_disable(vq);
 		sc->tx_in_progress = 1;
 		pthread_mutex_unlock(&sc->tx_mtx);
 

Modified: head/usr.sbin/bhyve/pci_virtio_scsi.c
==============================================================================
--- head/usr.sbin/bhyve/pci_virtio_scsi.c	Tue Jun 11 15:36:36 2019	(r348928)
+++ head/usr.sbin/bhyve/pci_virtio_scsi.c	Tue Jun 11 15:52:41 2019	(r348929)
@@ -581,7 +581,7 @@ static void
 pci_vtscsi_eventq_notify(void *vsc, struct vqueue_info *vq)
 {
 
-	vq->vq_used->vu_flags |= VRING_USED_F_NO_NOTIFY;
+	vq_kick_disable(vq);
 }
 
 static void

Modified: head/usr.sbin/bhyve/virtio.c
==============================================================================
--- head/usr.sbin/bhyve/virtio.c	Tue Jun 11 15:36:36 2019	(r348928)
+++ head/usr.sbin/bhyve/virtio.c	Tue Jun 11 15:52:41 2019	(r348929)
@@ -428,7 +428,8 @@ vq_relchain(struct vqueue_info *vq, uint16_t idx, uint
 
 	/*
 	 * Ensure the used descriptor is visible before updating the index.
-	 * This is necessary on ISAs with memory ordering less strict than x86.
+	 * This is necessary on ISAs with memory ordering less strict than x86
+	 * (and even on x86 to act as a compiler barrier).
 	 */
 	atomic_thread_fence_rel();
 	vuh->vu_idx = uidx;

Modified: head/usr.sbin/bhyve/virtio.h
==============================================================================
--- head/usr.sbin/bhyve/virtio.h	Tue Jun 11 15:36:36 2019	(r348928)
+++ head/usr.sbin/bhyve/virtio.h	Tue Jun 11 15:52:41 2019	(r348929)
@@ -31,6 +31,8 @@
 #ifndef	_VIRTIO_H_
 #define	_VIRTIO_H_
 
+#include <machine/atomic.h>
+
 /*
  * These are derived from several virtio specifications.
  *
@@ -445,6 +447,26 @@ vq_interrupt(struct virtio_softc *vs, struct vqueue_in
 		pci_lintr_assert(vs->vs_pi);
 		VS_UNLOCK(vs);
 	}
+}
+
+static inline void
+vq_kick_enable(struct vqueue_info *vq)
+{
+
+	vq->vq_used->vu_flags &= ~VRING_USED_F_NO_NOTIFY;
+	/*
+	 * Full memory barrier to make sure the store to vu_flags
+	 * happens before the load from va_idx, which results from
+	 * a subsequent call to vq_has_descs().
+	 */
+	atomic_thread_fence_seq_cst();
+}
+
+static inline void
+vq_kick_disable(struct vqueue_info *vq)
+{
+
+	vq->vq_used->vu_flags |= VRING_USED_F_NO_NOTIFY;
 }
 
 struct iovec;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201906111552.x5BFqf3l062530>