Benjamin Herrenschmidt
2018-Aug-08 22:13 UTC
[RFC 0/4] Virtio uses DMA API for all devices
On Wed, 2018-08-08 at 23:31 +0300, Michael S. Tsirkin wrote:> On Wed, Aug 08, 2018 at 11:18:13PM +1000, Benjamin Herrenschmidt wrote: > > Sure, but all of this is just the configuration of the iommu. But I > > think we agree here, and your point remains valid, indeed my proposed > > hack: > > > > > if ((flags & VIRTIO_F_IOMMU_PLATFORM) || arch_virtio_wants_dma_ops()) > > > > Will only work if the IOMMU and non-IOMMU path are completely equivalent. > > > > We can provide that guarantee for our secure VM case, but not generally so if > > we were to go down the route of a quirk in virtio, it might be better to > > make it painfully obvious that it's specific to that one case with a different > > kind of turd: > > > > - if (xen_domain()) > > + if (xen_domain() || pseries_secure_vm()) > > return true; > > I don't think it's pseries specific actually. E.g. I suspect AMD SEV > might benefit from the same kind of hack.As long as they can provide the same guarantee that the DMA ops are completely equivalent between virtio and other PCI devices, at least on the same bus, ie, we don't have to go hack special DMA ops. I think the latter is really what Christoph wants to avoid for good reasons.> > So to summarize, and make sure I'm not missing something, the two approaches > > at hand are either: > > > > 1- The above, which is a one liner and contained in the guest, so that's nice, but > > also means another turd in virtio which isn't ... > > > > 2- We force pseries to always set VIRTIO_F_IOMMU_PLATFORM, but with the current > > architecture on our side that will force virtio to always go through an emulated > > iommu, as pseries doesn't have the concept of a real bypass window, and thus will > > impact performance for both secure and non-secure VMs. > > > > 3- Invent a property that can be put in selected PCI device tree nodes that > > indicates that for that device specifically, the iommu can be bypassed, along with > > a hypercall to turn that bypass on/off. Virtio would then use VIRTIO_F_IOMMU_PLATFORM > > but its DT nodes would also have that property and Linux would notice it and turn > > bypass on. > > For completeness, virtio could also have its own bounce buffer > outside of DMA API one. I don't see lots of benefits to this > though.Not fan of that either...> > The resulting properties of those options are: > > > > 1- Is what I want because it's the simplest, provides the best performance now, > > and works without code changes to qemu or non-secure Linux. However it does > > add a tiny turd to virtio which is annoying. > > > > 2- This works but it puts the iommu in the way always, thus reducing virtio performance > > accross the board for pseries unless we only do that for secure VMs but that is > > difficult (as discussed earlier). > > > > 3- This would recover the performance lost in -2-, however it requires qemu *and* > > guest changes. Specifically, existing guests (RHEL 7 etc...) would get the > > performance hit of -2- unless modified to call that 'enable bypass' call, which > > isn't great. > > > > So imho we have to chose one of 3 not-great solutions here... Unless I missed > > something in your ideas of course. > >
Benjamin Herrenschmidt
2018-Aug-09 02:00 UTC
[RFC 0/4] Virtio uses DMA API for all devices
On Thu, 2018-08-09 at 08:13 +1000, Benjamin Herrenschmidt wrote:> > For completeness, virtio could also have its own bounce buffer > > outside of DMA API one. I don't see lots of benefits to this > > though. > > Not fan of that either...To elaborate a bit ... For our secure VMs, we will need bounce buffering for everything anyway. virtio, emulated PCI, or vfio. By ensuring that we create an identity mapping in the IOMMU for the bounce buffering pool, we enable virtio "legacy/direct" to use the same mapping ops as things using the iommu. That said, we still need somewhere in arch/powerpc a set of dma ops which we'll attach to all PCI devices of a secure VM to force bouncing always, rather than just based on address (which is what the standard swiotlb ones do)... Unless we can tweak the swiotlb "threshold" for example by using an empty mask. We'll need the same set of DMA ops for VIO devices too, not just PCI. Cheers, Ben.
On Thu, Aug 09, 2018 at 08:13:32AM +1000, Benjamin Herrenschmidt wrote:> > > - if (xen_domain()) > > > + if (xen_domain() || pseries_secure_vm()) > > > return true; > > > > I don't think it's pseries specific actually. E.g. I suspect AMD SEV > > might benefit from the same kind of hack. > > As long as they can provide the same guarantee that the DMA ops are > completely equivalent between virtio and other PCI devices, at least on > the same bus, ie, we don't have to go hack special DMA ops. > > I think the latter is really what Christoph wants to avoid for good > reasons.Yes. I also generally want to avoid too much arch specific magic. FYI, I'm off to a week-long vacation today, don't expect quick replies.
On Thu, Sep 06, 2018 at 07:09:09PM -0500, Jiandi An wrote:> For virtio device we have to pass in iommu_platform=true flag for > this to set the VIRTIO_F_IOMMU_PLATFORM flag. But for example > QEMU has the use of iommu_platform attribute disabled for virtio-gpu > device. So would also like to move towards not having to specify > the VIRTIO_F_IOMMU_PLATFORM flag.Specifying VIRTIO_F_IOMMU_PLATFORM is the right thing for your platform given that you can't directly use physical addresses. Please fix qemu so that virtio-gpu works with VIRTIO_F_IOMMU_PLATFORM. Also just as I said for the power folks: you should really work with the qemu folks that VIRTIO_F_IOMMU_PLATFORM (or whatever we call the properly documented flag) can be set by default, and no pointless performance overhead is implied by having a sane and simple implementation.
> > this to set the VIRTIO_F_IOMMU_PLATFORM flag. But for example > > QEMU has the use of iommu_platform attribute disabled for virtio-gpu > > device. So would also like to move towards not having to specify > > the VIRTIO_F_IOMMU_PLATFORM flag. > > Specifying VIRTIO_F_IOMMU_PLATFORM is the right thing for your > platform given that you can't directly use physical addresses. > Please fix qemu so that virtio-gpu works with VIRTIO_F_IOMMU_PLATFORM.This needs both host and guest side changes btw. Guest side patch is in drm-misc (a3b815f09bb8) and should land in the next merge window. Host side patches are here: https://git.kraxel.org/cgit/qemu/log/?h=sirius/virtio-gpu-iommu Should also land in the next qemu version. cheers, Gerd
Possibly Parallel Threads
- [RFC 0/4] Virtio uses DMA API for all devices
- [PATCH V2 RFC] fixup! virtio: convert to use DMA api
- [PATCH V2 RFC] fixup! virtio: convert to use DMA api
- [PATCH V2 RFC] fixup! virtio: convert to use DMA api
- [RFC] virtio: Use DMA MAP API for devices without an IOMMU