[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] vdpa-dev: Fix initialisation order to restore VDUSE compatib
From: |
Kevin Wolf |
Subject: |
Re: [PATCH] vdpa-dev: Fix initialisation order to restore VDUSE compatibility |
Date: |
Mon, 5 Feb 2024 14:13:40 +0100 |
Am 02.02.2024 um 14:25 hat Kevin Wolf geschrieben:
> VDUSE requires that virtqueues are first enabled before the DRIVER_OK
> status flag is set; with the current API of the kernel module, it is
> impossible to enable the opposite order in our block export code because
> userspace is not notified when a virtqueue is enabled.
>
> This requirement also mathces the normal initialisation order as done by
> the generic vhost code in QEMU. However, commit 6c482547 accidentally
> changed the order for vdpa-dev and broke access to VDUSE devices with
> this.
>
> This changes vdpa-dev to use the normal order again and use the standard
> vhost callback .vhost_set_vring_enable for this. VDUSE devices can be
> used with vdpa-dev again after this fix.
>
> Cc: qemu-stable@nongnu.org
> Fixes: 6c4825476a4351530bcac17abab72295b75ffe98
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> hw/virtio/vdpa-dev.c | 5 +----
> hw/virtio/vhost-vdpa.c | 17 +++++++++++++++++
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index eb9ecea83b..13e87f06f6 100644
> --- a/hw/virtio/vdpa-dev.c
> +++ b/hw/virtio/vdpa-dev.c
> @@ -253,14 +253,11 @@ static int vhost_vdpa_device_start(VirtIODevice *vdev,
> Error **errp)
>
> s->dev.acked_features = vdev->guest_features;
>
> - ret = vhost_dev_start(&s->dev, vdev, false);
> + ret = vhost_dev_start(&s->dev, vdev, true);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Error starting vhost");
> goto err_guest_notifiers;
> }
> - for (i = 0; i < s->dev.nvqs; ++i) {
> - vhost_vdpa_set_vring_ready(&s->vdpa, i);
> - }
> s->started = true;
>
> /*
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 3a43beb312..c4574d56c5 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -904,6 +904,22 @@ int vhost_vdpa_set_vring_ready(struct vhost_vdpa *v,
> unsigned idx)
> return r;
> }
>
> +static int vhost_vdpa_set_vring_enable(struct vhost_dev *dev, int enable)
> +{
> + struct vhost_vdpa *v = dev->opaque;
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < dev->nvqs; ++i) {
> + ret = vhost_vdpa_set_vring_ready(v, i);
> + if (ret < 0) {
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
Oops, this forgets to actually use the @enable parameter, and always
enables the queue even if the caller wanted to disable it.
I'll fix this in a v2, but I'd first like to see if something has to
change to address Stefano's concern, too.
Kevin
Re: [PATCH] vdpa-dev: Fix initialisation order to restore VDUSE compatibility, Eugenio Perez Martin, 2024/02/05
Re: [PATCH] vdpa-dev: Fix initialisation order to restore VDUSE compatibility,
Kevin Wolf <=