All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/23] vhost refactoring and fixes
@ 2025-10-15 14:57 Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
                   ` (23 more replies)
  0 siblings, 24 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst; +Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin

Hi all. That's rebased and updated first part of
  [PATCH 00/33] vhost-user-blk: live-backend local migration

v3:
01-05,07-23: add r-b by Daniil
01,03,04,11-12,15,19,22,23: add r-b by Raphael
02: - add a-b by Markus and Raphael
    - touch-up commit message
06: carefully follow existing pattern
    of split into features / features_ex

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      |  63 +++++--
 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, 537 insertions(+), 412 deletions(-)

-- 
2.48.1



^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v3 01/23] vhost-user: rework enabling vrings
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz, 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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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] 36+ messages in thread

* [PATCH v3 02/23] vhost: drop backend_features field
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper Vladimir Sementsov-Ogievskiy
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	devel, Markus Armbruster, Raphael Norwitz, Kevin Wolf,
	Hanna Reitz, Jason Wang, Paolo Bonzini, Fam Zheng, Eric Blake,
	open list:Block layer core

This field is mostly unused and sometimes confusing. We even have
a TODO-like comment to drop it, the comment is removed in this commit.

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: devel@lists.libvirt.org
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Acked-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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] 36+ messages in thread

* [PATCH v3 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 04/23] vhost: move protocol_features to vhost_user Vladimir Sementsov-Ogievskiy
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

Make all protocol feature checks in the same way.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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] 36+ messages in thread

* [PATCH v3 04/23] vhost: move protocol_features to vhost_user
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 05/23] vhost-user-gpu: drop code duplication Vladimir Sementsov-Ogievskiy
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz, 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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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] 36+ messages in thread

* [PATCH v3 05/23] vhost-user-gpu: drop code duplication
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 04/23] vhost: move protocol_features to vhost_user Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Philippe Mathieu-Daudé, Raphael Norwitz,
	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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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] 36+ messages in thread

* [PATCH v3 06/23] vhost: make vhost_dev.features private
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 05/23] vhost-user-gpu: drop code duplication Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-17 10:32   ` Daniil Tatianin
  2025-10-15 14:57 ` [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code Vladimir Sementsov-Ogievskiy
                   ` (17 subsequent siblings)
  23 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, 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   | 39 +++++++++++++++++++++++++++++++++++--
 net/vhost-vdpa.c            |  7 +++----
 9 files changed, 65 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..94efa409aa 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_ex(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_ex(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..1ba1af1d86 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.h/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,40 @@ 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(dev->_features, feature);
+}
+
+static inline bool vhost_dev_has_feature_ex(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(&dev->_features, feature);
+}
+
+static inline void vhost_dev_clear_feature_ex(struct vhost_dev *dev,
+                                              uint64_t feature)
+{
+    virtio_clear_feature_ex(dev->_features_ex, 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] 36+ messages in thread

* [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (5 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-11-07 13:01   ` Michael S. Tsirkin
  2025-10-15 14:57 ` [PATCH v3 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers Vladimir Sementsov-Ogievskiy
                   ` (16 subsequent siblings)
  23 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst; +Cc: sgarzare, raphael, qemu-devel, 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>
Reviewed-by: Daniil Tatianin <d-tatianin@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] 36+ messages in thread

* [PATCH v3 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (6 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 09/23] vhost-user: keep QIOChannelSocket for backend channel Vladimir Sementsov-Ogievskiy
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst; +Cc: sgarzare, raphael, qemu-devel, 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>
Reviewed-by: Daniil Tatianin <d-tatianin@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] 36+ messages in thread

* [PATCH v3 09/23] vhost-user: keep QIOChannelSocket for backend channel
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (7 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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] 36+ messages in thread

* [PATCH v3 10/23] vhost: vhost_virtqueue_start(): fix failure path
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (8 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 09/23] vhost-user: keep QIOChannelSocket for backend channel Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-20 18:24   ` Raphael Norwitz
  2025-10-15 14:57 ` [PATCH v3 11/23] vhost: make vhost_memory_unmap() null-safe Vladimir Sementsov-Ogievskiy
                   ` (13 subsequent siblings)
  23 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst; +Cc: sgarzare, raphael, qemu-devel, 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>
Reviewed-by: Daniil Tatianin <d-tatianin@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 94efa409aa..be2245cc7e 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] 36+ messages in thread

* [PATCH v3 11/23] vhost: make vhost_memory_unmap() null-safe
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (9 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 12/23] vhost: simplify calls to vhost_memory_unmap() Vladimir Sementsov-Ogievskiy
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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 be2245cc7e..db6ea42f9b 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] 36+ messages in thread

* [PATCH v3 12/23] vhost: simplify calls to vhost_memory_unmap()
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (10 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 11/23] vhost: make vhost_memory_unmap() null-safe Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start() Vladimir Sementsov-Ogievskiy
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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 db6ea42f9b..c8ec6b4911 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] 36+ messages in thread

* [PATCH v3 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start()
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (11 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 12/23] vhost: simplify calls to vhost_memory_unmap() Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 14/23] vhost: vhost_virtqueue_start(): drop extra local variables Vladimir Sementsov-Ogievskiy
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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 c8ec6b4911..db93b93fb1 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] 36+ messages in thread

* [PATCH v3 14/23] vhost: vhost_virtqueue_start(): drop extra local variables
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (12 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start() Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:57 ` [PATCH v3 15/23] vhost: final refactoring of vhost vrings map/unmap Vladimir Sementsov-Ogievskiy
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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 db93b93fb1..52a3236fff 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] 36+ messages in thread

* [PATCH v3 15/23] vhost: final refactoring of vhost vrings map/unmap
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (13 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 14/23] vhost: vhost_virtqueue_start(): drop extra local variables Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:57 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:58 ` [PATCH v3 16/23] vhost: simplify vhost_dev_init() error-path Vladimir Sementsov-Ogievskiy
                   ` (8 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:57 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

Introduce helper functions vhost_vrings_map() and
vhost_vrings_unmap() and use them.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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 52a3236fff..8c5e434b89 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] 36+ messages in thread

* [PATCH v3 16/23] vhost: simplify vhost_dev_init() error-path
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (14 preceding siblings ...)
  2025-10-15 14:57 ` [PATCH v3 15/23] vhost: final refactoring of vhost vrings map/unmap Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:58 ` [PATCH v3 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init() Vladimir Sementsov-Ogievskiy
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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 8c5e434b89..2dba7d53c9 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] 36+ messages in thread

* [PATCH v3 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init()
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (15 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 16/23] vhost: simplify vhost_dev_init() error-path Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:58 ` [PATCH v3 18/23] vhost: introduce check_memslots() helper Vladimir Sementsov-Ogievskiy
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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 2dba7d53c9..34846edf36 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] 36+ messages in thread

* [PATCH v3 18/23] vhost: introduce check_memslots() helper
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (16 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init() Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-11-03 22:19   ` Michael S. Tsirkin
  2025-10-15 14:58 ` [PATCH v3 19/23] vhost: vhost_dev_init(): simplify features initialization Vladimir Sementsov-Ogievskiy
                   ` (5 subsequent siblings)
  23 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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 34846edf36..56a812b06b 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] 36+ messages in thread

* [PATCH v3 19/23] vhost: vhost_dev_init(): simplify features initialization
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (17 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 18/23] vhost: introduce check_memslots() helper Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:58 ` [PATCH v3 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier() Vladimir Sementsov-Ogievskiy
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

Drop extra variable and extra function parameter passing, initialize
dev._features directly.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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 56a812b06b..fb5c4ba1ca 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] 36+ messages in thread

* [PATCH v3 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier()
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (18 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 19/23] vhost: vhost_dev_init(): simplify features initialization Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-10-15 14:58 ` [PATCH v3 21/23] vhost-user: make trace events more readable Vladimir Sementsov-Ogievskiy
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Philippe Mathieu-Daudé, Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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] 36+ messages in thread

* [PATCH v3 21/23] vhost-user: make trace events more readable
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (19 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier() Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-11-03 22:22   ` Michael S. Tsirkin
  2025-10-15 14:58 ` [PATCH v3 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
                   ` (2 subsequent siblings)
  23 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Philippe Mathieu-Daudé, Raphael Norwitz

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>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 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] 36+ messages in thread

* [PATCH v3 22/23] vhost-user-blk: add some useful trace-points
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (20 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 21/23] vhost-user: make trace events more readable Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-11-03 21:28   ` Michael S. Tsirkin
  2025-10-15 14:58 ` [PATCH v3 23/23] vhost: " Vladimir Sementsov-Ogievskiy
  2025-10-18 15:33 ` [PATCH v3 00/23] vhost refactoring and fixes Lei Yang
  23 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz, Kevin Wolf, Hanna Reitz,
	open list:Block layer core

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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] 36+ messages in thread

* [PATCH v3 23/23] vhost: add some useful trace-points
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (21 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
@ 2025-10-15 14:58 ` Vladimir Sementsov-Ogievskiy
  2025-11-03 21:26   ` Michael S. Tsirkin
  2025-10-18 15:33 ` [PATCH v3 00/23] vhost refactoring and fixes Lei Yang
  23 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-15 14:58 UTC (permalink / raw)
  To: mst
  Cc: sgarzare, raphael, qemu-devel, vsementsov, yc-core, d-tatianin,
	Raphael Norwitz

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
---
 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 fb5c4ba1ca..7ba90c24db 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] 36+ messages in thread

* Re: [PATCH v3 06/23] vhost: make vhost_dev.features private
  2025-10-15 14:57 ` [PATCH v3 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
@ 2025-10-17 10:32   ` Daniil Tatianin
  2025-10-17 17:02     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 36+ messages in thread
From: Daniil Tatianin @ 2025-10-17 10:32 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, mst
  Cc: sgarzare, raphael, qemu-devel, yc-core, Marc-André Lureau,
	Jason Wang, Alex Bennée

On 10/15/25 5:57 PM, Vladimir Sementsov-Ogievskiy 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   | 39 +++++++++++++++++++++++++++++++++++--
>   net/vhost-vdpa.c            |  7 +++----
>   9 files changed, 65 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..94efa409aa 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);


