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

[Halil Pasic]

On Wed, 26 Feb 2020 11:52:26 -0500
"Michael S. Tsirkin" <address@hidden> wrote:

> On Wed, Feb 26, 2020 at 04:36:18PM +0100, Halil Pasic wrote:
> > On Wed, 26 Feb 2020 08:37:13 -0500
> > "Michael S. Tsirkin" <address@hidden> wrote:
> > 
> > > On Wed, Feb 26, 2020 at 02:28:39PM +0100, Halil Pasic wrote:
> > > > On Wed, 26 Feb 2020 17:43:57 +0800
> > > > Jason Wang <address@hidden> 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")
> > > > > Cc: address@hidden
> > > > > Signed-off-by: Jason Wang <address@hidden>
> > > > 
> > > > Tested-by: Halil Pasic <address@hidden>
> > > > Reviewed-by: Halil Pasic <address@hidden>
> > > > 
> > > > Thank you very much for fixing this! BTW as I mentioned before it
> > > > fixes vhost-vsock with iommu_platform=on as well.
> > > 
> > > Fixes as in improves performance?
> > 
> > No, fixes like one does not get something like:
> > qemu-system-s390x: vhost_set_features failed: Operation not supported (95)
> > qemu-system-s390x: Error starting vhost: 95
> > any more.
> > 
> > Regards,
> > Halil
> > 
> > [..]
> 
> But can commit c471ad0e9bd46 actually boot a secure guest
> where iommu_platform=on is required?
> 

No, of course it can not. But I'm not sure about AMD SEV. AFAIU without
Jason's patch it does not work for AMD SEV. Tom already stated that with
SEV they don't need the IOVA translation aspect of ACCESS_PLATFORM, but
I have no idea if the condition vdev->dma_as == &address_space_memory
catches them as well or not. They probably have !=.

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