From 1ecbb6a8da4e013f77a4eae34b802ded99a640a0 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Mon, 4 Mar 2019 23:44:43 +0100 Subject: [PATCH] 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: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/446086 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris --- lib/vhost/rte_vhost_compat.c | 54 ++++++++++++++++++++++++++++++++++++ lib/vhost/vhost.c | 48 +++++++++++++++++++------------- lib/vhost/vhost_internal.h | 1 + 3 files changed, 84 insertions(+), 19 deletions(-) diff --git a/lib/vhost/rte_vhost_compat.c b/lib/vhost/rte_vhost_compat.c index 8f8122b8c..001abf8d6 100644 --- a/lib/vhost/rte_vhost_compat.c +++ b/lib/vhost/rte_vhost_compat.c @@ -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; diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 20dab1feb..6ad8ed3ec 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -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); diff --git a/lib/vhost/vhost_internal.h b/lib/vhost/vhost_internal.h index 9080ef5b1..59c8e9b2c 100644 --- a/lib/vhost/vhost_internal.h +++ b/lib/vhost/vhost_internal.h @@ -127,6 +127,7 @@ struct spdk_vhost_session { int32_t lcore; bool needs_restart; + bool forced_polling; struct rte_vhost_memory *mem;