vhost: enable vrings in vhost_dev_start() for vhost-user devices

Commit 02b61f38d3 ("hw/virtio: incorporate backend features in features")
properly negotiates VHOST_USER_F_PROTOCOL_FEATURES with the vhost-user
backend, but we forgot to enable vrings as specified in
docs/interop/vhost-user.rst:

    If ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, the
    ring starts directly in the enabled state.

    If ``VHOST_USER_F_PROTOCOL_FEATURES`` has been negotiated, the ring is
    initialized in a disabled state and is enabled by
    ``VHOST_USER_SET_VRING_ENABLE`` with parameter 1.

Some vhost-user front-ends already did this by calling
vhost_ops.vhost_set_vring_enable() directly:
- backends/cryptodev-vhost.c
- hw/net/virtio-net.c
- hw/virtio/vhost-user-gpio.c

But most didn't do that, so we would leave the vrings disabled and some
backends would not work. We observed this issue with the rust version of
virtiofsd [1], which uses the event loop [2] provided by the
vhost-user-backend crate where requests are not processed if vring is
not enabled.

Let's fix this issue by enabling the vrings in vhost_dev_start() for
vhost-user front-ends that don't already do this directly. Same thing
also in vhost_dev_stop() where we disable vrings.

[1] https://gitlab.com/virtio-fs/virtiofsd
[2] https://github.com/rust-vmm/vhost/blob/240fc2966/crates/vhost-user-backend/src/event_loop.rs#L217

Fixes: 02b61f38d3 ("hw/virtio: incorporate backend features in features")
Reported-by: German Maglione <gmaglione@redhat.com>
Tested-by: German Maglione <gmaglione@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Message-Id: <20221123131630.52020-1-sgarzare@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20221130112439.2527228-3-alex.bennee@linaro.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This commit is contained in:
Stefano Garzarella 2022-11-30 11:24:36 +00:00 committed by Michael S. Tsirkin
parent 523e40022f
commit 4daa5054c5
13 changed files with 67 additions and 31 deletions

View File

@ -94,7 +94,7 @@ cryptodev_vhost_start_one(CryptoDevBackendVhost *crypto,
goto fail_notifiers;
}
r = vhost_dev_start(&crypto->dev, dev);
r = vhost_dev_start(&crypto->dev, dev, false);
if (r < 0) {
goto fail_start;
}
@ -111,7 +111,7 @@ static void
cryptodev_vhost_stop_one(CryptoDevBackendVhost *crypto,
VirtIODevice *dev)
{
vhost_dev_stop(&crypto->dev, dev);
vhost_dev_stop(&crypto->dev, dev, false);
vhost_dev_disable_notifiers(&crypto->dev, dev);
}

View File

@ -85,7 +85,7 @@ vhost_user_backend_start(VhostUserBackend *b)
}
b->dev.acked_features = b->vdev->guest_features;
ret = vhost_dev_start(&b->dev, b->vdev);
ret = vhost_dev_start(&b->dev, b->vdev, true);
if (ret < 0) {
error_report("Error start vhost dev");
goto err_guest_notifiers;
@ -120,7 +120,7 @@ vhost_user_backend_stop(VhostUserBackend *b)
return;
}
vhost_dev_stop(&b->dev, b->vdev);
vhost_dev_stop(&b->dev, b->vdev, true);
if (k->set_guest_notifiers) {
ret = k->set_guest_notifiers(qbus->parent,

View File

@ -178,7 +178,7 @@ static int vhost_user_blk_start(VirtIODevice *vdev, Error **errp)
}
s->dev.vq_index_end = s->dev.nvqs;
ret = vhost_dev_start(&s->dev, vdev);
ret = vhost_dev_start(&s->dev, vdev, true);
if (ret < 0) {
error_setg_errno(errp, -ret, "Error starting vhost");
goto err_guest_notifiers;
@ -213,7 +213,7 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
return;
}
vhost_dev_stop(&s->dev, vdev);
vhost_dev_stop(&s->dev, vdev, true);
ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
if (ret < 0) {

View File

@ -259,7 +259,7 @@ static int vhost_net_start_one(struct vhost_net *net,
goto fail_notifiers;
}
r = vhost_dev_start(&net->dev, dev);
r = vhost_dev_start(&net->dev, dev, false);
if (r < 0) {
goto fail_start;
}
@ -308,7 +308,7 @@ fail:
if (net->nc->info->poll) {
net->nc->info->poll(net->nc, true);
}
vhost_dev_stop(&net->dev, dev);
vhost_dev_stop(&net->dev, dev, false);
fail_start:
vhost_dev_disable_notifiers(&net->dev, dev);
fail_notifiers:
@ -329,7 +329,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
if (net->nc->info->poll) {
net->nc->info->poll(net->nc, true);
}
vhost_dev_stop(&net->dev, dev);
vhost_dev_stop(&net->dev, dev, false);
if (net->nc->info->stop) {
net->nc->info->stop(net->nc);
}
@ -606,7 +606,7 @@ err_start:
assert(r >= 0);
}
vhost_dev_stop(&net->dev, vdev);
vhost_dev_stop(&net->dev, vdev, false);
return r;
}