This should probably use the vhost_dev_features_ex getter

Otherwise, LGTM:
Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>

>   
>       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_ex(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_ex(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..1ba1af1d86 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.h/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,40 @@ 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(dev->_features, feature);
> +}
> +
> +static inline bool vhost_dev_has_feature_ex(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(&dev->_features, feature);
> +}
> +
> +static inline void vhost_dev_clear_feature_ex(struct vhost_dev *dev,
> +                                              uint64_t feature)
> +{
> +    virtio_clear_feature_ex(dev->_features_ex, 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;
>       }
>   


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 06/23] vhost: make vhost_dev.features private
  2025-10-17 10:32   ` Daniil Tatianin
@ 2025-10-17 17:02     ` Vladimir Sementsov-Ogievskiy
  2025-10-31 17:31       ` Daniil Tatianin
  0 siblings, 1 reply; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-10-17 17:02 UTC (permalink / raw)
  To: Daniil Tatianin, mst
  Cc: sgarzare, raphael, qemu-devel, yc-core, Marc-André Lureau,
	Jason Wang, Alex Bennée

On 17.10.25 13:32, Daniil Tatianin wrote:
> On 10/15/25 5:57 PM, Vladimir Sementsov-Ogievskiy 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   | 39 +++++++++++++++++++++++++++++++++++--
>>   net/vhost-vdpa.c            |  7 +++----
>>   9 files changed, 65 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..94efa409aa 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);
> 
> 
> This should probably use the vhost_dev_features_ex getter

Hmm, here we write into hdev->_features_ex.

> 
> Otherwise, LGTM:
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> 
>>       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_ex(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_ex(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..1ba1af1d86 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.h/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,40 @@ 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(dev->_features, feature);
>> +}
>> +
>> +static inline bool vhost_dev_has_feature_ex(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(&dev->_features, feature);
>> +}
>> +
>> +static inline void vhost_dev_clear_feature_ex(struct vhost_dev *dev,
>> +                                              uint64_t feature)
>> +{
>> +    virtio_clear_feature_ex(dev->_features_ex, 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;
>>       }


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 00/23] vhost refactoring and fixes
  2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
                   ` (22 preceding siblings ...)
  2025-10-15 14:58 ` [PATCH v3 23/23] vhost: " Vladimir Sementsov-Ogievskiy
@ 2025-10-18 15:33 ` Lei Yang
  23 siblings, 0 replies; 36+ messages in thread
From: Lei Yang @ 2025-10-18 15:33 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: mst, sgarzare, raphael, qemu-devel, yc-core, d-tatianin

Tested this series of patches with virtio-net regression tests,
everything works fine.

Tested-by: Lei Yang <leiyang@redhat.com>


On Wed, Oct 15, 2025 at 11:03 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Hi all. That's rebased and updated first part of
>   [PATCH 00/33] vhost-user-blk: live-backend local migration
>
> v3:
> 01-05,07-23: add r-b by Daniil
> 01,03,04,11-12,15,19,22,23: add r-b by Raphael
> 02: - add a-b by Markus and Raphael
>     - touch-up commit message
> 06: carefully follow existing pattern
>     of split into features / features_ex
>
> 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      |  63 +++++--
>  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, 537 insertions(+), 412 deletions(-)
>
> --
> 2.48.1
>
>



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 10/23] vhost: vhost_virtqueue_start(): fix failure path
  2025-10-15 14:57 ` [PATCH v3 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
@ 2025-10-20 18:24   ` Raphael Norwitz
  0 siblings, 0 replies; 36+ messages in thread
