[PATCH v2 01/23] vhost-user: rework enabling vrings

Vladimir Sementsov-Ogievskiy posted 23 patches 1 month ago
Maintainers: "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v2 01/23] vhost-user: rework enabling vrings
Posted by Vladimir Sementsov-Ogievskiy 1 month ago
We call the handler almost the same way in three places:

 - cryptodev-vhost.c
 - vhost_net.c
 - vhost.c

The only difference, is that in vhost.c we don't try to call the handler
for old vhost-user (when VHOST_USER_F_PROTOCOL_FEATURES is not supported).

cryptodev-vhost and vhost_net code will just fail in this case. Probably
they were developed only for newer vhost-user. Anyway, it doesn't seem
correct to rely on this error path, if these devices want to check,
that they don't communicate to old vhost-user protocol, they should
do that earlier.

Let's create the common helper, to call .vhost_set_vring_enable and
use in all three places. For vhost-user let's just always skip
enable/disable if it's unsupported.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 backends/cryptodev-vhost.c |  8 +-------
 hw/net/vhost_net.c         |  7 +------
 hw/virtio/vhost-user.c     |  7 ++++++-
 hw/virtio/vhost.c          | 21 ---------------------
 include/hw/virtio/vhost.h  |  9 +++++++++
 5 files changed, 17 insertions(+), 35 deletions(-)

diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
index 943680a23a..abdfce33af 100644
--- a/backends/cryptodev-vhost.c
+++ b/backends/cryptodev-vhost.c
@@ -152,7 +152,6 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
 {
     CryptoDevBackendVhost *crypto =
                        cryptodev_get_vhost(cc, b, queue);
-    const VhostOps *vhost_ops;
 
     cc->vring_enable = enable;
 
@@ -160,12 +159,7 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
         return 0;
     }
 
-    vhost_ops = crypto->dev.vhost_ops;
-    if (vhost_ops->vhost_set_vring_enable) {
-        return vhost_ops->vhost_set_vring_enable(&crypto->dev, enable);
-    }
-
-    return 0;
+    return vhost_dev_set_vring_enable(&crypto->dev, enable);
 }
 
 int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index a8ee18a912..25e9f1fd24 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -587,7 +587,6 @@ VHostNetState *get_vhost_net(NetClientState *nc)
 int vhost_net_set_vring_enable(NetClientState *nc, int enable)
 {
     VHostNetState *net = get_vhost_net(nc);
-    const VhostOps *vhost_ops = net->dev.vhost_ops;
 
     /*
      * vhost-vdpa network devices need to enable dataplane virtqueues after
@@ -601,11 +600,7 @@ int vhost_net_set_vring_enable(NetClientState *nc, int enable)
 
     nc->vring_enable = enable;
 
-    if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
-        return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
-    }
-
-    return 0;
+    return vhost_dev_set_vring_enable(&net->dev, enable);
 }
 
 int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 36c9c2e04d..f367ce06ce 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1235,7 +1235,12 @@ static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
     int i;
 
     if (!virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
-        return -EINVAL;
+        /*
+         * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
+         * been negotiated, the rings start directly in the enabled state,
+         * and can't be disabled.
+         */
+        return 0;
     }
 
     for (i = 0; i < dev->nvqs; ++i) {
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 266a11514a..414a48a218 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -2013,27 +2013,6 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
     return 0;
 }
 