View File

@ -68,7 +68,7 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
goto err_guest_notifiers;
}
ret = vhost_dev_start(&vsc->dev, vdev);
ret = vhost_dev_start(&vsc->dev, vdev, true);
if (ret < 0) {
error_report("Error start vhost dev");
goto err_guest_notifiers;
@ -101,7 +101,7 @@ void vhost_scsi_common_stop(VHostSCSICommon *vsc)
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
int ret = 0;
vhost_dev_stop(&vsc->dev, vdev);
vhost_dev_stop(&vsc->dev, vdev, true);
if (k->set_guest_notifiers) {
ret = k->set_guest_notifiers(qbus->parent, vsc->dev.nvqs, false);

View File

@ -9,8 +9,8 @@ vhost_section(const char *name) "%s"
vhost_reject_section(const char *name, int d) "%s:%d"
vhost_iotlb_miss(void *dev, int step) "%p step %d"
vhost_dev_cleanup(void *dev) "%p"
vhost_dev_start(void *dev, const char *name) "%p:%s"
vhost_dev_stop(void *dev, const char *name) "%p:%s"
vhost_dev_start(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
vhost_dev_stop(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
# vhost-user.c

View File

@ -76,7 +76,7 @@ static void vuf_start(VirtIODevice *vdev)
}
fs->vhost_dev.acked_features = vdev->guest_features;
ret = vhost_dev_start(&fs->vhost_dev, vdev);
ret = vhost_dev_start(&fs->vhost_dev, vdev, true);
if (ret < 0) {
error_report("Error starting vhost: %d", -ret);
goto err_guest_notifiers;
@ -110,7 +110,7 @@ static void vuf_stop(VirtIODevice *vdev)
return;
}
vhost_dev_stop(&fs->vhost_dev, vdev);
vhost_dev_stop(&fs->vhost_dev, vdev, true);
ret = k->set_guest_notifiers(qbus->parent, fs->vhost_dev.nvqs, false);
if (ret < 0) {

View File

@ -81,7 +81,7 @@ static int vu_gpio_start(VirtIODevice *vdev)
*/
vhost_ack_features(&gpio->vhost_dev, feature_bits, vdev->guest_features);
ret = vhost_dev_start(&gpio->vhost_dev, vdev);
ret = vhost_dev_start(&gpio->vhost_dev, vdev, false);
if (ret < 0) {
error_report("Error starting vhost-user-gpio: %d", ret);
goto err_guest_notifiers;
@ -139,7 +139,7 @@ static void vu_gpio_stop(VirtIODevice *vdev)
return;
}
vhost_dev_stop(vhost_dev, vdev);
vhost_dev_stop(vhost_dev, vdev, false);
ret = k->set_guest_notifiers(qbus->parent, vhost_dev->nvqs, false);
if (ret < 0) {

View File

@ -46,7 +46,7 @@ static void vu_i2c_start(VirtIODevice *vdev)
i2c->vhost_dev.acked_features = vdev->guest_features;
ret = vhost_dev_start(&i2c->vhost_dev, vdev);
ret = vhost_dev_start(&i2c->vhost_dev, vdev, true);
if (ret < 0) {
error_report("Error starting vhost-user-i2c: %d", -ret);
goto err_guest_notifiers;
@ -80,7 +80,7 @@ static void vu_i2c_stop(VirtIODevice *vdev)
return;
}
vhost_dev_stop(&i2c->vhost_dev, vdev);
vhost_dev_stop(&i2c->vhost_dev, vdev, true);
ret = k->set_guest_notifiers(qbus->parent, i2c->vhost_dev.nvqs, false);
if (ret < 0) {

View File

@ -47,7 +47,7 @@ static void vu_rng_start(VirtIODevice *vdev)
}
rng->vhost_dev.acked_features = vdev->guest_features;
ret = vhost_dev_start(&rng->vhost_dev, vdev);
ret = vhost_dev_start(&rng->vhost_dev, vdev, true);
if (ret < 0) {
error_report("Error starting vhost-user-rng: %d", -ret);
goto err_guest_notifiers;
@ -81,7 +81,7 @@ static void vu_rng_stop(VirtIODevice *vdev)
return;
}
vhost_dev_stop(&rng->vhost_dev, vdev);
vhost_dev_stop(&rng->vhost_dev, vdev, true);
ret = k->set_guest_notifiers(qbus->parent, rng->vhost_dev.nvqs, false);
if (ret < 0) {

View File

@ -70,7 +70,7 @@ int vhost_vsock_common_start(VirtIODevice *vdev)
}
vvc->vhost_dev.acked_features = vdev->guest_features;
ret = vhost_dev_start(&vvc->vhost_dev, vdev);
ret = vhost_dev_start(&vvc->vhost_dev, vdev, true);
if (ret < 0) {
error_report("Error starting vhost: %d", -ret);
goto err_guest_notifiers;
@ -105,7 +105,7 @@ void vhost_vsock_common_stop(VirtIODevice *vdev)
return;
}
vhost_dev_stop(&vvc->vhost_dev, vdev);
vhost_dev_stop(&vvc->vhost_dev, vdev, true);
ret = k->set_guest_notifiers(qbus->parent, vvc->vhost_dev.nvqs, false);
if (ret < 0) {

View File

@ -1777,15 +1777,36 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
return 0;
}
static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
{
if (!hdev->vhost_ops->vhost_set_vring_enable) {
return 0;
}
/*
* For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
* been negotiated, the rings start directly in the enabled state, and
* .vhost_set_vring_enable callback will fail since
* VHOST_USER_SET_VRING_ENABLE is not supported.
*/
if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER &&
!virtio_has_feature(hdev->backend_features,
VHOST_USER_F_PROTOCOL_FEATURES)) {
return 0;
}
return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);
}
/* Host notifiers must be enabled at this point. */
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
{
int i, r;
/* should only be called after backend is connected */
assert(hdev->vhost_ops);
trace_vhost_dev_start(hdev, vdev->name);
trace_vhost_dev_start(hdev, vdev->name, vrings);
vdev->vhost_started = true;
hdev->started = true;
@ -1830,10 +1851,16 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
goto fail_log;
}
}
if (vrings) {
r = vhost_dev_set_vring_enable(hdev, true);
if (r) {
goto fail_log;
}
}
if (hdev->vhost_ops->vhost_dev_start) {
r = hdev->vhost_ops->vhost_dev_start(hdev, true);
if (r) {
goto fail_log;
goto fail_start;
}
}
if (vhost_dev_has_iommu(hdev) &&
@ -1848,6 +1875,10 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev)
}
}
return 0;
fail_start:
if (vrings) {
vhost_dev_set_vring_enable(hdev, false);
}
fail_log:
vhost_log_put(hdev, false);
fail_vq:
@ -1866,18 +1897,21 @@ fail_features:
}
/* Host notifiers must be enabled at this point. */
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
{
int i;
/* should only be called after backend is connected */
assert(hdev->vhost_ops);
trace_vhost_dev_stop(hdev, vdev->name);
trace_vhost_dev_stop(hdev, vdev->name, vrings);
if (hdev->vhost_ops->vhost_dev_start) {
hdev->vhost_ops->vhost_dev_start(hdev, false);
}
if (vrings) {
vhost_dev_set_vring_enable(hdev, false);
}
for (i = 0; i < hdev->nvqs; ++i) {
vhost_virtqueue_stop(hdev,
vdev,

View File

@ -184,24 +184,26 @@ static inline bool vhost_dev_is_started(struct vhost_dev *hdev)
* vhost_dev_start() - start the vhost device
* @hdev: common vhost_dev structure
* @vdev: the VirtIODevice structure
* @vrings: true to have vrings enabled in this call
*
* Starts the vhost device. From this point VirtIO feature negotiation
* can start and the device can start processing VirtIO transactions.
*
* Return: 0 on success, < 0 on error.
*/
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings);
/**
* vhost_dev_stop() - stop the vhost device
* @hdev: common vhost_dev structure
* @vdev: the VirtIODevice structure
* @vrings: true to have vrings disabled in this call
*
* Stop the vhost device. After the device is stopped the notifiers
* can be disabled (@vhost_dev_disable_notifiers) and the device can
* be torn down (@vhost_dev_cleanup).
*/
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings);
/**
* DOC: vhost device configuration handling