On Thu, 27 Feb 2020 10:47:22 -0500 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Thu, Feb 27, 2020 at 02:02:15PM +0100, Halil Pasic wrote: > > On Wed, 26 Feb 2020 11:52:26 -0500 > > "Michael S. Tsirkin" <m...@redhat.com> 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" <m...@redhat.com> 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 <jasow...@redhat.com> 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 <pa...@linux.ibm.com> > > > > > > > Fixes: c471ad0e9bd46 ("vhost_net: device IOTLB support") > > > > > > > Cc: qemu-sta...@nongnu.org > > > > > > > Signed-off-by: Jason Wang <jasow...@redhat.com> > > > > > > > > > > > > Tested-by: Halil Pasic <pa...@linux.ibm.com> > > > > > > Reviewed-by: Halil Pasic <pa...@linux.ibm.com> > > > > > > > > > > > > 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 > > > Let's just say if we do have a Fixes: tag we want to set it correctly to > the commit that needs this fix. > You are absolutely correct. And c471ad0e9bd46 has nothing to do with vsock. On s390x the situation with virtio-net + vhost + iommu_platform=on seems rather complex. I did some digging, but I have no conclusive results yet. And I don't think we care all that much for iommu_platform=on for old qemus. Regards, Halil