-static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
-{
-    if (!hdev->vhost_ops->vhost_set_vring_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)) {
-        return 0;
-    }
-
-    return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);
-}
-
 /*
  * Host notifiers must be enabled at this point.
  *
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 08bbb4dfe9..1ee639dd7e 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -215,6 +215,15 @@ static inline bool vhost_dev_is_started(struct vhost_dev *hdev)
     return hdev->started;
 }
 
+static inline int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
+{
+    if (!hdev->vhost_ops->vhost_set_vring_enable) {
+        return 0;
+    }
+
+    return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);
+}
+
 /**
  * vhost_dev_start() - start the vhost device
  * @hdev: common vhost_dev structure
-- 
2.48.1
Re: [PATCH v2 01/23] vhost-user: rework enabling vrings
Posted by Raphael Norwitz 1 month ago
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>

On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> We call the handler almost the same way in three places:
>
>  - cryptodev-vhost.c
>  - vhost_net.c
>  - vhost.c
>
> The only difference, is that in vhost.c we don't try to call the handler
> for old vhost-user (when VHOST_USER_F_PROTOCOL_FEATURES is not supported).
>
> cryptodev-vhost and vhost_net code will just fail in this case. Probably
> they were developed only for newer vhost-user. Anyway, it doesn't seem
> correct to rely on this error path, if these devices want to check,
> that they don't communicate to old vhost-user protocol, they should
> do that earlier.
>
> Let's create the common helper, to call .vhost_set_vring_enable and
> use in all three places. For vhost-user let's just always skip
> enable/disable if it's unsupported.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  backends/cryptodev-vhost.c |  8 +-------
>  hw/net/vhost_net.c         |  7 +------
>  hw/virtio/vhost-user.c     |  7 ++++++-
>  hw/virtio/vhost.c          | 21 ---------------------
>  include/hw/virtio/vhost.h  |  9 +++++++++
>  5 files changed, 17 insertions(+), 35 deletions(-)
>
> diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
> index 943680a23a..abdfce33af 100644
> --- a/backends/cryptodev-vhost.c
> +++ b/backends/cryptodev-vhost.c
> @@ -152,7 +152,6 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
>  {
>      CryptoDevBackendVhost *crypto =
>                         cryptodev_get_vhost(cc, b, queue);
> -    const VhostOps *vhost_ops;
>
>      cc->vring_enable = enable;
>
> @@ -160,12 +159,7 @@ vhost_set_vring_enable(CryptoDevBackendClient *cc,
>          return 0;
>      }
>
> -    vhost_ops = crypto->dev.vhost_ops;
> -    if (vhost_ops->vhost_set_vring_enable) {
> -        return vhost_ops->vhost_set_vring_enable(&crypto->dev, enable);
> -    }
> -
> -    return 0;
> +    return vhost_dev_set_vring_enable(&crypto->dev, enable);
>  }
>
>  int cryptodev_vhost_start(VirtIODevice *dev, int total_queues)
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index a8ee18a912..25e9f1fd24 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -587,7 +587,6 @@ VHostNetState *get_vhost_net(NetClientState *nc)
>  int vhost_net_set_vring_enable(NetClientState *nc, int enable)
>  {
>      VHostNetState *net = get_vhost_net(nc);
> -    const VhostOps *vhost_ops = net->dev.vhost_ops;
>
>      /*
>       * vhost-vdpa network devices need to enable dataplane virtqueues after
> @@ -601,11 +600,7 @@ int vhost_net_set_vring_enable(NetClientState *nc, int enable)
>
>      nc->vring_enable = enable;
>
> -    if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
> -        return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
> -    }
> -
> -    return 0;
> +    return vhost_dev_set_vring_enable(&net->dev, enable);
>  }
>
>  int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu)
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 36c9c2e04d..f367ce06ce 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1235,7 +1235,12 @@ static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
>      int i;
>
>      if (!virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> -        return -EINVAL;
> +        /*
> +         * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
> +         * been negotiated, the rings start directly in the enabled state,
> +         * and can't be disabled.
> +         */
> +        return 0;
>      }
>
>      for (i = 0; i < dev->nvqs; ++i) {
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 266a11514a..414a48a218 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -2013,27 +2013,6 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
>      return 0;
>  }
>
> -static int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
> -{
> -    if (!hdev->vhost_ops->vhost_set_vring_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)) {
> -        return 0;
> -    }
> -
> -    return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);
> -}
> -
>  /*
>   * Host notifiers must be enabled at this point.
>   *
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 08bbb4dfe9..1ee639dd7e 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -215,6 +215,15 @@ static inline bool vhost_dev_is_started(struct vhost_dev *hdev)
>      return hdev->started;
>  }
>
> +static inline int vhost_dev_set_vring_enable(struct vhost_dev *hdev, int enable)
> +{
> +    if (!hdev->vhost_ops->vhost_set_vring_enable) {
> +        return 0;
> +    }
> +
> +    return hdev->vhost_ops->vhost_set_vring_enable(hdev, enable);
> +}
> +
>  /**
>   * vhost_dev_start() - start the vhost device
>   * @hdev: common vhost_dev structure
> --
> 2.48.1
>