From: Raphael Norwitz @ 2025-10-20 18:24 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 Wed, Oct 15, 2025 at 11:07 AM 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>
> Reviewed-by: Daniil Tatianin <d-tatianin@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 94efa409aa..be2245cc7e 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	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 06/23] vhost: make vhost_dev.features private
  2025-10-17 17:02     ` Vladimir Sementsov-Ogievskiy
@ 2025-10-31 17:31       ` Daniil Tatianin
  0 siblings, 0 replies; 36+ messages in thread
From: Daniil Tatianin @ 2025-10-31 17:31 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, mst
  Cc: sgarzare, raphael, qemu-devel, yc-core, Marc-André Lureau,
	Jason Wang, Alex Bennée


On 10/17/25 8:02 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 17.10.25 13:32, Daniil Tatianin wrote:
>> On 10/15/25 5:57 PM, Vladimir Sementsov-Ogievskiy 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   | 39 
>>> +++++++++++++++++++++++++++++++++++--
>>>   net/vhost-vdpa.c            |  7 +++----
>>>   9 files changed, 65 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..94efa409aa 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);
>>
>>
>> This should probably use the vhost_dev_features_ex getter
>
> Hmm, here we write into hdev->_features_ex.

Right, I missed that it returns a const pointer.

Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>

>
>>
>> Otherwise, LGTM:
>> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
>>
>>>       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_ex(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_ex(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..1ba1af1d86 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.h/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,40 @@ 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(dev->_features, feature);
>>> +}
>>> +
>>> +static inline bool vhost_dev_has_feature_ex(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(&dev->_features, feature);
>>> +}
>>> +
>>> +static inline void vhost_dev_clear_feature_ex(struct vhost_dev *dev,
>>> +                                              uint64_t feature)
>>> +{
>>> +    virtio_clear_feature_ex(dev->_features_ex, 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;
>>>       }
>
>


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 23/23] vhost: add some useful trace-points
  2025-10-15 14:58 ` [PATCH v3 23/23] vhost: " Vladimir Sementsov-Ogievskiy
@ 2025-11-03 21:26   ` Michael S. Tsirkin
  0 siblings, 0 replies; 36+ messages in thread
From: Michael S. Tsirkin @ 2025-11-03 21:26 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
	Raphael Norwitz

On Wed, Oct 15, 2025 at 05:58:07PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
> ---
>  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"

virtqueue_ not virtque_

> +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 fb5c4ba1ca..7ba90c24db 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);
> +

