[PATCH 01/33] vhost: introduce vhost_ops->vhost_set_vring_enable_supported method

Vladimir Sementsov-Ogievskiy posted 33 patches 3 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, Jason Wang <jasowang@redhat.com>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH 01/33] vhost: introduce vhost_ops->vhost_set_vring_enable_supported method
Posted by Vladimir Sementsov-Ogievskiy 3 months ago
Remove vhost-user specific hack from generic code.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/virtio/vhost-user.c            |  8 ++++++++
 hw/virtio/vhost.c                 | 15 ++++++---------
 include/hw/virtio/vhost-backend.h |  2 ++
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 1e1d6b0d6e..1b2879a90c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1230,6 +1230,12 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev,
     return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring, false);
 }
 
+static bool vhost_user_set_vring_enable_supported(struct vhost_dev *dev)
+{
+    return virtio_has_feature(dev->backend_features,
+                              VHOST_USER_F_PROTOCOL_FEATURES);
+}
+
 static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
 {
     int i;
@@ -3032,6 +3038,8 @@ const VhostOps user_ops = {
         .vhost_reset_device = vhost_user_reset_device,
         .vhost_get_vq_index = vhost_user_get_vq_index,
         .vhost_set_vring_enable = vhost_user_set_vring_enable,
+        .vhost_set_vring_enable_supported =
+            vhost_user_set_vring_enable_supported,
         .vhost_requires_shm_log = vhost_user_requires_shm_log,
         .vhost_migration_done = vhost_user_migration_done,
         .vhost_net_set_mtu = vhost_user_net_set_mtu,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6557c58d12..c33dad4acd 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1986,15 +1986,12 @@ static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
         return 0;
     }
 
-    /*
-     * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
-     * been negotiated, the rings start directly in the enabled state, and
-     * .vhost_set_vring_enable callback will fail since
-     * VHOST_USER_SET_VRING_ENABLE is not supported.
-     */
-    if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER &&
-        !virtio_has_feature(hdev->backend_features,
-                            VHOST_USER_F_PROTOCOL_FEATURES)) {
+    if (hdev->vhost_ops->vhost_set_vring_enable_supported &&
+        !hdev->vhost_ops->vhost_set_vring_enable_supported(hdev)) {
+        /*
+         * This means, that rings are always enabled, and disable/enable
+         * API is not supported.
+         */
         return 0;
     }
 
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index d6df209a2f..f65fa26298 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -105,6 +105,7 @@ typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
 typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
 typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
                                          int enable);
+typedef bool (*vhost_set_vring_enable_supported_op)(struct vhost_dev *dev);
 typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
 typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
                                        char *mac_addr);
