* [PATCH v2 01/23] vhost-user: rework enabling vrings
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 18:25 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
` (22 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, Gonglei (Arei), Zhenwei Pi, Jason Wang
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
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 01/23] vhost-user: rework enabling vrings
2025-10-11 23:23 ` [PATCH v2 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
@ 2025-10-13 18:25 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 18:25 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
Gonglei (Arei), Zhenwei Pi, Jason Wang
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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 02/23] vhost: drop backend_features field
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 4:42 ` Markus Armbruster
` (2 more replies)
2025-10-11 23:23 ` [PATCH v2 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper Vladimir Sementsov-Ogievskiy
` (21 subsequent siblings)
23 siblings, 3 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, qemu-stable, Kevin Wolf, Hanna Reitz,
Jason Wang, Paolo Bonzini, Fam Zheng, Eric Blake,
Markus Armbruster, open list:Block layer core
This field is mostly unused and sometimes confusing (we even have
a TODO-like comment to drop it). Let's finally do.
The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
vhost-user-net). But we can simply recalculate these two flags in place
from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
Note: removing field from x-query-virtio-status result is incompatible
change. We can do it because the command is unstable.
Cc: qemu-stable@nongnu.org
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/block/vhost-user-blk.c | 1 -
hw/net/vhost_net.c | 26 ++++++++++++++------------
hw/scsi/vhost-scsi.c | 1 -
hw/scsi/vhost-user-scsi.c | 1 -
hw/virtio/vdpa-dev.c | 1 -
hw/virtio/vhost-user.c | 17 ++++++++---------
hw/virtio/virtio-hmp-cmds.c | 2 --
hw/virtio/virtio-qmp.c | 4 ----
include/hw/virtio/vhost.h | 7 -------
qapi/virtio.json | 3 ---
10 files changed, 22 insertions(+), 41 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index c0cc5f6942..de7a810c93 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -348,7 +348,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
s->dev.nvqs = s->num_queues;
s->dev.vqs = s->vhost_vqs;
s->dev.vq_index = 0;
- s->dev.backend_features = 0;
vhost_dev_set_config_notifier(&s->dev, &blk_ops);
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 25e9f1fd24..fda90e231e 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -52,8 +52,14 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
void vhost_net_ack_features_ex(struct vhost_net *net, const uint64_t *features)
{
- virtio_features_copy(net->dev.acked_features_ex,
- net->dev.backend_features_ex);
+ virtio_features_clear(net->dev.acked_features_ex);
+ if (net->backend == -1) {
+ net->dev.acked_features =
+ net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
+ } else if (!qemu_has_vnet_hdr(net->nc)) {
+ net->dev.acked_features = 1ULL << VHOST_NET_F_VIRTIO_NET_HDR;
+ }
+
vhost_ack_features_ex(&net->dev, net->feature_bits, features);
}
@@ -258,12 +264,9 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
if (r < 0) {
goto fail;
}
- net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend)
- ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
net->backend = r;
net->dev.protocol_features = 0;
} else {
- virtio_features_clear(net->dev.backend_features_ex);
net->dev.protocol_features = 0;
net->backend = -1;
@@ -284,13 +287,12 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
}
- if (virtio_features_andnot(missing_features,
- net->dev.backend_features_ex,
- net->dev.features_ex)) {
- fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
- " for backend\n", VIRTIO_FEATURES_PR(missing_features));
- goto fail;
- }
+ if (!qemu_has_vnet_hdr(options->net_backend) &&
+ (~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR))) {
+ fprintf(stderr, "vhost lacks feature mask 0x%llx for backend\n",
+ ~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR));
+ goto fail;
+ }
}
/* Set sane init value. Override when guest acks. */
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index cdf405b0f8..d694a25fe2 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -276,7 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
vsc->dev.vqs = vqs;
vsc->dev.vq_index = 0;
- vsc->dev.backend_features = 0;
ret = vhost_dev_init(&vsc->dev, (void *)(uintptr_t)vhostfd,
VHOST_BACKEND_TYPE_KERNEL, 0, errp);
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 25f2d894e7..0c80a271d8 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -159,7 +159,6 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
vsc->dev.vqs = s->vhost_vqs;
vsc->dev.vq_index = 0;
- vsc->dev.backend_features = 0;
ret = vhost_dev_init(&vsc->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
errp);
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
index 4a7b970976..efd9f68420 100644
--- a/hw/virtio/vdpa-dev.c
+++ b/hw/virtio/vdpa-dev.c
@@ -104,7 +104,6 @@ static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
v->dev.vqs = vqs;
v->dev.vq_index = 0;
v->dev.vq_index_end = v->dev.nvqs;
- v->dev.backend_features = 0;
v->started = false;
ret = vhost_vdpa_get_iova_range(v->vhostfd, &iova_range);
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index f367ce06ce..b80174f489 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1448,14 +1448,15 @@ static int vhost_user_set_features(struct vhost_dev *dev,
int ret;
/*
- * We need to include any extra backend only feature bits that
- * might be needed by our device. Currently this includes the
- * VHOST_USER_F_PROTOCOL_FEATURES bit for enabling protocol
- * features.
+ * Don't lose VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user
+ * specific.
*/
- ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
- features | dev->backend_features,
- log_enabled);
+ if (virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
+ features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
+ }
+
+ ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
+ log_enabled);
if (virtio_has_feature(dev->protocol_features,
VHOST_USER_PROTOCOL_F_STATUS)) {
@@ -2189,8 +2190,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
(dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
uint64_t protocol_features;
- dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
-
err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
&protocol_features);
if (err < 0) {
diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
index 1daae482d3..4bf9a3109d 100644
--- a/hw/virtio/virtio-hmp-cmds.c
+++ b/hw/virtio/virtio-hmp-cmds.c
@@ -176,8 +176,6 @@ void hmp_virtio_status(Monitor *mon, const QDict *qdict)
hmp_virtio_dump_features(mon, s->vhost_dev->features);
monitor_printf(mon, " Acked features:\n");
hmp_virtio_dump_features(mon, s->vhost_dev->acked_features);
- monitor_printf(mon, " Backend features:\n");
- hmp_virtio_dump_features(mon, s->vhost_dev->backend_features);
monitor_printf(mon, " Protocol features:\n");
hmp_virtio_dump_protocols(mon, s->vhost_dev->protocol_features);
}
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index b338344c6c..dd1a38e792 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -780,8 +780,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
vdev->guest_features_ex);
status->host_features = qmp_decode_features(vdev->device_id,
vdev->host_features_ex);
- status->backend_features = qmp_decode_features(vdev->device_id,
- vdev->backend_features_ex);
switch (vdev->device_endian) {
case VIRTIO_DEVICE_ENDIAN_LITTLE:
@@ -822,8 +820,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
qmp_decode_features(vdev->device_id, hdev->features_ex);
status->vhost_dev->acked_features =
qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
- status->vhost_dev->backend_features =
- qmp_decode_features(vdev->device_id, hdev->backend_features_ex);
status->vhost_dev->protocol_features =
qmp_decode_protocols(hdev->protocol_features);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 1ee639dd7e..3e69e47833 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -100,16 +100,9 @@ struct vhost_dev {
*
* @features: available features provided by the backend
* @acked_features: final negotiated features with front-end driver
- *
- * @backend_features: this is used in a couple of places to either
- * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
- * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
- * future use should be discouraged and the variable retired as
- * its easy to confuse with the VirtIO backend_features.
*/
VIRTIO_DECLARE_FEATURES(features);
VIRTIO_DECLARE_FEATURES(acked_features);
- VIRTIO_DECLARE_FEATURES(backend_features);
/**
* @protocol_features: is the vhost-user only feature set by
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 05295ab665..b995a5bb6d 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -85,8 +85,6 @@
#
# @acked-features: vhost_dev acked_features
#
-# @backend-features: vhost_dev backend_features
-#
# @protocol-features: vhost_dev protocol_features
#
# @max-queues: vhost_dev max_queues
@@ -106,7 +104,6 @@
'vq-index': 'int',
'features': 'VirtioDeviceFeatures',
'acked-features': 'VirtioDeviceFeatures',
- 'backend-features': 'VirtioDeviceFeatures',
'protocol-features': 'VhostDeviceProtocols',
'max-queues': 'uint64',
'backend-cap': 'uint64',
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 02/23] vhost: drop backend_features field
2025-10-11 23:23 ` [PATCH v2 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
@ 2025-10-13 4:42 ` Markus Armbruster
2025-10-13 8:32 ` Vladimir Sementsov-Ogievskiy
2025-10-13 18:40 ` Raphael Norwitz
2025-10-14 15:28 ` Michael Tokarev
2 siblings, 1 reply; 43+ messages in thread
From: Markus Armbruster @ 2025-10-13 4:42 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, raphael.s.norwitz, yc-core,
d-tatianin, qemu-stable, Kevin Wolf, Hanna Reitz, Jason Wang,
Paolo Bonzini, Fam Zheng, Eric Blake, open list:Block layer core
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> This field is mostly unused and sometimes confusing (we even have
> a TODO-like comment to drop it). Let's finally do.
>
> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
> vhost-user-net). But we can simply recalculate these two flags in place
> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>
> Note: removing field from x-query-virtio-status result is incompatible
> change. We can do it because the command is unstable.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
You improved the commit message like I suggested, but lost my
Acked-by: Markus Armbruster <armbru@redhat.com>
:)
[...]
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 1ee639dd7e..3e69e47833 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -100,16 +100,9 @@ struct vhost_dev {
> *
> * @features: available features provided by the backend
> * @acked_features: final negotiated features with front-end driver
> - *
> - * @backend_features: this is used in a couple of places to either
> - * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
> - * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
> - * future use should be discouraged and the variable retired as
> - * its easy to confuse with the VirtIO backend_features.
> */
I guess this is the TODO-like comment mentioned in the commit message.
> VIRTIO_DECLARE_FEATURES(features);
> VIRTIO_DECLARE_FEATURES(acked_features);
> - VIRTIO_DECLARE_FEATURES(backend_features);
>
> /**
> * @protocol_features: is the vhost-user only feature set by
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 05295ab665..b995a5bb6d 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -85,8 +85,6 @@
> #
> # @acked-features: vhost_dev acked_features
> #
> -# @backend-features: vhost_dev backend_features
> -#
> # @protocol-features: vhost_dev protocol_features
> #
> # @max-queues: vhost_dev max_queues
> @@ -106,7 +104,6 @@
> 'vq-index': 'int',
> 'features': 'VirtioDeviceFeatures',
> 'acked-features': 'VirtioDeviceFeatures',
> - 'backend-features': 'VirtioDeviceFeatures',
> 'protocol-features': 'VhostDeviceProtocols',
> 'max-queues': 'uint64',
> 'backend-cap': 'uint64',
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 02/23] vhost: drop backend_features field
2025-10-13 4:42 ` Markus Armbruster
@ 2025-10-13 8:32 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-13 8:32 UTC (permalink / raw)
To: Markus Armbruster
Cc: mst, sgarzare, raphael, qemu-devel, raphael.s.norwitz, yc-core,
d-tatianin, qemu-stable, Kevin Wolf, Hanna Reitz, Jason Wang,
Paolo Bonzini, Fam Zheng, Eric Blake, open list:Block layer core
On 13.10.25 07:42, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>
>> This field is mostly unused and sometimes confusing (we even have
>> a TODO-like comment to drop it). Let's finally do.
>>
>> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
>> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
>> vhost-user-net). But we can simply recalculate these two flags in place
>> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>>
>> Note: removing field from x-query-virtio-status result is incompatible
>> change. We can do it because the command is unstable.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>
> You improved the commit message like I suggested, but lost my
I decided that commit is changed enough, so that it's not safe to save any r-b/a-b marks.
> Acked-by: Markus Armbruster <armbru@redhat.com>
> :)
Thanks!
>
> [...]
>
>> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
>> index 1ee639dd7e..3e69e47833 100644
>> --- a/include/hw/virtio/vhost.h
>> +++ b/include/hw/virtio/vhost.h
>> @@ -100,16 +100,9 @@ struct vhost_dev {
>> *
>> * @features: available features provided by the backend
>> * @acked_features: final negotiated features with front-end driver
>> - *
>> - * @backend_features: this is used in a couple of places to either
>> - * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
>> - * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
>> - * future use should be discouraged and the variable retired as
>> - * its easy to confuse with the VirtIO backend_features.
>> */
>
> I guess this is the TODO-like comment mentioned in the commit message.
yes)
>
>> VIRTIO_DECLARE_FEATURES(features);
>> VIRTIO_DECLARE_FEATURES(acked_features);
>> - VIRTIO_DECLARE_FEATURES(backend_features);
>>
>> /**
>> * @protocol_features: is the vhost-user only feature set by
>> diff --git a/qapi/virtio.json b/qapi/virtio.json
>> index 05295ab665..b995a5bb6d 100644
>> --- a/qapi/virtio.json
>> +++ b/qapi/virtio.json
>> @@ -85,8 +85,6 @@
>> #
>> # @acked-features: vhost_dev acked_features
>> #
>> -# @backend-features: vhost_dev backend_features
>> -#
>> # @protocol-features: vhost_dev protocol_features
>> #
>> # @max-queues: vhost_dev max_queues
>> @@ -106,7 +104,6 @@
>> 'vq-index': 'int',
>> 'features': 'VirtioDeviceFeatures',
>> 'acked-features': 'VirtioDeviceFeatures',
>> - 'backend-features': 'VirtioDeviceFeatures',
>> 'protocol-features': 'VhostDeviceProtocols',
>> 'max-queues': 'uint64',
>> 'backend-cap': 'uint64',
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 02/23] vhost: drop backend_features field
2025-10-11 23:23 ` [PATCH v2 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
2025-10-13 4:42 ` Markus Armbruster
@ 2025-10-13 18:40 ` Raphael Norwitz
2025-10-14 15:28 ` Michael Tokarev
2 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 18:40 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
qemu-stable, Kevin Wolf, Hanna Reitz, Jason Wang, Paolo Bonzini,
Fam Zheng, Eric Blake, Markus Armbruster,
open list:Block layer core
Acked-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:
>
> This field is mostly unused and sometimes confusing (we even have
> a TODO-like comment to drop it). Let's finally do.
>
> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
> vhost-user-net). But we can simply recalculate these two flags in place
> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>
> Note: removing field from x-query-virtio-status result is incompatible
> change. We can do it because the command is unstable.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/block/vhost-user-blk.c | 1 -
> hw/net/vhost_net.c | 26 ++++++++++++++------------
> hw/scsi/vhost-scsi.c | 1 -
> hw/scsi/vhost-user-scsi.c | 1 -
> hw/virtio/vdpa-dev.c | 1 -
> hw/virtio/vhost-user.c | 17 ++++++++---------
> hw/virtio/virtio-hmp-cmds.c | 2 --
> hw/virtio/virtio-qmp.c | 4 ----
> include/hw/virtio/vhost.h | 7 -------
> qapi/virtio.json | 3 ---
> 10 files changed, 22 insertions(+), 41 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index c0cc5f6942..de7a810c93 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -348,7 +348,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> s->dev.nvqs = s->num_queues;
> s->dev.vqs = s->vhost_vqs;
> s->dev.vq_index = 0;
> - s->dev.backend_features = 0;
>
> vhost_dev_set_config_notifier(&s->dev, &blk_ops);
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 25e9f1fd24..fda90e231e 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -52,8 +52,14 @@ int vhost_net_set_config(struct vhost_net *net, const uint8_t *data,
>
> void vhost_net_ack_features_ex(struct vhost_net *net, const uint64_t *features)
> {
> - virtio_features_copy(net->dev.acked_features_ex,
> - net->dev.backend_features_ex);
> + virtio_features_clear(net->dev.acked_features_ex);
> + if (net->backend == -1) {
> + net->dev.acked_features =
> + net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> + } else if (!qemu_has_vnet_hdr(net->nc)) {
> + net->dev.acked_features = 1ULL << VHOST_NET_F_VIRTIO_NET_HDR;
> + }
> +
> vhost_ack_features_ex(&net->dev, net->feature_bits, features);
> }
>
> @@ -258,12 +264,9 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
> if (r < 0) {
> goto fail;
> }
> - net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend)
> - ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
> net->backend = r;
> net->dev.protocol_features = 0;
> } else {
> - virtio_features_clear(net->dev.backend_features_ex);
> net->dev.protocol_features = 0;
> net->backend = -1;
>
> @@ -284,13 +287,12 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
> net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
> }
>
> - if (virtio_features_andnot(missing_features,
> - net->dev.backend_features_ex,
> - net->dev.features_ex)) {
> - fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
> - " for backend\n", VIRTIO_FEATURES_PR(missing_features));
> - goto fail;
> - }
> + if (!qemu_has_vnet_hdr(options->net_backend) &&
> + (~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR))) {
> + fprintf(stderr, "vhost lacks feature mask 0x%llx for backend\n",
> + ~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR));
> + goto fail;
> + }
> }
>
> /* Set sane init value. Override when guest acks. */
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index cdf405b0f8..d694a25fe2 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -276,7 +276,6 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
> vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
> vsc->dev.vqs = vqs;
> vsc->dev.vq_index = 0;
> - vsc->dev.backend_features = 0;
>
> ret = vhost_dev_init(&vsc->dev, (void *)(uintptr_t)vhostfd,
> VHOST_BACKEND_TYPE_KERNEL, 0, errp);
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 25f2d894e7..0c80a271d8 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -159,7 +159,6 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
> vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
> vsc->dev.vqs = s->vhost_vqs;
> vsc->dev.vq_index = 0;
> - vsc->dev.backend_features = 0;
>
> ret = vhost_dev_init(&vsc->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0,
> errp);
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index 4a7b970976..efd9f68420 100644
> --- a/hw/virtio/vdpa-dev.c
> +++ b/hw/virtio/vdpa-dev.c
> @@ -104,7 +104,6 @@ static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
> v->dev.vqs = vqs;
> v->dev.vq_index = 0;
> v->dev.vq_index_end = v->dev.nvqs;
> - v->dev.backend_features = 0;
> v->started = false;
>
> ret = vhost_vdpa_get_iova_range(v->vhostfd, &iova_range);
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index f367ce06ce..b80174f489 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1448,14 +1448,15 @@ static int vhost_user_set_features(struct vhost_dev *dev,
> int ret;
>
> /*
> - * We need to include any extra backend only feature bits that
> - * might be needed by our device. Currently this includes the
> - * VHOST_USER_F_PROTOCOL_FEATURES bit for enabling protocol
> - * features.
> + * Don't lose VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user
> + * specific.
> */
> - ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
> - features | dev->backend_features,
> - log_enabled);
> + if (virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> + features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> + }
> +
> + ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
> + log_enabled);
>
> if (virtio_has_feature(dev->protocol_features,
> VHOST_USER_PROTOCOL_F_STATUS)) {
> @@ -2189,8 +2190,6 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> (dev->config_ops && dev->config_ops->vhost_dev_config_notifier);
> uint64_t protocol_features;
>
> - dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> -
> err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES,
> &protocol_features);
> if (err < 0) {
> diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
> index 1daae482d3..4bf9a3109d 100644
> --- a/hw/virtio/virtio-hmp-cmds.c
> +++ b/hw/virtio/virtio-hmp-cmds.c
> @@ -176,8 +176,6 @@ void hmp_virtio_status(Monitor *mon, const QDict *qdict)
> hmp_virtio_dump_features(mon, s->vhost_dev->features);
> monitor_printf(mon, " Acked features:\n");
> hmp_virtio_dump_features(mon, s->vhost_dev->acked_features);
> - monitor_printf(mon, " Backend features:\n");
> - hmp_virtio_dump_features(mon, s->vhost_dev->backend_features);
> monitor_printf(mon, " Protocol features:\n");
> hmp_virtio_dump_protocols(mon, s->vhost_dev->protocol_features);
> }
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index b338344c6c..dd1a38e792 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -780,8 +780,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
> vdev->guest_features_ex);
> status->host_features = qmp_decode_features(vdev->device_id,
> vdev->host_features_ex);
> - status->backend_features = qmp_decode_features(vdev->device_id,
> - vdev->backend_features_ex);
>
> switch (vdev->device_endian) {
> case VIRTIO_DEVICE_ENDIAN_LITTLE:
> @@ -822,8 +820,6 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
> qmp_decode_features(vdev->device_id, hdev->features_ex);
> status->vhost_dev->acked_features =
> qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
> - status->vhost_dev->backend_features =
> - qmp_decode_features(vdev->device_id, hdev->backend_features_ex);
>
> status->vhost_dev->protocol_features =
> qmp_decode_protocols(hdev->protocol_features);
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 1ee639dd7e..3e69e47833 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -100,16 +100,9 @@ struct vhost_dev {
> *
> * @features: available features provided by the backend
> * @acked_features: final negotiated features with front-end driver
> - *
> - * @backend_features: this is used in a couple of places to either
> - * store VHOST_USER_F_PROTOCOL_FEATURES to apply to
> - * VHOST_USER_SET_FEATURES or VHOST_NET_F_VIRTIO_NET_HDR. Its
> - * future use should be discouraged and the variable retired as
> - * its easy to confuse with the VirtIO backend_features.
> */
> VIRTIO_DECLARE_FEATURES(features);
> VIRTIO_DECLARE_FEATURES(acked_features);
> - VIRTIO_DECLARE_FEATURES(backend_features);
>
> /**
> * @protocol_features: is the vhost-user only feature set by
> diff --git a/qapi/virtio.json b/qapi/virtio.json
> index 05295ab665..b995a5bb6d 100644
> --- a/qapi/virtio.json
> +++ b/qapi/virtio.json
> @@ -85,8 +85,6 @@
> #
> # @acked-features: vhost_dev acked_features
> #
> -# @backend-features: vhost_dev backend_features
> -#
> # @protocol-features: vhost_dev protocol_features
> #
> # @max-queues: vhost_dev max_queues
> @@ -106,7 +104,6 @@
> 'vq-index': 'int',
> 'features': 'VirtioDeviceFeatures',
> 'acked-features': 'VirtioDeviceFeatures',
> - 'backend-features': 'VirtioDeviceFeatures',
> 'protocol-features': 'VhostDeviceProtocols',
> 'max-queues': 'uint64',
> 'backend-cap': 'uint64',
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 02/23] vhost: drop backend_features field
2025-10-11 23:23 ` [PATCH v2 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
2025-10-13 4:42 ` Markus Armbruster
2025-10-13 18:40 ` Raphael Norwitz
@ 2025-10-14 15:28 ` Michael Tokarev
2025-10-14 15:34 ` Vladimir Sementsov-Ogievskiy
2 siblings, 1 reply; 43+ messages in thread
From: Michael Tokarev @ 2025-10-14 15:28 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, yc-core,
d-tatianin, qemu-stable, Kevin Wolf, Hanna Reitz, Jason Wang,
Paolo Bonzini, Fam Zheng, Eric Blake, Markus Armbruster,
open list:Block layer core
On 10/12/25 02:23, Vladimir Sementsov-Ogievskiy wrote:
> This field is mostly unused and sometimes confusing (we even have
> a TODO-like comment to drop it). Let's finally do.
>
> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
> vhost-user-net). But we can simply recalculate these two flags in place
> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>
> Note: removing field from x-query-virtio-status result is incompatible
> change. We can do it because the command is unstable.
>
> Cc: qemu-stable@nongnu.org
Why do you think this change deserves qemu-stable? Does it fix a bug or
is it just a clean-up?
Thanks,
/mjt
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 02/23] vhost: drop backend_features field
2025-10-14 15:28 ` Michael Tokarev
@ 2025-10-14 15:34 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-14 15:34 UTC (permalink / raw)
To: Michael Tokarev, mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, yc-core,
d-tatianin, qemu-stable, Kevin Wolf, Hanna Reitz, Jason Wang,
Paolo Bonzini, Fam Zheng, Eric Blake, Markus Armbruster,
open list:Block layer core
On 14.10.25 18:28, Michael Tokarev wrote:
> On 10/12/25 02:23, Vladimir Sementsov-Ogievskiy wrote:
>> This field is mostly unused and sometimes confusing (we even have
>> a TODO-like comment to drop it). Let's finally do.
>>
>> The field is used to held VHOST_USER_F_PROTOCOL_FEATURES for vhost-user
>> and/or VHOST_NET_F_VIRTIO_NET_HDR for vhost-net (which may be
>> vhost-user-net). But we can simply recalculate these two flags in place
>> from hdev->features, and from net-client for VHOST_NET_F_VIRTIO_NET_HDR.
>>
>> Note: removing field from x-query-virtio-status result is incompatible
>> change. We can do it because the command is unstable.
>>
>> Cc: qemu-stable@nongnu.org
>
> Why do you think this change deserves qemu-stable? Does it fix a bug or
> is it just a clean-up?
>
Oops, it's a mistake (working too late).
I wanted to write
Cc: devel@lists.libvirt.org
:)
Seems, v3 is needed anyway, will fix it.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 18:46 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 04/23] vhost: move protocol_features to vhost_user Vladimir Sementsov-Ogievskiy
` (20 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Make all protocol feature checks in the same way.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/vhost-user.c | 120 +++++++++++++++++++++--------------------
1 file changed, 62 insertions(+), 58 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b80174f489..b8231dcbcf 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -272,6 +272,12 @@ struct scrub_regions {
int fd_idx;
};
+static bool vhost_user_has_protocol_feature(struct vhost_dev *dev,
+ uint64_t feature)
+{
+ return virtio_has_feature(dev->protocol_features, feature);
+}
+
static int vhost_user_read_header(struct vhost_dev *dev, VhostUserMsg *msg)
{
struct vhost_user *u = dev->opaque;
@@ -435,8 +441,8 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
{
int fds[VHOST_USER_MAX_RAM_SLOTS];
size_t fd_num = 0;
- bool shmfd = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_LOG_SHMFD);
+ bool shmfd =
+ vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_LOG_SHMFD);
int ret;
VhostUserMsg msg = {
.hdr.request = VHOST_USER_SET_LOG_BASE,
@@ -1006,11 +1012,11 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
int fds[VHOST_MEMORY_BASELINE_NREGIONS];
size_t fd_num = 0;
bool do_postcopy = u->postcopy_listen && u->postcopy_fd.handler;
- bool reply_supported = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_REPLY_ACK);
+ bool reply_supported =
+ vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
bool config_mem_slots =
- virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS);
+ vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS);
int ret;
if (do_postcopy) {
@@ -1058,8 +1064,9 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
static int vhost_user_set_vring_endian(struct vhost_dev *dev,
struct vhost_vring_state *ring)
{
- bool cross_endian = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_CROSS_ENDIAN);
+ bool cross_endian =
+ vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_CROSS_ENDIAN);
VhostUserMsg msg = {
.hdr.request = VHOST_USER_SET_VRING_ENDIAN,
.hdr.flags = VHOST_USER_VERSION,
@@ -1129,8 +1136,9 @@ static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
int ret;
if (wait_for_reply) {
- bool reply_supported = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_REPLY_ACK);
+ bool reply_supported =
+ vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
if (reply_supported) {
msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
}
@@ -1458,8 +1466,7 @@ static int vhost_user_set_features(struct vhost_dev *dev,
ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
log_enabled);
- if (virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_STATUS)) {
+ if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_STATUS)) {
if (!ret) {
return vhost_user_add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
}
@@ -1513,8 +1520,8 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
* Historically, reset was not implemented so only reset devices
* that are expecting it.
*/
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
+ if (!vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
return -ENOSYS;
}
@@ -1571,8 +1578,8 @@ static int vhost_user_backend_handle_vring_host_notifier(struct vhost_dev *dev,
void *addr;
char *name;
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_HOST_NOTIFIER) ||
+ if (!vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_HOST_NOTIFIER) ||
vdev == NULL || queue_idx >= virtio_get_num_queues(vdev)) {
return -EINVAL;
}
@@ -1884,13 +1891,13 @@ static int vhost_setup_backend_channel(struct vhost_dev *dev)
};
struct vhost_user *u = dev->opaque;
int sv[2], ret = 0;
- bool reply_supported = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_REPLY_ACK);
+ bool reply_supported =
+ vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
Error *local_err = NULL;
QIOChannel *ioc;
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_BACKEND_REQ)) {
+ if (!vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_BACKEND_REQ)) {
return 0;
}
@@ -2138,8 +2145,8 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
switch (pnd->reason) {
case POSTCOPY_NOTIFY_PROBE:
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_PAGEFAULT)) {
+ if (!vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_PAGEFAULT)) {
/* TODO: Get the device name into this error somehow */
error_setg(errp,
"vhost-user backend not capable of postcopy");
@@ -2230,7 +2237,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
}
/* query the max queues we support if backend supports Multiple Queue */
- if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)) {
+ if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_MQ)) {
err = vhost_user_get_u64(dev, VHOST_USER_GET_QUEUE_NUM,
&dev->max_queues);
if (err < 0) {
@@ -2248,18 +2255,18 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
}
if (virtio_has_feature(features, VIRTIO_F_IOMMU_PLATFORM) &&
- !(virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_BACKEND_REQ) &&
- virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_REPLY_ACK))) {
+ !(vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_BACKEND_REQ) &&
+ vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_REPLY_ACK))) {
error_setg(errp, "IOMMU support requires reply-ack and "
"backend-req protocol features.");
return -EINVAL;
}
/* get max memory regions if backend supports configurable RAM slots */
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS)) {
+ if (!vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS)) {
u->user->memory_slots = VHOST_MEMORY_BASELINE_NREGIONS;
} else {
err = vhost_user_get_max_memslots(dev, &ram_slots);
@@ -2281,8 +2288,8 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
}
if (dev->migration_blocker == NULL &&
- !virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_LOG_SHMFD)) {
+ !vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_LOG_SHMFD)) {
error_setg(&dev->migration_blocker,
"Migration disabled: vhost-user backend lacks "
"VHOST_USER_PROTOCOL_F_LOG_SHMFD feature.");
@@ -2351,8 +2358,8 @@ static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
{
assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
- return virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_LOG_SHMFD);
+ return vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_LOG_SHMFD);
}
static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
@@ -2367,8 +2374,7 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
}
/* if backend supports VHOST_USER_PROTOCOL_F_RARP ask it to send the RARP */
- if (virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_RARP)) {
+ if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_RARP)) {
msg.hdr.request = VHOST_USER_SEND_RARP;
msg.hdr.flags = VHOST_USER_VERSION;
memcpy((char *)&msg.payload.u64, mac_addr, 6);
@@ -2382,11 +2388,11 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
{
VhostUserMsg msg;
- bool reply_supported = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_REPLY_ACK);
+ bool reply_supported =
+ vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
int ret;
- if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_NET_MTU))) {
+ if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_NET_MTU)) {
return 0;
}
@@ -2446,8 +2452,7 @@ static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
.hdr.size = VHOST_USER_CONFIG_HDR_SIZE + config_len,
};
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_CONFIG)) {
+ if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_CONFIG)) {
error_setg(errp, "VHOST_USER_PROTOCOL_F_CONFIG not supported");
return -EINVAL;
}
@@ -2490,8 +2495,8 @@ static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data,
{
int ret;
uint8_t *p;
- bool reply_supported = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_REPLY_ACK);
+ bool reply_supported =
+ vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
VhostUserMsg msg = {
.hdr.request = VHOST_USER_SET_CONFIG,
@@ -2499,8 +2504,7 @@ static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data,
.hdr.size = VHOST_USER_CONFIG_HDR_SIZE + size,
};
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_CONFIG)) {
+ if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_CONFIG)) {
return -ENOTSUP;
}
@@ -2535,8 +2539,9 @@ static int vhost_user_crypto_create_session(struct vhost_dev *dev,
uint64_t *session_id)
{
int ret;
- bool crypto_session = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
+ bool crypto_session =
+ vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
CryptoDevBackendSessionInfo *backend_info = session_info;
VhostUserMsg msg = {
.hdr.request = VHOST_USER_CREATE_CRYPTO_SESSION,
@@ -2637,8 +2642,9 @@ static int
vhost_user_crypto_close_session(struct vhost_dev *dev, uint64_t session_id)
{
int ret;
- bool crypto_session = virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
+ bool crypto_session =
+ vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
VhostUserMsg msg = {
.hdr.request = VHOST_USER_CLOSE_CRYPTO_SESSION,
.hdr.flags = VHOST_USER_VERSION,
@@ -2683,8 +2689,8 @@ static int vhost_user_get_inflight_fd(struct vhost_dev *dev,
.hdr.size = sizeof(msg.payload.inflight),
};
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
+ if (!vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
return 0;
}
@@ -2751,8 +2757,8 @@ static int vhost_user_set_inflight_fd(struct vhost_dev *dev,
.hdr.size = sizeof(msg.payload.inflight),
};
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
+ if (!vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
return 0;
}
@@ -2851,8 +2857,7 @@ void vhost_user_async_close(DeviceState *d,
static int vhost_user_dev_start(struct vhost_dev *dev, bool started)
{
- if (!virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_STATUS)) {
+ if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_STATUS)) {
return 0;
}
@@ -2877,16 +2882,15 @@ static void vhost_user_reset_status(struct vhost_dev *dev)
return;
}
- if (virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_STATUS)) {
+ if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_STATUS)) {
vhost_user_set_status(dev, 0);
}
}
static bool vhost_user_supports_device_state(struct vhost_dev *dev)
{
- return virtio_has_feature(dev->protocol_features,
- VHOST_USER_PROTOCOL_F_DEVICE_STATE);
+ return vhost_user_has_protocol_feature(
+ dev, VHOST_USER_PROTOCOL_F_DEVICE_STATE);
}
static int vhost_user_set_device_state_fd(struct vhost_dev *dev,
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper
2025-10-11 23:23 ` [PATCH v2 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper Vladimir Sementsov-Ogievskiy
@ 2025-10-13 18:46 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 18:46 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
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:
>
> Make all protocol feature checks in the same way.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/vhost-user.c | 120 +++++++++++++++++++++--------------------
> 1 file changed, 62 insertions(+), 58 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index b80174f489..b8231dcbcf 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -272,6 +272,12 @@ struct scrub_regions {
> int fd_idx;
> };
>
> +static bool vhost_user_has_protocol_feature(struct vhost_dev *dev,
> + uint64_t feature)
> +{
> + return virtio_has_feature(dev->protocol_features, feature);
> +}
> +
> static int vhost_user_read_header(struct vhost_dev *dev, VhostUserMsg *msg)
> {
> struct vhost_user *u = dev->opaque;
> @@ -435,8 +441,8 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
> {
> int fds[VHOST_USER_MAX_RAM_SLOTS];
> size_t fd_num = 0;
> - bool shmfd = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_LOG_SHMFD);
> + bool shmfd =
> + vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_LOG_SHMFD);
> int ret;
> VhostUserMsg msg = {
> .hdr.request = VHOST_USER_SET_LOG_BASE,
> @@ -1006,11 +1012,11 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
> int fds[VHOST_MEMORY_BASELINE_NREGIONS];
> size_t fd_num = 0;
> bool do_postcopy = u->postcopy_listen && u->postcopy_fd.handler;
> - bool reply_supported = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_REPLY_ACK);
> + bool reply_supported =
> + vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
> bool config_mem_slots =
> - virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS);
> + vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS);
> int ret;
>
> if (do_postcopy) {
> @@ -1058,8 +1064,9 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
> static int vhost_user_set_vring_endian(struct vhost_dev *dev,
> struct vhost_vring_state *ring)
> {
> - bool cross_endian = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_CROSS_ENDIAN);
> + bool cross_endian =
> + vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_CROSS_ENDIAN);
> VhostUserMsg msg = {
> .hdr.request = VHOST_USER_SET_VRING_ENDIAN,
> .hdr.flags = VHOST_USER_VERSION,
> @@ -1129,8 +1136,9 @@ static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg,
> int ret;
>
> if (wait_for_reply) {
> - bool reply_supported = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_REPLY_ACK);
> + bool reply_supported =
> + vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
> if (reply_supported) {
> msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
> }
> @@ -1458,8 +1466,7 @@ static int vhost_user_set_features(struct vhost_dev *dev,
> ret = vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
> log_enabled);
>
> - if (virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_STATUS)) {
> + if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_STATUS)) {
> if (!ret) {
> return vhost_user_add_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);
> }
> @@ -1513,8 +1520,8 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
> * Historically, reset was not implemented so only reset devices
> * that are expecting it.
> */
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
> + if (!vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_RESET_DEVICE)) {
> return -ENOSYS;
> }
>
> @@ -1571,8 +1578,8 @@ static int vhost_user_backend_handle_vring_host_notifier(struct vhost_dev *dev,
> void *addr;
> char *name;
>
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_HOST_NOTIFIER) ||
> + if (!vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_HOST_NOTIFIER) ||
> vdev == NULL || queue_idx >= virtio_get_num_queues(vdev)) {
> return -EINVAL;
> }
> @@ -1884,13 +1891,13 @@ static int vhost_setup_backend_channel(struct vhost_dev *dev)
> };
> struct vhost_user *u = dev->opaque;
> int sv[2], ret = 0;
> - bool reply_supported = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_REPLY_ACK);
> + bool reply_supported =
> + vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
> Error *local_err = NULL;
> QIOChannel *ioc;
>
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_BACKEND_REQ)) {
> + if (!vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_BACKEND_REQ)) {
> return 0;
> }
>
> @@ -2138,8 +2145,8 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
>
> switch (pnd->reason) {
> case POSTCOPY_NOTIFY_PROBE:
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_PAGEFAULT)) {
> + if (!vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_PAGEFAULT)) {
> /* TODO: Get the device name into this error somehow */
> error_setg(errp,
> "vhost-user backend not capable of postcopy");
> @@ -2230,7 +2237,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> }
>
> /* query the max queues we support if backend supports Multiple Queue */
> - if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)) {
> + if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_MQ)) {
> err = vhost_user_get_u64(dev, VHOST_USER_GET_QUEUE_NUM,
> &dev->max_queues);
> if (err < 0) {
> @@ -2248,18 +2255,18 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> }
>
> if (virtio_has_feature(features, VIRTIO_F_IOMMU_PLATFORM) &&
> - !(virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_BACKEND_REQ) &&
> - virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_REPLY_ACK))) {
> + !(vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_BACKEND_REQ) &&
> + vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_REPLY_ACK))) {
> error_setg(errp, "IOMMU support requires reply-ack and "
> "backend-req protocol features.");
> return -EINVAL;
> }
>
> /* get max memory regions if backend supports configurable RAM slots */
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS)) {
> + if (!vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS)) {
> u->user->memory_slots = VHOST_MEMORY_BASELINE_NREGIONS;
> } else {
> err = vhost_user_get_max_memslots(dev, &ram_slots);
> @@ -2281,8 +2288,8 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque,
> }
>
> if (dev->migration_blocker == NULL &&
> - !virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_LOG_SHMFD)) {
> + !vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_LOG_SHMFD)) {
> error_setg(&dev->migration_blocker,
> "Migration disabled: vhost-user backend lacks "
> "VHOST_USER_PROTOCOL_F_LOG_SHMFD feature.");
> @@ -2351,8 +2358,8 @@ static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
> {
> assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>
> - return virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_LOG_SHMFD);
> + return vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_LOG_SHMFD);
> }
>
> static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
> @@ -2367,8 +2374,7 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
> }
>
> /* if backend supports VHOST_USER_PROTOCOL_F_RARP ask it to send the RARP */
> - if (virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_RARP)) {
> + if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_RARP)) {
> msg.hdr.request = VHOST_USER_SEND_RARP;
> msg.hdr.flags = VHOST_USER_VERSION;
> memcpy((char *)&msg.payload.u64, mac_addr, 6);
> @@ -2382,11 +2388,11 @@ static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
> static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
> {
> VhostUserMsg msg;
> - bool reply_supported = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_REPLY_ACK);
> + bool reply_supported =
> + vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
> int ret;
>
> - if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_NET_MTU))) {
> + if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_NET_MTU)) {
> return 0;
> }
>
> @@ -2446,8 +2452,7 @@ static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
> .hdr.size = VHOST_USER_CONFIG_HDR_SIZE + config_len,
> };
>
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_CONFIG)) {
> + if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_CONFIG)) {
> error_setg(errp, "VHOST_USER_PROTOCOL_F_CONFIG not supported");
> return -EINVAL;
> }
> @@ -2490,8 +2495,8 @@ static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data,
> {
> int ret;
> uint8_t *p;
> - bool reply_supported = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_REPLY_ACK);
> + bool reply_supported =
> + vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
>
> VhostUserMsg msg = {
> .hdr.request = VHOST_USER_SET_CONFIG,
> @@ -2499,8 +2504,7 @@ static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data,
> .hdr.size = VHOST_USER_CONFIG_HDR_SIZE + size,
> };
>
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_CONFIG)) {
> + if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_CONFIG)) {
> return -ENOTSUP;
> }
>
> @@ -2535,8 +2539,9 @@ static int vhost_user_crypto_create_session(struct vhost_dev *dev,
> uint64_t *session_id)
> {
> int ret;
> - bool crypto_session = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
> + bool crypto_session =
> + vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
> CryptoDevBackendSessionInfo *backend_info = session_info;
> VhostUserMsg msg = {
> .hdr.request = VHOST_USER_CREATE_CRYPTO_SESSION,
> @@ -2637,8 +2642,9 @@ static int
> vhost_user_crypto_close_session(struct vhost_dev *dev, uint64_t session_id)
> {
> int ret;
> - bool crypto_session = virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
> + bool crypto_session =
> + vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_CRYPTO_SESSION);
> VhostUserMsg msg = {
> .hdr.request = VHOST_USER_CLOSE_CRYPTO_SESSION,
> .hdr.flags = VHOST_USER_VERSION,
> @@ -2683,8 +2689,8 @@ static int vhost_user_get_inflight_fd(struct vhost_dev *dev,
> .hdr.size = sizeof(msg.payload.inflight),
> };
>
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
> + if (!vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
> return 0;
> }
>
> @@ -2751,8 +2757,8 @@ static int vhost_user_set_inflight_fd(struct vhost_dev *dev,
> .hdr.size = sizeof(msg.payload.inflight),
> };
>
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
> + if (!vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) {
> return 0;
> }
>
> @@ -2851,8 +2857,7 @@ void vhost_user_async_close(DeviceState *d,
>
> static int vhost_user_dev_start(struct vhost_dev *dev, bool started)
> {
> - if (!virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_STATUS)) {
> + if (!vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_STATUS)) {
> return 0;
> }
>
> @@ -2877,16 +2882,15 @@ static void vhost_user_reset_status(struct vhost_dev *dev)
> return;
> }
>
> - if (virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_STATUS)) {
> + if (vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_STATUS)) {
> vhost_user_set_status(dev, 0);
> }
> }
>
> static bool vhost_user_supports_device_state(struct vhost_dev *dev)
> {
> - return virtio_has_feature(dev->protocol_features,
> - VHOST_USER_PROTOCOL_F_DEVICE_STATE);
> + return vhost_user_has_protocol_feature(
> + dev, VHOST_USER_PROTOCOL_F_DEVICE_STATE);
> }
>
> static int vhost_user_set_device_state_fd(struct vhost_dev *dev,
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 04/23] vhost: move protocol_features to vhost_user
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 18:53 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 05/23] vhost-user-gpu: drop code duplication Vladimir Sementsov-Ogievskiy
` (19 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, Gonglei (Arei), Zhenwei Pi, Jason Wang
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 <vsementsov@yandex-team.ru>
---
backends/cryptodev-vhost.c | 1 -
hw/net/vhost_net.c | 2 --
hw/virtio/vhost-user.c | 26 +++++++++++++++++++++++---
hw/virtio/virtio-qmp.c | 6 ++++--
include/hw/virtio/vhost-user.h | 3 +++
include/hw/virtio/vhost.h | 8 --------
6 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
index abdfce33af..c6069f4e5b 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 fda90e231e..ca19983126 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -265,9 +265,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 b8231dcbcf..3fd11a3b57 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 {
@@ -275,7 +284,8 @@ struct scrub_regions {
static bool vhost_user_has_protocol_feature(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)
@@ -2229,8 +2239,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;
@@ -3021,6 +3031,16 @@ static int vhost_user_check_device_state(struct vhost_dev *dev, Error **errp)
return 0;
}
+void vhost_user_qmp_status(struct vhost_dev *dev, VirtioStatus *status)
+{
+ struct vhost_user *u = dev->opaque;
+
+ assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+ status->vhost_dev->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,
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index dd1a38e792..82acb6d232 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -821,12 +821,14 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
status->vhost_dev->acked_features =
qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
- 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->backend_type == VHOST_BACKEND_TYPE_USER) {
+ vhost_user_qmp_status(hdev, status);
+ }
}
return status;
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index 9a3f238b43..36d96296a3 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -10,6 +10,7 @@
#include "chardev/char-fe.h"
#include "hw/virtio/virtio.h"
+#include "qapi/qapi-types-virtio.h"
enum VhostUserProtocolFeature {
VHOST_USER_PROTOCOL_F_MQ = 0,
@@ -111,4 +112,6 @@ void vhost_user_async_close(DeviceState *d,
CharBackend *chardev, struct vhost_dev *vhost,
vu_async_close_fn cb);
+void vhost_user_qmp_status(struct vhost_dev *dev, VirtioStatus *status);
+
#endif
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 3e69e47833..e308bc4556 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -104,14 +104,6 @@ struct vhost_dev {
VIRTIO_DECLARE_FEATURES(features);
VIRTIO_DECLARE_FEATURES(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
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 04/23] vhost: move protocol_features to vhost_user
2025-10-11 23:23 ` [PATCH v2 04/23] vhost: move protocol_features to vhost_user Vladimir Sementsov-Ogievskiy
@ 2025-10-13 18:53 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 18:53 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
Gonglei (Arei), Zhenwei Pi, Jason Wang
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:
>
> 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 <vsementsov@yandex-team.ru>
> ---
> backends/cryptodev-vhost.c | 1 -
> hw/net/vhost_net.c | 2 --
> hw/virtio/vhost-user.c | 26 +++++++++++++++++++++++---
> hw/virtio/virtio-qmp.c | 6 ++++--
> include/hw/virtio/vhost-user.h | 3 +++
> include/hw/virtio/vhost.h | 8 --------
> 6 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c
> index abdfce33af..c6069f4e5b 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 fda90e231e..ca19983126 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -265,9 +265,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 b8231dcbcf..3fd11a3b57 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 {
> @@ -275,7 +284,8 @@ struct scrub_regions {
> static bool vhost_user_has_protocol_feature(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)
> @@ -2229,8 +2239,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;
> @@ -3021,6 +3031,16 @@ static int vhost_user_check_device_state(struct vhost_dev *dev, Error **errp)
> return 0;
> }
>
> +void vhost_user_qmp_status(struct vhost_dev *dev, VirtioStatus *status)
> +{
> + struct vhost_user *u = dev->opaque;
> +
> + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
> +
> + status->vhost_dev->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,
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index dd1a38e792..82acb6d232 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -821,12 +821,14 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
> status->vhost_dev->acked_features =
> qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
>
> - 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->backend_type == VHOST_BACKEND_TYPE_USER) {
> + vhost_user_qmp_status(hdev, status);
> + }
> }
>
> return status;
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index 9a3f238b43..36d96296a3 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -10,6 +10,7 @@
>
> #include "chardev/char-fe.h"
> #include "hw/virtio/virtio.h"
> +#include "qapi/qapi-types-virtio.h"
>
> enum VhostUserProtocolFeature {
> VHOST_USER_PROTOCOL_F_MQ = 0,
> @@ -111,4 +112,6 @@ void vhost_user_async_close(DeviceState *d,
> CharBackend *chardev, struct vhost_dev *vhost,
> vu_async_close_fn cb);
>
> +void vhost_user_qmp_status(struct vhost_dev *dev, VirtioStatus *status);
> +
> #endif
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 3e69e47833..e308bc4556 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -104,14 +104,6 @@ struct vhost_dev {
> VIRTIO_DECLARE_FEATURES(features);
> VIRTIO_DECLARE_FEATURES(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
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 05/23] vhost-user-gpu: drop code duplication
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 04/23] vhost: move protocol_features to vhost_user Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
` (18 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, Philippe Mathieu-Daudé,
Marc-André Lureau
Obviously, this duplicated fragment doesn't make any sense.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/display/vhost-user-gpu.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 9fc6bbcd2c..79ea64b12c 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -644,10 +644,6 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
VIRTIO_GPU_F_RESOURCE_UUID)) {
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED;
}
- if (virtio_has_feature(g->vhost->dev.features,
- VIRTIO_GPU_F_RESOURCE_UUID)) {
- g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED;
- }
if (!virtio_gpu_base_device_realize(qdev, NULL, NULL, errp)) {
return;
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 06/23] vhost: make vhost_dev.features private
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 05/23] vhost-user-gpu: drop code duplication Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 19:52 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 07/23] virtio: move common part of _set_guest_notifier to generic code Vladimir Sementsov-Ogievskiy
` (17 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, Marc-André Lureau, Jason Wang,
Alex Bennée
It's hard to control where and how do we use this field. Let's
cover all usages by getters/setters, and keep direct access to the
field only in vhost.c. It will help to control migration of this
field in further commits.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/display/vhost-user-gpu.c | 7 +++----
hw/net/vhost_net.c | 18 +++++++++---------
hw/virtio/vdpa-dev.c | 2 +-
hw/virtio/vhost-user-base.c | 8 ++++++--
hw/virtio/vhost-user.c | 4 ++--
hw/virtio/vhost.c | 6 +++---
hw/virtio/virtio-qmp.c | 2 +-
include/hw/virtio/vhost.h | 27 +++++++++++++++++++++++++--
net/vhost-vdpa.c | 7 +++----
9 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 79ea64b12c..146620e0a3 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -631,17 +631,16 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
/* existing backend may send DMABUF, so let's add that requirement */
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED;
- if (virtio_has_feature(g->vhost->dev.features, VIRTIO_GPU_F_VIRGL)) {
+ if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_VIRGL)) {
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED;
}
- if (virtio_has_feature(g->vhost->dev.features, VIRTIO_GPU_F_EDID)) {
+ if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_EDID)) {
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_EDID_ENABLED;
} else {
error_report("EDID requested but the backend doesn't support it.");
g->parent_obj.conf.flags &= ~(1 << VIRTIO_GPU_FLAG_EDID_ENABLED);
}
- if (virtio_has_feature(g->vhost->dev.features,
- VIRTIO_GPU_F_RESOURCE_UUID)) {
+ if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_RESOURCE_UUID)) {
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED;
}
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ca19983126..323d117735 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -54,8 +54,8 @@ void vhost_net_ack_features_ex(struct vhost_net *net, const uint64_t *features)
{
virtio_features_clear(net->dev.acked_features_ex);
if (net->backend == -1) {
- net->dev.acked_features =
- net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
+ net->dev.acked_features = (vhost_dev_features(&net->dev) &
+ (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
} else if (!qemu_has_vnet_hdr(net->nc)) {
net->dev.acked_features = 1ULL << VHOST_NET_F_VIRTIO_NET_HDR;
}
@@ -282,15 +282,15 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
if (backend_kernel) {
if (!qemu_has_vnet_hdr_len(options->net_backend,
sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
- net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
+ vhost_dev_clear_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF);
}
if (!qemu_has_vnet_hdr(options->net_backend) &&
- (~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR))) {
- fprintf(stderr, "vhost lacks feature mask 0x%llx for backend\n",
- ~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR));
- goto fail;
- }
+ !vhost_dev_has_feature(&net->dev, VHOST_NET_F_VIRTIO_NET_HDR)) {
+ fprintf(stderr, "vhost lacks VHOST_NET_F_VIRTIO_NET_HDR "
+ "feature for backend\n");
+ goto fail;
+ }
}
/* Set sane init value. Override when guest acks. */
@@ -298,7 +298,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
virtio_features_from_u64(features,
options->get_acked_features(net->nc));
if (virtio_features_andnot(missing_features, features,
- net->dev.features_ex)) {
+ vhost_dev_features_ex(&net->dev))) {
fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
" for backend\n", VIRTIO_FEATURES_PR(missing_features));
goto fail;
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
index efd9f68420..e1a2ff433d 100644
--- a/hw/virtio/vdpa-dev.c
+++ b/hw/virtio/vdpa-dev.c
@@ -224,7 +224,7 @@ static uint64_t vhost_vdpa_device_get_features(VirtIODevice *vdev,
Error **errp)
{
VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev);
- uint64_t backend_features = s->dev.features;
+ uint64_t backend_features = vhost_dev_features(&s->dev);
if (!virtio_has_feature(features, VIRTIO_F_IOMMU_PLATFORM)) {
virtio_clear_feature(&backend_features, VIRTIO_F_IOMMU_PLATFORM);
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index ff67a020b4..cf311c3bfc 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -118,9 +118,13 @@ static uint64_t vub_get_features(VirtIODevice *vdev,
uint64_t requested_features, Error **errp)
{
VHostUserBase *vub = VHOST_USER_BASE(vdev);
+ uint64_t backend_features = vhost_dev_features(&vub->vhost_dev);
+
/* This should be set when the vhost connection initialises */
- g_assert(vub->vhost_dev.features);
- return vub->vhost_dev.features & ~(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
+ g_assert(backend_features);
+ virtio_clear_feature(&backend_features, VHOST_USER_F_PROTOCOL_FEATURES);
+
+ return backend_features;
}
/*
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 3fd11a3b57..9f26515fd4 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1252,7 +1252,7 @@ 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)) {
+ if (!vhost_dev_has_feature(dev, VHOST_USER_F_PROTOCOL_FEATURES)) {
/*
* For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
* been negotiated, the rings start directly in the enabled state,
@@ -1469,7 +1469,7 @@ static int vhost_user_set_features(struct vhost_dev *dev,
* Don't lose VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user
* specific.
*/
- if (virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
+ if (vhost_dev_has_feature(dev, VHOST_USER_F_PROTOCOL_FEATURES)) {
features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 414a48a218..773b91c0a0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1603,7 +1603,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
}
}
- virtio_features_copy(hdev->features_ex, features);
+ virtio_features_copy(hdev->_features_ex, features);
hdev->memory_listener = (MemoryListener) {
.name = "vhost",
@@ -1626,7 +1626,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
};
if (hdev->migration_blocker == NULL) {
- if (!virtio_has_feature_ex(hdev->features_ex, VHOST_F_LOG_ALL)) {
+ if (!vhost_dev_has_feature(hdev, VHOST_F_LOG_ALL)) {
error_setg(&hdev->migration_blocker,
"Migration disabled: vhost lacks VHOST_F_LOG_ALL feature.");
} else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_alloc_check()) {
@@ -1900,7 +1900,7 @@ void vhost_get_features_ex(struct vhost_dev *hdev,
const int *bit = feature_bits;
while (*bit != VHOST_INVALID_FEATURE_BIT) {
- if (!virtio_has_feature_ex(hdev->features_ex, *bit)) {
+ if (!vhost_dev_has_feature(hdev, *bit)) {
virtio_clear_feature_ex(features, *bit);
}
bit++;
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 82acb6d232..33d32720e1 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -817,7 +817,7 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
status->vhost_dev->nvqs = hdev->nvqs;
status->vhost_dev->vq_index = hdev->vq_index;
status->vhost_dev->features =
- qmp_decode_features(vdev->device_id, hdev->features_ex);
+ qmp_decode_features(vdev->device_id, vhost_dev_features_ex(hdev));
status->vhost_dev->acked_features =
qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index e308bc4556..e4e4526ca8 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -98,10 +98,11 @@ struct vhost_dev {
* offered by a backend which may be a subset of the total
* features eventually offered to the guest.
*
- * @features: available features provided by the backend
+ * @_features: available features provided by the backend, private,
+ * direct access only in vhost.c
* @acked_features: final negotiated features with front-end driver
*/
- VIRTIO_DECLARE_FEATURES(features);
+ VIRTIO_DECLARE_FEATURES(_features);
VIRTIO_DECLARE_FEATURES(acked_features);
uint64_t max_queues;
@@ -403,6 +404,28 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
struct vhost_inflight *inflight);
bool vhost_dev_has_iommu(struct vhost_dev *dev);
+static inline bool vhost_dev_has_feature(struct vhost_dev *dev,
+ uint64_t feature)
+{
+ return virtio_has_feature_ex(dev->_features_ex, feature);
+}
+
+static inline uint64_t vhost_dev_features(struct vhost_dev *dev)
+{
+ return dev->_features;
+}
+
+static inline const uint64_t *vhost_dev_features_ex(struct vhost_dev *dev)
+{
+ return dev->_features_ex;
+}
+
+static inline void vhost_dev_clear_feature(struct vhost_dev *dev,
+ uint64_t feature)
+{
+ virtio_clear_feature_ex(&dev->_features, feature);
+}
+
#ifdef CONFIG_VHOST
int vhost_reset_device(struct vhost_dev *hdev);
#else
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 74d26a9497..0af0d3bdd3 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -256,15 +256,14 @@ static bool vhost_vdpa_get_vnet_hash_supported_types(NetClientState *nc,
{
assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
- uint64_t features = s->vhost_vdpa.dev->features;
int fd = s->vhost_vdpa.shared->device_fd;
struct {
struct vhost_vdpa_config hdr;
uint32_t supported_hash_types;
} config;
- if (!virtio_has_feature(features, VIRTIO_NET_F_HASH_REPORT) &&
- !virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
+ if (!vhost_dev_has_feature(s->vhost_vdpa.dev, VIRTIO_NET_F_HASH_REPORT) &&
+ !vhost_dev_has_feature(s->vhost_vdpa.dev, VIRTIO_NET_F_RSS)) {
return false;
}
@@ -585,7 +584,7 @@ static int vhost_vdpa_net_cvq_start(NetClientState *nc)
* If we early return in these cases SVQ will not be enabled. The migration
* will be blocked as long as vhost-vdpa backends will not offer _F_LOG.
*/
- if (!vhost_vdpa_net_valid_svq_features(v->dev->features, NULL)) {
+ if (!vhost_vdpa_net_valid_svq_features(vhost_dev_features(v->dev), NULL)) {
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 06/23] vhost: make vhost_dev.features private
2025-10-11 23:23 ` [PATCH v2 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
@ 2025-10-13 19:52 ` Raphael Norwitz
2025-10-14 9:11 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 19:52 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
Marc-André Lureau, Jason Wang, Alex Bennée
There are places where the code switches logic to set/check
features_ex rather than _features. If I'm reading the code correctly I
don't think it makes a functional difference but it does make things
pretty confusing.
I'd suggest having vhost_dev_has_feature() check _features rather than
_features_ex or add a separate call to check
vhost_dev_has_feature_ex()? At the least there should be a comment
explaining that checking the start of _features_ex is the same as
checking features because of the union definition?
On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> It's hard to control where and how do we use this field. Let's
> cover all usages by getters/setters, and keep direct access to the
> field only in vhost.c. It will help to control migration of this
> field in further commits.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/display/vhost-user-gpu.c | 7 +++----
> hw/net/vhost_net.c | 18 +++++++++---------
> hw/virtio/vdpa-dev.c | 2 +-
> hw/virtio/vhost-user-base.c | 8 ++++++--
> hw/virtio/vhost-user.c | 4 ++--
> hw/virtio/vhost.c | 6 +++---
> hw/virtio/virtio-qmp.c | 2 +-
> include/hw/virtio/vhost.h | 27 +++++++++++++++++++++++++--
> net/vhost-vdpa.c | 7 +++----
> 9 files changed, 53 insertions(+), 28 deletions(-)
>
> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> index 79ea64b12c..146620e0a3 100644
> --- a/hw/display/vhost-user-gpu.c
> +++ b/hw/display/vhost-user-gpu.c
> @@ -631,17 +631,16 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
>
> /* existing backend may send DMABUF, so let's add that requirement */
> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED;
Here's one example of where things get confusing to read. We are now
checking g->vhost->dev.features_ex rather than g->vhost->dev.features.
> - if (virtio_has_feature(g->vhost->dev.features, VIRTIO_GPU_F_VIRGL)) {
> + if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_VIRGL)) {
> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED;
> }
> - if (virtio_has_feature(g->vhost->dev.features, VIRTIO_GPU_F_EDID)) {
> + if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_EDID)) {
> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_EDID_ENABLED;
> } else {
> error_report("EDID requested but the backend doesn't support it.");
> g->parent_obj.conf.flags &= ~(1 << VIRTIO_GPU_FLAG_EDID_ENABLED);
> }
> - if (virtio_has_feature(g->vhost->dev.features,
> - VIRTIO_GPU_F_RESOURCE_UUID)) {
> + if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_RESOURCE_UUID)) {
> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED;
> }
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index ca19983126..323d117735 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -54,8 +54,8 @@ void vhost_net_ack_features_ex(struct vhost_net *net, const uint64_t *features)
> {
> virtio_features_clear(net->dev.acked_features_ex);
> if (net->backend == -1) {
> - net->dev.acked_features =
> - net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> + net->dev.acked_features = (vhost_dev_features(&net->dev) &
> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
> } else if (!qemu_has_vnet_hdr(net->nc)) {
> net->dev.acked_features = 1ULL << VHOST_NET_F_VIRTIO_NET_HDR;
> }
> @@ -282,15 +282,15 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
> if (backend_kernel) {
> if (!qemu_has_vnet_hdr_len(options->net_backend,
> sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
> - net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
> + vhost_dev_clear_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF);
> }
>
> if (!qemu_has_vnet_hdr(options->net_backend) &&
> - (~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR))) {
> - fprintf(stderr, "vhost lacks feature mask 0x%llx for backend\n",
> - ~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR));
> - goto fail;
> - }
ditto here.
> + !vhost_dev_has_feature(&net->dev, VHOST_NET_F_VIRTIO_NET_HDR)) {
> + fprintf(stderr, "vhost lacks VHOST_NET_F_VIRTIO_NET_HDR "
> + "feature for backend\n");
> + goto fail;
> + }
> }
>
> /* Set sane init value. Override when guest acks. */
> @@ -298,7 +298,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
> virtio_features_from_u64(features,
> options->get_acked_features(net->nc));
> if (virtio_features_andnot(missing_features, features,
> - net->dev.features_ex)) {
> + vhost_dev_features_ex(&net->dev))) {
> fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
> " for backend\n", VIRTIO_FEATURES_PR(missing_features));
> goto fail;
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index efd9f68420..e1a2ff433d 100644
> --- a/hw/virtio/vdpa-dev.c
> +++ b/hw/virtio/vdpa-dev.c
> @@ -224,7 +224,7 @@ static uint64_t vhost_vdpa_device_get_features(VirtIODevice *vdev,
> Error **errp)
> {
> VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev);
> - uint64_t backend_features = s->dev.features;
> + uint64_t backend_features = vhost_dev_features(&s->dev);
>
> if (!virtio_has_feature(features, VIRTIO_F_IOMMU_PLATFORM)) {
> virtio_clear_feature(&backend_features, VIRTIO_F_IOMMU_PLATFORM);
> diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> index ff67a020b4..cf311c3bfc 100644
> --- a/hw/virtio/vhost-user-base.c
> +++ b/hw/virtio/vhost-user-base.c
> @@ -118,9 +118,13 @@ static uint64_t vub_get_features(VirtIODevice *vdev,
> uint64_t requested_features, Error **errp)
> {
> VHostUserBase *vub = VHOST_USER_BASE(vdev);
> + uint64_t backend_features = vhost_dev_features(&vub->vhost_dev);
> +
> /* This should be set when the vhost connection initialises */
> - g_assert(vub->vhost_dev.features);
> - return vub->vhost_dev.features & ~(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> + g_assert(backend_features);
> + virtio_clear_feature(&backend_features, VHOST_USER_F_PROTOCOL_FEATURES);
> +
> + return backend_features;
> }
>
> /*
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 3fd11a3b57..9f26515fd4 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1252,7 +1252,7 @@ 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)) {
> + if (!vhost_dev_has_feature(dev, VHOST_USER_F_PROTOCOL_FEATURES)) {
> /*
> * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
> * been negotiated, the rings start directly in the enabled state,
> @@ -1469,7 +1469,7 @@ static int vhost_user_set_features(struct vhost_dev *dev,
> * Don't lose VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user
> * specific.
> */
> - if (virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
> + if (vhost_dev_has_feature(dev, VHOST_USER_F_PROTOCOL_FEATURES)) {
> features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> }
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 414a48a218..773b91c0a0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1603,7 +1603,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> }
> }
>
> - virtio_features_copy(hdev->features_ex, features);
> + virtio_features_copy(hdev->_features_ex, features);
>
> hdev->memory_listener = (MemoryListener) {
> .name = "vhost",
> @@ -1626,7 +1626,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> };
>
> if (hdev->migration_blocker == NULL) {
> - if (!virtio_has_feature_ex(hdev->features_ex, VHOST_F_LOG_ALL)) {
> + if (!vhost_dev_has_feature(hdev, VHOST_F_LOG_ALL)) {
> error_setg(&hdev->migration_blocker,
> "Migration disabled: vhost lacks VHOST_F_LOG_ALL feature.");
> } else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_alloc_check()) {
> @@ -1900,7 +1900,7 @@ void vhost_get_features_ex(struct vhost_dev *hdev,
> const int *bit = feature_bits;
>
> while (*bit != VHOST_INVALID_FEATURE_BIT) {
> - if (!virtio_has_feature_ex(hdev->features_ex, *bit)) {
> + if (!vhost_dev_has_feature(hdev, *bit)) {
> virtio_clear_feature_ex(features, *bit);
> }
> bit++;
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index 82acb6d232..33d32720e1 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -817,7 +817,7 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
> status->vhost_dev->nvqs = hdev->nvqs;
> status->vhost_dev->vq_index = hdev->vq_index;
> status->vhost_dev->features =
> - qmp_decode_features(vdev->device_id, hdev->features_ex);
> + qmp_decode_features(vdev->device_id, vhost_dev_features_ex(hdev));
> status->vhost_dev->acked_features =
> qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index e308bc4556..e4e4526ca8 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -98,10 +98,11 @@ struct vhost_dev {
> * offered by a backend which may be a subset of the total
> * features eventually offered to the guest.
> *
> - * @features: available features provided by the backend
> + * @_features: available features provided by the backend, private,
> + * direct access only in vhost.c
> * @acked_features: final negotiated features with front-end driver
> */
> - VIRTIO_DECLARE_FEATURES(features);
> + VIRTIO_DECLARE_FEATURES(_features);
> VIRTIO_DECLARE_FEATURES(acked_features);
>
> uint64_t max_queues;
> @@ -403,6 +404,28 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
> struct vhost_inflight *inflight);
> bool vhost_dev_has_iommu(struct vhost_dev *dev);
>
> +static inline bool vhost_dev_has_feature(struct vhost_dev *dev,
> + uint64_t feature)
> +{
For simplicity should this be:
return virtio_has_feature_ex(dev->_features, feature);
?
> + return virtio_has_feature_ex(dev->_features_ex, feature);
> +}
> +
> +static inline uint64_t vhost_dev_features(struct vhost_dev *dev)
> +{
> + return dev->_features;
> +}
> +
> +static inline const uint64_t *vhost_dev_features_ex(struct vhost_dev *dev)
> +{
> + return dev->_features_ex;
> +}
> +
> +static inline void vhost_dev_clear_feature(struct vhost_dev *dev,
> + uint64_t feature)
> +{
> + virtio_clear_feature_ex(&dev->_features, feature);
> +}
> +
> #ifdef CONFIG_VHOST
> int vhost_reset_device(struct vhost_dev *hdev);
> #else
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 74d26a9497..0af0d3bdd3 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -256,15 +256,14 @@ static bool vhost_vdpa_get_vnet_hash_supported_types(NetClientState *nc,
> {
> assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> - uint64_t features = s->vhost_vdpa.dev->features;
> int fd = s->vhost_vdpa.shared->device_fd;
> struct {
> struct vhost_vdpa_config hdr;
> uint32_t supported_hash_types;
> } config;
>
> - if (!virtio_has_feature(features, VIRTIO_NET_F_HASH_REPORT) &&
> - !virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
> + if (!vhost_dev_has_feature(s->vhost_vdpa.dev, VIRTIO_NET_F_HASH_REPORT) &&
> + !vhost_dev_has_feature(s->vhost_vdpa.dev, VIRTIO_NET_F_RSS)) {
> return false;
> }
>
> @@ -585,7 +584,7 @@ static int vhost_vdpa_net_cvq_start(NetClientState *nc)
> * If we early return in these cases SVQ will not be enabled. The migration
> * will be blocked as long as vhost-vdpa backends will not offer _F_LOG.
> */
> - if (!vhost_vdpa_net_valid_svq_features(v->dev->features, NULL)) {
> + if (!vhost_vdpa_net_valid_svq_features(vhost_dev_features(v->dev), NULL)) {
> return 0;
> }
>
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 06/23] vhost: make vhost_dev.features private
2025-10-13 19:52 ` Raphael Norwitz
@ 2025-10-14 9:11 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-14 9:11 UTC (permalink / raw)
To: Raphael Norwitz
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
Marc-André Lureau, Jason Wang, Alex Bennée
On 13.10.25 22:52, Raphael Norwitz wrote:
> There are places where the code switches logic to set/check
> features_ex rather than _features. If I'm reading the code correctly I
> don't think it makes a functional difference but it does make things
> pretty confusing.
>
> I'd suggest having vhost_dev_has_feature() check _features rather than
> _features_ex or add a separate call to check
> vhost_dev_has_feature_ex()? At the least there should be a comment
> explaining that checking the start of _features_ex is the same as
> checking features because of the union definition?
>
Hmm agree.. Honestly, I just don't like the split into features and features_ex,
IMO, it would be better always use _ex semantics, and then rename it to old
name without suffix.
And if keep split semantics for now, it's better to follow it consistently.
> On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
>>
>> It's hard to control where and how do we use this field. Let's
>> cover all usages by getters/setters, and keep direct access to the
>> field only in vhost.c. It will help to control migration of this
>> field in further commits.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>> hw/display/vhost-user-gpu.c | 7 +++----
>> hw/net/vhost_net.c | 18 +++++++++---------
>> hw/virtio/vdpa-dev.c | 2 +-
>> hw/virtio/vhost-user-base.c | 8 ++++++--
>> hw/virtio/vhost-user.c | 4 ++--
>> hw/virtio/vhost.c | 6 +++---
>> hw/virtio/virtio-qmp.c | 2 +-
>> include/hw/virtio/vhost.h | 27 +++++++++++++++++++++++++--
>> net/vhost-vdpa.c | 7 +++----
>> 9 files changed, 53 insertions(+), 28 deletions(-)
>>
>> diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
>> index 79ea64b12c..146620e0a3 100644
>> --- a/hw/display/vhost-user-gpu.c
>> +++ b/hw/display/vhost-user-gpu.c
>> @@ -631,17 +631,16 @@ vhost_user_gpu_device_realize(DeviceState *qdev, Error **errp)
>>
>> /* existing backend may send DMABUF, so let's add that requirement */
>> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED;
>
> Here's one example of where things get confusing to read. We are now
> checking g->vhost->dev.features_ex rather than g->vhost->dev.features.
>
>> - if (virtio_has_feature(g->vhost->dev.features, VIRTIO_GPU_F_VIRGL)) {
>> + if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_VIRGL)) {
>> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED;
>> }
>> - if (virtio_has_feature(g->vhost->dev.features, VIRTIO_GPU_F_EDID)) {
>> + if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_EDID)) {
>> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_EDID_ENABLED;
>> } else {
>> error_report("EDID requested but the backend doesn't support it.");
>> g->parent_obj.conf.flags &= ~(1 << VIRTIO_GPU_FLAG_EDID_ENABLED);
>> }
>> - if (virtio_has_feature(g->vhost->dev.features,
>> - VIRTIO_GPU_F_RESOURCE_UUID)) {
>> + if (vhost_dev_has_feature(&g->vhost->dev, VIRTIO_GPU_F_RESOURCE_UUID)) {
>> g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED;
>> }
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index ca19983126..323d117735 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -54,8 +54,8 @@ void vhost_net_ack_features_ex(struct vhost_net *net, const uint64_t *features)
>> {
>> virtio_features_clear(net->dev.acked_features_ex);
>> if (net->backend == -1) {
>> - net->dev.acked_features =
>> - net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
>> + net->dev.acked_features = (vhost_dev_features(&net->dev) &
>> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
>> } else if (!qemu_has_vnet_hdr(net->nc)) {
>> net->dev.acked_features = 1ULL << VHOST_NET_F_VIRTIO_NET_HDR;
>> }
>> @@ -282,15 +282,15 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>> if (backend_kernel) {
>> if (!qemu_has_vnet_hdr_len(options->net_backend,
>> sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
>> - net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF);
>> + vhost_dev_clear_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF);
>> }
>>
>> if (!qemu_has_vnet_hdr(options->net_backend) &&
>> - (~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR))) {
>> - fprintf(stderr, "vhost lacks feature mask 0x%llx for backend\n",
>> - ~net->dev.features & (1ULL << VHOST_NET_F_VIRTIO_NET_HDR));
>> - goto fail;
>> - }
>
> ditto here.
>
>> + !vhost_dev_has_feature(&net->dev, VHOST_NET_F_VIRTIO_NET_HDR)) {
>> + fprintf(stderr, "vhost lacks VHOST_NET_F_VIRTIO_NET_HDR "
>> + "feature for backend\n");
>> + goto fail;
>> + }
>> }
>>
>> /* Set sane init value. Override when guest acks. */
>> @@ -298,7 +298,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
>> virtio_features_from_u64(features,
>> options->get_acked_features(net->nc));
>> if (virtio_features_andnot(missing_features, features,
>> - net->dev.features_ex)) {
>> + vhost_dev_features_ex(&net->dev))) {
>> fprintf(stderr, "vhost lacks feature mask 0x" VIRTIO_FEATURES_FMT
>> " for backend\n", VIRTIO_FEATURES_PR(missing_features));
>> goto fail;
>> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
>> index efd9f68420..e1a2ff433d 100644
>> --- a/hw/virtio/vdpa-dev.c
>> +++ b/hw/virtio/vdpa-dev.c
>> @@ -224,7 +224,7 @@ static uint64_t vhost_vdpa_device_get_features(VirtIODevice *vdev,
>> Error **errp)
>> {
>> VhostVdpaDevice *s = VHOST_VDPA_DEVICE(vdev);
>> - uint64_t backend_features = s->dev.features;
>> + uint64_t backend_features = vhost_dev_features(&s->dev);
>>
>> if (!virtio_has_feature(features, VIRTIO_F_IOMMU_PLATFORM)) {
>> virtio_clear_feature(&backend_features, VIRTIO_F_IOMMU_PLATFORM);
>> diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
>> index ff67a020b4..cf311c3bfc 100644
>> --- a/hw/virtio/vhost-user-base.c
>> +++ b/hw/virtio/vhost-user-base.c
>> @@ -118,9 +118,13 @@ static uint64_t vub_get_features(VirtIODevice *vdev,
>> uint64_t requested_features, Error **errp)
>> {
>> VHostUserBase *vub = VHOST_USER_BASE(vdev);
>> + uint64_t backend_features = vhost_dev_features(&vub->vhost_dev);
>> +
>> /* This should be set when the vhost connection initialises */
>> - g_assert(vub->vhost_dev.features);
>> - return vub->vhost_dev.features & ~(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
>> + g_assert(backend_features);
>> + virtio_clear_feature(&backend_features, VHOST_USER_F_PROTOCOL_FEATURES);
>> +
>> + return backend_features;
>> }
>>
>> /*
>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>> index 3fd11a3b57..9f26515fd4 100644
>> --- a/hw/virtio/vhost-user.c
>> +++ b/hw/virtio/vhost-user.c
>> @@ -1252,7 +1252,7 @@ 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)) {
>> + if (!vhost_dev_has_feature(dev, VHOST_USER_F_PROTOCOL_FEATURES)) {
>> /*
>> * For vhost-user devices, if VHOST_USER_F_PROTOCOL_FEATURES has not
>> * been negotiated, the rings start directly in the enabled state,
>> @@ -1469,7 +1469,7 @@ static int vhost_user_set_features(struct vhost_dev *dev,
>> * Don't lose VHOST_USER_F_PROTOCOL_FEATURES, which is vhost-user
>> * specific.
>> */
>> - if (virtio_has_feature(dev->features, VHOST_USER_F_PROTOCOL_FEATURES)) {
>> + if (vhost_dev_has_feature(dev, VHOST_USER_F_PROTOCOL_FEATURES)) {
>> features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>> }
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 414a48a218..773b91c0a0 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1603,7 +1603,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>> }
>> }
>>
>> - virtio_features_copy(hdev->features_ex, features);
>> + virtio_features_copy(hdev->_features_ex, features);
>>
>> hdev->memory_listener = (MemoryListener) {
>> .name = "vhost",
>> @@ -1626,7 +1626,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>> };
>>
>> if (hdev->migration_blocker == NULL) {
>> - if (!virtio_has_feature_ex(hdev->features_ex, VHOST_F_LOG_ALL)) {
>> + if (!vhost_dev_has_feature(hdev, VHOST_F_LOG_ALL)) {
>> error_setg(&hdev->migration_blocker,
>> "Migration disabled: vhost lacks VHOST_F_LOG_ALL feature.");
>> } else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_alloc_check()) {
>> @@ -1900,7 +1900,7 @@ void vhost_get_features_ex(struct vhost_dev *hdev,
>> const int *bit = feature_bits;
>>
>> while (*bit != VHOST_INVALID_FEATURE_BIT) {
>> - if (!virtio_has_feature_ex(hdev->features_ex, *bit)) {
>> + if (!vhost_dev_has_feature(hdev, *bit)) {
>> virtio_clear_feature_ex(features, *bit);
>> }
>> bit++;
>> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
>> index 82acb6d232..33d32720e1 100644
>> --- a/hw/virtio/virtio-qmp.c
>> +++ b/hw/virtio/virtio-qmp.c
>> @@ -817,7 +817,7 @@ VirtioStatus *qmp_x_query_virtio_status(const char *path, Error **errp)
>> status->vhost_dev->nvqs = hdev->nvqs;
>> status->vhost_dev->vq_index = hdev->vq_index;
>> status->vhost_dev->features =
>> - qmp_decode_features(vdev->device_id, hdev->features_ex);
>> + qmp_decode_features(vdev->device_id, vhost_dev_features_ex(hdev));
>> status->vhost_dev->acked_features =
>> qmp_decode_features(vdev->device_id, hdev->acked_features_ex);
>>
>> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
>> index e308bc4556..e4e4526ca8 100644
>> --- a/include/hw/virtio/vhost.h
>> +++ b/include/hw/virtio/vhost.h
>> @@ -98,10 +98,11 @@ struct vhost_dev {
>> * offered by a backend which may be a subset of the total
>> * features eventually offered to the guest.
>> *
>> - * @features: available features provided by the backend
>> + * @_features: available features provided by the backend, private,
>> + * direct access only in vhost.c
>> * @acked_features: final negotiated features with front-end driver
>> */
>> - VIRTIO_DECLARE_FEATURES(features);
>> + VIRTIO_DECLARE_FEATURES(_features);
>> VIRTIO_DECLARE_FEATURES(acked_features);
>>
>> uint64_t max_queues;
>> @@ -403,6 +404,28 @@ int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
>> struct vhost_inflight *inflight);
>> bool vhost_dev_has_iommu(struct vhost_dev *dev);
>>
>> +static inline bool vhost_dev_has_feature(struct vhost_dev *dev,
>> + uint64_t feature)
>> +{
>
> For simplicity should this be:
>
> return virtio_has_feature_ex(dev->_features, feature);
>
> ?
>
>> + return virtio_has_feature_ex(dev->_features_ex, feature);
>> +}
>> +
>> +static inline uint64_t vhost_dev_features(struct vhost_dev *dev)
>> +{
>> + return dev->_features;
>> +}
>> +
>> +static inline const uint64_t *vhost_dev_features_ex(struct vhost_dev *dev)
>> +{
>> + return dev->_features_ex;
>> +}
>> +
>> +static inline void vhost_dev_clear_feature(struct vhost_dev *dev,
>> + uint64_t feature)
>> +{
>> + virtio_clear_feature_ex(&dev->_features, feature);
>> +}
>> +
>> #ifdef CONFIG_VHOST
>> int vhost_reset_device(struct vhost_dev *hdev);
>> #else
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index 74d26a9497..0af0d3bdd3 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -256,15 +256,14 @@ static bool vhost_vdpa_get_vnet_hash_supported_types(NetClientState *nc,
>> {
>> assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
>> VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
>> - uint64_t features = s->vhost_vdpa.dev->features;
>> int fd = s->vhost_vdpa.shared->device_fd;
>> struct {
>> struct vhost_vdpa_config hdr;
>> uint32_t supported_hash_types;
>> } config;
>>
>> - if (!virtio_has_feature(features, VIRTIO_NET_F_HASH_REPORT) &&
>> - !virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
>> + if (!vhost_dev_has_feature(s->vhost_vdpa.dev, VIRTIO_NET_F_HASH_REPORT) &&
>> + !vhost_dev_has_feature(s->vhost_vdpa.dev, VIRTIO_NET_F_RSS)) {
>> return false;
>> }
>>
>> @@ -585,7 +584,7 @@ static int vhost_vdpa_net_cvq_start(NetClientState *nc)
>> * If we early return in these cases SVQ will not be enabled. The migration
>> * will be blocked as long as vhost-vdpa backends will not offer _F_LOG.
>> */
>> - if (!vhost_vdpa_net_valid_svq_features(v->dev->features, NULL)) {
>> + if (!vhost_vdpa_net_valid_svq_features(vhost_dev_features(v->dev), NULL)) {
>> return 0;
>> }
>>
>> --
>> 2.48.1
>>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 07/23] virtio: move common part of _set_guest_notifier to generic code
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers Vladimir Sementsov-Ogievskiy
` (16 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
virtio-pci and virtio-mmio handle config notifier equally but
with different code (mmio adds a separate function, when pci
use common function). Let's chose the more compact way (pci)
and reuse it for mmio.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/virtio-mmio.c | 41 +++++------------------------
hw/virtio/virtio-pci.c | 34 +++---------------------
hw/virtio/virtio.c | 48 +++++++++++++++++++++++++++++++---
include/hw/virtio/virtio-pci.h | 3 ---
include/hw/virtio/virtio.h | 16 +++++++++---
5 files changed, 67 insertions(+), 75 deletions(-)
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index fb58c36452..1f1d96129b 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -659,18 +659,11 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
- VirtQueue *vq = virtio_get_queue(vdev, n);
- EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
+ int r;
- if (assign) {
- int r = event_notifier_init(notifier, 0);
- if (r < 0) {
- return r;
- }
- virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
- } else {
- virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
- event_notifier_cleanup(notifier);
+ r = virtio_queue_set_guest_notifier(vdev, n, assign, with_irqfd);
+ if (r < 0) {
+ return r;
}
if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
@@ -679,30 +672,7 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
return 0;
}
-static int virtio_mmio_set_config_guest_notifier(DeviceState *d, bool assign,
- bool with_irqfd)
-{
- VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
- VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
- VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
- EventNotifier *notifier = virtio_config_get_guest_notifier(vdev);
- int r = 0;
- if (assign) {
- r = event_notifier_init(notifier, 0);
- if (r < 0) {
- return r;
- }
- virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
- } else {
- virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
- event_notifier_cleanup(notifier);
- }
- if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
- vdc->guest_notifier_mask(vdev, VIRTIO_CONFIG_IRQ_IDX, !assign);
- }
- return r;
-}
static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
bool assign)
{
@@ -724,7 +694,8 @@ static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
goto assign_error;
}
}
- r = virtio_mmio_set_config_guest_notifier(d, assign, with_irqfd);
+ r = virtio_mmio_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, assign,
+ with_irqfd);
if (r < 0) {
goto assign_error;
}
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 937e22f08a..6c4576a17f 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1197,43 +1197,17 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
}
}
-void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
- int n, bool assign,
- bool with_irqfd)
-{
- if (n == VIRTIO_CONFIG_IRQ_IDX) {
- virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
- } else {
- virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
- }
-}
-
static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
bool with_irqfd)
{
VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
- VirtQueue *vq = NULL;
- EventNotifier *notifier = NULL;
+ int r;
- if (n == VIRTIO_CONFIG_IRQ_IDX) {
- notifier = virtio_config_get_guest_notifier(vdev);
- } else {
- vq = virtio_get_queue(vdev, n);
- notifier = virtio_queue_get_guest_notifier(vq);
- }
-
- if (assign) {
- int r = event_notifier_init(notifier, 0);
- if (r < 0) {
- return r;
- }
- virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
- } else {
- virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
- with_irqfd);
- event_notifier_cleanup(notifier);
+ r = virtio_queue_set_guest_notifier(vdev, n, assign, with_irqfd);
+ if (r < 0) {
+ return r;
}
if (!msix_enabled(&proxy->pci_dev) &&
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 153ee0a0cf..704bc7943f 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3803,8 +3803,10 @@ static void virtio_config_guest_notifier_read(EventNotifier *n)
virtio_notify_config(vdev);
}
}
-void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
- bool with_irqfd)
+
+static void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq,
+ bool assign,
+ bool with_irqfd)
{
if (assign && !with_irqfd) {
event_notifier_set_handler(&vq->guest_notifier,
@@ -3819,7 +3821,7 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
}
}
-void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
+static void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
bool assign, bool with_irqfd)
{
EventNotifier *n;
@@ -3836,6 +3838,46 @@ void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
}
}
+static void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev,
+ VirtQueue *vq,
+ int n, bool assign,
+ bool with_irqfd)
+{
+ if (n == VIRTIO_CONFIG_IRQ_IDX) {
+ virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
+ } else {
+ virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
+ }
+}
+
+int virtio_queue_set_guest_notifier(VirtIODevice *vdev, int n, bool assign,
+ bool with_irqfd)
+{
+ VirtQueue *vq = NULL;
+ EventNotifier *notifier = NULL;
+
+ if (n == VIRTIO_CONFIG_IRQ_IDX) {
+ notifier = virtio_config_get_guest_notifier(vdev);
+ } else {
+ vq = virtio_get_queue(vdev, n);
+ notifier = virtio_queue_get_guest_notifier(vq);
+ }
+
+ if (assign) {
+ int r = event_notifier_init(notifier, 0);
+ if (r < 0) {
+ return r;
+ }
+ virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
+ } else {
+ virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
+ with_irqfd);
+ event_notifier_cleanup(notifier);
+ }
+
+ return 0;
+}
+
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
{
return &vq->guest_notifier;
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 639752977e..2a5b65f374 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -263,9 +263,6 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t);
* @fixed_queues.
*/
unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
-void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
- int n, bool assign,
- bool with_irqfd);
int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, uint8_t bar, uint64_t offset,
uint64_t length, uint8_t id);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d97529c3f1..7db8046766 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -420,8 +420,6 @@ void virtio_queue_update_used_idx(VirtIODevice *vdev, int n);
VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
uint16_t virtio_get_queue_index(VirtQueue *vq);
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
-void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
- bool with_irqfd);
int virtio_device_start_ioeventfd(VirtIODevice *vdev);
int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
void virtio_device_release_ioeventfd(VirtIODevice *vdev);
@@ -435,8 +433,18 @@ void virtio_queue_aio_detach_host_notifier(VirtQueue *vq, AioContext *ctx);
VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
EventNotifier *virtio_config_get_guest_notifier(VirtIODevice *vdev);
-void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
- bool assign, bool with_irqfd);
+
+/**
+ * virtio_queue_set_guest_notifier - set/unset queue or config guest
+ * notifier
+ *
+ * @vdev: the VirtIO device
+ * @n: queue number, or VIRTIO_CONFIG_IRQ_IDX to set config notifer
+ * @assign: true to set notifier, false to unset
+ * @with_irqfd: irqfd enabled
+ */
+int virtio_queue_set_guest_notifier(VirtIODevice *vdev, int n, bool assign,
+ bool with_irqfd);
static inline void virtio_add_feature(uint64_t *features, unsigned int fbit)
{
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 07/23] virtio: move common part of _set_guest_notifier to generic code Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 09/23] vhost-user: keep QIOChannelSocket for backend channel Vladimir Sementsov-Ogievskiy
` (15 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Now they don't make code more readable. Let's better put the whole
logic into virtio_queue_set_guest_notifier().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/virtio.c | 76 +++++++++++-----------------------------------
1 file changed, 17 insertions(+), 59 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 704bc7943f..4184aff75c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3804,74 +3804,32 @@ static void virtio_config_guest_notifier_read(EventNotifier *n)
}
}
-static void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq,
- bool assign,
- bool with_irqfd)
-{
- if (assign && !with_irqfd) {
- event_notifier_set_handler(&vq->guest_notifier,
- virtio_queue_guest_notifier_read);
- } else {
- event_notifier_set_handler(&vq->guest_notifier, NULL);
- }
- if (!assign) {
- /* Test and clear notifier before closing it,
- * in case poll callback didn't have time to run. */
- virtio_queue_guest_notifier_read(&vq->guest_notifier);
- }
-}
-
-static void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
- bool assign, bool with_irqfd)
-{
- EventNotifier *n;
- n = &vdev->config_notifier;
- if (assign && !with_irqfd) {
- event_notifier_set_handler(n, virtio_config_guest_notifier_read);
- } else {
- event_notifier_set_handler(n, NULL);
- }
- if (!assign) {
- /* Test and clear notifier before closing it,*/
- /* in case poll callback didn't have time to run. */
- virtio_config_guest_notifier_read(n);
- }
-}
-
-static void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev,
- VirtQueue *vq,
- int n, bool assign,
- bool with_irqfd)
-{
- if (n == VIRTIO_CONFIG_IRQ_IDX) {
- virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
- } else {
- virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
- }
-}
-
int virtio_queue_set_guest_notifier(VirtIODevice *vdev, int n, bool assign,
bool with_irqfd)
{
- VirtQueue *vq = NULL;
- EventNotifier *notifier = NULL;
-
- if (n == VIRTIO_CONFIG_IRQ_IDX) {
- notifier = virtio_config_get_guest_notifier(vdev);
- } else {
- vq = virtio_get_queue(vdev, n);
- notifier = virtio_queue_get_guest_notifier(vq);
- }
+ bool is_config = n == VIRTIO_CONFIG_IRQ_IDX;
+ VirtQueue *vq = is_config ? NULL : virtio_get_queue(vdev, n);
+ EventNotifier *notifier = is_config ?
+ virtio_config_get_guest_notifier(vdev) :
+ virtio_queue_get_guest_notifier(vq);
+ EventNotifierHandler *read_fn = is_config ?
+ virtio_config_guest_notifier_read :
+ virtio_queue_guest_notifier_read;
if (assign) {
int r = event_notifier_init(notifier, 0);
if (r < 0) {
return r;
}
- virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
- } else {
- virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
- with_irqfd);
+ }
+
+ event_notifier_set_handler(notifier,
+ (assign && !with_irqfd) ? read_fn : NULL);
+
+ if (!assign) {
+ /* Test and clear notifier before closing it,*/
+ /* in case poll callback didn't have time to run. */
+ read_fn(notifier);
event_notifier_cleanup(notifier);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 09/23] vhost-user: keep QIOChannelSocket for backend channel
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
` (14 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Keep QIOChannelSocket pointer instead of more generic
QIOChannel. No real difference for now, but it would
be simpler to migrate socket fd in further commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/vhost-user.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 9f26515fd4..23e7c12b16 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -244,7 +244,7 @@ struct vhost_user {
struct vhost_dev *dev;
/* Shared between vhost devs of the same virtio device */
VhostUserState *user;
- QIOChannel *backend_ioc;
+ QIOChannelSocket *backend_sioc;
GSource *backend_src;
NotifierWithReturn postcopy_notifier;
struct PostCopyFD postcopy_fd;
@@ -1796,8 +1796,8 @@ static void close_backend_channel(struct vhost_user *u)
g_source_destroy(u->backend_src);
g_source_unref(u->backend_src);
u->backend_src = NULL;
- object_unref(OBJECT(u->backend_ioc));
- u->backend_ioc = NULL;
+ object_unref(OBJECT(u->backend_sioc));
+ u->backend_sioc = NULL;
}
static gboolean backend_read(QIOChannel *ioc, GIOCondition condition,
@@ -1904,7 +1904,6 @@ static int vhost_setup_backend_channel(struct vhost_dev *dev)
bool reply_supported =
vhost_user_has_protocol_feature(dev, VHOST_USER_PROTOCOL_F_REPLY_ACK);
Error *local_err = NULL;
- QIOChannel *ioc;
if (!vhost_user_has_protocol_feature(
dev, VHOST_USER_PROTOCOL_F_BACKEND_REQ)) {
@@ -1917,15 +1916,15 @@ static int vhost_setup_backend_channel(struct vhost_dev *dev)
return -saved_errno;
}
- ioc = QIO_CHANNEL(qio_channel_socket_new_fd(sv[0], &local_err));
- if (!ioc) {
+ u->backend_sioc = qio_channel_socket_new_fd(sv[0], &local_err);
+ if (!u->backend_sioc) {
error_report_err(local_err);
return -ECONNREFUSED;
}
- u->backend_ioc = ioc;
- u->backend_src = qio_channel_add_watch_source(u->backend_ioc,
- G_IO_IN | G_IO_HUP,
- backend_read, dev, NULL, NULL);
+ u->backend_src = qio_channel_add_watch_source(QIO_CHANNEL(u->backend_sioc),
+ G_IO_IN | G_IO_HUP,
+ backend_read, dev,
+ NULL, NULL);
if (reply_supported) {
msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
@@ -2336,7 +2335,7 @@ static int vhost_user_backend_cleanup(struct vhost_dev *dev)
close(u->postcopy_fd.fd);
u->postcopy_fd.handler = NULL;
}
- if (u->backend_ioc) {
+ if (u->backend_sioc) {
close_backend_channel(u);
}
g_free(u->region_rb);
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 10/23] vhost: vhost_virtqueue_start(): fix failure path
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 09/23] vhost-user: keep QIOChannelSocket for backend channel Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 20:13 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 11/23] vhost: make vhost_memory_unmap() null-safe Vladimir Sementsov-Ogievskiy
` (13 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
We miss call to unmap in cases when vhost_memory_map() returns
lenght less than requested (still we consider such cases as an
error). Let's fix it in vhost_memory_map().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/vhost.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 773b91c0a0..8031c74e7b 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -453,11 +453,20 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
}
static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
- hwaddr *plen, bool is_write)
+ hwaddr len, bool is_write)
{
if (!vhost_dev_has_iommu(dev)) {
- return address_space_map(dev->vdev->dma_as, addr, plen, is_write,
- MEMTXATTRS_UNSPECIFIED);
+ hwaddr mapped_len = len;
+ void *res = address_space_map(dev->vdev->dma_as, addr, &mapped_len,
+ is_write, MEMTXATTRS_UNSPECIFIED);
+ if (!res) {
+ return NULL;
+ }
+ if (len != mapped_len) {
+ address_space_unmap(dev->vdev->dma_as, res, mapped_len, 0, 0);
+ return NULL;
+ }
+ return res;
} else {
return (void *)(uintptr_t)addr;
}
@@ -1261,7 +1270,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
VirtioBusState *vbus = VIRTIO_BUS(qbus);
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
- hwaddr s, l, a;
+ hwaddr l, a;
int r;
int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
struct vhost_vring_file file = {
@@ -1301,24 +1310,24 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
}
}
- vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
+ vq->desc_size = l = virtio_queue_get_desc_size(vdev, idx);
vq->desc_phys = a;
- vq->desc = vhost_memory_map(dev, a, &l, false);
- if (!vq->desc || l != s) {
+ vq->desc = vhost_memory_map(dev, a, l, false);
+ if (!vq->desc) {
r = -ENOMEM;
goto fail_alloc_desc;
}
- vq->avail_size = s = l = virtio_queue_get_avail_size(vdev, idx);
+ vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
- vq->avail = vhost_memory_map(dev, a, &l, false);
- if (!vq->avail || l != s) {
+ vq->avail = vhost_memory_map(dev, a, l, false);
+ if (!vq->avail) {
r = -ENOMEM;
goto fail_alloc_avail;
}
- vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
+ vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
- vq->used = vhost_memory_map(dev, a, &l, true);
- if (!vq->used || l != s) {
+ vq->used = vhost_memory_map(dev, a, l, true);
+ if (!vq->used) {
r = -ENOMEM;
goto fail_alloc_used;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 10/23] vhost: vhost_virtqueue_start(): fix failure path
2025-10-11 23:23 ` [PATCH v2 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
@ 2025-10-13 20:13 ` Raphael Norwitz
2025-10-14 16:23 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 20:13 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
Overall looks good just once comment.
On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> We miss call to unmap in cases when vhost_memory_map() returns
> lenght less than requested (still we consider such cases as an
> error). Let's fix it in vhost_memory_map().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/vhost.c | 35 ++++++++++++++++++++++-------------
> 1 file changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 773b91c0a0..8031c74e7b 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -453,11 +453,20 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
> }
>
> static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
> - hwaddr *plen, bool is_write)
> + hwaddr len, bool is_write)
> {
> if (!vhost_dev_has_iommu(dev)) {
> - return address_space_map(dev->vdev->dma_as, addr, plen, is_write,
> - MEMTXATTRS_UNSPECIFIED);
> + hwaddr mapped_len = len;
> + void *res = address_space_map(dev->vdev->dma_as, addr, &mapped_len,
> + is_write, MEMTXATTRS_UNSPECIFIED);
> + if (!res) {
> + return NULL;
> + }
> + if (len != mapped_len) {
Should this be:
address_space_unmap(dev->vdev->dma_as, res, mapped_len, is_write,
MEMTXATTRS_UNSPECIFIED);
rather than address_space_unmap(...0,0)?
> + address_space_unmap(dev->vdev->dma_as, res, mapped_len, 0, 0);
> + return NULL;
> + }
> + return res;
> } else {
> return (void *)(uintptr_t)addr;
> }
> @@ -1261,7 +1270,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
> VirtioBusState *vbus = VIRTIO_BUS(qbus);
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
> - hwaddr s, l, a;
> + hwaddr l, a;
> int r;
> int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
> struct vhost_vring_file file = {
> @@ -1301,24 +1310,24 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> }
> }
>
> - vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
> + vq->desc_size = l = virtio_queue_get_desc_size(vdev, idx);
> vq->desc_phys = a;
> - vq->desc = vhost_memory_map(dev, a, &l, false);
> - if (!vq->desc || l != s) {
> + vq->desc = vhost_memory_map(dev, a, l, false);
> + if (!vq->desc) {
> r = -ENOMEM;
> goto fail_alloc_desc;
> }
> - vq->avail_size = s = l = virtio_queue_get_avail_size(vdev, idx);
> + vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
> vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
> - vq->avail = vhost_memory_map(dev, a, &l, false);
> - if (!vq->avail || l != s) {
> + vq->avail = vhost_memory_map(dev, a, l, false);
> + if (!vq->avail) {
> r = -ENOMEM;
> goto fail_alloc_avail;
> }
> - vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
> + vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
> vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
> - vq->used = vhost_memory_map(dev, a, &l, true);
> - if (!vq->used || l != s) {
> + vq->used = vhost_memory_map(dev, a, l, true);
> + if (!vq->used) {
> r = -ENOMEM;
> goto fail_alloc_used;
> }
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [PATCH v2 10/23] vhost: vhost_virtqueue_start(): fix failure path
2025-10-13 20:13 ` Raphael Norwitz
@ 2025-10-14 16:23 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-14 16:23 UTC (permalink / raw)
To: Raphael Norwitz; +Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
On 13.10.25 23:13, Raphael Norwitz wrote:
> Overall looks good just once comment.
>
> On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
>>
>> We miss call to unmap in cases when vhost_memory_map() returns
>> lenght less than requested (still we consider such cases as an
>> error). Let's fix it in vhost_memory_map().
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>> hw/virtio/vhost.c | 35 ++++++++++++++++++++++-------------
>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 773b91c0a0..8031c74e7b 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -453,11 +453,20 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
>> }
>>
>> static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
>> - hwaddr *plen, bool is_write)
>> + hwaddr len, bool is_write)
>> {
>> if (!vhost_dev_has_iommu(dev)) {
>> - return address_space_map(dev->vdev->dma_as, addr, plen, is_write,
>> - MEMTXATTRS_UNSPECIFIED);
>> + hwaddr mapped_len = len;
>> + void *res = address_space_map(dev->vdev->dma_as, addr, &mapped_len,
>> + is_write, MEMTXATTRS_UNSPECIFIED);
>> + if (!res) {
>> + return NULL;
>> + }
>> + if (len != mapped_len) {
>
> Should this be:
>
> address_space_unmap(dev->vdev->dma_as, res, mapped_len, is_write,
> MEMTXATTRS_UNSPECIFIED);
>
> rather than address_space_unmap(...0,0)?
Pre-patch we do "...0,0)" for already mapped memories.
Moreover, following commit above address_space_unmap() definition,
is_write and access_len are here to mark the memory as dirty.
In our case, that's an early fail, and we don't actually access the memory,
so it's correct to set is_write to zero, I think.
The latter 0 is also correct, as it's just unused when is_write==0.
>
>> + address_space_unmap(dev->vdev->dma_as, res, mapped_len, 0, 0);
>> + return NULL;
>> + }
>> + return res;
>> } else {
>> return (void *)(uintptr_t)addr;
>> }
>> @@ -1261,7 +1270,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
>> BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
>> VirtioBusState *vbus = VIRTIO_BUS(qbus);
>> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
>> - hwaddr s, l, a;
>> + hwaddr l, a;
>> int r;
>> int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
>> struct vhost_vring_file file = {
>> @@ -1301,24 +1310,24 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
>> }
>> }
>>
>> - vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
>> + vq->desc_size = l = virtio_queue_get_desc_size(vdev, idx);
>> vq->desc_phys = a;
>> - vq->desc = vhost_memory_map(dev, a, &l, false);
>> - if (!vq->desc || l != s) {
>> + vq->desc = vhost_memory_map(dev, a, l, false);
>> + if (!vq->desc) {
>> r = -ENOMEM;
>> goto fail_alloc_desc;
>> }
>> - vq->avail_size = s = l = virtio_queue_get_avail_size(vdev, idx);
>> + vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
>> vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
>> - vq->avail = vhost_memory_map(dev, a, &l, false);
>> - if (!vq->avail || l != s) {
>> + vq->avail = vhost_memory_map(dev, a, l, false);
>> + if (!vq->avail) {
>> r = -ENOMEM;
>> goto fail_alloc_avail;
>> }
>> - vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
>> + vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
>> vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
>> - vq->used = vhost_memory_map(dev, a, &l, true);
>> - if (!vq->used || l != s) {
>> + vq->used = vhost_memory_map(dev, a, l, true);
>> + if (!vq->used) {
>> r = -ENOMEM;
>> goto fail_alloc_used;
>> }
>> --
>> 2.48.1
>>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 11/23] vhost: make vhost_memory_unmap() null-safe
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 20:23 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 12/23] vhost: simplify calls to vhost_memory_unmap() Vladimir Sementsov-Ogievskiy
` (12 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
This helps to simplify failure paths of vhost_virtqueue_start()
a lot. We also need to zero-out pointers on unmap, to not try
to unmap invalid pointers.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/vhost.c | 41 +++++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 8031c74e7b..6fec193d5f 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -472,14 +472,20 @@ static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
}
}
-static void vhost_memory_unmap(struct vhost_dev *dev, void *buffer,
+static void vhost_memory_unmap(struct vhost_dev *dev, void **buffer,
hwaddr len, int is_write,
hwaddr access_len)
{
+ if (!*buffer) {
+ return;
+ }
+
if (!vhost_dev_has_iommu(dev)) {
- address_space_unmap(dev->vdev->dma_as, buffer, len, is_write,
+ address_space_unmap(dev->vdev->dma_as, *buffer, len, is_write,
access_len);
}
+
+ *buffer = NULL;
}
static int vhost_verify_ring_part_mapping(void *ring_hva,
@@ -1315,33 +1321,33 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
vq->desc = vhost_memory_map(dev, a, l, false);
if (!vq->desc) {
r = -ENOMEM;
- goto fail_alloc_desc;
+ goto fail;
}
vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
vq->avail = vhost_memory_map(dev, a, l, false);
if (!vq->avail) {
r = -ENOMEM;
- goto fail_alloc_avail;
+ goto fail;
}
vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
vq->used = vhost_memory_map(dev, a, l, true);
if (!vq->used) {
r = -ENOMEM;
- goto fail_alloc_used;
+ goto fail;
}
r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
if (r < 0) {
- goto fail_alloc;
+ goto fail;
}
file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
if (r) {
VHOST_OPS_DEBUG(r, "vhost_set_vring_kick failed");
- goto fail_kick;
+ goto fail;
}
/* Clear and discard previous events if any. */
@@ -1361,24 +1367,19 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
file.fd = -1;
r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
if (r) {
- goto fail_vector;
+ goto fail;
}
}
return 0;
-fail_vector:
-fail_kick:
-fail_alloc:
- vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
+fail:
+ vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
0, 0);
-fail_alloc_used:
- vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
+ vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
0, 0);
-fail_alloc_avail:
- vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
+ vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
0, 0);
-fail_alloc_desc:
return r;
}
@@ -1425,11 +1426,11 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
vhost_vq_index);
}
- vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
+ vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
1, virtio_queue_get_used_size(vdev, idx));
- vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
+ vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
0, virtio_queue_get_avail_size(vdev, idx));
- vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
+ vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
0, virtio_queue_get_desc_size(vdev, idx));
return r;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 11/23] vhost: make vhost_memory_unmap() null-safe
2025-10-11 23:23 ` [PATCH v2 11/23] vhost: make vhost_memory_unmap() null-safe Vladimir Sementsov-Ogievskiy
@ 2025-10-13 20:23 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 20:23 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
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:
>
> This helps to simplify failure paths of vhost_virtqueue_start()
> a lot. We also need to zero-out pointers on unmap, to not try
> to unmap invalid pointers.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/vhost.c | 41 +++++++++++++++++++++--------------------
> 1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 8031c74e7b..6fec193d5f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -472,14 +472,20 @@ static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
> }
> }
>
> -static void vhost_memory_unmap(struct vhost_dev *dev, void *buffer,
> +static void vhost_memory_unmap(struct vhost_dev *dev, void **buffer,
> hwaddr len, int is_write,
> hwaddr access_len)
> {
> + if (!*buffer) {
> + return;
> + }
> +
> if (!vhost_dev_has_iommu(dev)) {
> - address_space_unmap(dev->vdev->dma_as, buffer, len, is_write,
> + address_space_unmap(dev->vdev->dma_as, *buffer, len, is_write,
> access_len);
> }
> +
> + *buffer = NULL;
> }
>
> static int vhost_verify_ring_part_mapping(void *ring_hva,
> @@ -1315,33 +1321,33 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> vq->desc = vhost_memory_map(dev, a, l, false);
> if (!vq->desc) {
> r = -ENOMEM;
> - goto fail_alloc_desc;
> + goto fail;
> }
> vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
> vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
> vq->avail = vhost_memory_map(dev, a, l, false);
> if (!vq->avail) {
> r = -ENOMEM;
> - goto fail_alloc_avail;
> + goto fail;
> }
> vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
> vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
> vq->used = vhost_memory_map(dev, a, l, true);
> if (!vq->used) {
> r = -ENOMEM;
> - goto fail_alloc_used;
> + goto fail;
> }
>
> r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
> if (r < 0) {
> - goto fail_alloc;
> + goto fail;
> }
>
> file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
> r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
> if (r) {
> VHOST_OPS_DEBUG(r, "vhost_set_vring_kick failed");
> - goto fail_kick;
> + goto fail;
> }
>
> /* Clear and discard previous events if any. */
> @@ -1361,24 +1367,19 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> file.fd = -1;
> r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> if (r) {
> - goto fail_vector;
> + goto fail;
> }
> }
>
> return 0;
>
> -fail_vector:
> -fail_kick:
> -fail_alloc:
> - vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> +fail:
> + vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
> 0, 0);
> -fail_alloc_used:
> - vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
> + vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
> 0, 0);
> -fail_alloc_avail:
> - vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> + vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
> 0, 0);
> -fail_alloc_desc:
> return r;
> }
>
> @@ -1425,11 +1426,11 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
> vhost_vq_index);
> }
>
> - vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> + vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
> 1, virtio_queue_get_used_size(vdev, idx));
> - vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
> + vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
> 0, virtio_queue_get_avail_size(vdev, idx));
> - vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> + vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
> 0, virtio_queue_get_desc_size(vdev, idx));
> return r;
> }
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 12/23] vhost: simplify calls to vhost_memory_unmap()
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (10 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 11/23] vhost: make vhost_memory_unmap() null-safe Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 20:26 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start() Vladimir Sementsov-Ogievskiy
` (11 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
No reason to calculate memory size again, as we have corresponding
variable for each vring.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/vhost.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6fec193d5f..e00ba9ecc8 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1374,12 +1374,9 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
return 0;
fail:
- vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
- 0, 0);
- vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
- 0, 0);
- vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
- 0, 0);
+ vhost_memory_unmap(dev, &vq->used, vq->used_size, 0, 0);
+ vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, 0);
+ vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, 0);
return r;
}
@@ -1426,12 +1423,9 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
vhost_vq_index);
}
- vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
- 1, virtio_queue_get_used_size(vdev, idx));
- vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
- 0, virtio_queue_get_avail_size(vdev, idx));
- vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
- 0, virtio_queue_get_desc_size(vdev, idx));
+ vhost_memory_unmap(dev, &vq->used, vq->used_size, 1, vq->used_size);
+ vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, vq->avail_size);
+ vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, vq->desc_size);
return r;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 12/23] vhost: simplify calls to vhost_memory_unmap()
2025-10-11 23:23 ` [PATCH v2 12/23] vhost: simplify calls to vhost_memory_unmap() Vladimir Sementsov-Ogievskiy
@ 2025-10-13 20:26 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 20:26 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
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:
>
> No reason to calculate memory size again, as we have corresponding
> variable for each vring.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/vhost.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6fec193d5f..e00ba9ecc8 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1374,12 +1374,9 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> return 0;
>
> fail:
> - vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
> - 0, 0);
> - vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
> - 0, 0);
> - vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
> - 0, 0);
> + vhost_memory_unmap(dev, &vq->used, vq->used_size, 0, 0);
> + vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, 0);
> + vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, 0);
> return r;
> }
>
> @@ -1426,12 +1423,9 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
> vhost_vq_index);
> }
>
> - vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
> - 1, virtio_queue_get_used_size(vdev, idx));
> - vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
> - 0, virtio_queue_get_avail_size(vdev, idx));
> - vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
> - 0, virtio_queue_get_desc_size(vdev, idx));
> + vhost_memory_unmap(dev, &vq->used, vq->used_size, 1, vq->used_size);
> + vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, vq->avail_size);
> + vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, vq->desc_size);
> return r;
> }
>
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start()
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (11 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 12/23] vhost: simplify calls to vhost_memory_unmap() Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 14/23] vhost: vhost_virtqueue_start(): drop extra local variables Vladimir Sementsov-Ogievskiy
` (10 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
This simplifies further refactoring and final introduction
of vhost backend live migration.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/vhost.c | 47 +++++++++++++++++++++++------------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e00ba9ecc8..7390385876 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1292,30 +1292,6 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
/* Queue might not be ready for start */
return 0;
}
-
- vq->num = state.num = virtio_queue_get_num(vdev, idx);
- r = dev->vhost_ops->vhost_set_vring_num(dev, &state);
- if (r) {
- VHOST_OPS_DEBUG(r, "vhost_set_vring_num failed");
- return r;
- }
-
- state.num = virtio_queue_get_last_avail_idx(vdev, idx);
- r = dev->vhost_ops->vhost_set_vring_base(dev, &state);
- if (r) {
- VHOST_OPS_DEBUG(r, "vhost_set_vring_base failed");
- return r;
- }
-
- if (vhost_needs_vring_endian(vdev)) {
- r = vhost_virtqueue_set_vring_endian_legacy(dev,
- virtio_is_big_endian(vdev),
- vhost_vq_index);
- if (r) {
- return r;
- }
- }
-
vq->desc_size = l = virtio_queue_get_desc_size(vdev, idx);
vq->desc_phys = a;
vq->desc = vhost_memory_map(dev, a, l, false);
@@ -1338,6 +1314,29 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
goto fail;
}
+ vq->num = state.num = virtio_queue_get_num(vdev, idx);
+ r = dev->vhost_ops->vhost_set_vring_num(dev, &state);
+ if (r) {
+ VHOST_OPS_DEBUG(r, "vhost_set_vring_num failed");
+ goto fail;
+ }
+
+ state.num = virtio_queue_get_last_avail_idx(vdev, idx);
+ r = dev->vhost_ops->vhost_set_vring_base(dev, &state);
+ if (r) {
+ VHOST_OPS_DEBUG(r, "vhost_set_vring_base failed");
+ goto fail;
+ }
+
+ if (vhost_needs_vring_endian(vdev)) {
+ r = vhost_virtqueue_set_vring_endian_legacy(dev,
+ virtio_is_big_endian(vdev),
+ vhost_vq_index);
+ if (r) {
+ goto fail;
+ }
+ }
+
r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
if (r < 0) {
goto fail;
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 14/23] vhost: vhost_virtqueue_start(): drop extra local variables
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (12 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start() Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
One letter named variables doesn't really help to read the code,
and they simply duplicate structure fields.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/vhost.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7390385876..cc07450dcb 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1276,7 +1276,6 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
VirtioBusState *vbus = VIRTIO_BUS(qbus);
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
- hwaddr l, a;
int r;
int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
struct vhost_vring_file file = {
@@ -1287,28 +1286,27 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
};
struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
- a = virtio_queue_get_desc_addr(vdev, idx);
- if (a == 0) {
+ vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
+ if (vq->desc_phys == 0) {
/* Queue might not be ready for start */
return 0;
}
- vq->desc_size = l = virtio_queue_get_desc_size(vdev, idx);
- vq->desc_phys = a;
- vq->desc = vhost_memory_map(dev, a, l, false);
+ vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
+ vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
if (!vq->desc) {
r = -ENOMEM;
goto fail;
}
- vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
- vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
- vq->avail = vhost_memory_map(dev, a, l, false);
+ vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
+ vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
+ vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
if (!vq->avail) {
r = -ENOMEM;
goto fail;
}
- vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
- vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
- vq->used = vhost_memory_map(dev, a, l, true);
+ vq->used_size = virtio_queue_get_used_size(vdev, idx);
+ vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
+ vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
if (!vq->used) {
r = -ENOMEM;
goto fail;
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (13 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 14/23] vhost: vhost_virtqueue_start(): drop extra local variables Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 20:39 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 16/23] vhost: simplify vhost_dev_init() error-path Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Introduce helper functions vhost_vrings_map() and
vhost_vrings_unmap() and use them.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/vhost.c | 82 ++++++++++++++++++++++++++++++-----------------
1 file changed, 52 insertions(+), 30 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index cc07450dcb..ebdea096aa 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -488,6 +488,53 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void **buffer,
*buffer = NULL;
}
+static void vhost_vrings_unmap(struct vhost_dev *dev,
+ struct vhost_virtqueue *vq, bool touched)
+{
+ vhost_memory_unmap(dev, &vq->used, vq->used_size, touched,
+ touched ? vq->used_size : 0);
+ vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0,
+ touched ? vq->avail_size : 0);
+ vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0,
+ touched ? vq->desc_size : 0);
+}
+
+static int vhost_vrings_map(struct vhost_dev *dev,
+ struct VirtIODevice *vdev,
+ struct vhost_virtqueue *vq,
+ unsigned idx)
+{
+ vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
+ if (vq->desc_phys == 0) {
+ /* Queue might not be ready for start */
+ return 0;
+ }
+ vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
+ vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
+ if (!vq->desc) {
+ return -ENOMEM;
+ }
+
+ vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
+ vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
+ vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
+ if (!vq->avail) {
+ goto fail;
+ }
+
+ vq->used_size = virtio_queue_get_used_size(vdev, idx);
+ vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
+ vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
+ if (!vq->used) {
+ goto fail;
+ }
+
+ return 1;
+fail:
+ vhost_vrings_unmap(dev, vq, false);
+ return -ENOMEM;
+}
+
static int vhost_verify_ring_part_mapping(void *ring_hva,
uint64_t ring_gpa,
uint64_t ring_size,
@@ -1286,30 +1333,9 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
};
struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
- vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
- if (vq->desc_phys == 0) {
- /* Queue might not be ready for start */
- return 0;
- }
- vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
- vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
- if (!vq->desc) {
- r = -ENOMEM;
- goto fail;
- }
- vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
- vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
- vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
- if (!vq->avail) {
- r = -ENOMEM;
- goto fail;
- }
- vq->used_size = virtio_queue_get_used_size(vdev, idx);
- vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
- vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
- if (!vq->used) {
- r = -ENOMEM;
- goto fail;
+ r = vhost_vrings_map(dev, vdev, vq, idx);
+ if (r <= 0) {
+ return r;
}
vq->num = state.num = virtio_queue_get_num(vdev, idx);
@@ -1371,9 +1397,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
return 0;
fail:
- vhost_memory_unmap(dev, &vq->used, vq->used_size, 0, 0);
- vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, 0);
- vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, 0);
+ vhost_vrings_unmap(dev, vq, false);
return r;
}
@@ -1420,9 +1444,7 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
vhost_vq_index);
}
- vhost_memory_unmap(dev, &vq->used, vq->used_size, 1, vq->used_size);
- vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, vq->avail_size);
- vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, vq->desc_size);
+ vhost_vrings_unmap(dev, vq, true);
return r;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap
2025-10-11 23:23 ` [PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap Vladimir Sementsov-Ogievskiy
@ 2025-10-13 20:39 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 20:39 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
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:
>
> Introduce helper functions vhost_vrings_map() and
> vhost_vrings_unmap() and use them.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/vhost.c | 82 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 52 insertions(+), 30 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index cc07450dcb..ebdea096aa 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -488,6 +488,53 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void **buffer,
> *buffer = NULL;
> }
>
> +static void vhost_vrings_unmap(struct vhost_dev *dev,
> + struct vhost_virtqueue *vq, bool touched)
> +{
> + vhost_memory_unmap(dev, &vq->used, vq->used_size, touched,
> + touched ? vq->used_size : 0);
> + vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0,
> + touched ? vq->avail_size : 0);
> + vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0,
> + touched ? vq->desc_size : 0);
> +}
> +
> +static int vhost_vrings_map(struct vhost_dev *dev,
> + struct VirtIODevice *vdev,
> + struct vhost_virtqueue *vq,
> + unsigned idx)
> +{
> + vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
> + if (vq->desc_phys == 0) {
> + /* Queue might not be ready for start */
> + return 0;
> + }
> + vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
> + vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
> + if (!vq->desc) {
> + return -ENOMEM;
> + }
> +
> + vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
> + vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
> + vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
> + if (!vq->avail) {
> + goto fail;
> + }
> +
> + vq->used_size = virtio_queue_get_used_size(vdev, idx);
> + vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
> + vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
> + if (!vq->used) {
> + goto fail;
> + }
> +
> + return 1;
> +fail:
> + vhost_vrings_unmap(dev, vq, false);
> + return -ENOMEM;
> +}
> +
> static int vhost_verify_ring_part_mapping(void *ring_hva,
> uint64_t ring_gpa,
> uint64_t ring_size,
> @@ -1286,30 +1333,9 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> };
> struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
>
> - vq->desc_phys = virtio_queue_get_desc_addr(vdev, idx);
> - if (vq->desc_phys == 0) {
> - /* Queue might not be ready for start */
> - return 0;
> - }
> - vq->desc_size = virtio_queue_get_desc_size(vdev, idx);
> - vq->desc = vhost_memory_map(dev, vq->desc_phys, vq->desc_size, false);
> - if (!vq->desc) {
> - r = -ENOMEM;
> - goto fail;
> - }
> - vq->avail_size = virtio_queue_get_avail_size(vdev, idx);
> - vq->avail_phys = virtio_queue_get_avail_addr(vdev, idx);
> - vq->avail = vhost_memory_map(dev, vq->avail_phys, vq->avail_size, false);
> - if (!vq->avail) {
> - r = -ENOMEM;
> - goto fail;
> - }
> - vq->used_size = virtio_queue_get_used_size(vdev, idx);
> - vq->used_phys = virtio_queue_get_used_addr(vdev, idx);
> - vq->used = vhost_memory_map(dev, vq->used_phys, vq->used_size, true);
> - if (!vq->used) {
> - r = -ENOMEM;
> - goto fail;
> + r = vhost_vrings_map(dev, vdev, vq, idx);
> + if (r <= 0) {
> + return r;
> }
>
> vq->num = state.num = virtio_queue_get_num(vdev, idx);
> @@ -1371,9 +1397,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> return 0;
>
> fail:
> - vhost_memory_unmap(dev, &vq->used, vq->used_size, 0, 0);
> - vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, 0);
> - vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, 0);
> + vhost_vrings_unmap(dev, vq, false);
> return r;
> }
>
> @@ -1420,9 +1444,7 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
> vhost_vq_index);
> }
>
> - vhost_memory_unmap(dev, &vq->used, vq->used_size, 1, vq->used_size);
> - vhost_memory_unmap(dev, &vq->avail, vq->avail_size, 0, vq->avail_size);
> - vhost_memory_unmap(dev, &vq->desc, vq->desc_size, 0, vq->desc_size);
> + vhost_vrings_unmap(dev, vq, true);
> return r;
> }
>
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 16/23] vhost: simplify vhost_dev_init() error-path
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (14 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 15/23] vhost: final refactoring of vhost vrings map/unmap Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init() Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
No reason to rollback setting up busyloop timeout on failure.
We don't do such rollback for other things we setup in backend.
Also, look at vhost_net_init() in hw/net/vhost_net.c: we may fail
after successfully called vhost_dev_init(), and in this case we'll
just call vhost_dev_cleanup(), which doesn't rollback busyloop
timeout.
So, let's keep it simple.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/vhost.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ebdea096aa..188ea0f955 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1621,7 +1621,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
busyloop_timeout);
if (r < 0) {
error_setg_errno(errp, -r, "Failed to set busyloop timeout");
- goto fail_busyloop;
+ goto fail;
}
}
}
@@ -1661,7 +1661,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
if (hdev->migration_blocker != NULL) {
r = migrate_add_blocker_normal(&hdev->migration_blocker, errp);
if (r < 0) {
- goto fail_busyloop;
+ goto fail;
}
}
@@ -1693,17 +1693,11 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
" than current number of used (%d) and reserved (%d)"
" memory slots for memory devices.", limit, used, reserved);
r = -EINVAL;
- goto fail_busyloop;
+ goto fail;
}
return 0;
-fail_busyloop:
- if (busyloop_timeout) {
- while (--i >= 0) {
- vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, 0);
- }
- }
fail:
hdev->nvqs = n_initialized_vqs;
vhost_dev_cleanup(hdev);
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init()
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (15 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 16/23] vhost: simplify vhost_dev_init() error-path Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 18/23] vhost: introduce check_memslots() helper Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Let's all per-virtqueue initializations be in one place.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/vhost.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 188ea0f955..8ba97c231b 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1493,7 +1493,8 @@ static void vhost_virtqueue_error_notifier(EventNotifier *n)
}
static int vhost_virtqueue_init(struct vhost_dev *dev,
- struct vhost_virtqueue *vq, int n)
+ struct vhost_virtqueue *vq, int n,
+ bool busyloop_timeout)
{
int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, n);
struct vhost_vring_file file = {
@@ -1530,6 +1531,14 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
vhost_virtqueue_error_notifier);
}
+ if (busyloop_timeout) {
+ r = vhost_virtqueue_set_busyloop_timeout(dev, n, busyloop_timeout);
+ if (r < 0) {
+ VHOST_OPS_DEBUG(r, "Failed to set busyloop timeout");
+ goto fail_err;
+ }
+ }
+
return 0;
fail_err:
@@ -1608,24 +1617,14 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
}
for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) {
- r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i);
+ r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i,
+ busyloop_timeout);
if (r < 0) {
error_setg_errno(errp, -r, "Failed to initialize virtqueue %d", i);
goto fail;
}
}
- if (busyloop_timeout) {
- for (i = 0; i < hdev->nvqs; ++i) {
- r = vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i,
- busyloop_timeout);
- if (r < 0) {
- error_setg_errno(errp, -r, "Failed to set busyloop timeout");
- goto fail;
- }
- }
- }
-
virtio_features_copy(hdev->_features_ex, features);
hdev->memory_listener = (MemoryListener) {
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 18/23] vhost: introduce check_memslots() helper
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (16 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init() Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:23 ` [PATCH v2 19/23] vhost: vhost_dev_init(): simplify features initialization Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/vhost.c | 71 ++++++++++++++++++++++++++---------------------
1 file changed, 40 insertions(+), 31 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 8ba97c231b..c990029756 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1572,12 +1572,50 @@ static int vhost_dev_get_features(struct vhost_dev *hdev,
return r;
}
+static bool check_memslots(struct vhost_dev *hdev, Error **errp)
+{
+ unsigned int used, reserved, limit;
+
+ limit = hdev->vhost_ops->vhost_backend_memslots_limit(hdev);
+ if (limit < MEMORY_DEVICES_SAFE_MAX_MEMSLOTS &&
+ memory_devices_memslot_auto_decision_active()) {
+ error_setg(errp, "some memory device (like virtio-mem)"
+ " decided how many memory slots to use based on the overall"
+ " number of memory slots; this vhost backend would further"
+ " restricts the overall number of memory slots");
+ error_append_hint(errp, "Try plugging this vhost backend before"
+ " plugging such memory devices.\n");
+ return false;
+ }
+
+ /*
+ * The listener we registered properly setup the number of required
+ * memslots in vhost_commit().
+ */
+ used = hdev->mem->nregions;
+
+ /*
+ * We assume that all reserved memslots actually require a real memslot
+ * in our vhost backend. This might not be true, for example, if the
+ * memslot would be ROM. If ever relevant, we can optimize for that --
+ * but we'll need additional information about the reservations.
+ */
+ reserved = memory_devices_get_reserved_memslots();
+ if (used + reserved > limit) {
+ error_setg(errp, "vhost backend memory slots limit (%d) is less"
+ " than current number of used (%d) and reserved (%d)"
+ " memory slots for memory devices.", limit, used, reserved);
+ return false;
+ }
+
+ return true;
+}
+
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type, uint32_t busyloop_timeout,
Error **errp)
{
uint64_t features[VIRTIO_FEATURES_NU64S];
- unsigned int used, reserved, limit;
int i, r, n_initialized_vqs = 0;
hdev->vdev = NULL;
@@ -1603,19 +1641,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
goto fail;
}
- limit = hdev->vhost_ops->vhost_backend_memslots_limit(hdev);
- if (limit < MEMORY_DEVICES_SAFE_MAX_MEMSLOTS &&
- memory_devices_memslot_auto_decision_active()) {
- error_setg(errp, "some memory device (like virtio-mem)"
- " decided how many memory slots to use based on the overall"
- " number of memory slots; this vhost backend would further"
- " restricts the overall number of memory slots");
- error_append_hint(errp, "Try plugging this vhost backend before"
- " plugging such memory devices.\n");
- r = -EINVAL;
- goto fail;
- }
-
for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) {
r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i,
busyloop_timeout);
@@ -1674,23 +1699,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
memory_listener_register(&hdev->memory_listener, &address_space_memory);
QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
- /*
- * The listener we registered properly setup the number of required
- * memslots in vhost_commit().
- */
- used = hdev->mem->nregions;
-
- /*
- * We assume that all reserved memslots actually require a real memslot
- * in our vhost backend. This might not be true, for example, if the
- * memslot would be ROM. If ever relevant, we can optimize for that --
- * but we'll need additional information about the reservations.
- */
- reserved = memory_devices_get_reserved_memslots();
- if (used + reserved > limit) {
- error_setg(errp, "vhost backend memory slots limit (%d) is less"
- " than current number of used (%d) and reserved (%d)"
- " memory slots for memory devices.", limit, used, reserved);
+ if (!check_memslots(hdev, errp)) {
r = -EINVAL;
goto fail;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 19/23] vhost: vhost_dev_init(): simplify features initialization
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (17 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 18/23] vhost: introduce check_memslots() helper Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-13 20:51 ` Raphael Norwitz
2025-10-11 23:23 ` [PATCH v2 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier() Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Drop extra variable and extra function parameter passing, initialize
dev._features directly.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/vhost.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index c990029756..d02d1d4c34 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1557,18 +1557,17 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
}
}
-static int vhost_dev_get_features(struct vhost_dev *hdev,
- uint64_t *features)
+static int vhost_dev_init_features(struct vhost_dev *hdev)
{
uint64_t features64;
int r;
if (hdev->vhost_ops->vhost_get_features_ex) {
- return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
+ return hdev->vhost_ops->vhost_get_features_ex(hdev, hdev->_features_ex);
}
r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
- virtio_features_from_u64(features, features64);
+ virtio_features_from_u64(hdev->_features_ex, features64);
return r;
}
@@ -1615,7 +1614,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type, uint32_t busyloop_timeout,
Error **errp)
{
- uint64_t features[VIRTIO_FEATURES_NU64S];
int i, r, n_initialized_vqs = 0;
hdev->vdev = NULL;
@@ -1635,9 +1633,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
goto fail;
}
- r = vhost_dev_get_features(hdev, features);
+ r = vhost_dev_init_features(hdev);
if (r < 0) {
- error_setg_errno(errp, -r, "vhost_get_features failed");
+ error_setg_errno(errp, -r, "vhost_init_features failed");
goto fail;
}
@@ -1650,8 +1648,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
}
}
- virtio_features_copy(hdev->_features_ex, features);
-
hdev->memory_listener = (MemoryListener) {
.name = "vhost",
.begin = vhost_begin,
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 19/23] vhost: vhost_dev_init(): simplify features initialization
2025-10-11 23:23 ` [PATCH v2 19/23] vhost: vhost_dev_init(): simplify features initialization Vladimir Sementsov-Ogievskiy
@ 2025-10-13 20:51 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 20:51 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
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:
>
> Drop extra variable and extra function parameter passing, initialize
> dev._features directly.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/vhost.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index c990029756..d02d1d4c34 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1557,18 +1557,17 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> }
> }
>
> -static int vhost_dev_get_features(struct vhost_dev *hdev,
> - uint64_t *features)
> +static int vhost_dev_init_features(struct vhost_dev *hdev)
> {
> uint64_t features64;
> int r;
>
> if (hdev->vhost_ops->vhost_get_features_ex) {
> - return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
> + return hdev->vhost_ops->vhost_get_features_ex(hdev, hdev->_features_ex);
> }
>
> r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
> - virtio_features_from_u64(features, features64);
> + virtio_features_from_u64(hdev->_features_ex, features64);
> return r;
> }
>
> @@ -1615,7 +1614,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> VhostBackendType backend_type, uint32_t busyloop_timeout,
> Error **errp)
> {
> - uint64_t features[VIRTIO_FEATURES_NU64S];
> int i, r, n_initialized_vqs = 0;
>
> hdev->vdev = NULL;
> @@ -1635,9 +1633,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> goto fail;
> }
>
> - r = vhost_dev_get_features(hdev, features);
> + r = vhost_dev_init_features(hdev);
> if (r < 0) {
> - error_setg_errno(errp, -r, "vhost_get_features failed");
> + error_setg_errno(errp, -r, "vhost_init_features failed");
> goto fail;
> }
>
> @@ -1650,8 +1648,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> }
> }
>
> - virtio_features_copy(hdev->_features_ex, features);
> -
> hdev->memory_listener = (MemoryListener) {
> .name = "vhost",
> .begin = vhost_begin,
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier()
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (18 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 19/23] vhost: vhost_dev_init(): simplify features initialization Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:23 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:24 ` [PATCH v2 21/23] vhost-user: make trace events more readable Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:23 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, Philippe Mathieu-Daudé
The logic kept as is. Refactor to simplify further changes.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/virtio-bus.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
index cef944e015..9b545acda3 100644
--- a/hw/virtio/virtio-bus.c
+++ b/hw/virtio/virtio-bus.c
@@ -298,20 +298,18 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, int n, bool assign)
__func__, strerror(-r), r);
return r;
}
- r = k->ioeventfd_assign(proxy, notifier, n, true);
- if (r < 0) {
- error_report("%s: unable to assign ioeventfd: %d", __func__, r);
- virtio_bus_cleanup_host_notifier(bus, n);
- }
- } else {
- k->ioeventfd_assign(proxy, notifier, n, false);
}
- if (r == 0) {
- virtio_queue_set_host_notifier_enabled(vq, assign);
+ r = k->ioeventfd_assign(proxy, notifier, n, assign);
+ if (r < 0 && assign) {
+ error_report("%s: unable to assign ioeventfd: %d", __func__, r);
+ virtio_bus_cleanup_host_notifier(bus, n);
+ return r;
}
- return r;
+ virtio_queue_set_host_notifier_enabled(vq, assign);
+
+ return 0;
}
void virtio_bus_cleanup_host_notifier(VirtioBusState *bus, int n)
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 21/23] vhost-user: make trace events more readable
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (19 preceding siblings ...)
2025-10-11 23:23 ` [PATCH v2 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier() Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:24 ` Vladimir Sementsov-Ogievskiy
2025-10-11 23:24 ` [PATCH v2 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
23 siblings, 0 replies; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:24 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, Philippe Mathieu-Daudé
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
hw/virtio/trace-events | 4 +-
hw/virtio/vhost-user.c | 94 +++++++++++++++++++++++++++++++++++++++++-
2 files changed, 94 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 658cc365e7..aa1ffa5e94 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -25,8 +25,8 @@ vhost_user_set_mem_table_withfd(int index, const char *name, uint64_t memory_siz
vhost_user_postcopy_waker(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t rb_offset) "%s + 0x%"PRIx64
-vhost_user_read(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
-vhost_user_write(uint32_t req, uint32_t flags) "req:%d flags:0x%"PRIx32""
+vhost_user_read(uint32_t req, const char *req_name, uint32_t flags) "req:%d (%s) flags:0x%"PRIx32""
+vhost_user_write(uint32_t req, const char *req_name, uint32_t flags) "req:%d (%s) flags:0x%"PRIx32""
vhost_user_create_notifier(int idx, void *n) "idx:%d n:%p"
# vhost-vdpa.c
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 23e7c12b16..e45b74eddd 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -119,6 +119,94 @@ typedef enum VhostUserBackendRequest {
VHOST_USER_BACKEND_MAX
} VhostUserBackendRequest;
+static const char *vhost_req_name(VhostUserRequest req)
+{
+ switch (req) {
+ case VHOST_USER_NONE:
+ return "NONE";
+ case VHOST_USER_GET_FEATURES:
+ return "GET_FEATURES";
+ case VHOST_USER_SET_FEATURES:
+ return "SET_FEATURES";
+ case VHOST_USER_SET_OWNER:
+ return "SET_OWNER";
+ case VHOST_USER_RESET_OWNER:
+ return "RESET_OWNER";
+ case VHOST_USER_SET_MEM_TABLE:
+ return "SET_MEM_TABLE";
+ case VHOST_USER_SET_LOG_BASE:
+ return "SET_LOG_BASE";
+ case VHOST_USER_SET_LOG_FD:
+ return "SET_LOG_FD";
+ case VHOST_USER_SET_VRING_NUM:
+ return "SET_VRING_NUM";
+ case VHOST_USER_SET_VRING_ADDR:
+ return "SET_VRING_ADDR";
+ case VHOST_USER_SET_VRING_BASE:
+ return "SET_VRING_BASE";
+ case VHOST_USER_GET_VRING_BASE:
+ return "GET_VRING_BASE";
+ case VHOST_USER_SET_VRING_KICK:
+ return "SET_VRING_KICK";
+ case VHOST_USER_SET_VRING_CALL:
+ return "SET_VRING_CALL";
+ case VHOST_USER_SET_VRING_ERR:
+ return "SET_VRING_ERR";
+ case VHOST_USER_GET_PROTOCOL_FEATURES:
+ return "GET_PROTOCOL_FEATURES";
+ case VHOST_USER_SET_PROTOCOL_FEATURES:
+ return "SET_PROTOCOL_FEATURES";
+ case VHOST_USER_GET_QUEUE_NUM:
+ return "GET_QUEUE_NUM";
+ case VHOST_USER_SET_VRING_ENABLE:
+ return "SET_VRING_ENABLE";
+ case VHOST_USER_SEND_RARP:
+ return "SEND_RARP";
+ case VHOST_USER_NET_SET_MTU:
+ return "NET_SET_MTU";
+ case VHOST_USER_SET_BACKEND_REQ_FD:
+ return "SET_BACKEND_REQ_FD";
+ case VHOST_USER_IOTLB_MSG:
+ return "IOTLB_MSG";
+ case VHOST_USER_SET_VRING_ENDIAN:
+ return "SET_VRING_ENDIAN";
+ case VHOST_USER_GET_CONFIG:
+ return "GET_CONFIG";
+ case VHOST_USER_SET_CONFIG:
+ return "SET_CONFIG";
+ case VHOST_USER_CREATE_CRYPTO_SESSION:
+ return "CREATE_CRYPTO_SESSION";
+ case VHOST_USER_CLOSE_CRYPTO_SESSION:
+ return "CLOSE_CRYPTO_SESSION";
+ case VHOST_USER_POSTCOPY_ADVISE:
+ return "POSTCOPY_ADVISE";
+ case VHOST_USER_POSTCOPY_LISTEN:
+ return "POSTCOPY_LISTEN";
+ case VHOST_USER_POSTCOPY_END:
+ return "POSTCOPY_END";
+ case VHOST_USER_GET_INFLIGHT_FD:
+ return "GET_INFLIGHT_FD";
+ case VHOST_USER_SET_INFLIGHT_FD:
+ return "SET_INFLIGHT_FD";
+ case VHOST_USER_GPU_SET_SOCKET:
+ return "GPU_SET_SOCKET";
+ case VHOST_USER_RESET_DEVICE:
+ return "RESET_DEVICE";
+ case VHOST_USER_GET_MAX_MEM_SLOTS:
+ return "GET_MAX_MEM_SLOTS";
+ case VHOST_USER_ADD_MEM_REG:
+ return "ADD_MEM_REG";
+ case VHOST_USER_REM_MEM_REG:
+ return "REM_MEM_REG";
+ case VHOST_USER_SET_STATUS:
+ return "SET_STATUS";
+ case VHOST_USER_GET_STATUS:
+ return "GET_STATUS";
+ default:
+ return "<unknown>";
+ }
+}
+
typedef struct VhostUserMemoryRegion {
uint64_t guest_phys_addr;
uint64_t memory_size;
@@ -311,7 +399,8 @@ static int vhost_user_read_header(struct vhost_dev *dev, VhostUserMsg *msg)
return -EPROTO;
}
- trace_vhost_user_read(msg->hdr.request, msg->hdr.flags);
+ trace_vhost_user_read(msg->hdr.request,
+ vhost_req_name(msg->hdr.request), msg->hdr.flags);
return 0;
}
@@ -431,7 +520,8 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg,
return ret < 0 ? -saved_errno : -EIO;
}
- trace_vhost_user_write(msg->hdr.request, msg->hdr.flags);
+ trace_vhost_user_write(msg->hdr.request, vhost_req_name(msg->hdr.request),
+ msg->hdr.flags);
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* [PATCH v2 22/23] vhost-user-blk: add some useful trace-points
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (20 preceding siblings ...)
2025-10-11 23:24 ` [PATCH v2 21/23] vhost-user: make trace events more readable Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:24 ` Vladimir Sementsov-Ogievskiy
2025-10-13 20:55 ` Raphael Norwitz
2025-10-11 23:24 ` [PATCH v2 23/23] vhost: " Vladimir Sementsov-Ogievskiy
2025-10-13 14:04 ` [PATCH v2 00/23] vhost refactoring and fixes Daniil Tatianin
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:24 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin, Kevin Wolf, Hanna Reitz,
open list:Block layer core
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/block/trace-events | 10 ++++++++++
hw/block/vhost-user-blk.c | 19 +++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/hw/block/trace-events b/hw/block/trace-events
index cc9a9f2460..dbaa5ca6cb 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -58,6 +58,16 @@ virtio_blk_handle_zone_mgmt(void *vdev, void *req, uint8_t op, int64_t sector, i
virtio_blk_handle_zone_reset_all(void *vdev, void *req, int64_t sector, int64_t len) "vdev %p req %p sector 0x%" PRIx64 " cap 0x%" PRIx64 ""
virtio_blk_handle_zone_append(void *vdev, void *req, int64_t sector) "vdev %p req %p, append sector 0x%" PRIx64 ""
+# vhost-user-blk.c
+vhost_user_blk_start_in(void *vdev) "vdev %p"
+vhost_user_blk_start_out(void *vdev) "vdev %p"
+vhost_user_blk_stop_in(void *vdev) "vdev %p"
+vhost_user_blk_stop_out(void *vdev) "vdev %p"
+vhost_user_blk_connect_in(void *vdev) "vdev %p"
+vhost_user_blk_connect_out(void *vdev) "vdev %p"
+vhost_user_blk_device_realize_in(void *vdev) "vdev %p"
+vhost_user_blk_device_realize_out(void *vdev) "vdev %p"
+
# hd-geometry.c
hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p LCHS %d %d %d"
hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "blk %p CHS %u %u %u trans %d"
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index de7a810c93..a5daed4346 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -31,6 +31,7 @@
#include "hw/virtio/virtio-access.h"
#include "system/system.h"
#include "system/runstate.h"
+#include "trace.h"
static const int user_feature_bits[] = {
VIRTIO_BLK_F_SIZE_MAX,
@@ -137,6 +138,8 @@ static int vhost_user_blk_start(VirtIODevice *vdev, Error **errp)
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
int i, ret;
+ trace_vhost_user_blk_start_in(vdev);
+
if (!k->set_guest_notifiers) {
error_setg(errp, "binding does not support guest notifiers");
return -ENOSYS;
@@ -192,6 +195,8 @@ static int vhost_user_blk_start(VirtIODevice *vdev, Error **errp)
}
s->started_vu = true;
+ trace_vhost_user_blk_start_out(vdev);
+
return ret;
err_guest_notifiers:
@@ -212,6 +217,8 @@ static int vhost_user_blk_stop(VirtIODevice *vdev)
int ret;
bool force_stop = false;
+ trace_vhost_user_blk_stop_in(vdev);
+
if (!s->started_vu) {
return 0;
}
@@ -233,6 +240,9 @@ static int vhost_user_blk_stop(VirtIODevice *vdev)
}
vhost_dev_disable_notifiers(&s->dev, vdev);
+
+ trace_vhost_user_blk_stop_out(vdev);
+
return ret;
}
@@ -340,6 +350,8 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
VHostUserBlk *s = VHOST_USER_BLK(vdev);
int ret = 0;
+ trace_vhost_user_blk_connect_in(vdev);
+
if (s->connected) {
return 0;
}
@@ -365,6 +377,8 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
ret = vhost_user_blk_start(vdev, errp);
}
+ trace_vhost_user_blk_connect_out(vdev);
+
return ret;
}
@@ -455,6 +469,8 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
int retries;
int i, ret;
+ trace_vhost_user_blk_device_realize_in(vdev);
+
if (!s->chardev.chr) {
error_setg(errp, "chardev is mandatory");
return;
@@ -514,6 +530,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL,
vhost_user_blk_event, NULL, (void *)dev,
NULL, true);
+
+ trace_vhost_user_blk_device_realize_out(vdev);
+
return;
virtio_err:
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 22/23] vhost-user-blk: add some useful trace-points
2025-10-11 23:24 ` [PATCH v2 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
@ 2025-10-13 20:55 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 20:55 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
Kevin Wolf, Hanna Reitz, open list:Block layer core
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:
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/block/trace-events | 10 ++++++++++
> hw/block/vhost-user-blk.c | 19 +++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index cc9a9f2460..dbaa5ca6cb 100644
> --- a/hw/block/trace-events
> +++ b/hw/block/trace-events
> @@ -58,6 +58,16 @@ virtio_blk_handle_zone_mgmt(void *vdev, void *req, uint8_t op, int64_t sector, i
> virtio_blk_handle_zone_reset_all(void *vdev, void *req, int64_t sector, int64_t len) "vdev %p req %p sector 0x%" PRIx64 " cap 0x%" PRIx64 ""
> virtio_blk_handle_zone_append(void *vdev, void *req, int64_t sector) "vdev %p req %p, append sector 0x%" PRIx64 ""
>
> +# vhost-user-blk.c
> +vhost_user_blk_start_in(void *vdev) "vdev %p"
> +vhost_user_blk_start_out(void *vdev) "vdev %p"
> +vhost_user_blk_stop_in(void *vdev) "vdev %p"
> +vhost_user_blk_stop_out(void *vdev) "vdev %p"
> +vhost_user_blk_connect_in(void *vdev) "vdev %p"
> +vhost_user_blk_connect_out(void *vdev) "vdev %p"
> +vhost_user_blk_device_realize_in(void *vdev) "vdev %p"
> +vhost_user_blk_device_realize_out(void *vdev) "vdev %p"
> +
> # hd-geometry.c
> hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p LCHS %d %d %d"
> hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "blk %p CHS %u %u %u trans %d"
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index de7a810c93..a5daed4346 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -31,6 +31,7 @@
> #include "hw/virtio/virtio-access.h"
> #include "system/system.h"
> #include "system/runstate.h"
> +#include "trace.h"
>
> static const int user_feature_bits[] = {
> VIRTIO_BLK_F_SIZE_MAX,
> @@ -137,6 +138,8 @@ static int vhost_user_blk_start(VirtIODevice *vdev, Error **errp)
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> int i, ret;
>
> + trace_vhost_user_blk_start_in(vdev);
> +
> if (!k->set_guest_notifiers) {
> error_setg(errp, "binding does not support guest notifiers");
> return -ENOSYS;
> @@ -192,6 +195,8 @@ static int vhost_user_blk_start(VirtIODevice *vdev, Error **errp)
> }
> s->started_vu = true;
>
> + trace_vhost_user_blk_start_out(vdev);
> +
> return ret;
>
> err_guest_notifiers:
> @@ -212,6 +217,8 @@ static int vhost_user_blk_stop(VirtIODevice *vdev)
> int ret;
> bool force_stop = false;
>
> + trace_vhost_user_blk_stop_in(vdev);
> +
> if (!s->started_vu) {
> return 0;
> }
> @@ -233,6 +240,9 @@ static int vhost_user_blk_stop(VirtIODevice *vdev)
> }
>
> vhost_dev_disable_notifiers(&s->dev, vdev);
> +
> + trace_vhost_user_blk_stop_out(vdev);
> +
> return ret;
> }
>
> @@ -340,6 +350,8 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> int ret = 0;
>
> + trace_vhost_user_blk_connect_in(vdev);
> +
> if (s->connected) {
> return 0;
> }
> @@ -365,6 +377,8 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> ret = vhost_user_blk_start(vdev, errp);
> }
>
> + trace_vhost_user_blk_connect_out(vdev);
> +
> return ret;
> }
>
> @@ -455,6 +469,8 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> int retries;
> int i, ret;
>
> + trace_vhost_user_blk_device_realize_in(vdev);
> +
> if (!s->chardev.chr) {
> error_setg(errp, "chardev is mandatory");
> return;
> @@ -514,6 +530,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL,
> vhost_user_blk_event, NULL, (void *)dev,
> NULL, true);
> +
> + trace_vhost_user_blk_device_realize_out(vdev);
> +
> return;
>
> virtio_err:
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH v2 23/23] vhost: add some useful trace-points
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (21 preceding siblings ...)
2025-10-11 23:24 ` [PATCH v2 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
@ 2025-10-11 23:24 ` Vladimir Sementsov-Ogievskiy
2025-10-13 21:04 ` Raphael Norwitz
2025-10-13 14:04 ` [PATCH v2 00/23] vhost refactoring and fixes Daniil Tatianin
23 siblings, 1 reply; 43+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-11 23:24 UTC (permalink / raw)
To: mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, vsementsov,
yc-core, d-tatianin
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
hw/virtio/trace-events | 12 ++++++++++--
hw/virtio/vhost.c | 20 ++++++++++++++++++--
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index aa1ffa5e94..c2529814f9 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -9,8 +9,16 @@ vhost_section(const char *name) "%s"
vhost_reject_section(const char *name, int d) "%s:%d"
vhost_iotlb_miss(void *dev, int step) "%p step %d"
vhost_dev_cleanup(void *dev) "%p"
-vhost_dev_start(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
-vhost_dev_stop(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
+vhost_dev_start_in(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
+vhost_dev_start_out(void *dev, const char *name) "%p:%s"
+vhost_dev_stop_in(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
+vhost_dev_stop_out(void *dev, const char *name) "%p:%s"
+vhost_virtque_start_in(void *dev, const char *name, int idx) "%p:%s %d"
+vhost_virtque_start_out(void *dev, const char *name, int idx) "%p:%s %d"
+vhost_virtque_stop_in(void *dev, const char *name, int idx) "%p:%s %d"
+vhost_virtque_stop_out(void *dev, const char *name, int idx) "%p:%s %d"
+vhost_dev_init_in(void *dev) "%p"
+vhost_dev_init_out(void *dev) "%p"
# vhost-user.c
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index d02d1d4c34..f15ef8cff6 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1333,6 +1333,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
};
struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
+ trace_vhost_virtque_start_in(dev, vdev->name, idx);
+
r = vhost_vrings_map(dev, vdev, vq, idx);
if (r <= 0) {
return r;
@@ -1394,6 +1396,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
}
}
+ trace_vhost_virtque_start_out(dev, vdev->name, idx);
+
return 0;
fail:
@@ -1412,6 +1416,8 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
};
int r = 0;
+ trace_vhost_virtque_stop_in(dev, vdev->name, idx);
+
if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
/* Don't stop the virtqueue which might have not been started */
return 0;
@@ -1445,6 +1451,8 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
}
vhost_vrings_unmap(dev, vq, true);
+
+ trace_vhost_virtque_stop_out(dev, vdev->name, idx);
return r;
}
@@ -1616,6 +1624,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
{
int i, r, n_initialized_vqs = 0;
+ trace_vhost_dev_init_in(hdev);
+
hdev->vdev = NULL;
hdev->migration_blocker = NULL;
@@ -1700,6 +1710,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
goto fail;
}
+ trace_vhost_dev_init_out(hdev);
+
return 0;
fail:
@@ -2048,7 +2060,7 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
/* should only be called after backend is connected */
assert(hdev->vhost_ops);
- trace_vhost_dev_start(hdev, vdev->name, vrings);
+ trace_vhost_dev_start_in(hdev, vdev->name, vrings);
vdev->vhost_started = true;
hdev->started = true;
@@ -2133,6 +2145,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
}
}
vhost_start_config_intr(hdev);
+
+ trace_vhost_dev_start_out(hdev, vdev->name);
return 0;
fail_iotlb:
if (vhost_dev_has_iommu(hdev) &&
@@ -2182,7 +2196,7 @@ static int do_vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev,
event_notifier_cleanup(
&hdev->vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier);
- trace_vhost_dev_stop(hdev, vdev->name, vrings);
+ trace_vhost_dev_stop_in(hdev, vdev->name, vrings);
if (hdev->vhost_ops->vhost_dev_start) {
hdev->vhost_ops->vhost_dev_start(hdev, false);
@@ -2212,6 +2226,8 @@ static int do_vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev,
hdev->started = false;
vdev->vhost_started = false;
hdev->vdev = NULL;
+
+ trace_vhost_dev_stop_out(hdev, vdev->name);
return rc;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 23/23] vhost: add some useful trace-points
2025-10-11 23:24 ` [PATCH v2 23/23] vhost: " Vladimir Sementsov-Ogievskiy
@ 2025-10-13 21:04 ` Raphael Norwitz
0 siblings, 0 replies; 43+ messages in thread
From: Raphael Norwitz @ 2025-10-13 21:04 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin
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:
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/trace-events | 12 ++++++++++--
> hw/virtio/vhost.c | 20 ++++++++++++++++++--
> 2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index aa1ffa5e94..c2529814f9 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -9,8 +9,16 @@ vhost_section(const char *name) "%s"
> vhost_reject_section(const char *name, int d) "%s:%d"
> vhost_iotlb_miss(void *dev, int step) "%p step %d"
> vhost_dev_cleanup(void *dev) "%p"
> -vhost_dev_start(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
> -vhost_dev_stop(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
> +vhost_dev_start_in(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
> +vhost_dev_start_out(void *dev, const char *name) "%p:%s"
> +vhost_dev_stop_in(void *dev, const char *name, bool vrings) "%p:%s vrings:%d"
> +vhost_dev_stop_out(void *dev, const char *name) "%p:%s"
> +vhost_virtque_start_in(void *dev, const char *name, int idx) "%p:%s %d"
> +vhost_virtque_start_out(void *dev, const char *name, int idx) "%p:%s %d"
> +vhost_virtque_stop_in(void *dev, const char *name, int idx) "%p:%s %d"
> +vhost_virtque_stop_out(void *dev, const char *name, int idx) "%p:%s %d"
> +vhost_dev_init_in(void *dev) "%p"
> +vhost_dev_init_out(void *dev) "%p"
>
>
> # vhost-user.c
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index d02d1d4c34..f15ef8cff6 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1333,6 +1333,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> };
> struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
>
> + trace_vhost_virtque_start_in(dev, vdev->name, idx);
> +
> r = vhost_vrings_map(dev, vdev, vq, idx);
> if (r <= 0) {
> return r;
> @@ -1394,6 +1396,8 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> }
> }
>
> + trace_vhost_virtque_start_out(dev, vdev->name, idx);
> +
> return 0;
>
> fail:
> @@ -1412,6 +1416,8 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
> };
> int r = 0;
>
> + trace_vhost_virtque_stop_in(dev, vdev->name, idx);
> +
> if (virtio_queue_get_desc_addr(vdev, idx) == 0) {
> /* Don't stop the virtqueue which might have not been started */
> return 0;
> @@ -1445,6 +1451,8 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
> }
>
> vhost_vrings_unmap(dev, vq, true);
> +
> + trace_vhost_virtque_stop_out(dev, vdev->name, idx);
> return r;
> }
>
> @@ -1616,6 +1624,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> {
> int i, r, n_initialized_vqs = 0;
>
> + trace_vhost_dev_init_in(hdev);
> +
> hdev->vdev = NULL;
> hdev->migration_blocker = NULL;
>
> @@ -1700,6 +1710,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> goto fail;
> }
>
> + trace_vhost_dev_init_out(hdev);
> +
> return 0;
>
> fail:
> @@ -2048,7 +2060,7 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
> /* should only be called after backend is connected */
> assert(hdev->vhost_ops);
>
> - trace_vhost_dev_start(hdev, vdev->name, vrings);
> + trace_vhost_dev_start_in(hdev, vdev->name, vrings);
>
> vdev->vhost_started = true;
> hdev->started = true;
> @@ -2133,6 +2145,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
> }
> }
> vhost_start_config_intr(hdev);
> +
> + trace_vhost_dev_start_out(hdev, vdev->name);
> return 0;
> fail_iotlb:
> if (vhost_dev_has_iommu(hdev) &&
> @@ -2182,7 +2196,7 @@ static int do_vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev,
> event_notifier_cleanup(
> &hdev->vqs[VHOST_QUEUE_NUM_CONFIG_INR].masked_config_notifier);
>
> - trace_vhost_dev_stop(hdev, vdev->name, vrings);
> + trace_vhost_dev_stop_in(hdev, vdev->name, vrings);
>
> if (hdev->vhost_ops->vhost_dev_start) {
> hdev->vhost_ops->vhost_dev_start(hdev, false);
> @@ -2212,6 +2226,8 @@ static int do_vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev,
> hdev->started = false;
> vdev->vhost_started = false;
> hdev->vdev = NULL;
> +
> + trace_vhost_dev_stop_out(hdev, vdev->name);
> return rc;
> }
>
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 00/23] vhost refactoring and fixes
2025-10-11 23:23 [PATCH v2 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
` (22 preceding siblings ...)
2025-10-11 23:24 ` [PATCH v2 23/23] vhost: " Vladimir Sementsov-Ogievskiy
@ 2025-10-13 14:04 ` Daniil Tatianin
23 siblings, 0 replies; 43+ messages in thread
From: Daniil Tatianin @ 2025-10-13 14:04 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, mst
Cc: sgarzare, raphael, qemu-devel, raphael.s.norwitz, yc-core
Hi!
For all patches:
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Thanks,
Daniil
On 10/12/25 2:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all. That's rebased and updated first part of
> [PATCH 00/33] vhost-user-blk: live-backend local migration
>
> v2:
> 01: rework
> 02: - rebaose on _ex features, more accurate
> - change in vhost_net_ack_features_ex()
> - add note to commit message
> 03: rename to vhost_user_has_protocol_feature,
> to follow existing vhost_user_set_protocol_features
> 04: rework to vhost-user only helper
> 05: add r-b by Philippe and Raphael
> 06: rebase on _ex features, make new helpers static inliners
> 07: add docstring
> 09: a-b by Raphael
> 10: rebase on address_space_map
> 11: fix double-free
> 13,14: r-b by Raphael
> 16: a-b by Raphael
> 17,18: r-b by Raphael
> 19: reworked
> 20: add r-b by Philippe and Raphael
> 21: - add r-b by Philippe and Raphael
> - fix indent
> 22: rename to _in/_out, add vdev ptr
> 23: rename to _in/_out, add dev ptr
>
> Vladimir Sementsov-Ogievskiy (23):
> vhost-user: rework enabling vrings
> vhost: drop backend_features field
> vhost-user: introduce vhost_user_has_protocol_feature() helper
> vhost: move protocol_features to vhost_user
> vhost-user-gpu: drop code duplication
> vhost: make vhost_dev.features private
> virtio: move common part of _set_guest_notifier to generic code
> virtio: drop *_set_guest_notifier_fd_handler() helpers
> vhost-user: keep QIOChannelSocket for backend channel
> vhost: vhost_virtqueue_start(): fix failure path
> vhost: make vhost_memory_unmap() null-safe
> vhost: simplify calls to vhost_memory_unmap()
> vhost: move vrings mapping to the top of vhost_virtqueue_start()
> vhost: vhost_virtqueue_start(): drop extra local variables
> vhost: final refactoring of vhost vrings map/unmap
> vhost: simplify vhost_dev_init() error-path
> vhost: move busyloop timeout initialization to vhost_virtqueue_init()
> vhost: introduce check_memslots() helper
> vhost: vhost_dev_init(): simplify features initialization
> hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier()
> vhost-user: make trace events more readable
> vhost-user-blk: add some useful trace-points
> vhost: add some useful trace-points
>
> backends/cryptodev-vhost.c | 9 +-
> hw/block/trace-events | 10 ++
> hw/block/vhost-user-blk.c | 20 ++-
> hw/display/vhost-user-gpu.c | 11 +-
> hw/net/vhost_net.c | 35 ++--
> hw/scsi/vhost-scsi.c | 1 -
> hw/scsi/vhost-user-scsi.c | 1 -
> hw/virtio/trace-events | 16 +-
> hw/virtio/vdpa-dev.c | 3 +-
> hw/virtio/vhost-user-base.c | 8 +-
> hw/virtio/vhost-user.c | 285 ++++++++++++++++++++++---------
> hw/virtio/vhost.c | 300 +++++++++++++++++----------------
> hw/virtio/virtio-bus.c | 18 +-
> hw/virtio/virtio-hmp-cmds.c | 2 -
> hw/virtio/virtio-mmio.c | 41 +----
> hw/virtio/virtio-pci.c | 34 +---
> hw/virtio/virtio-qmp.c | 12 +-
> hw/virtio/virtio.c | 48 +++---
> include/hw/virtio/vhost-user.h | 3 +
> include/hw/virtio/vhost.h | 51 ++++--
> include/hw/virtio/virtio-pci.h | 3 -
> include/hw/virtio/virtio.h | 16 +-
> net/vhost-vdpa.c | 7 +-
> qapi/virtio.json | 3 -
> 24 files changed, 525 insertions(+), 412 deletions(-)
>
^ permalink raw reply [flat|nested] 43+ messages in thread