[PATCH for 7.1 v2 0/4] virtio fixes (split from virtio-gpio series)

[PATCH for 7.1 v2 0/4] virtio fixes (split from virtio-gpio series)

[Alex Bennée]

Hi,

This is just a split out series based on:

   Subject: [PATCH v3 for 7.2 00/21] virtio-gpio and various virtio cleanups
   Date: Tue, 26 Jul 2022 20:21:29 +0100
   Message-Id: <20220726192150.2435175-1-alex.bennee@linaro.org>

with the patches identified by mst:

  Right. Still I think some are fixes we should merge now.
  I am thinking patches 5, 7,8,9 ? 6 if it makes backporting
  much easier. WDYT? If you agree pls separate bugfixes in
  series I can apply. Thanks!

but I've dropped the first patch as that was actually a backend and
I've added one of Jason's acked-bys.

Alex Bennée (4):
  hw/virtio: incorporate backend features in features
  hw/virtio: gracefully handle unset vhost_dev vdev
  hw/virtio: handle un-configured shutdown in virtio-pci
  hw/virtio: fix vhost_user_read tracepoint

 hw/virtio/vhost-user.c | 13 ++++++++++---
 hw/virtio/vhost.c      | 10 +++++++---
 hw/virtio/virtio-pci.c |  9 +++++++--
 3 files changed, 24 insertions(+), 8 deletions(-)

-- 
2.30.2

[PATCH v2 1/4] hw/virtio: incorporate backend features in features

[Alex Bennée]

There are some extra bits used over a vhost-user connection which are
hidden from the device itself. We need to set them here to ensure we
enable things like the protocol extensions.

Currently net/vhost-user.c has it's own inscrutable way of persisting
this data but it really should live in the core vhost_user code.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-7-alex.bennee@linaro.org>
---
 hw/virtio/vhost-user.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 75b8df21a4..1936a44e82 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1460,7 +1460,14 @@ static int vhost_user_set_features(struct vhost_dev *dev,
      */
     bool log_enabled = features & (0x1ULL << VHOST_F_LOG_ALL);
 
-    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
+    /*
+     * 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.
+     */
+    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
+                              features | dev->backend_features,
                               log_enabled);
 }
 
-- 
2.30.2

[PATCH v2 2/4] hw/virtio: gracefully handle unset vhost_dev vdev

[Alex Bennée]

I've noticed asserts firing because we query the status of vdev after
a vhost connection is closed down. Rather than faulting on the NULL
indirect just quietly reply false.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-8-alex.bennee@linaro.org>
---
 hw/virtio/vhost.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 0827d631c0..f758f177bb 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -306,7 +306,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
     dev->log_size = size;
 }
 
-static int vhost_dev_has_iommu(struct vhost_dev *dev)
+static bool vhost_dev_has_iommu(struct vhost_dev *dev)
 {
     VirtIODevice *vdev = dev->vdev;
 
@@ -316,8 +316,12 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
      * does not have IOMMU, there's no need to enable this feature
      * which may cause unnecessary IOTLB miss/update transactions.
      */
-    return virtio_bus_device_iommu_enabled(vdev) &&
-           virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+    if (vdev) {
+        return virtio_bus_device_iommu_enabled(vdev) &&
+            virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+    } else {
+        return false;
+    }
 }
 
 static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
-- 
2.30.2

[PATCH v2 3/4] hw/virtio: handle un-configured shutdown in virtio-pci

[Alex Bennée]

The assert() protecting against leakage is a little aggressive and
causes needless crashes if a device is shutdown without having been
configured. In this case no descriptors are lost because none have
been assigned.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-9-alex.bennee@linaro.org>
---
 hw/virtio/virtio-pci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 45327f0b31..5ce61f9b45 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -996,9 +996,14 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
 
     nvqs = MIN(nvqs, VIRTIO_QUEUE_MAX);
 
