Commit 1ecbb6a8 authored by Darek Stojaczyk's avatar Darek Stojaczyk Committed by Jim Harris
Browse files

vhost/compat: start polling queues prematurely 



rte_vhost requires all queues to be fully initialized
in order to start I/O processing. This behavior is not
compliant with the vhost-user specification and doesn't
work with QEMU 2.12+, which will only initialize 1 I/O
queue for the SeaBIOS boot. Theoretically, we should
start polling each virtqueue individually after
receiving its SET_VRING_KICK message, but rte_vhost is
not designed to poll individual queues. So we use
a workaround to detect when a vhost session could be
potentially at that SeaBIOS stage and we mark it to
start polling as soon as its first virtqueue gets
initialized. This doesn't hurt any non-QEMU vhost slaves
and allows QEMU 2.12+ to boot correctly. SET_FEATURES
could be sent at any time, but QEMU will send it at
least once on SeaBIOS initialization - whenever
powered-up or rebooted.

Vhost sessions are still mostly started/stopped from
within rte_vhost callbacks, but now there's additional
concept of "forced" polling, in which SPDK starts
sessions manually, while rte_vhost still thinks the
sessions are stopped. This can potentially lead to cases
where a session is "started" twice, or gets destroyed
while it's still being polled (by force). Those cases
also need to be handled within this patch.

Change-Id: I70636d63e27914906ddece59cec34f1dd37ec5cd
Signed-off-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446086


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent d11aa873

lib/vhost/rte_vhost_compat.c

+54 −0
Original line number Diff line number Diff line
@@ -65,6 +65,28 @@ spdk_extern_vhost_pre_msg_handler(int vid, void *_msg) 
	}



	switch (msg->request) {

	case VHOST_USER_GET_VRING_BASE:

		if (vsession->forced_polling && vsession->lcore != -1) {

			/* Our queue is stopped for whatever reason, but we may still

			 * need to poll it after it's initialized again.

			 */

			g_spdk_vhost_ops.destroy_device(vid);

		}

		break;

	case VHOST_USER_SET_VRING_BASE:

	case VHOST_USER_SET_VRING_ADDR:

	case VHOST_USER_SET_VRING_NUM:

	case VHOST_USER_SET_VRING_KICK:

		if (vsession->forced_polling && vsession->lcore != -1) {

			/* Additional queues are being initialized, so we either processed

			 * enough I/Os and are switching from SeaBIOS to the OS now, or

			 * we were never in SeaBIOS in the first place. Either way, we

			 * don't need our workaround anymore.

			 */

			g_spdk_vhost_ops.destroy_device(vid);

			vsession->forced_polling = false;

		}

		break;

	case VHOST_USER_SET_VRING_CALL:

		/* rte_vhost will close the previous callfd and won't notify

		 * us about any change. This will effectively make SPDK fail
@@ -98,6 +120,7 @@ spdk_extern_vhost_pre_msg_handler(int vid, void *_msg) 
static enum rte_vhost_msg_result

spdk_extern_vhost_post_msg_handler(int vid, void *_msg)

{

	struct vhost_user_msg *msg = _msg;

	struct spdk_vhost_session *vsession;



	vsession = spdk_vhost_session_find_by_vid(vid);
@@ -110,6 +133,37 @@ spdk_extern_vhost_post_msg_handler(int vid, void *_msg) 
	if (vsession->needs_restart) {

		g_spdk_vhost_ops.new_device(vid);

		vsession->needs_restart = false;

		return RTE_VHOST_MSG_RESULT_NOT_HANDLED;

	}



	switch (msg->request) {

	case VHOST_USER_SET_FEATURES:

		/* rte_vhost requires all queues to be fully initialized in order

		 * to start I/O processing. This behavior is not compliant with the

		 * vhost-user specification and doesn't work with QEMU 2.12+, which

		 * will only initialize 1 I/O queue for the SeaBIOS boot.

		 * Theoretically, we should start polling each virtqueue individually

		 * after receiving its SET_VRING_KICK message, but rte_vhost is not

		 * designed to poll individual queues. So here we use a workaround

		 * to detect when the vhost session could be potentially at that SeaBIOS

		 * stage and we mark it to start polling as soon as its first virtqueue

		 * gets initialized. This doesn't hurt any non-QEMU vhost slaves

		 * and allows QEMU 2.12+ to boot correctly. SET_FEATURES could be sent

		 * at any time, but QEMU will send it at least once on SeaBIOS

		 * initialization - whenever powered-up or rebooted.

		 */

		vsession->forced_polling = true;

		break;

	case VHOST_USER_SET_VRING_KICK:

		/* vhost-user spec tells us to start polling a queue after receiving

		 * its SET_VRING_KICK message. Let's do it!

		 */

		if (vsession->forced_polling && vsession->lcore == -1) {

			g_spdk_vhost_ops.new_device(vid);

		}

		break;

	default:

		break;

	}



	return RTE_VHOST_MSG_RESULT_NOT_HANDLED;

