Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM

[Michael S. Tsirkin]

On Wed, Feb 26, 2020 at 01:55:39PM +0100, Halil Pasic wrote:
> On Wed, 26 Feb 2020 04:53:33 -0500
> "Michael S. Tsirkin" <address@hidden> wrote:
> 
> > On Wed, Feb 26, 2020 at 05:43:57PM +0800, Jason Wang wrote:
> > > We turn on device IOTLB via VIRTIO_F_IOMMU_PLATFORM unconditionally on
> > > platform without IOMMU support. This can lead unnecessary IOTLB
> > > transactions which will damage the performance.
> > > 
> > > Fixing this by check whether the device is backed by IOMMU and disable
> > > device IOTLB.
> > > 
> > > Reported-by: Halil Pasic <address@hidden>
> > > Fixes: c471ad0e9bd46 ("vhost_net: device IOTLB support")
> > 
> > Well it's just an optimization, isn't it?
> > I don't think it's justified to push this to everyone using
> > vhost with IOTLB, is it?
> 
> IMHO we need this for everyone using vhost! For instance vhost-vsock
> currently does not work with iommu_platform=on, because unlike vhost-net
> vhost does not offer F_ACCESS_PLATFORM, so set features IOCTL fails. 

You mean vsock does not offer it? OK but that's still not a bugfix.
Making new configs work is great, but that's a feature almost
by definition.

> > If you disagree, could you comment a bit on which configurations where 
> > tested?
> > 
> > > Cc: address@hidden
> > > Signed-off-by: Jason Wang <address@hidden>
> > 
> > Halil could you test this pls? Does this help your performance issue?
> > 
> 
> I'm pretty sure it does, but I will re-test. The previous version where
> it was done virtio-net certainly did.
> 
> Regards,
> Halil
> 
>