-    /* When deassigning, pass a consistent nvqs value
-     * to avoid leaking notifiers.
+    /*
+     * When deassigning, pass a consistent nvqs value to avoid leaking
+     * notifiers. But first check we've actually been configured, exit
+     * early if we haven't.
      */
+    if (!assign && !proxy->nvqs_with_notifiers) {
+        return 0;
+    }
     assert(assign || nvqs == proxy->nvqs_with_notifiers);
 
     proxy->nvqs_with_notifiers = nvqs;
-- 
2.30.2

[PATCH v2 4/4] hw/virtio: fix vhost_user_read tracepoint

[Alex Bennée]

As reads happen in the callback we were never seeing them. We only
really care about the header so move the tracepoint to when the header
is complete.

Fixes: 6ca6d8ee9d (hw/virtio: add vhost_user_[read|write] trace points)
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Jason Wang <jasowang@redhat.com>
Message-Id: <20220726192150.2435175-10-alex.bennee@linaro.org>
---
 hw/virtio/vhost-user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 1936a44e82..c0b50deaf2 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -295,6 +295,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);
+
     return 0;
 }
 
@@ -544,8 +546,6 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
         }
     }
 
-    trace_vhost_user_read(msg.hdr.request, msg.hdr.flags);
-
     return 0;
 }
 
-- 
2.30.2

Re: [PATCH v2 1/4] hw/virtio: incorporate backend features in features

[Michael S. Tsirkin]

On Thu, Jul 28, 2022 at 02:55:00PM +0100, Alex Bennée wrote:
> There are some extra bits used over a vhost-user connection which are
> hidden from the device itself. We need to set them here to ensure we
> enable things like the protocol extensions.
> 
> Currently net/vhost-user.c has it's own inscrutable way of persisting
> this data but it really should live in the core vhost_user code.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20220726192150.2435175-7-alex.bennee@linaro.org>

I went back and forth on this, but in the end I feel
it is safer to defer this one until after the release.

> ---
>  hw/virtio/vhost-user.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 75b8df21a4..1936a44e82 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1460,7 +1460,14 @@ static int vhost_user_set_features(struct vhost_dev *dev,
>       */
>      bool log_enabled = features & (0x1ULL << VHOST_F_LOG_ALL);
>  
> -    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
> +    /*
> +     * 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.
> +     */
> +    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
> +                              features | dev->backend_features,
>                                log_enabled);
>  }
>  
> -- 
> 2.30.2

Re: [PATCH v2 2/4] hw/virtio: gracefully handle unset vhost_dev vdev

[Michael S. Tsirkin]

On Thu, Jul 28, 2022 at 02:55:01PM +0100, Alex Bennée wrote:
> I've noticed asserts firing because we query the status of vdev after
> a vhost connection is closed down. Rather than faulting on the NULL
> indirect just quietly reply false.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20220726192150.2435175-8-alex.bennee@linaro.org>

Please do not include this header when you post.


> ---
>  hw/virtio/vhost.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 0827d631c0..f758f177bb 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -306,7 +306,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
>      dev->log_size = size;
>  }
>  
> -static int vhost_dev_has_iommu(struct vhost_dev *dev)
> +static bool vhost_dev_has_iommu(struct vhost_dev *dev)
>  {
>      VirtIODevice *vdev = dev->vdev;
>  
> @@ -316,8 +316,12 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
>       * does not have IOMMU, there's no need to enable this feature
>       * which may cause unnecessary IOTLB miss/update transactions.
>       */
> -    return virtio_bus_device_iommu_enabled(vdev) &&
> -           virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> +    if (vdev) {
> +        return virtio_bus_device_iommu_enabled(vdev) &&
> +            virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
> +    } else {
> +        return false;
> +    }
>  }
>  
>  static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
> -- 
> 2.30.2

Re: [PATCH v2 2/4] hw/virtio: gracefully handle unset vhost_dev vdev