should be "virtqueue"

>      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);
> +


same


>      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);
> +

same

>      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);

same

>      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] 36+ messages in thread

* Re: [PATCH v3 22/23] vhost-user-blk: add some useful trace-points
  2025-10-15 14:58 ` [PATCH v3 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
@ 2025-11-03 21:28   ` Michael S. Tsirkin
  2025-11-04 11:08     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2025-11-03 21:28 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
	Raphael Norwitz, Kevin Wolf, Hanna Reitz,
	open list:Block layer core

On Wed, Oct 15, 2025 at 05:58:06PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
> ---
>  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:


Is it ok that on error there's an in trace but not an out trace?

> -- 
> 2.48.1



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 18/23] vhost: introduce check_memslots() helper
  2025-10-15 14:58 ` [PATCH v3 18/23] vhost: introduce check_memslots() helper Vladimir Sementsov-Ogievskiy
@ 2025-11-03 22:19   ` Michael S. Tsirkin
  2025-11-04 10:59     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 36+ messages in thread
From: Michael S. Tsirkin @ 2025-11-03 22:19 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
	Raphael Norwitz

On Wed, Oct 15, 2025 at 05:58:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>


This needs a better commit log.
the subject makes it look like it's merely adding a helper
but it is not the case: e.g. errors are now handled
somewhat differently.

