On Thu, Oct 9, 2025 at 3:35 PM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
>
> On 09.10.25 21:57, Raphael Norwitz wrote:
> > On Wed, Aug 13, 2025 at 12:57 PM Vladimir Sementsov-Ogievskiy
> > <[email protected]> wrote:
> >>
> >> As comment says: it's only for vhost-user. So, let's move it
> >> to corresponding vhost backend realization.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> >> ---
> >> backends/cryptodev-vhost.c | 1 -
> >> hw/net/vhost_net.c | 2 --
> >> hw/virtio/vhost-user.c | 23 ++++++++++++++++++++---
> >> hw/virtio/virtio-qmp.c | 6 ++++--
> >> include/hw/virtio/vhost-backend.h | 3 +++
> >> include/hw/virtio/vhost.h | 8 --------
> >> 6 files changed, 27 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
> >> index 943680a23a..3bcdc494d8 100644
> >> --- a/backends/cryptodev-vhost.c
> >> +++ b/backends/cryptodev-vhost.c
> >> @@ -60,7 +60,6 @@ cryptodev_vhost_init(
> >>
> >> crypto->cc = options->cc;
> >>
> >> - crypto->dev.protocol_features = 0;
> >> crypto->backend = -1;
> >>
> >> /* vhost-user needs vq_index to initiate a specific queue pair */
> >> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> >> index fcee279f0b..ce30b6e197 100644
> >> --- a/hw/net/vhost_net.c
> >> +++ b/hw/net/vhost_net.c
> >> @@ -260,9 +260,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions
> >> *options)
> >> goto fail;
> >> }
> >> net->backend = r;
> >> - net->dev.protocol_features = 0;
> >> } else {
> >> - net->dev.protocol_features = 0;
> >> net->backend = -1;
> >>
> >> /* vhost-user needs vq_index to initiate a specific queue pair */
> >> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >> index 6fa5b8a8bd..abdf47ee7b 100644
> >> --- a/hw/virtio/vhost-user.c
> >> +++ b/hw/virtio/vhost-user.c
> >> @@ -11,6 +11,7 @@
> >> #include "qemu/osdep.h"
> >> #include "qapi/error.h"
> >> #include "hw/virtio/virtio-dmabuf.h"
> >> +#include "hw/virtio/virtio-qmp.h"
> >> #include "hw/virtio/vhost.h"
> >> #include "hw/virtio/virtio-crypto.h"
> >> #include "hw/virtio/vhost-user.h"
> >> @@ -264,6 +265,14 @@ struct vhost_user {
> >> /* Our current regions */
> >> int num_shadow_regions;
> >> struct vhost_memory_region shadow_regions[VHOST_USER_MAX_RAM_SLOTS];
> >> +
> >> + /**
> >> + * @protocol_features: the vhost-user protocol feature set by
> >> + * VHOST_USER_SET_PROTOCOL_FEATURES. Protocol features are only
> >> + * negotiated if VHOST_USER_F_PROTOCOL_FEATURES has been offered
> >> + * by the backend (see @features).
> >> + */
> >> + uint64_t protocol_features;
> >> };
> >>
> >> struct scrub_regions {
> >> @@ -274,7 +283,8 @@ struct scrub_regions {
> >>
> >> static bool vhost_user_has_prot(struct vhost_dev *dev, uint64_t feature)
> >> {
> >> - return virtio_has_feature(dev->protocol_features, feature);
> >> + struct vhost_user *u = dev->opaque;
> >> + return virtio_has_feature(u->protocol_features, feature);
> >> }
> >>
> >> static int vhost_user_read_header(struct vhost_dev *dev, VhostUserMsg
> >> *msg)
> >> @@ -2218,8 +2228,8 @@ static int vhost_user_backend_init(struct vhost_dev
> >> *dev, void *opaque,
> >> }
> >>
> >> /* final set of protocol features */
> >> - dev->protocol_features = protocol_features;
> >> - err = vhost_user_set_protocol_features(dev,
> >> dev->protocol_features);
> >> + u->protocol_features = protocol_features;
> >> + err = vhost_user_set_protocol_features(dev, u->protocol_features);
> >> if (err < 0) {
> >> error_setg_errno(errp, EPROTO, "vhost_backend_init failed");
> >> return -EPROTO;
> >> @@ -3001,6 +3011,12 @@ static int vhost_user_check_device_state(struct
> >> vhost_dev *dev, Error **errp)
> >> return 0;
> >> }
> >>
> >> +static void vhost_user_qmp_status(struct vhost_dev *dev, VhostStatus
> >> *status)
> >> +{
> >> + struct vhost_user *u = dev->opaque;
> >> + status->protocol_features =
> >> qmp_decode_protocols(u->protocol_features);
> >> +}
> >> +
> >> const VhostOps user_ops = {
> >> .backend_type = VHOST_BACKEND_TYPE_USER,
> >> .vhost_backend_init = vhost_user_backend_init,
> >> @@ -3041,4 +3057,5 @@ const VhostOps user_ops = {
> >> .vhost_supports_device_state = vhost_user_supports_device_state,
> >> .vhost_set_device_state_fd = vhost_user_set_device_state_fd,
> >> .vhost_check_device_state = vhost_user_check_device_state,
> >> + .vhost_qmp_status = vhost_user_qmp_status,
> >> };
> >> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> >> index e514a4797e..d55b12f9f3 100644
> >> --- a/hw/virtio/virtio-qmp.c
> >> +++ b/hw/virtio/virtio-qmp.c
> >> @@ -788,12 +788,14 @@ VirtioStatus *qmp_x_query_virtio_status(const char
> >> *path, Error **errp)
> >> qmp_decode_features(vdev->device_id, hdev->features);
> >> status->vhost_dev->acked_features =
> >> qmp_decode_features(vdev->device_id, hdev->acked_features);
> >> - status->vhost_dev->protocol_features =
> >> - qmp_decode_protocols(hdev->protocol_features);
> >> status->vhost_dev->max_queues = hdev->max_queues;
> >> status->vhost_dev->backend_cap = hdev->backend_cap;
> >> status->vhost_dev->log_enabled = hdev->log_enabled;
> >> status->vhost_dev->log_size = hdev->log_size;
> >> +
> >> + if (hdev->vhost_ops->vhost_qmp_status) {
> >> + hdev->vhost_ops->vhost_qmp_status(hdev, status->vhost_dev);
> >> + }
> >
> > Same comment as patch 1/33 - why have it in vhost_ops if it is
> > vhost_user specific, rather than checking the backend type and calling
> > a helper?
> >
>
> Aha, I think now, I undestand you correctly in 1/33.
>
> No specific reason, but a try to keep generic code backend-agnostic, without
> knowledge about specific backends.
>
> Not a problem for me to swith to "if (backend_type == ", if it's preferable
> in this case.
>
Yes, that is what I meant. I think it makes the code more readable but
if others disagree we can leave it.
> >
> >> }
> >>
> >> return status;
> >> diff --git a/include/hw/virtio/vhost-backend.h
> >> b/include/hw/virtio/vhost-backend.h
> >> index f65fa26298..0785fc764d 100644
> >> --- a/include/hw/virtio/vhost-backend.h
> >> +++ b/include/hw/virtio/vhost-backend.h
> >> @@ -12,6 +12,7 @@
> >> #define VHOST_BACKEND_H
> >>
> >> #include "system/memory.h"
> >> +#include "qapi/qapi-commands-virtio.h"
> >>
> >> typedef enum VhostBackendType {
> >> VHOST_BACKEND_TYPE_NONE = 0,
> >> @@ -160,6 +161,7 @@ typedef int (*vhost_set_device_state_fd_op)(struct
> >> vhost_dev *dev,
> >> int *reply_fd,
> >> Error **errp);
> >> typedef int (*vhost_check_device_state_op)(struct vhost_dev *dev, Error
> >> **errp);
> >> +typedef void (*vhost_qmp_status_op)(struct vhost_dev *dev, VhostStatus
> >> *status);
> >>
> >> typedef struct VhostOps {
> >> VhostBackendType backend_type;
> >> @@ -216,6 +218,7 @@ typedef struct VhostOps {
> >> vhost_supports_device_state_op vhost_supports_device_state;
> >> vhost_set_device_state_fd_op vhost_set_device_state_fd;
> >> vhost_check_device_state_op vhost_check_device_state;
> >> + vhost_qmp_status_op vhost_qmp_status;
> >> } VhostOps;
> >>
> >> int vhost_backend_update_device_iotlb(struct vhost_dev *dev,
> >> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> >> index 9f9dd2d46d..15bc287a9d 100644
> >> --- a/include/hw/virtio/vhost.h
> >> +++ b/include/hw/virtio/vhost.h
> >> @@ -104,14 +104,6 @@ struct vhost_dev {
> >> uint64_t features;
> >> uint64_t acked_features;
> >>
> >> - /**
> >> - * @protocol_features: is the vhost-user only feature set by
> >> - * VHOST_USER_SET_PROTOCOL_FEATURES. Protocol features are only
> >> - * negotiated if VHOST_USER_F_PROTOCOL_FEATURES has been offered
> >> - * by the backend (see @features).
> >> - */
> >> - uint64_t protocol_features;
> >> -
> >> uint64_t max_queues;
> >> uint64_t backend_cap;
> >> /* @started: is the vhost device started? */
> >> --
> >> 2.48.1
> >>
> >>
>
>
> --
> Best regards,
> Vladimir
