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

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Mar 13, 2020 at 01:44:46PM +0100, Halil Pasic wrote:
> [..]
> > > 
> > > CCing Tom. @Tom does vhost-vsock work for you with SEV and current qemu?
> > > 
> > > Also, one can specify iommu_platform=on on a device that ain't a part of
> > > a secure-capable VM, just for the fun of it. And that breaks
> > > vhost-vsock. Or is setting iommu_platform=on only valid if
> > > qemu-system-s390x is protected virtualization capable?
> > > 
> > > BTW, I don't have a strong opinion on the fixes tag. We currently do not
> > > recommend setting iommu_platform, and thus I don't think we care too
> > > much about past qemus having problems with it.
> > > 
> > > Regards,
> > > Halil
> > 
> > 
> > Let's just say if we do have a Fixes: tag we want to set it correctly to
> > the commit that needs this fix.
> > 
> 
> I finally did some digging regarding the performance degradation. For
> s390x the performance degradation on vhost-net was introduced by commit
> 076a93d797 ("exec: simplify address_space_get_iotlb_entry"). Before
> IOMMUTLBEntry.addr_mask used to be based on plen, which in turn was
> calculated as the rest of the memory regions size (from address), and
> covered most of the guest address space. That is we didn't have a whole
> lot of IOTLB API overhead.
> 
> With commit 076a93d797 I see IOMMUTLBEntry.addr_mask == 0xfff which comes
> as ~TARGET_PAGE_MASK from flatview_do_translate(). To have things working
> properly I applied 75e5b70e6, b021d1c044, and d542800d1e on the level of
> 076a93d797 and 076a93d797~1.

Peter, what's your take on this one?


> Regarding vhost-vsock. It does not work with iommu_platform=on since the
> very beginning (i.e. 8607f5c307 ("virtio: convert to use DMA api")). Not
> sure if that is a good or a bad thing. (If the vhost driver in the kernel
> would actually have to do the IOTLB translation, then failing in case
> where it does not support it seems sane. The problem is that
> ACCESS_PLATFORM is used for more than one thing (needs translation, and
> restricted memory access).)
> 
> I don't think I've heard back from AMD whether vsock works with SEV or
> not... I don't have access to HW to test it myself.
> 
> We (s390) don't require this being backported to the stable qemus,
> because for us iommu_platform=on becomes relevant with protected
> virtualization, and those qemu versions don't support it.
> 
> Cheers,
> Halil