[PATCH v2] vdpa: fix vhost-vdpa suspended state not be shared

Wafer Xie posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251107064320.4633-1-wafer@jaguarmicro.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>
There is a newer version of this series
hw/virtio/vhost-vdpa.c         | 6 +++---
include/hw/virtio/vhost-vdpa.h | 8 ++++++--
2 files changed, 9 insertions(+), 5 deletions(-)
[PATCH v2] vdpa: fix vhost-vdpa suspended state not be shared
Posted by Wafer Xie 1 week ago
When stopping a vhost-vdpa device, only the first queue pair is marked as suspended,
while the remaining queues are not updated to the suspended state.
As a result, when stopping a multi-queue vhost-vdpa device,
the following error message will be printed.

qemu-system-x86_64:vhost VQ 2 ring restore failed: -1: Operation not permitted (1)

qemu-system-x86_64:vhost VQ 3 ring restore failed: -1: Operation not permitted (1)

So move v->suspended to v->shared, and then all the vhost_vdpa devices cannot
have different suspended states.

Fixes: 0bb302a9960a ("vdpa: add vhost_vdpa_suspend")

Suggested-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Wafer Xie <wafer@jaguarmicro.com>

--
Changes in v2:
 -Move v->suspended to v->shared
--
---
 hw/virtio/vhost-vdpa.c         | 6 +++---
 include/hw/virtio/vhost-vdpa.h | 8 ++++++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 7061b6e1a3..2f8f11df86 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -905,7 +905,7 @@ static int vhost_vdpa_reset_device(struct vhost_dev *dev)
 
     memory_listener_unregister(&v->shared->listener);
     v->shared->listener_registered = false;
-    v->suspended = false;
+    v->shared->suspended = false;
     return 0;
 }
 
@@ -1354,7 +1354,7 @@ static void vhost_vdpa_suspend(struct vhost_dev *dev)
         if (unlikely(r)) {
             error_report("Cannot suspend: %s(%d)", g_strerror(errno), errno);
         } else {
-            v->suspended = true;
+            v->shared->suspended = true;
             return;
         }
     }
@@ -1481,7 +1481,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev *dev,
         return 0;
     }
 
-    if (!v->suspended) {
+    if (!v->shared->suspended) {
         /*
          * Cannot trust in value returned by device, let vhost recover used
          * idx from guest.
diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 449bf5c840..6ad371f424 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -76,6 +76,12 @@ typedef struct vhost_vdpa_shared {
 
     /* SVQ switching is in progress, or already completed? */
     SVQTransitionState svq_switching;
+
+    /*
+     * Device suspended successfully.
+     * The device can only be suspended once, so it needs to be shared.
+     */
+    bool suspended;
 } VhostVDPAShared;
 
 typedef struct vhost_vdpa {
@@ -83,8 +89,6 @@ typedef struct vhost_vdpa {
     uint32_t address_space_id;
     uint64_t acked_features;
     bool shadow_vqs_enabled;
-    /* Device suspended successfully */
-    bool suspended;
     VhostVDPAShared *shared;
     GPtrArray *shadow_vqs;
     const VhostShadowVirtqueueOps *shadow_vq_ops;
-- 
2.34.1


Re: [PATCH v2] vdpa: fix vhost-vdpa suspended state not be shared
Posted by Eugenio Perez Martin 1 week ago
On Fri, Nov 7, 2025 at 7:45 AM Wafer Xie <wafer@jaguarmicro.com> wrote:
>
> When stopping a vhost-vdpa device, only the first queue pair is marked as suspended,
> while the remaining queues are not updated to the suspended state.
> As a result, when stopping a multi-queue vhost-vdpa device,
> the following error message will be printed.
>
> qemu-system-x86_64:vhost VQ 2 ring restore failed: -1: Operation not permitted (1)
>
> qemu-system-x86_64:vhost VQ 3 ring restore failed: -1: Operation not permitted (1)
>
> So move v->suspended to v->shared, and then all the vhost_vdpa devices cannot
> have different suspended states.
>
> Fixes: 0bb302a9960a ("vdpa: add vhost_vdpa_suspend")
>
> Suggested-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Wafer Xie <wafer@jaguarmicro.com>
>
> --
> Changes in v2:
>  -Move v->suspended to v->shared
> --
> ---
>  hw/virtio/vhost-vdpa.c         | 6 +++---
>  include/hw/virtio/vhost-vdpa.h | 8 ++++++--
>  2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 7061b6e1a3..2f8f11df86 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -905,7 +905,7 @@ static int vhost_vdpa_reset_device(struct vhost_dev *dev)
>
>      memory_listener_unregister(&v->shared->listener);
>      v->shared->listener_registered = false;
> -    v->suspended = false;
> +    v->shared->suspended = false;
>      return 0;
>  }
>
> @@ -1354,7 +1354,7 @@ static void vhost_vdpa_suspend(struct vhost_dev *dev)
>          if (unlikely(r)) {
>              error_report("Cannot suspend: %s(%d)", g_strerror(errno), errno);
>          } else {
> -            v->suspended = true;
> +            v->shared->suspended = true;
>              return;
>          }
>      }
> @@ -1481,7 +1481,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev *dev,
>          return 0;
>      }
>
> -    if (!v->suspended) {
> +    if (!v->shared->suspended) {
>          /*
>           * Cannot trust in value returned by device, let vhost recover used
>           * idx from guest.
> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> index 449bf5c840..6ad371f424 100644
> --- a/include/hw/virtio/vhost-vdpa.h
> +++ b/include/hw/virtio/vhost-vdpa.h
> @@ -76,6 +76,12 @@ typedef struct vhost_vdpa_shared {
>
>      /* SVQ switching is in progress, or already completed? */
>      SVQTransitionState svq_switching;
> +
> +    /*
> +     * Device suspended successfully.
> +     * The device can only be suspended once, so it needs to be shared.

I think I prefer the "the vhost_vdpa devices cannot have different
suspended states" comment here, but I'm fine with this one.

Either if you change it or not,

Acked-by: Eugenio Pérez <eperezma@redhat.com>

Thanks!

> +     */
> +    bool suspended;
>  } VhostVDPAShared;
>
>  typedef struct vhost_vdpa {
> @@ -83,8 +89,6 @@ typedef struct vhost_vdpa {
>      uint32_t address_space_id;
>      uint64_t acked_features;
>      bool shadow_vqs_enabled;
> -    /* Device suspended successfully */
> -    bool suspended;
>      VhostVDPAShared *shared;
>      GPtrArray *shadow_vqs;
>      const VhostShadowVirtqueueOps *shadow_vq_ops;
> --
> 2.34.1
>