Hi Stefano, On Sat, Aug 06, 2022 at 09:48:28AM +0200, Stefano Garzarella wrote: > On Fri, Aug 05, 2022 at 07:11:06PM +0100, Will Deacon wrote: > > The fundamental issue is, I think, that VIRTIO_F_ACCESS_PLATFORM is > > being used for two very different things within the same device; for the > > guest it basically means "use the DMA API, it knows what to do" but for > > vhost it very specifically means "enable IOTLB". We've recently had > > other problems with this flag [3] but in this case it used to work > > reliably and now it doesn't anymore. > > > > So how should we fix this? One possibility is for us to hack crosvm to > > clear the VIRTIO_F_ACCESS_PLATFORM flag when setting the vhost > > Why do you consider this a hack? I think it's a hack for two reasons: (1) We're changing userspace to avoid a breaking change in kernel behaviour (2) I think that crosvm's approach is actually pretty reasonable To elaborate on (2), crosvm has a set of device features that it has negotiated with the guest. It then takes the intersection of these features with those advertised by VHOST_GET_FEATURES and calls VHOST_SET_FEATURES with the result. If there was a common interpretation of what these features do, then this would work and would mean we wouldn't have to opt-in on a per-flag basis for vhost. Since VIRTIO_F_ACCESS_PLATFORM is being overloaded to mean two completely different things, then it breaks and I think masking out that specific flag is a hack because it's basically crosvm saying "yeah, I may have negotiated this with the driver but vhost _actually_ means 'IOTLB' when it says it supports this flag so I'll mask it out because I know better". > If the VMM implements the translation feature, it is right in my opinion > that it does not enable the feature for the vhost device. Otherwise, if it > wants the vhost device to do the translation, enable the feature and send > the IOTLB messages to set the translation. > > QEMU for example masks features when not required or supported. > crosvm should negotiate only the features it supports. > > @Michael and @Jason can correct me, but if a vhost device negotiates > VIRTIO_F_ACCESS_PLATFORM, then it expects the VMM to send IOTLB messages to > set the translation. As above, the issue is that vhost now unconditionally advertises this in VHOST_GET_FEATURES and so a VMM with no knowledge of IOTLB can end up enabling it by accident. Will
