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

Eugenio Pérez posted 3 patches 3 weeks, 2 days ago
[PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Eugenio Pérez 3 weeks, 2 days ago
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 */
+		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

Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Jason Wang 3 weeks ago
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);
        }

> +               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
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Eugenio Perez Martin 3 weeks ago
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
>
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Jason Wang 2 weeks, 6 days ago
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.

Thanks

>
> > > +               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
> >
>
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Eugenio Perez Martin 2 weeks, 6 days ago
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?

> Thanks
>
> >
> > > > +               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
> > >
> >
>
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Eugenio Perez Martin 1 week, 2 days ago
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.
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Michael S. Tsirkin 1 week, 2 days ago
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.

-- 
MST

Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Jason Wang 1 week ago
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
>
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Eugenio Perez Martin 1 week ago
On Thu, Mar 26, 2026 at 3:44 AM Jason Wang <jasowang@redhat.com> wrote:
>
> 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.
>

If you mean something like this:
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 6202f6902fcd..23b850023417 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -816,7 +816,8 @@ static int vduse_vdpa_reset(struct vdpa_device *vdpa)
        struct vduse_dev *dev = vdpa_to_vduse(vdpa);
        int ret = vduse_dev_set_status(dev, 0);

-       vduse_dev_reset(dev);
+       if (ret == 0)
+               vduse_dev_reset(dev);

        return ret;
 }

---

I totally agree. But in my opinion, it's outside this series' scope. I
can send a patch on top.
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Jason Wang 6 days, 22 hours ago
On Thu, Mar 26, 2026 at 2:57 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Thu, Mar 26, 2026 at 3:44 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > 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.
> >
>
> If you mean something like this:
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
> b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 6202f6902fcd..23b850023417 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -816,7 +816,8 @@ static int vduse_vdpa_reset(struct vdpa_device *vdpa)
>         struct vduse_dev *dev = vdpa_to_vduse(vdpa);
>         int ret = vduse_dev_set_status(dev, 0);
>
> -       vduse_dev_reset(dev);
> +       if (ret == 0)
> +               vduse_dev_reset(dev);

Something like this but we need to check if it's a timeout as queue ready.

>
>         return ret;
>  }
>
> ---
>
> I totally agree. But in my opinion, it's outside this series' scope. I
> can send a patch on top.
>

Works for me.

Thanks
Re: [PATCH v3 3/3] vduse: add F_QUEUE_READY feature
Posted by Eugenio Perez Martin 6 days, 17 hours ago
On Fri, Mar 27, 2026 at 2:15 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Thu, Mar 26, 2026 at 2:57 PM Eugenio Perez Martin
> <eperezma@redhat.com> wrote:
> >
> > On Thu, Mar 26, 2026 at 3:44 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > 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.
> > >
> >
> > If you mean something like this:
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
> > b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 6202f6902fcd..23b850023417 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -816,7 +816,8 @@ static int vduse_vdpa_reset(struct vdpa_device *vdpa)
> >         struct vduse_dev *dev = vdpa_to_vduse(vdpa);
> >         int ret = vduse_dev_set_status(dev, 0);
> >
> > -       vduse_dev_reset(dev);
> > +       if (ret == 0)
> > +               vduse_dev_reset(dev);
>
> Something like this but we need to check if it's a timeout as queue ready.
>

What should VDUSE or vdpa layer do differently if it is a timeout vs
returning an error?

> >
> >         return ret;
> >  }
> >
> > ---
> >
> > I totally agree. But in my opinion, it's outside this series' scope. I
> > can send a patch on top.
> >
>
> Works for me.
>
> Thanks
>