Pls document the thinking process explaining why it does not
matter. CB.


> ---
>  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 34846edf36..56a812b06b 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

This comment worked in the original code but not now.

The listener here is memory_listener_register which yes happens to
be called before check_memslots but it is far from obvious.



> +     * 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);


So it looks like with your change
this will temporarily register the listener
restricting the number of slots, then check and unregister.

Is this ever a problem?

commit log needs better documentation why.



>      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	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 21/23] vhost-user: make trace events more readable
  2025-10-15 14:58 ` [PATCH v3 21/23] vhost-user: make trace events more readable Vladimir Sementsov-Ogievskiy
@ 2025-11-03 22:22   ` Michael S. Tsirkin
  0 siblings, 0 replies; 36+ messages in thread
From: Michael S. Tsirkin @ 2025-11-03 22:22 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
	Philippe Mathieu-Daudé, Raphael Norwitz

On Wed, Oct 15, 2025 at 05:58:05PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 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>
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> ---
>  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>";
> +    }
> +}
> +

Please use a macro so we don't have this duplication.


E.g.

  #define VHOST_USER_CASE(name) \
      case VHOST_USER_##name: \
          return #name;



>  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	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 18/23] vhost: introduce check_memslots() helper
  2025-11-03 22:19   ` Michael S. Tsirkin
@ 2025-11-04 10:59     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-11-04 10:59 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
	Raphael Norwitz

On 04.11.25 01:19, Michael S. Tsirkin wrote:
> On Wed, Oct 15, 2025 at 05:58:02PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
>> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
> 
> 
> This needs a better commit log.
> the subject makes it look like it's merely adding a helper
> but it is not the case: e.g. errors are now handled
> somewhat differently.
> 
> Pls document the thinking process explaining why it does not
> matter. CB.
> 

Will do. Or, split into separate commits for helper and for logic change.

> 
>> ---
>>   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 34846edf36..56a812b06b 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
> 
> This comment worked in the original code but not now.
> 
> The listener here is memory_listener_register which yes happens to
> be called before check_memslots but it is far from obvious.
> 
> 
> 
>> +     * 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);
> 
> 
> So it looks like with your change
> this will temporarily register the listener
> restricting the number of slots, then check and unregister.
> 
> Is this ever a problem?
> 
> commit log needs better documentation why.
> 
> 
> 
>>       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
> 


-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 22/23] vhost-user-blk: add some useful trace-points
  2025-11-03 21:28   ` Michael S. Tsirkin