[Alex Bennée]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Jul 28, 2022 at 02:55:01PM +0100, Alex Bennée wrote:
>> I've noticed asserts firing because we query the status of vdev after
>> a vhost connection is closed down. Rather than faulting on the NULL
>> indirect just quietly reply false.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20220726192150.2435175-8-alex.bennee@linaro.org>
>
> Please do not include this header when you post.

What the message id? It's added by b4 to track when the message was last
posted. On my PRs I often have two message id's, one from the series
where I picked it up and one from the last time my series was posted. I
usually only edit them down so I'm not repeating it for each iteration
of a series.

>
>
>> ---
>>  hw/virtio/vhost.c | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>> 
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index 0827d631c0..f758f177bb 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -306,7 +306,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
>>      dev->log_size = size;
>>  }
>>  
>> -static int vhost_dev_has_iommu(struct vhost_dev *dev)
>> +static bool vhost_dev_has_iommu(struct vhost_dev *dev)
>>  {
>>      VirtIODevice *vdev = dev->vdev;
>>  
>> @@ -316,8 +316,12 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
>>       * does not have IOMMU, there's no need to enable this feature
>>       * which may cause unnecessary IOTLB miss/update transactions.
>>       */
>> -    return virtio_bus_device_iommu_enabled(vdev) &&
>> -           virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
>> +    if (vdev) {
>> +        return virtio_bus_device_iommu_enabled(vdev) &&
>> +            virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
>> +    } else {
>> +        return false;
>> +    }
>>  }
>>  
>>  static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
>> -- 
>> 2.30.2


-- 
Alex Bennée

RE: [PATCH v2 1/4] hw/virtio: incorporate backend features in features

[Yajun Wu]

Hi Alex,

With this change, VHOST_USER_F_PROTOCOL_FEATURES bit will be set to backend for virtio block device (previously not).

From https://www.qemu.org/docs/master/interop/vhost-user.html spec:
If VHOST_USER_F_PROTOCOL_FEATURES has not been negotiated, the ring starts directly in the enabled state.
If VHOST_USER_F_PROTOCOL_FEATURES has been negotiated, the ring is initialized in a disabled state and is enabled by VHOST_USER_SET_VRING_ENABLE with parameter 1.

Vhost-user-blk won't send out VHOST_USER_SET_VRING_ENABLE today. 
Backend gets VHOST_USER_F_PROTOCOL_FEATURES negotiated and can't get VHOST_USER_SET_VRING_ENABLE.
VQs keep in disabled state.

Can you check on this scenario?

Thanks

-----Original Message-----
From: Qemu-devel <qemu-devel-bounces+yajunw=nvidia.com@nongnu.org> On Behalf Of Alex Bennée
Sent: Thursday, July 28, 2022 9:55 PM
To: qemu-devel@nongnu.org
Cc: mst@redhat.com; Alex Bennée <alex.bennee@linaro.org>
Subject: [PATCH v2 1/4] hw/virtio: incorporate backend features in features

External email: Use caution opening links or attachments


There are some extra bits used over a vhost-user connection which are hidden from the device itself. We need to set them here to ensure we enable things like the protocol extensions.

Currently net/vhost-user.c has it's own inscrutable way of persisting this data but it really should live in the core vhost_user code.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20220726192150.2435175-7-alex.bennee@linaro.org>
---
 hw/virtio/vhost-user.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 75b8df21a4..1936a44e82 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1460,7 +1460,14 @@ static int vhost_user_set_features(struct vhost_dev *dev,
      */
     bool log_enabled = features & (0x1ULL << VHOST_F_LOG_ALL);

-    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
+    /*
+     * 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.
+     */
+    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
+                              features | dev->backend_features,
                               log_enabled);  }

--
2.30.2

Re: [PATCH v2 1/4] hw/virtio: incorporate backend features in features

[Alex Bennée]

Yajun Wu <yajunw@nvidia.com> writes:

