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

[Halil Pasic]

[..]
> > 
> > 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.

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