@ 2025-11-04 11:08     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 36+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2025-11-04 11:08 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: sgarzare, raphael, qemu-devel, yc-core, d-tatianin,
	Raphael Norwitz, Kevin Wolf, Hanna Reitz,
	open list:Block layer core

On 04.11.25 00:28, Michael S. Tsirkin wrote:
> On Wed, Oct 15, 2025 at 05:58:06PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
>> Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
>> ---
>>   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:
> 
> 
> Is it ok that on error there's an in trace but not an out trace?
> 

I thought about it.

In my opinion, switching to use common "out: " and gotos instead of simple returns,
is too much pay for proper out trace-points.

So, it's a compromise: we have at least trace-point on success-out, and don't make
the code more complicated just for good trace points.

Probably, we can invent some smart macro, which will inject out tracepoint using
g_auto...

-- 
Best regards,
Vladimir


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code
  2025-10-15 14:57 ` [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code Vladimir Sementsov-Ogievskiy
@ 2025-11-07 13:01   ` Michael S. Tsirkin
  0 siblings, 0 replies; 36+ messages in thread
From: Michael S. Tsirkin @ 2025-11-07 13:01 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: sgarzare, raphael, qemu-devel, yc-core, d-tatianin

On Wed, Oct 15, 2025 at 05:57:51PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 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>
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>


Breaks virtio-ccw which also uses this.

> ---
>  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	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2025-11-07 13:02 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 04/23] vhost: move protocol_features to vhost_user Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 05/23] vhost-user-gpu: drop code duplication Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
2025-10-17 10:32   ` Daniil Tatianin
2025-10-17 17:02     ` Vladimir Sementsov-Ogievskiy
2025-10-31 17:31       ` Daniil Tatianin
2025-10-15 14:57 ` [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code Vladimir Sementsov-Ogievskiy
2025-11-07 13:01   ` Michael S. Tsirkin
2025-10-15 14:57 ` [PATCH v3 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 09/23] vhost-user: keep QIOChannelSocket for backend channel Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
2025-10-20 18:24   ` Raphael Norwitz
2025-10-15 14:57 ` [PATCH v3 11/23] vhost: make vhost_memory_unmap() null-safe Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 12/23] vhost: simplify calls to vhost_memory_unmap() Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start() Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 14/23] vhost: vhost_virtqueue_start(): drop extra local variables Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 15/23] vhost: final refactoring of vhost vrings map/unmap Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 16/23] vhost: simplify vhost_dev_init() error-path Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init() Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 18/23] vhost: introduce check_memslots() helper Vladimir Sementsov-Ogievskiy
2025-11-03 22:19   ` Michael S. Tsirkin
2025-11-04 10:59     ` Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 19/23] vhost: vhost_dev_init(): simplify features initialization Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier() Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 21/23] vhost-user: make trace events more readable Vladimir Sementsov-Ogievskiy
2025-11-03 22:22   ` Michael S. Tsirkin
2025-10-15 14:58 ` [PATCH v3 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
2025-11-03 21:28   ` Michael S. Tsirkin
2025-11-04 11:08     ` Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 23/23] vhost: " Vladimir Sementsov-Ogievskiy
2025-11-03 21:26   ` Michael S. Tsirkin
2025-10-18 15:33 ` [PATCH v3 00/23] vhost refactoring and fixes Lei Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.