> Hi Alex,
>
> With this change, VHOST_USER_F_PROTOCOL_FEATURES bit will be set to
> backend for virtio block device (previously not).
>
> From https://www.qemu.org/docs/master/interop/vhost-user.html spec:
> If VHOST_USER_F_PROTOCOL_FEATURES has not been negotiated, the ring starts directly in the enabled state.
> If VHOST_USER_F_PROTOCOL_FEATURES has been negotiated, the ring is
> initialized in a disabled state and is enabled by
> VHOST_USER_SET_VRING_ENABLE with parameter 1.
>
> Vhost-user-blk won't send out VHOST_USER_SET_VRING_ENABLE today. 
> Backend gets VHOST_USER_F_PROTOCOL_FEATURES negotiated and can't get VHOST_USER_SET_VRING_ENABLE.
> VQs keep in disabled state.

If the backend advertises protocol features but the stub doesn't support
it how does it get enabled?

The testing I did was mostly by hand with the gpio backend and using the
qtests. I Think we need to add some acceptance testing into avocado with
some real daemons because I don't think we have enough coverage with the
current qtest approach.

>
> Can you check on this scenario?
>
> Thanks
>
> -----Original Message-----
> From: Qemu-devel <qemu-devel-bounces+yajunw=nvidia.com@nongnu.org> On Behalf Of Alex Bennée
> Sent: Thursday, July 28, 2022 9:55 PM
> To: qemu-devel@nongnu.org
> Cc: mst@redhat.com; Alex Bennée <alex.bennee@linaro.org>
> Subject: [PATCH v2 1/4] hw/virtio: incorporate backend features in features
>
> External email: Use caution opening links or attachments
>
>
> There are some extra bits used over a vhost-user connection which are hidden from the device itself. We need to set them here to ensure we enable things like the protocol extensions.
>
> Currently net/vhost-user.c has it's own inscrutable way of persisting this data but it really should live in the core vhost_user code.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20220726192150.2435175-7-alex.bennee@linaro.org>
> ---
>  hw/virtio/vhost-user.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 75b8df21a4..1936a44e82 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1460,7 +1460,14 @@ static int vhost_user_set_features(struct vhost_dev *dev,
>       */
>      bool log_enabled = features & (0x1ULL << VHOST_F_LOG_ALL);
>
> -    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
> +    /*
> +     * 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.
> +     */
> +    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
> +                              features | dev->backend_features,
>                                log_enabled);  }


-- 
Alex Bennée

RE: [PATCH v2 1/4] hw/virtio: incorporate backend features in features

[Yajun Wu]

Hi Alex,

Sorry for the late response, I missed your mail.
You can test together with dpdk and have reproduce.

Steps:
1. DPDK 
git clone https://github.com/DPDK/dpdk.git
git checkout v22.07
meson build -Dexamples=vhost_blk
ninja -C build
cd /tmp/
sudo dpdk/build/examples/dpdk-vhost_blk # it's a daemon 

2. Add blk device to qemu, then bootup VM.
  <qemu:commandline>
    <qemu:arg value='-chardev'/>
    <qemu:arg value='socket,id=char0,path=/tmp/vhost.socket'/>
    <qemu:arg value='-device'/>
    <qemu:arg value='vhost-user-blk-pci,chardev=char0,num-queues=1'/>
  </qemu:commandline>

Without this commit, you can get device ready log from dpdk-vhost_blk:

VHOST_CONFIG: (/tmp/vhost.socket) negotiated Vhost-user protocol features: 0x11ebf
VHOST_CONFIG: (/tmp/vhost.socket) negotiated Virtio features: 0x100000000
VHOST_CONFIG: (/tmp/vhost.socket) virtio is now ready for processing.
New Device /tmp/vhost.socket, Device ID 0
Ctrlr Worker Thread start

With this commit, device won't be ready and VM will hang. 
You can see VHOST_USER_F_PROTOCOL_FEATURES bit is added.

