Similar to virtio infra, vhost core maintains the features status
in the full extended format and allows the devices to implement
extended version of the getter/setter.
Note that 'protocol_features' are not extended: they are only
used by vhost-user, and the latter device is not going to implement
extended features soon.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v2 -> v3:
- fix compile warning
- _array -> _ex
v1 -> v2:
- uint128_t -> uint64_t[]
- add _ex() variant of features manipulation helpers
---
hw/virtio/vhost.c | 73 +++++++++++++++++++++++++++----
include/hw/virtio/vhost-backend.h | 6 +++
include/hw/virtio/vhost.h | 33 ++++++++++++--
3 files changed, 100 insertions(+), 12 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index c30ea1156e..85ae1e4d4c 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
static int vhost_dev_set_features(struct vhost_dev *dev,
bool enable_log)
{
- uint64_t features = dev->acked_features;
+ uint64_t features[VIRTIO_FEATURES_DWORDS];
int r;
+
+ virtio_features_copy(features, dev->acked_features_ex);
if (enable_log) {
- features |= 0x1ULL << VHOST_F_LOG_ALL;
+ virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
}
if (!vhost_dev_has_iommu(dev)) {
- features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
+ virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
}
if (dev->vhost_ops->vhost_force_iommu) {
if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
- features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
+ virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
}
}
- r = dev->vhost_ops->vhost_set_features(dev, features);
+
+ if (virtio_features_use_extended(features) &&
+ !dev->vhost_ops->vhost_set_features_ex) {
+ VHOST_OPS_DEBUG(r, "extended features without device support");
+ r = -EINVAL;
+ goto out;
+ }
+
+ if (dev->vhost_ops->vhost_set_features_ex) {
+ r = dev->vhost_ops->vhost_set_features_ex(dev, features);
+ } else {
+ r = dev->vhost_ops->vhost_set_features(dev, features[0]);
+ }
if (r < 0) {
VHOST_OPS_DEBUG(r, "vhost_set_features failed");
goto out;
@@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
}
}
+static int vhost_dev_get_features(struct vhost_dev *hdev,
+ uint64_t *features)
+{
+ uint64_t features64;
+ int r;
+
+ if (hdev->vhost_ops->vhost_get_features_ex) {
+ return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
+ }
+
+ r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
+ virtio_features_from_u64(features, features64);
+ return r;
+}
+
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type, uint32_t busyloop_timeout,
Error **errp)
{
+ uint64_t features[VIRTIO_FEATURES_DWORDS];
unsigned int used, reserved, limit;
- uint64_t features;
int i, r, n_initialized_vqs = 0;
hdev->vdev = NULL;
@@ -1531,7 +1560,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
goto fail;
}
- r = hdev->vhost_ops->vhost_get_features(hdev, &features);
+ r = vhost_dev_get_features(hdev, features);
if (r < 0) {
error_setg_errno(errp, -r, "vhost_get_features failed");
goto fail;
@@ -1569,7 +1598,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
}
}
- hdev->features = features;
+ virtio_features_copy(hdev->features_ex, features);
hdev->memory_listener = (MemoryListener) {
.name = "vhost",
@@ -1592,7 +1621,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 (!virtio_has_feature_ex(hdev->features_ex, 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()) {
@@ -1871,6 +1900,20 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
return features;
}
+void vhost_get_features_ex(struct vhost_dev *hdev,
+ const int *feature_bits,
+ uint64_t *features)
+{
+ const int *bit = feature_bits;
+
+ while (*bit != VHOST_INVALID_FEATURE_BIT) {
+ if (!virtio_has_feature_ex(hdev->features_ex, *bit)) {
+ virtio_clear_feature_ex(features, *bit);
+ }
+ bit++;
+ }
+}
+
void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features)
{
@@ -1884,6 +1927,18 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
}
}
+void vhost_ack_features_ex(struct vhost_dev *hdev, const int *feature_bits,
+ const uint64_t *features)
+{
+ const int *bit = feature_bits;
+ while (*bit != VHOST_INVALID_FEATURE_BIT) {
+ if (virtio_has_feature_ex(features, *bit)) {
+ virtio_add_feature_ex(hdev->acked_features_ex, *bit);
+ }
+ bit++;
+ }
+}
+
int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
uint32_t config_len, Error **errp)
{
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index d6df209a2f..ff94fa1734 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -95,6 +95,10 @@ typedef int (*vhost_new_worker_op)(struct vhost_dev *dev,
struct vhost_worker_state *worker);
typedef int (*vhost_free_worker_op)(struct vhost_dev *dev,
struct vhost_worker_state *worker);
+typedef int (*vhost_set_features_ex_op)(struct vhost_dev *dev,
+ const uint64_t *features);
+typedef int (*vhost_get_features_ex_op)(struct vhost_dev *dev,
+ uint64_t *features);
typedef int (*vhost_set_features_op)(struct vhost_dev *dev,
uint64_t features);
typedef int (*vhost_get_features_op)(struct vhost_dev *dev,
@@ -186,6 +190,8 @@ typedef struct VhostOps {
vhost_free_worker_op vhost_free_worker;
vhost_get_vring_worker_op vhost_get_vring_worker;
vhost_attach_vring_worker_op vhost_attach_vring_worker;
+ vhost_set_features_ex_op vhost_set_features_ex;
+ vhost_get_features_ex_op vhost_get_features_ex;
vhost_set_features_op vhost_set_features;
vhost_get_features_op vhost_get_features;
vhost_set_backend_cap_op vhost_set_backend_cap;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 66be6afc88..39fbffc6bc 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -107,9 +107,9 @@ struct vhost_dev {
* future use should be discouraged and the variable retired as
* its easy to confuse with the VirtIO backend_features.
*/
- uint64_t features;
- uint64_t acked_features;
- uint64_t 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
@@ -333,6 +333,21 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features);
+/**
+ * vhost_get_features_ex() - sanitize the extended features set
+ * @hdev: common vhost_dev structure
+ * @feature_bits: pointer to terminated table of feature bits
+ * @features: original features set, filtered out on return
+ *
+ * This is the extended variant of vhost_get_features(), supporting the
+ * the extended features set. Filter it with the intersection of what is
+ * supported by the vhost backend (hdev->features) and the supported
+ * feature_bits.
+ */
+void vhost_get_features_ex(struct vhost_dev *hdev,
+ const int *feature_bits,
+ uint64_t *features);
+
/**
* vhost_ack_features() - set vhost acked_features
* @hdev: common vhost_dev structure
@@ -344,6 +359,18 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
*/
void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features);
+
+/**
+ * vhost_ack_features_ex() - set vhost full set of acked_features
+ * @hdev: common vhost_dev structure
+ * @feature_bits: pointer to terminated table of feature bits
+ * @features: requested feature set
+ *
+ * This sets the internal hdev->acked_features to the intersection of
+ * the backends advertised features and the supported feature_bits.
+ */
+void vhost_ack_features_ex(struct vhost_dev *hdev, const int *feature_bits,
+ const uint64_t *features);
unsigned int vhost_get_max_memslots(void);
unsigned int vhost_get_free_memslots(void);
--
2.50.0On Fri, Jul 18, 2025 at 4:53 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Similar to virtio infra, vhost core maintains the features status
> in the full extended format and allows the devices to implement
> extended version of the getter/setter.
>
> Note that 'protocol_features' are not extended: they are only
> used by vhost-user, and the latter device is not going to implement
> extended features soon.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v2 -> v3:
> - fix compile warning
> - _array -> _ex
>
> v1 -> v2:
> - uint128_t -> uint64_t[]
> - add _ex() variant of features manipulation helpers
> ---
> hw/virtio/vhost.c | 73 +++++++++++++++++++++++++++----
> include/hw/virtio/vhost-backend.h | 6 +++
> include/hw/virtio/vhost.h | 33 ++++++++++++--
> 3 files changed, 100 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index c30ea1156e..85ae1e4d4c 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
> static int vhost_dev_set_features(struct vhost_dev *dev,
> bool enable_log)
> {
> - uint64_t features = dev->acked_features;
> + uint64_t features[VIRTIO_FEATURES_DWORDS];
> int r;
> +
> + virtio_features_copy(features, dev->acked_features_ex);
> if (enable_log) {
> - features |= 0x1ULL << VHOST_F_LOG_ALL;
> + virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
> }
> if (!vhost_dev_has_iommu(dev)) {
> - features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
> + virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> }
> if (dev->vhost_ops->vhost_force_iommu) {
> if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
> - features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
> + virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> }
> }
> - r = dev->vhost_ops->vhost_set_features(dev, features);
> +
> + if (virtio_features_use_extended(features) &&
> + !dev->vhost_ops->vhost_set_features_ex) {
> + VHOST_OPS_DEBUG(r, "extended features without device support");
> + r = -EINVAL;
> + goto out;
> + }
> +
> + if (dev->vhost_ops->vhost_set_features_ex) {
> + r = dev->vhost_ops->vhost_set_features_ex(dev, features);
> + } else {
> + r = dev->vhost_ops->vhost_set_features(dev, features[0]);
> + }
> if (r < 0) {
> VHOST_OPS_DEBUG(r, "vhost_set_features failed");
> goto out;
> @@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> }
> }
>
> +static int vhost_dev_get_features(struct vhost_dev *hdev,
> + uint64_t *features)
> +{
> + uint64_t features64;
> + int r;
> +
> + if (hdev->vhost_ops->vhost_get_features_ex) {
> + return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
> + }
> +
> + r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
> + virtio_features_from_u64(features, features64);
> + return r;
> +}
Nit: let's have a vhost_dev_set_features() as well?
Thanks
On 7/22/25 5:32 AM, Jason Wang wrote:
> On Fri, Jul 18, 2025 at 4:53 PM Paolo Abeni <pabeni@redhat.com> wrote:
>>
>> Similar to virtio infra, vhost core maintains the features status
>> in the full extended format and allows the devices to implement
>> extended version of the getter/setter.
>>
>> Note that 'protocol_features' are not extended: they are only
>> used by vhost-user, and the latter device is not going to implement
>> extended features soon.
>>
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> ---
>> v2 -> v3:
>> - fix compile warning
>> - _array -> _ex
>>
>> v1 -> v2:
>> - uint128_t -> uint64_t[]
>> - add _ex() variant of features manipulation helpers
>> ---
>> hw/virtio/vhost.c | 73 +++++++++++++++++++++++++++----
>> include/hw/virtio/vhost-backend.h | 6 +++
>> include/hw/virtio/vhost.h | 33 ++++++++++++--
>> 3 files changed, 100 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index c30ea1156e..85ae1e4d4c 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
>> static int vhost_dev_set_features(struct vhost_dev *dev,
>> bool enable_log)
>> {
>> - uint64_t features = dev->acked_features;
>> + uint64_t features[VIRTIO_FEATURES_DWORDS];
>> int r;
>> +
>> + virtio_features_copy(features, dev->acked_features_ex);
>> if (enable_log) {
>> - features |= 0x1ULL << VHOST_F_LOG_ALL;
>> + virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
>> }
>> if (!vhost_dev_has_iommu(dev)) {
>> - features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
>> + virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
>> }
>> if (dev->vhost_ops->vhost_force_iommu) {
>> if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
>> - features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
>> + virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
>> }
>> }
>> - r = dev->vhost_ops->vhost_set_features(dev, features);
>> +
>> + if (virtio_features_use_extended(features) &&
>> + !dev->vhost_ops->vhost_set_features_ex) {
>> + VHOST_OPS_DEBUG(r, "extended features without device support");
>> + r = -EINVAL;
>> + goto out;
>> + }
>> +
>> + if (dev->vhost_ops->vhost_set_features_ex) {
>> + r = dev->vhost_ops->vhost_set_features_ex(dev, features);
>> + } else {
>> + r = dev->vhost_ops->vhost_set_features(dev, features[0]);
>> + }
>> if (r < 0) {
>> VHOST_OPS_DEBUG(r, "vhost_set_features failed");
>> goto out;
>> @@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
>> }
>> }
>>
>> +static int vhost_dev_get_features(struct vhost_dev *hdev,
>> + uint64_t *features)
>> +{
>> + uint64_t features64;
>> + int r;
>> +
>> + if (hdev->vhost_ops->vhost_get_features_ex) {
>> + return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
>> + }
>> +
>> + r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
>> + virtio_features_from_u64(features, features64);
>> + return r;
>> +}
>
> Nit: let's have a vhost_dev_set_features() as well?
I guess you mean to factor out the
vhost_set_features_ex()/vhost_set_features() in a specific helper am I
correct?
Note that there is already a vhost_dev_set_features() function. It's
touched by the previous chunk. I opted for not creating the mentioned
helper to avoid some weird naming issues, as such helper would not lead
to any code deduplication.
Please LMK if you have strong opinion for a different choice.
Thanks,
Paolo
On Wed, Jul 23, 2025 at 12:55 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 7/22/25 5:32 AM, Jason Wang wrote:
> > On Fri, Jul 18, 2025 at 4:53 PM Paolo Abeni <pabeni@redhat.com> wrote:
> >>
> >> Similar to virtio infra, vhost core maintains the features status
> >> in the full extended format and allows the devices to implement
> >> extended version of the getter/setter.
> >>
> >> Note that 'protocol_features' are not extended: they are only
> >> used by vhost-user, and the latter device is not going to implement
> >> extended features soon.
> >>
> >> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> >> ---
> >> v2 -> v3:
> >> - fix compile warning
> >> - _array -> _ex
> >>
> >> v1 -> v2:
> >> - uint128_t -> uint64_t[]
> >> - add _ex() variant of features manipulation helpers
> >> ---
> >> hw/virtio/vhost.c | 73 +++++++++++++++++++++++++++----
> >> include/hw/virtio/vhost-backend.h | 6 +++
> >> include/hw/virtio/vhost.h | 33 ++++++++++++--
> >> 3 files changed, 100 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >> index c30ea1156e..85ae1e4d4c 100644
> >> --- a/hw/virtio/vhost.c
> >> +++ b/hw/virtio/vhost.c
> >> @@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
> >> static int vhost_dev_set_features(struct vhost_dev *dev,
> >> bool enable_log)
> >> {
> >> - uint64_t features = dev->acked_features;
> >> + uint64_t features[VIRTIO_FEATURES_DWORDS];
> >> int r;
> >> +
> >> + virtio_features_copy(features, dev->acked_features_ex);
> >> if (enable_log) {
> >> - features |= 0x1ULL << VHOST_F_LOG_ALL;
> >> + virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
> >> }
> >> if (!vhost_dev_has_iommu(dev)) {
> >> - features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
> >> + virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> >> }
> >> if (dev->vhost_ops->vhost_force_iommu) {
> >> if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
> >> - features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
> >> + virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> >> }
> >> }
> >> - r = dev->vhost_ops->vhost_set_features(dev, features);
> >> +
> >> + if (virtio_features_use_extended(features) &&
> >> + !dev->vhost_ops->vhost_set_features_ex) {
> >> + VHOST_OPS_DEBUG(r, "extended features without device support");
> >> + r = -EINVAL;
> >> + goto out;
> >> + }
> >> +
> >> + if (dev->vhost_ops->vhost_set_features_ex) {
> >> + r = dev->vhost_ops->vhost_set_features_ex(dev, features);
> >> + } else {
> >> + r = dev->vhost_ops->vhost_set_features(dev, features[0]);
> >> + }
> >> if (r < 0) {
> >> VHOST_OPS_DEBUG(r, "vhost_set_features failed");
> >> goto out;
> >> @@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> >> }
> >> }
> >>
> >> +static int vhost_dev_get_features(struct vhost_dev *hdev,
> >> + uint64_t *features)
> >> +{
> >> + uint64_t features64;
> >> + int r;
> >> +
> >> + if (hdev->vhost_ops->vhost_get_features_ex) {
> >> + return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
> >> + }
> >> +
> >> + r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
> >> + virtio_features_from_u64(features, features64);
> >> + return r;
> >> +}
> >
> > Nit: let's have a vhost_dev_set_features() as well?
>
> I guess you mean to factor out the
> vhost_set_features_ex()/vhost_set_features() in a specific helper am I
> correct?
Yes.
>
> Note that there is already a vhost_dev_set_features() function. It's
> touched by the previous chunk. I opted for not creating the mentioned
> helper to avoid some weird naming issues, as such helper would not lead
> to any code deduplication.
I'm fine not having that then.
>
> Please LMK if you have strong opinion for a different choice.
>
> Thanks,
>
> Paolo
Thanks
>
On Fri, Jul 18, 2025 at 10:52:33AM +0200, Paolo Abeni wrote:
>Similar to virtio infra, vhost core maintains the features status
>in the full extended format and allows the devices to implement
>extended version of the getter/setter.
>
>Note that 'protocol_features' are not extended: they are only
>used by vhost-user, and the latter device is not going to implement
>extended features soon.
>
>Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>---
>v2 -> v3:
> - fix compile warning
> - _array -> _ex
>
>v1 -> v2:
> - uint128_t -> uint64_t[]
> - add _ex() variant of features manipulation helpers
>---
> hw/virtio/vhost.c | 73 +++++++++++++++++++++++++++----
> include/hw/virtio/vhost-backend.h | 6 +++
> include/hw/virtio/vhost.h | 33 ++++++++++++--
> 3 files changed, 100 insertions(+), 12 deletions(-)
>
>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>index c30ea1156e..85ae1e4d4c 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
> static int vhost_dev_set_features(struct vhost_dev *dev,
> bool enable_log)
> {
>- uint64_t features = dev->acked_features;
>+ uint64_t features[VIRTIO_FEATURES_DWORDS];
> int r;
>+
>+ virtio_features_copy(features, dev->acked_features_ex);
> if (enable_log) {
>- features |= 0x1ULL << VHOST_F_LOG_ALL;
>+ virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
> }
> if (!vhost_dev_has_iommu(dev)) {
>- features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
>+ virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> }
> if (dev->vhost_ops->vhost_force_iommu) {
> if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
>- features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
>+ virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> }
> }
>- r = dev->vhost_ops->vhost_set_features(dev, features);
>+
>+ if (virtio_features_use_extended(features) &&
>+ !dev->vhost_ops->vhost_set_features_ex) {
>+ VHOST_OPS_DEBUG(r, "extended features without device support");
>+ r = -EINVAL;
>+ goto out;
>+ }
>+
>+ if (dev->vhost_ops->vhost_set_features_ex) {
>+ r = dev->vhost_ops->vhost_set_features_ex(dev, features);
>+ } else {
>+ r = dev->vhost_ops->vhost_set_features(dev, features[0]);
>+ }
> if (r < 0) {
> VHOST_OPS_DEBUG(r, "vhost_set_features failed");
> goto out;
>@@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> }
> }
>
>+static int vhost_dev_get_features(struct vhost_dev *hdev,
>+ uint64_t *features)
>+{
>+ uint64_t features64;
>+ int r;
>+
>+ if (hdev->vhost_ops->vhost_get_features_ex) {
>+ return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
>+ }
>+
>+ r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
>+ virtio_features_from_u64(features, features64);
>+ return r;
>+}
>+
> int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> VhostBackendType backend_type, uint32_t busyloop_timeout,
> Error **errp)
> {
>+ uint64_t features[VIRTIO_FEATURES_DWORDS];
> unsigned int used, reserved, limit;
>- uint64_t features;
> int i, r, n_initialized_vqs = 0;
>
> hdev->vdev = NULL;
>@@ -1531,7 +1560,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> goto fail;
> }
>
>- r = hdev->vhost_ops->vhost_get_features(hdev, &features);
>+ r = vhost_dev_get_features(hdev, features);
> if (r < 0) {
> error_setg_errno(errp, -r, "vhost_get_features failed");
> goto fail;
>@@ -1569,7 +1598,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> }
> }
>
>- hdev->features = features;
>+ virtio_features_copy(hdev->features_ex, features);
>
> hdev->memory_listener = (MemoryListener) {
> .name = "vhost",
>@@ -1592,7 +1621,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 (!virtio_has_feature_ex(hdev->features_ex, 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()) {
>@@ -1871,6 +1900,20 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> return features;
> }
>
>+void vhost_get_features_ex(struct vhost_dev *hdev,
>+ const int *feature_bits,
>+ uint64_t *features)
>+{
>+ const int *bit = feature_bits;
>+
>+ while (*bit != VHOST_INVALID_FEATURE_BIT) {
>+ if (!virtio_has_feature_ex(hdev->features_ex, *bit)) {
>+ virtio_clear_feature_ex(features, *bit);
>+ }
>+ bit++;
>+ }
>+}
>+
Can we do something similar of what we do in hw/virtio/virtio.c where
the old virtio_set_features() use the new virtio_set_features_ex()?
> void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> uint64_t features)
> {
>@@ -1884,6 +1927,18 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> }
> }
>
>+void vhost_ack_features_ex(struct vhost_dev *hdev, const int *feature_bits,
>+ const uint64_t *features)
>+{
>+ const int *bit = feature_bits;
>+ while (*bit != VHOST_INVALID_FEATURE_BIT) {
>+ if (virtio_has_feature_ex(features, *bit)) {
>+ virtio_add_feature_ex(hdev->acked_features_ex, *bit);
>+ }
>+ bit++;
>+ }
>+}
>+
Ditto.
Not a strong opinion, but just to reduce code duplication.
Thanks,
Stefano
> int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
> uint32_t config_len, Error **errp)
> {
>diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
>index d6df209a2f..ff94fa1734 100644
>--- a/include/hw/virtio/vhost-backend.h
>+++ b/include/hw/virtio/vhost-backend.h
>@@ -95,6 +95,10 @@ typedef int (*vhost_new_worker_op)(struct vhost_dev *dev,
> struct vhost_worker_state *worker);
> typedef int (*vhost_free_worker_op)(struct vhost_dev *dev,
> struct vhost_worker_state *worker);
>+typedef int (*vhost_set_features_ex_op)(struct vhost_dev *dev,
>+ const uint64_t *features);
>+typedef int (*vhost_get_features_ex_op)(struct vhost_dev *dev,
>+ uint64_t *features);
> typedef int (*vhost_set_features_op)(struct vhost_dev *dev,
> uint64_t features);
> typedef int (*vhost_get_features_op)(struct vhost_dev *dev,
>@@ -186,6 +190,8 @@ typedef struct VhostOps {
> vhost_free_worker_op vhost_free_worker;
> vhost_get_vring_worker_op vhost_get_vring_worker;
> vhost_attach_vring_worker_op vhost_attach_vring_worker;
>+ vhost_set_features_ex_op vhost_set_features_ex;
>+ vhost_get_features_ex_op vhost_get_features_ex;
> vhost_set_features_op vhost_set_features;
> vhost_get_features_op vhost_get_features;
> vhost_set_backend_cap_op vhost_set_backend_cap;
>diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
>index 66be6afc88..39fbffc6bc 100644
>--- a/include/hw/virtio/vhost.h
>+++ b/include/hw/virtio/vhost.h
>@@ -107,9 +107,9 @@ struct vhost_dev {
> * future use should be discouraged and the variable retired as
> * its easy to confuse with the VirtIO backend_features.
> */
>- uint64_t features;
>- uint64_t acked_features;
>- uint64_t 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
>@@ -333,6 +333,21 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
> uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> uint64_t features);
>
>+/**
>+ * vhost_get_features_ex() - sanitize the extended features set
>+ * @hdev: common vhost_dev structure
>+ * @feature_bits: pointer to terminated table of feature bits
>+ * @features: original features set, filtered out on return
>+ *
>+ * This is the extended variant of vhost_get_features(), supporting the
>+ * the extended features set. Filter it with the intersection of what is
>+ * supported by the vhost backend (hdev->features) and the supported
>+ * feature_bits.
>+ */
>+void vhost_get_features_ex(struct vhost_dev *hdev,
>+ const int *feature_bits,
>+ uint64_t *features);
>+
> /**
> * vhost_ack_features() - set vhost acked_features
> * @hdev: common vhost_dev structure
>@@ -344,6 +359,18 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> */
> void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> uint64_t features);
>+
>+/**
>+ * vhost_ack_features_ex() - set vhost full set of acked_features
>+ * @hdev: common vhost_dev structure
>+ * @feature_bits: pointer to terminated table of feature bits
>+ * @features: requested feature set
>+ *
>+ * This sets the internal hdev->acked_features to the intersection of
>+ * the backends advertised features and the supported feature_bits.
>+ */
>+void vhost_ack_features_ex(struct vhost_dev *hdev, const int *feature_bits,
>+ const uint64_t *features);
> unsigned int vhost_get_max_memslots(void);
> unsigned int vhost_get_free_memslots(void);
>
>--
>2.50.0
>
On 7/18/25 4:36 PM, Stefano Garzarella wrote:
> On Fri, Jul 18, 2025 at 10:52:33AM +0200, Paolo Abeni wrote:
>> @@ -1871,6 +1900,20 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
>> return features;
>> }
>>
>> +void vhost_get_features_ex(struct vhost_dev *hdev,
>> + const int *feature_bits,
>> + uint64_t *features)
>> +{
>> + const int *bit = feature_bits;
>> +
>> + while (*bit != VHOST_INVALID_FEATURE_BIT) {
>> + if (!virtio_has_feature_ex(hdev->features_ex, *bit)) {
>> + virtio_clear_feature_ex(features, *bit);
>> + }
>> + bit++;
>> + }
>> +}
>> +
>
> Can we do something similar of what we do in hw/virtio/virtio.c where
> the old virtio_set_features() use the new virtio_set_features_ex()?
>
>> void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
>> uint64_t features)
>> {
>> @@ -1884,6 +1927,18 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
>> }
>> }
>>
>> +void vhost_ack_features_ex(struct vhost_dev *hdev, const int *feature_bits,
>> + const uint64_t *features)
>> +{
>> + const int *bit = feature_bits;
>> + while (*bit != VHOST_INVALID_FEATURE_BIT) {
>> + if (virtio_has_feature_ex(features, *bit)) {
>> + virtio_add_feature_ex(hdev->acked_features_ex, *bit);
>> + }
>> + bit++;
>> + }
>> +}
>> +
>
> Ditto.
>
> Not a strong opinion, but just to reduce code duplication.
The incremental diffstat with such cleanup looks good, so I'll include
that in the next revision, thanks!
Paolo
On Fri, Jul 18, 2025 at 10:37 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Fri, Jul 18, 2025 at 10:52:33AM +0200, Paolo Abeni wrote:
> >Similar to virtio infra, vhost core maintains the features status
> >in the full extended format and allows the devices to implement
> >extended version of the getter/setter.
> >
> >Note that 'protocol_features' are not extended: they are only
> >used by vhost-user, and the latter device is not going to implement
> >extended features soon.
> >
> >Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> >---
> >v2 -> v3:
> > - fix compile warning
> > - _array -> _ex
> >
> >v1 -> v2:
> > - uint128_t -> uint64_t[]
> > - add _ex() variant of features manipulation helpers
> >---
> > hw/virtio/vhost.c | 73 +++++++++++++++++++++++++++----
> > include/hw/virtio/vhost-backend.h | 6 +++
> > include/hw/virtio/vhost.h | 33 ++++++++++++--
> > 3 files changed, 100 insertions(+), 12 deletions(-)
> >
> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >index c30ea1156e..85ae1e4d4c 100644
> >--- a/hw/virtio/vhost.c
> >+++ b/hw/virtio/vhost.c
> >@@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
> > static int vhost_dev_set_features(struct vhost_dev *dev,
> > bool enable_log)
> > {
> >- uint64_t features = dev->acked_features;
> >+ uint64_t features[VIRTIO_FEATURES_DWORDS];
> > int r;
> >+
> >+ virtio_features_copy(features, dev->acked_features_ex);
> > if (enable_log) {
> >- features |= 0x1ULL << VHOST_F_LOG_ALL;
> >+ virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
> > }
> > if (!vhost_dev_has_iommu(dev)) {
> >- features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
> >+ virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> > }
> > if (dev->vhost_ops->vhost_force_iommu) {
> > if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
> >- features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
> >+ virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
> > }
> > }
> >- r = dev->vhost_ops->vhost_set_features(dev, features);
> >+
Hi Paolo
> >+ if (virtio_features_use_extended(features) &&
> >+ !dev->vhost_ops->vhost_set_features_ex) {
> >+ VHOST_OPS_DEBUG(r, "extended features without device support");
> >+ r = -EINVAL;
> >+ goto out;
> >+ }
As we discussed in version 2, this code should be changed to: [1],
otherwise the problem mentioned last time will occur when compiling.
[1] if (virtio_features_use_extended(features) &&
!dev->vhost_ops->vhost_set_features_ex) {
- VHOST_OPS_DEBUG(r, "extended features without device support");
r = -EINVAL;
+ VHOST_OPS_DEBUG(r, "extended features without device support");
goto out;
}
Thanks
Lei
> >+
> >+ if (dev->vhost_ops->vhost_set_features_ex) {
> >+ r = dev->vhost_ops->vhost_set_features_ex(dev, features);
> >+ } else {
> >+ r = dev->vhost_ops->vhost_set_features(dev, features[0]);
> >+ }
> > if (r < 0) {
> > VHOST_OPS_DEBUG(r, "vhost_set_features failed");
> > goto out;
> >@@ -1506,12 +1520,27 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq)
> > }
> > }
> >
> >+static int vhost_dev_get_features(struct vhost_dev *hdev,
> >+ uint64_t *features)
> >+{
> >+ uint64_t features64;
> >+ int r;
> >+
> >+ if (hdev->vhost_ops->vhost_get_features_ex) {
> >+ return hdev->vhost_ops->vhost_get_features_ex(hdev, features);
> >+ }
> >+
> >+ r = hdev->vhost_ops->vhost_get_features(hdev, &features64);
> >+ virtio_features_from_u64(features, features64);
> >+ return r;
> >+}
> >+
> > int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> > VhostBackendType backend_type, uint32_t busyloop_timeout,
> > Error **errp)
> > {
> >+ uint64_t features[VIRTIO_FEATURES_DWORDS];
> > unsigned int used, reserved, limit;
> >- uint64_t features;
> > int i, r, n_initialized_vqs = 0;
> >
> > hdev->vdev = NULL;
> >@@ -1531,7 +1560,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> > goto fail;
> > }
> >
> >- r = hdev->vhost_ops->vhost_get_features(hdev, &features);
> >+ r = vhost_dev_get_features(hdev, features);
> > if (r < 0) {
> > error_setg_errno(errp, -r, "vhost_get_features failed");
> > goto fail;
> >@@ -1569,7 +1598,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> > }
> > }
> >
> >- hdev->features = features;
> >+ virtio_features_copy(hdev->features_ex, features);
> >
> > hdev->memory_listener = (MemoryListener) {
> > .name = "vhost",
> >@@ -1592,7 +1621,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 (!virtio_has_feature_ex(hdev->features_ex, 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()) {
> >@@ -1871,6 +1900,20 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> > return features;
> > }
> >
> >+void vhost_get_features_ex(struct vhost_dev *hdev,
> >+ const int *feature_bits,
> >+ uint64_t *features)
> >+{
> >+ const int *bit = feature_bits;
> >+
> >+ while (*bit != VHOST_INVALID_FEATURE_BIT) {
> >+ if (!virtio_has_feature_ex(hdev->features_ex, *bit)) {
> >+ virtio_clear_feature_ex(features, *bit);
> >+ }
> >+ bit++;
> >+ }
> >+}
> >+
>
> Can we do something similar of what we do in hw/virtio/virtio.c where
> the old virtio_set_features() use the new virtio_set_features_ex()?
>
> > void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> > uint64_t features)
> > {
> >@@ -1884,6 +1927,18 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> > }
> > }
> >
> >+void vhost_ack_features_ex(struct vhost_dev *hdev, const int *feature_bits,
> >+ const uint64_t *features)
> >+{
> >+ const int *bit = feature_bits;
> >+ while (*bit != VHOST_INVALID_FEATURE_BIT) {
> >+ if (virtio_has_feature_ex(features, *bit)) {
> >+ virtio_add_feature_ex(hdev->acked_features_ex, *bit);
> >+ }
> >+ bit++;
> >+ }
> >+}
> >+
>
> Ditto.
>
> Not a strong opinion, but just to reduce code duplication.
>
> Thanks,
> Stefano
>
> > int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config,
> > uint32_t config_len, Error **errp)
> > {
> >diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
> >index d6df209a2f..ff94fa1734 100644
> >--- a/include/hw/virtio/vhost-backend.h
> >+++ b/include/hw/virtio/vhost-backend.h
> >@@ -95,6 +95,10 @@ typedef int (*vhost_new_worker_op)(struct vhost_dev *dev,
> > struct vhost_worker_state *worker);
> > typedef int (*vhost_free_worker_op)(struct vhost_dev *dev,
> > struct vhost_worker_state *worker);
> >+typedef int (*vhost_set_features_ex_op)(struct vhost_dev *dev,
> >+ const uint64_t *features);
> >+typedef int (*vhost_get_features_ex_op)(struct vhost_dev *dev,
> >+ uint64_t *features);
> > typedef int (*vhost_set_features_op)(struct vhost_dev *dev,
> > uint64_t features);
> > typedef int (*vhost_get_features_op)(struct vhost_dev *dev,
> >@@ -186,6 +190,8 @@ typedef struct VhostOps {
> > vhost_free_worker_op vhost_free_worker;
> > vhost_get_vring_worker_op vhost_get_vring_worker;
> > vhost_attach_vring_worker_op vhost_attach_vring_worker;
> >+ vhost_set_features_ex_op vhost_set_features_ex;
> >+ vhost_get_features_ex_op vhost_get_features_ex;
> > vhost_set_features_op vhost_set_features;
> > vhost_get_features_op vhost_get_features;
> > vhost_set_backend_cap_op vhost_set_backend_cap;
> >diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> >index 66be6afc88..39fbffc6bc 100644
> >--- a/include/hw/virtio/vhost.h
> >+++ b/include/hw/virtio/vhost.h
> >@@ -107,9 +107,9 @@ struct vhost_dev {
> > * future use should be discouraged and the variable retired as
> > * its easy to confuse with the VirtIO backend_features.
> > */
> >- uint64_t features;
> >- uint64_t acked_features;
> >- uint64_t 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
> >@@ -333,6 +333,21 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
> > uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> > uint64_t features);
> >
> >+/**
> >+ * vhost_get_features_ex() - sanitize the extended features set
> >+ * @hdev: common vhost_dev structure
> >+ * @feature_bits: pointer to terminated table of feature bits
> >+ * @features: original features set, filtered out on return
> >+ *
> >+ * This is the extended variant of vhost_get_features(), supporting the
> >+ * the extended features set. Filter it with the intersection of what is
> >+ * supported by the vhost backend (hdev->features) and the supported
> >+ * feature_bits.
> >+ */
> >+void vhost_get_features_ex(struct vhost_dev *hdev,
> >+ const int *feature_bits,
> >+ uint64_t *features);
> >+
> > /**
> > * vhost_ack_features() - set vhost acked_features
> > * @hdev: common vhost_dev structure
> >@@ -344,6 +359,18 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
> > */
> > void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
> > uint64_t features);
> >+
> >+/**
> >+ * vhost_ack_features_ex() - set vhost full set of acked_features
> >+ * @hdev: common vhost_dev structure
> >+ * @feature_bits: pointer to terminated table of feature bits
> >+ * @features: requested feature set
> >+ *
> >+ * This sets the internal hdev->acked_features to the intersection of
> >+ * the backends advertised features and the supported feature_bits.
> >+ */
> >+void vhost_ack_features_ex(struct vhost_dev *hdev, const int *feature_bits,
> >+ const uint64_t *features);
> > unsigned int vhost_get_max_memslots(void);
> > unsigned int vhost_get_free_memslots(void);
> >
> >--
> >2.50.0
> >
>
>
On 7/21/25 4:53 AM, Lei Yang wrote:
> On Fri, Jul 18, 2025 at 10:37 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> On Fri, Jul 18, 2025 at 10:52:33AM +0200, Paolo Abeni wrote:
>>> Similar to virtio infra, vhost core maintains the features status
>>> in the full extended format and allows the devices to implement
>>> extended version of the getter/setter.
>>>
>>> Note that 'protocol_features' are not extended: they are only
>>> used by vhost-user, and the latter device is not going to implement
>>> extended features soon.
>>>
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>> ---
>>> v2 -> v3:
>>> - fix compile warning
>>> - _array -> _ex
>>>
>>> v1 -> v2:
>>> - uint128_t -> uint64_t[]
>>> - add _ex() variant of features manipulation helpers
>>> ---
>>> hw/virtio/vhost.c | 73 +++++++++++++++++++++++++++----
>>> include/hw/virtio/vhost-backend.h | 6 +++
>>> include/hw/virtio/vhost.h | 33 ++++++++++++--
>>> 3 files changed, 100 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>>> index c30ea1156e..85ae1e4d4c 100644
>>> --- a/hw/virtio/vhost.c
>>> +++ b/hw/virtio/vhost.c
>>> @@ -972,20 +972,34 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
>>> static int vhost_dev_set_features(struct vhost_dev *dev,
>>> bool enable_log)
>>> {
>>> - uint64_t features = dev->acked_features;
>>> + uint64_t features[VIRTIO_FEATURES_DWORDS];
>>> int r;
>>> +
>>> + virtio_features_copy(features, dev->acked_features_ex);
>>> if (enable_log) {
>>> - features |= 0x1ULL << VHOST_F_LOG_ALL;
>>> + virtio_add_feature_ex(features, VHOST_F_LOG_ALL);
>>> }
>>> if (!vhost_dev_has_iommu(dev)) {
>>> - features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
>>> + virtio_clear_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
>>> }
>>> if (dev->vhost_ops->vhost_force_iommu) {
>>> if (dev->vhost_ops->vhost_force_iommu(dev) == true) {
>>> - features |= 0x1ULL << VIRTIO_F_IOMMU_PLATFORM;
>>> + virtio_add_feature_ex(features, VIRTIO_F_IOMMU_PLATFORM);
>>> }
>>> }
>>> - r = dev->vhost_ops->vhost_set_features(dev, features);
>>> +
>
> Hi Paolo
>
>>> + if (virtio_features_use_extended(features) &&
>>> + !dev->vhost_ops->vhost_set_features_ex) {
>>> + VHOST_OPS_DEBUG(r, "extended features without device support");
>>> + r = -EINVAL;
>>> + goto out;
>>> + }
>
> As we discussed in version 2, this code should be changed to: [1],
> otherwise the problem mentioned last time will occur when compiling.
>
> [1] if (virtio_features_use_extended(features) &&
> !dev->vhost_ops->vhost_set_features_ex) {
> - VHOST_OPS_DEBUG(r, "extended features without device support");
> r = -EINVAL;
> + VHOST_OPS_DEBUG(r, "extended features without device support");
> goto out;
> }
Yes, I'm sorry, something went wrong here. Likely ENOCOFFEE or similar.
Will hopefully address for good in the next revision.
Thanks,
Paolo
© 2016 - 2025 Red Hat, Inc.