Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature

[Jason Wang <jasowang@redhat.com>]

On Tue, Mar 24, 2026 at 11:24 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Mar 24, 2026 at 03:01:47PM +0100, Eugenio Perez Martin wrote:
> > On Fri, Mar 13, 2026 at 8:08 AM Eugenio Perez Martin
> > <eperezma@redhat.com> wrote:
> > >
> > > On Fri, Mar 13, 2026 at 7:05 AM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > On Thu, Mar 12, 2026 at 2:24 PM Eugenio Perez Martin
> > > > <eperezma@redhat.com> wrote:
> > > > >
> > > > > On Thu, Mar 12, 2026 at 4:58 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > > >
> > > > > > On Wed, Mar 11, 2026 at 3:08 AM Eugenio Pérez <eperezma@redhat.com> wrote:
> > > > > > >
> > > > > > > Add the VDUSE_F_QUEUE_READY feature flag. This allows the kernel module
> > > > > > > to explicitly signal userspace when a specific virtqueue has been
> > > > > > > enabled.
> > > > > > >
> > > > > > > In scenarios like Live Migration of VirtIO net devices, the dataplane
> > > > > > > starts after the control virtqueue allowing QEMU to apply configuration
> > > > > > > in the destination device.
> > > > > > >
> > > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > > ---
> > > > > > > v2:
> > > > > > > * Fix comment of vduse_dev_request.vq_ready
> > > > > > > * Set vq_ready before sending the message to the VDUSE userland
> > > > > > >   instance, avoiding the need for SMP sync after receiving the message.
> > > > > > > ---
> > > > > > >  drivers/vdpa/vdpa_user/vduse_dev.c | 28 +++++++++++++++++++++++++++-
> > > > > > >  include/uapi/linux/vduse.h         | 18 ++++++++++++++++++
> > > > > > >  2 files changed, 45 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > index 17e0358d3a68..4f642b95a7cb 100644
> > > > > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > > > > > @@ -9,6 +9,7 @@
> > > > > > >   */
> > > > > > >
> > > > > > >  #include "linux/virtio_net.h"
> > > > > > > +#include <linux/bits.h>
> > > > > > >  #include <linux/cleanup.h>
> > > > > > >  #include <linux/init.h>
> > > > > > >  #include <linux/module.h>
> > > > > > > @@ -53,7 +54,7 @@
> > > > > > >  #define IRQ_UNBOUND -1
> > > > > > >
> > > > > > >  /* Supported VDUSE features */
> > > > > > > -static const uint64_t vduse_features;
> > > > > > > +static const uint64_t vduse_features = BIT_U64(VDUSE_F_QUEUE_READY);
> > > > > > >
> > > > > > >  /*
> > > > > > >   * VDUSE instance have not asked the vduse API version, so assume 0.
> > > > > > > @@ -120,6 +121,7 @@ struct vduse_dev {
> > > > > > >         char *name;
> > > > > > >         struct mutex lock;
> > > > > > >         spinlock_t msg_lock;
> > > > > > > +       u64 vduse_features;
> > > > > > >         u64 msg_unique;
> > > > > > >         u32 msg_timeout;
> > > > > > >         wait_queue_head_t waitq;
> > > > > > > @@ -601,8 +603,29 @@ static void vduse_vdpa_set_vq_ready(struct vdpa_device *vdpa,
> > > > > > >  {
> > > > > > >         struct vduse_dev *dev = vdpa_to_vduse(vdpa);
> > > > > > >         struct vduse_virtqueue *vq = dev->vqs[idx];
> > > > > > > +       struct vduse_dev_msg msg = { 0 };
> > > > > > > +       int r;
> > > > > > >
> > > > > > >         vq->ready = ready;
> > > > > > > +
> > > > > > > +       if (!(dev->vduse_features & BIT_U64(VDUSE_F_QUEUE_READY)))
> > > > > > > +               return;
> > > > > > > +
> > > > > > > +       msg.req.type = VDUSE_SET_VQ_READY;
> > > > > > > +       msg.req.vq_ready.num = idx;
> > > > > > > +       msg.req.vq_ready.ready = !!ready;
> > > > > > > +
> > > > > > > +       r = vduse_dev_msg_sync(dev, &msg);
> > > > > > > +
> > > > > > > +       if (r < 0) {
> > > > > > > +               dev_dbg(&vdpa->dev, "device refuses to set vq %u ready %u",
> > > > > > > +                       idx, ready);
> > > > > > > +
> > > > > > > +               /* We can't do better than break the device in this case */
> > > > > >
> > > > > > It's better to explain why we can't depend on vduse_dev_msg_sync() here.
> > > > > >
> > > > > > For example it did:
> > > > > >
> > > > > >         if (unlikely(dev->broken))
> > > > > >                 return -EIO;
> > > > > >
> > > > > >         init_waitqueue_head(&msg->waitq);
> > > > > >         spin_lock(&dev->msg_lock);
> > > > > >         if (unlikely(dev->broken)) {
> > > > > >                 spin_unlock(&dev->msg_lock);
> > > > > >                 return -EIO;
> > > > > >         }
> > > > > >
> > > > > > and
> > > > > >
> > > > > >         if (!msg->completed) {
> > > > > >                 list_del(&msg->list);
> > > > > >                 msg->resp.result = VDUSE_REQ_RESULT_FAILED;
> > > > > >                 /* Mark the device as malfunction when there is a timeout */
> > > > > >                 if (!ret)
> > > > > >                         vduse_dev_broken(dev);
> > > > > >         }
> > > > > >
> > > > >
> > > > > I'm not sure I follow you here.
> > > > >
> > > > > We can't do better than breaking the device because the function
> > > > > returns no error state, and the caller does not expect an error code.
> > > > > Do you mean we can't depend on vduse_dev_msg_sync to call
> > > > > vduse_dev_broken(dev) by itself?
> > > >
> > > > I think I meant, reset seems to be more heavyweight than suspend.
> > > >
> > > > So if reset can fail, I don't see reason ot break device only for
> > > > suspend failure.
> > > >
> > >
> > > Sorry I still don't get you.
> > >
> > > This series does not implement suspend at all. It doesn't modify the
> > > VDUSE device reset or the virtio reset behavior. It only implements
> > > the vq ready message for the device. If the device returns an error
> > > from that operation, what is your proposal for when the driver sends
> > > new messages like resume?
> > >
> >
> > Friendly ping.
>
> Jason, more comments? If this is to go in it has to go into linux next.

A typo, basically I meant that reset is more heavyweight than queue
ready. If we decide to check the response for queue ready, we need to
check the reset as well.

But I'm fine if you think we can start from this.

Thanks

>
> --
> MST
>