[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM
From: |
Peter Xu |
Subject: |
Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM |
Date: |
Tue, 17 Mar 2020 10:13:16 -0400 |
On Tue, Mar 17, 2020 at 11:04:26AM +0800, Jason Wang wrote:
>
> On 2020/3/17 上午2:14, Peter Xu wrote:
> > On Mon, Mar 16, 2020 at 01:19:54PM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Mar 13, 2020 at 12:31:22PM -0400, Peter Xu wrote:
> > > > On Fri, Mar 13, 2020 at 11:29:59AM -0400, Michael S. Tsirkin wrote:
> > > > > 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?
> > > > Commit 076a93d797 was one of the patchset where we want to provide
> > > > sensible IOTLB entries and also that should start to work with huge
> > > > pages.
> > > So the issue bundamentally is that it
> > > never produces entries larger than page size.
> > >
> > > Wasteful even just with huge pages, all the more
> > > so which passthrough which could have giga-byte
> > > entries.
> > >
> > > Want to try fixing that?
> > Yes we can fix that, but I'm still not sure whether changing the
> > interface of address_space_get_iotlb_entry() to cover adhoc regions is
> > a good idea, because I think it's still a memory core API and imho it
> > would still be good to have IOTLBs returned to be what the hardware
> > will be using (always page aligned IOTLBs). Also it would still be
> > not ideal because vhost backend will still need to send the MISSING
> > messages and block for each of the continuous guest memory ranges
> > registered, so there will still be misterious delay. Not to say
> > logically all the caches can be invalidated too so in that sense I
> > think it's as hacky as the vhost speedup patch mentioned below..
> >
> > Ideally I think vhost should be able to know when PT is enabled or
> > disabled for the device, so the vhost backend (kernel or userspace)
> > should be able to directly use GPA for DMA. That might need some new
> > vhost interface.
>
>
> Yes but I think we don't need another API since we can send GPA->HVA mapping
> via device IOTLB API when we find there's no DMA translation at all (either
> PT or no vIOMMU).
Jason,
Do you mean what we've worked on before?
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg00574.html
(I just read the previous discussion on that patch, it seems to be
exactly what we've discussed again...)
Thanks,
>
> Vhost doesn't need to know whether an address is an IOVA (vIOMMU) , GPA (no
> vIOMMU), or even HVA (dpdk virtio-user).
>
> Thanks
>
>
> >
> > For the s390's specific issue, I would think Jason's patch an simple
> > and ideal solution already.
> >
> > Thanks,
> >
>
--
Peter Xu
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Halil Pasic, 2020/03/03
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Halil Pasic, 2020/03/13
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Michael S. Tsirkin, 2020/03/13
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Peter Xu, 2020/03/13
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Halil Pasic, 2020/03/16
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Peter Xu, 2020/03/16
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Michael S. Tsirkin, 2020/03/16
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Peter Xu, 2020/03/16
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Jason Wang, 2020/03/16
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM,
Peter Xu <=
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Jason Wang, 2020/03/17
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Michael S. Tsirkin, 2020/03/17
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Peter Xu, 2020/03/17
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Michael S. Tsirkin, 2020/03/17
- Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM, Brijesh Singh, 2020/03/13