Stefan Hajnoczi <[email protected]> writes:
> [[PGP Signed Part:Undecided]] > On Tue, May 24, 2022 at 04:40:41PM +0100, Alex Bennée wrote: >> Hi, >> >> This series ostensibly adds virtio-user-gpio stubs to the build for >> use with an external vhost-user daemon. We've been testing it with our >> rust daemons from: >> >> https://github.com/rust-vmm/vhost-device >> >> Getting the test enabled took some doing most likely because the need >> for CONFIG support exercised additional paths in the code that were >> not used for the simpler virtio-net tests. As a result the series has >> a number of cleanup and documentation patches. >> >> The final thing that needed fixing was the ensuring that >> VHOST_USER_F_PROTOCOL_FEATURES didn't get squashed in the negotiation >> process. This was the hardest thing to track down as we store the >> feature bits in several places variously as: >> >> in VirtIODevice as: >> uint64_t guest_features; >> uint64_t host_features; >> uint64_t backend_features; > > None of these know about VHOST_USER_F_PROTOCOL_FEATURES and vhost-user's > unfiltered feature bits should never be passed to VirtIODevice. > >> >> in vhost_dev as: >> uint64_t features; >> uint64_t acked_features; >> uint64_t backend_features; > > I don't think these should know about VHOST_USER_F_PROTOCOL_FEATURES > either. AFAIK vhost_dev deals with VIRTIO feature bits, not raw > vhost-user GET_FEATURES. So where does VHOST_USER_F_PROTOCOL_FEATURES get set before it's set with the VHOST_USER_SET_FEATURES message? Currently it's fed via: uint64_t features = vhost_dev->acked_features; in vhost_dev_set_features() which does apply a few extra bits (VHOST_F_LOG_ALL/VIRTIO_F_IOMMU_PLATFORM). Maybe it should be adding VHOST_USER_F_PROTOCOL_FEATURES here? How should it be signalled by the vhost-user backend that this should be done? Overload the function? > > Stefan > > [[End of PGP Signed Part]] -- Alex Bennée