VHOST_CONFIG: (/tmp/vhost.socket) negotiated Virtio features: 0x140000000

Dpdk code related:
./lib/vhost/vhost_user.c

2044     /*                                                                          
2045      * When VHOST_USER_F_PROTOCOL_FEATURES is not negotiated,                   
2046      * the ring starts already enabled. Otherwise, it is enabled via            
2047      * the SET_VRING_ENABLE message.                                            
2048      */                                                                         
2049     if (!(dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) {          
2050         vq->enabled = true;                                                     
2051     }


Thanks,
Yajun 




-----Original Message-----
From: Alex Bennée <alex.bennee@linaro.org> 
Sent: Wednesday, October 26, 2022 3:42 PM
To: Yajun Wu <yajunw@nvidia.com>
Cc: qemu-devel@nongnu.org; mst@redhat.com; Parav Pandit <parav@nvidia.com>
Subject: Re: [PATCH v2 1/4] hw/virtio: incorporate backend features in features

External email: Use caution opening links or attachments


Yajun Wu <yajunw@nvidia.com> writes:

> Hi Alex,
>
> With this change, VHOST_USER_F_PROTOCOL_FEATURES bit will be set to 
> backend for virtio block device (previously not).
>
> From https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.qemu.org%2Fdocs%2Fmaster%2Finterop%2Fvhost-user.html&amp;data=05%7C01%7Cyajunw%40nvidia.com%7C2e8901540bf441248ec608dab725ca87%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638023670196631779%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=y5g9wwTfh%2BxrkESRypoo4pg3eKYInyDerDmI844PBSE%3D&amp;reserved=0 spec:
> If VHOST_USER_F_PROTOCOL_FEATURES has not been negotiated, the ring starts directly in the enabled state.
> If VHOST_USER_F_PROTOCOL_FEATURES has been negotiated, the ring is 
> initialized in a disabled state and is enabled by 
> VHOST_USER_SET_VRING_ENABLE with parameter 1.
>
> Vhost-user-blk won't send out VHOST_USER_SET_VRING_ENABLE today.
> Backend gets VHOST_USER_F_PROTOCOL_FEATURES negotiated and can't get VHOST_USER_SET_VRING_ENABLE.
> VQs keep in disabled state.

If the backend advertises protocol features but the stub doesn't support it how does it get enabled?

The testing I did was mostly by hand with the gpio backend and using the qtests. I Think we need to add some acceptance testing into avocado with some real daemons because I don't think we have enough coverage with the current qtest approach.

>
> Can you check on this scenario?
>
> Thanks
>
> -----Original Message-----
> From: Qemu-devel <qemu-devel-bounces+yajunw=nvidia.com@nongnu.org> On 
> Behalf Of Alex Bennée
> Sent: Thursday, July 28, 2022 9:55 PM
> To: qemu-devel@nongnu.org
> Cc: mst@redhat.com; Alex Bennée <alex.bennee@linaro.org>
> Subject: [PATCH v2 1/4] hw/virtio: incorporate backend features in 
> features
>
> External email: Use caution opening links or attachments
>
>
> There are some extra bits used over a vhost-user connection which are hidden from the device itself. We need to set them here to ensure we enable things like the protocol extensions.
>
> Currently net/vhost-user.c has it's own inscrutable way of persisting this data but it really should live in the core vhost_user code.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20220726192150.2435175-7-alex.bennee@linaro.org>
> ---
>  hw/virtio/vhost-user.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 
> 75b8df21a4..1936a44e82 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1460,7 +1460,14 @@ static int vhost_user_set_features(struct vhost_dev *dev,
>       */
>      bool log_enabled = features & (0x1ULL << VHOST_F_LOG_ALL);
>
> -    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
> +    /*
> +     * 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.
> +     */
> +    return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES,
> +                              features | dev->backend_features,
>                                log_enabled);  }


--
Alex Bennée