@@ -193,6 +194,7 @@ typedef struct VhostOps {
     vhost_reset_device_op vhost_reset_device;
     vhost_get_vq_index_op vhost_get_vq_index;
     vhost_set_vring_enable_op vhost_set_vring_enable;
+    vhost_set_vring_enable_supported_op vhost_set_vring_enable_supported;
     vhost_requires_shm_log_op vhost_requires_shm_log;
     vhost_migration_done_op vhost_migration_done;
     vhost_vsock_set_guest_cid_op vhost_vsock_set_guest_cid;
-- 
2.48.1
Re: [PATCH 01/33] vhost: introduce vhost_ops->vhost_set_vring_enable_supported method
Posted by Raphael Norwitz 1 month ago
On Wed, Aug 13, 2025 at 12:53 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Remove vhost-user specific hack from generic code.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  hw/virtio/vhost-user.c            |  8 ++++++++
>  hw/virtio/vhost.c                 | 15 ++++++---------
>  include/hw/virtio/vhost-backend.h |  2 ++
>  3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 1e1d6b0d6e..1b2879a90c 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1230,6 +1230,12 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev,
>      return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring, false);
>  }
>
> +static bool vhost_user_set_vring_enable_supported(struct vhost_dev *dev)
> +{
> +    return virtio_has_feature(dev->backend_features,
> +                              VHOST_USER_F_PROTOCOL_FEATURES);
> +}
> +
>  static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
>  {
>      int i;
> @@ -3032,6 +3038,8 @@ const VhostOps user_ops = {
>          .vhost_reset_device = vhost_user_reset_device,
>          .vhost_get_vq_index = vhost_user_get_vq_index,
>          .vhost_set_vring_enable = vhost_user_set_vring_enable,
> +        .vhost_set_vring_enable_supported =
> +            vhost_user_set_vring_enable_supported,

Why not make this a callback like vhost_user_gpu_{set|shared}_socket()
in vhost_backend.h instead?


>          .vhost_requires_shm_log = vhost_user_requires_shm_log,
>          .vhost_migration_done = vhost_user_migration_done,
>          .vhost_net_set_mtu = vhost_user_net_set_mtu,
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6557c58d12..c33dad4acd 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1986,15 +1986,12 @@ static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
>          return 0;
>      }
>
> -    /*
> -     * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
> -     * been negotiated, the rings start directly in the enabled state, and
> -     * .vhost_set_vring_enable callback will fail since
> -     * VHOST_USER_SET_VRING_ENABLE is not supported.
> -     */
> -    if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER &&
> -        !virtio_has_feature(hdev->backend_features,
> -                            VHOST_USER_F_PROTOCOL_FEATURES)) {
> +    if (hdev->vhost_ops->vhost_set_vring_enable_supported &&
> +        !hdev->vhost_ops->vhost_set_vring_enable_supported(hdev)) {
> +        /*
> +         * This means, that rings are always enabled, and disable/enable
> +         * API is not supported.
> +         */
>          return 0;
>      }
>
> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> index d6df209a2f..f65fa26298 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -105,6 +105,7 @@ typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
>  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
>  typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
>                                           int enable);
> +typedef bool (*vhost_set_vring_enable_supported_op)(struct vhost_dev *dev);
>  typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
>  typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
>                                         char *mac_addr);
> @@ -193,6 +194,7 @@ typedef struct VhostOps {
>      vhost_reset_device_op vhost_reset_device;
>      vhost_get_vq_index_op vhost_get_vq_index;
>      vhost_set_vring_enable_op vhost_set_vring_enable;
> +    vhost_set_vring_enable_supported_op vhost_set_vring_enable_supported;
>      vhost_requires_shm_log_op vhost_requires_shm_log;
>      vhost_migration_done_op vhost_migration_done;
>      vhost_vsock_set_guest_cid_op vhost_vsock_set_guest_cid;
> --
> 2.48.1
>
>
Re: [PATCH 01/33] vhost: introduce vhost_ops->vhost_set_vring_enable_supported method
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
On 09.10.25 21:56, Raphael Norwitz wrote:
> On Wed, Aug 13, 2025 at 12:53 PM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
>>
>> Remove vhost-user specific hack from generic code.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   hw/virtio/vhost-user.c            |  8 ++++++++
>>   hw/virtio/vhost.c                 | 15 ++++++---------
>>   include/hw/virtio/vhost-backend.h |  2 ++
>>   3 files changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index 1e1d6b0d6e..1b2879a90c 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -1230,6 +1230,12 @@ static int vhost_user_set_vring_base(struct vhost_dev *dev,
>>       return vhost_set_vring(dev, VHOST_USER_SET_VRING_BASE, ring, false);
>>   }
>>
>> +static bool vhost_user_set_vring_enable_supported(struct vhost_dev *dev)
>> +{
>> +    return virtio_has_feature(dev->backend_features,
>> +                              VHOST_USER_F_PROTOCOL_FEATURES);
>> +}
>> +
>>   static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
>>   {
>>       int i;
>> @@ -3032,6 +3038,8 @@ const VhostOps user_ops = {
>>           .vhost_reset_device = vhost_user_reset_device,
>>           .vhost_get_vq_index = vhost_user_get_vq_index,
>>           .vhost_set_vring_enable = vhost_user_set_vring_enable,
>> +        .vhost_set_vring_enable_supported =
>> +            vhost_user_set_vring_enable_supported,
> 
> Why not make this a callback like vhost_user_gpu_{set|shared}_socket()
> in vhost_backend.h instead?

You mean make it just a separate function vhost_user_set_vring_enable_supported()?

But this way we'll have to keep "hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER"
below..

Or what do you mean?

> 
> 
>>           .vhost_requires_shm_log = vhost_user_requires_shm_log,
>>           .vhost_migration_done = vhost_user_migration_done,
>>           .vhost_net_set_mtu = vhost_user_net_set_mtu,
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 6557c58d12..c33dad4acd 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1986,15 +1986,12 @@ static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
>>           return 0;
>>       }
>>
>> -    /*
>> -     * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
>> -     * been negotiated, the rings start directly in the enabled state, and
>> -     * .vhost_set_vring_enable callback will fail since
>> -     * VHOST_USER_SET_VRING_ENABLE is not supported.
>> -     */
>> -    if (hdev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER &&
>> -        !virtio_has_feature(hdev->backend_features,
>> -                            VHOST_USER_F_PROTOCOL_FEATURES)) {
>> +    if (hdev->vhost_ops->vhost_set_vring_enable_supported &&
>> +        !hdev->vhost_ops->vhost_set_vring_enable_supported(hdev)) {
>> +        /*
>> +         * This means, that rings are always enabled, and disable/enable
>> +         * API is not supported.
>> +         */
>>           return 0;
>>       }
>>
>> diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
>> index d6df209a2f..f65fa26298 100644
>> --- a/include/hw/virtio/vhost-backend.h
>> +++ b/include/hw/virtio/vhost-backend.h
>> @@ -105,6 +105,7 @@ typedef int (*vhost_reset_device_op)(struct vhost_dev *dev);
>>   typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
>>   typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
>>                                            int enable);
>> +typedef bool (*vhost_set_vring_enable_supported_op)(struct vhost_dev *dev);
>>   typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
>>   typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
>>                                          char *mac_addr);
>> @@ -193,6 +194,7 @@ typedef struct VhostOps {
>>       vhost_reset_device_op vhost_reset_device;
>>       vhost_get_vq_index_op vhost_get_vq_index;
>>       vhost_set_vring_enable_op vhost_set_vring_enable;
>> +    vhost_set_vring_enable_supported_op vhost_set_vring_enable_supported;
>>       vhost_requires_shm_log_op vhost_requires_shm_log;
>>       vhost_migration_done_op vhost_migration_done;
>>       vhost_vsock_set_guest_cid_op vhost_vsock_set_guest_cid;
>> --
>> 2.48.1
>>
>>


-- 
Best regards,
Vladimir