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-sta...@nongnu.org > Fixes: 6c4825476a4351530bcac17abab72295b75ffe98 > Signed-off-by: Kevin Wolf <kw...@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