lib/vhost/vhost.c

+29 −19
Original line number Diff line number Diff line
@@ -1000,32 +1000,16 @@ spdk_vhost_event_async_send_foreach_continue(struct spdk_vhost_session *vsession 
}



static void

stop_device(int vid)

_stop_session(struct spdk_vhost_session *vsession)

{

	struct spdk_vhost_dev *vdev;

	struct spdk_vhost_session *vsession;

	struct spdk_vhost_dev *vdev = vsession->vdev;

	struct spdk_vhost_virtqueue *q;

	int rc;

	uint16_t i;



	pthread_mutex_lock(&g_spdk_vhost_mutex);

	vsession = spdk_vhost_session_find_by_vid(vid);

	if (vsession == NULL) {

		SPDK_ERRLOG("Couldn't find session with vid %d.\n", vid);

		pthread_mutex_unlock(&g_spdk_vhost_mutex);

		return;

	}



	vdev = vsession->vdev;

	if (vsession->lcore == -1) {

		/* already stopped, nothing to do */

		pthread_mutex_unlock(&g_spdk_vhost_mutex);

		return;

	}



	rc = vdev->backend->stop_session(vsession);

	if (rc != 0) {

		SPDK_ERRLOG("Couldn't stop device with vid %d.\n", vid);

		SPDK_ERRLOG("Couldn't stop device with vid %d.\n", vsession->vid);

		pthread_mutex_unlock(&g_spdk_vhost_mutex);

		return;

	}
@@ -1043,6 +1027,28 @@ stop_device(int vid) 
	vsession->lcore = -1;

	assert(vdev->active_session_num > 0);

	vdev->active_session_num--;

}



static void

stop_device(int vid)

{

	struct spdk_vhost_session *vsession;



	pthread_mutex_lock(&g_spdk_vhost_mutex);

	vsession = spdk_vhost_session_find_by_vid(vid);

	if (vsession == NULL) {

		SPDK_ERRLOG("Couldn't find session with vid %d.\n", vid);

		pthread_mutex_unlock(&g_spdk_vhost_mutex);

		return;

	}



	if (vsession->lcore == -1) {

		/* already stopped, nothing to do */

		pthread_mutex_unlock(&g_spdk_vhost_mutex);

		return;

	}



	_stop_session(vsession);

	pthread_mutex_unlock(&g_spdk_vhost_mutex);

}
@@ -1322,6 +1328,10 @@ destroy_connection(int vid) 
		return;

	}



	if (vsession->lcore != -1) {

		_stop_session(vsession);

	}



	TAILQ_REMOVE(&vsession->vdev->vsessions, vsession, tailq);

	spdk_dma_free(vsession);

	pthread_mutex_unlock(&g_spdk_vhost_mutex);

lib/vhost/vhost_internal.h

+1 −0
Original line number Diff line number Diff line
@@ -127,6 +127,7 @@ struct spdk_vhost_session { 
	int32_t lcore;



	bool needs_restart;

	bool forced_polling;



	struct rte_vhost_memory *mem;