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

[Eugenio Perez Martin <eperezma@redhat.com>]

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?

> > +               spin_lock(&dev->msg_lock);
> > +               vduse_dev_broken(dev);
> > +               spin_unlock(&dev->msg_lock);
> > +       }
> >  }
> >
> >  static bool vduse_vdpa_get_vq_ready(struct vdpa_device *vdpa, u16 idx)
> > @@ -2078,6 +2101,9 @@ static int vduse_create_dev(struct vduse_dev_config *config,
> >         dev->device_features = config->features;
> >         dev->device_id = config->device_id;
> >         dev->vendor_id = config->vendor_id;
> > +       dev->vduse_features = config->vduse_features;
> > +       dev_dbg(vduse_ctrl_dev, "Creating device %s with features 0x%llx",
> > +               config->name, config->vduse_features);
> >
> >         dev->nas = (dev->api_version < VDUSE_API_VERSION_1) ? 1 : config->nas;
> >         dev->as = kcalloc(dev->nas, sizeof(dev->as[0]), GFP_KERNEL);
> > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > index e9b5f32a452d..7324faea5df4 100644
> > --- a/include/uapi/linux/vduse.h
> > +++ b/include/uapi/linux/vduse.h
> > @@ -14,6 +14,9 @@
> >
> >  #define VDUSE_API_VERSION_1    1
> >
> > +/* The VDUSE instance expects a request for vq ready */
> > +#define VDUSE_F_QUEUE_READY    0
> > +
> >  /*
> >   * Get the version of VDUSE API that kernel supported (VDUSE_API_VERSION).
> >   * This is used for future extension.
> > @@ -330,6 +333,7 @@ enum vduse_req_type {
> >         VDUSE_SET_STATUS,
> >         VDUSE_UPDATE_IOTLB,
> >         VDUSE_SET_VQ_GROUP_ASID,
> > +       VDUSE_SET_VQ_READY,
> >  };
> >
> >  /**
> > @@ -377,6 +381,15 @@ struct vduse_iova_range_v2 {
> >         __u32 padding;
> >  };
> >
> > +/**
> > + * struct vduse_vq_ready - Virtqueue ready request message
> > + * @num: Virtqueue number
> > + */
> > +struct vduse_vq_ready {
> > +       __u32 num;
> > +       __u32 ready;
> > +};
> > +
> >  /**
> >   * struct vduse_dev_request - control request
> >   * @type: request type
> > @@ -387,6 +400,7 @@ struct vduse_iova_range_v2 {
> >   * @iova: IOVA range for updating
> >   * @iova_v2: IOVA range for updating if API_VERSION >= 1
> >   * @vq_group_asid: ASID of a virtqueue group
> > + * @vq_ready: Virtqueue ready request
> >   * @padding: padding
> >   *
> >   * Structure used by read(2) on /dev/vduse/$NAME.
> > @@ -404,6 +418,10 @@ struct vduse_dev_request {
> >                  */
> >                 struct vduse_iova_range_v2 iova_v2;
> >                 struct vduse_vq_group_asid vq_group_asid;
> > +
> > +               /* Only if VDUSE_F_QUEUE_READY is negotiated */
> > +               struct vduse_vq_ready vq_ready;
> > +
> >                 __u32 padding[32];
> >         };
> >  };
> > --
> > 2.53.0
> >
>
> Thanks
>