On Wed, Aug 13, 2025 at 12:58 PM Vladimir Sementsov-Ogievskiy
<[email protected]> 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 <[email protected]>
> ---
> hw/display/vhost-user-gpu.c | 7 +++----
> hw/net/vhost_net.c | 17 +++++++++--------
> hw/virtio/vdpa-dev.c | 2 +-
> hw/virtio/vhost-user-base.c | 8 ++++++--
> hw/virtio/vhost-user.c | 7 +++----
> hw/virtio/vhost.c | 21 ++++++++++++++++++---
> hw/virtio/virtio-qmp.c | 2 +-
> include/hw/virtio/vhost.h | 21 +++++++++++++++++++--
> net/vhost-vdpa.c | 7 +++----
> 9 files changed, 63 insertions(+), 29 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 ce30b6e197..5269533864 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -55,7 +55,8 @@ void vhost_net_ack_features(struct vhost_net *net, uint64_t
> features)
> {
> net->dev.acked_features =
> (qemu_has_vnet_hdr(net->nc) ? 0 : (1ULL <<
> VHOST_NET_F_VIRTIO_NET_HDR))
> - | (net->dev.features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
> + | (vhost_dev_features(&net->dev) &
> + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES));
>
> vhost_ack_features(&net->dev, net->feature_bits, features);
> }
> @@ -277,23 +278,23 @@ 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));
> + !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. */
> if (options->get_acked_features) {
> + uint64_t backend_features = vhost_dev_features(&net->dev);
> features = options->get_acked_features(net->nc);
> - if (~net->dev.features & features) {
> + if (~backend_features & features) {
> fprintf(stderr, "vhost lacks feature mask 0x%" PRIx64
> - " for backend\n",
> - (uint64_t)(~net->dev.features & features));
> + " for backend\n", ~backend_features & features);
> goto fail;
> }
> }
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index 3c0eed3e8e..4dfb03aaa7 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 abdf47ee7b..46f09f5988 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1245,15 +1245,14 @@ static int vhost_user_set_vring_base(struct vhost_dev
> *dev,
>
> static bool vhost_user_set_vring_enable_supported(struct vhost_dev *dev)
> {
> - return virtio_has_feature(dev->features,
> - VHOST_USER_F_PROTOCOL_FEATURES);
> + return vhost_dev_has_feature(dev, VHOST_USER_F_PROTOCOL_FEATURES);
> }
>
> 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)) {
> return -EINVAL;
> }
>
> @@ -1465,7 +1464,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 c33dad4acd..2631bbabcf 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -50,6 +50,21 @@ static QLIST_HEAD(, vhost_dev)
> vhost_log_devs[VHOST_BACKEND_TYPE_MAX];
> static QLIST_HEAD(, vhost_dev) vhost_devices =
> QLIST_HEAD_INITIALIZER(vhost_devices);
>
> +bool vhost_dev_has_feature(struct vhost_dev *dev, uint64_t feature)
> +{
> + return virtio_has_feature(dev->_features, feature);
> +}
> +
> +uint64_t vhost_dev_features(struct vhost_dev *dev)
> +{
> + return dev->_features;
> +}
> +
> +void vhost_dev_clear_feature(struct vhost_dev *dev, uint64_t feature)
> +{
> + virtio_clear_feature(&dev->_features, feature);
> +}
> +
> unsigned int vhost_get_max_memslots(void)
> {
> unsigned int max = UINT_MAX;
> @@ -1571,7 +1586,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> }
> }
>
> - hdev->features = features;
> + hdev->_features = features;
>
> hdev->memory_listener = (MemoryListener) {
> .name = "vhost",
> @@ -1594,7 +1609,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> };
>
> if (hdev->migration_blocker == NULL) {
> - if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
> + if (!vhost_dev_has_feature(hdev, VHOST_F_LOG_ALL)) {
> error_setg(&hdev->migration_blocker,
> "Migration disabled: vhost lacks VHOST_F_LOG_ALL
> feature.");
> } else if (vhost_dev_log_is_shared(hdev) &&
> !qemu_memfd_alloc_check()) {
> @@ -1865,7 +1880,7 @@ uint64_t vhost_get_features(struct vhost_dev *hdev,
> const int *feature_bits,
> const int *bit = feature_bits;
> while (*bit != VHOST_INVALID_FEATURE_BIT) {
> uint64_t bit_mask = (1ULL << *bit);
> - if (!(hdev->features & bit_mask)) {
> + if (!vhost_dev_has_feature(hdev, *bit)) {
> features &= ~bit_mask;
> }
> bit++;
> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
> index d55b12f9f3..4bd23c015e 100644
> --- a/hw/virtio/virtio-qmp.c
> +++ b/hw/virtio/virtio-qmp.c
> @@ -785,7 +785,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);
> + qmp_decode_features(vdev->device_id, vhost_dev_features(hdev));
> status->vhost_dev->acked_features =
> qmp_decode_features(vdev->device_id, hdev->acked_features);
> status->vhost_dev->max_queues = hdev->max_queues;
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 15bc287a9d..8a4c8c3502 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -98,10 +98,11 @@ struct vhost_dev {
> * offered by a backend which may be a subset of the total
> * features eventually offered to the guest.
> *
> - * @features: available features provided by the backend
> + * @_features: available features provided by the backend, private,
> + * direct access only in vhost.c
> * @acked_features: final negotiated features with front-end driver
> */
> - uint64_t features;
> + uint64_t _features;
> uint64_t acked_features;
>
> uint64_t max_queues;
> @@ -352,6 +353,22 @@ 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);
>
> +/**
> + * vhost_dev_has_feature() - check if vhost device has a specific feature
> + * @dev: common vhost_dev structure
> + * @feature: feature bit to check
> + *
> + * Return: true if the feature is supported, false otherwise
> + */
> +bool vhost_dev_has_feature(struct vhost_dev *dev, uint64_t feature);
> +
> +/**
> + * vhost_dev_features() - simple getter for dev->features
> + */
> +uint64_t vhost_dev_features(struct vhost_dev *dev);
> +
> +void vhost_dev_clear_feature(struct vhost_dev *dev, uint64_t feature);
Why not define these as static inline helpers in the header file?
> +
> #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
>
>
