Linux Virtualization - Sep 2018 - [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.
> >

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