On 7/21/25 4:53 AM, Lei Yang wrote:
> On Fri, Jul 18, 2025 at 10:37 PM Stefano Garzarella <sgarz...@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 <pab...@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
