Re: [PATCH] On unmap, flush IOMMU TLB and return correct size

[Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Wed, 2013-09-25 at 19:52 +0100, David Woodhouse wrote:
> On Wed, 2013-09-25 at 11:36 -0600, Alex Williamson wrote:
> > On Wed, 2013-09-25 at 17:05 +0100, David Woodhouse wrote:
> > > Why would it ever care? If it *happens* to map something that can use
> > > large pages, yay!. If it subsequently breaks apart those large pages by
> > > unmapping 4KiB in the middle, let the IOMMU driver break that apart.
> > 
> > Can this be done atomically?  I thought part of the reason for this
> > interface was that iommu drivers typically couldn't replace a huge page
> > with multiple smaller pages in the presence of DMA.
> 
> For the Intel IOMMU it can. You can atomically change from a large page
> entry, to a pointer to a full set of smaller page tables. Do the IOTLB
> flush, and at no time is there an interruption in service.

Cool

> Not sure if this is true for *all* IOMMU hardware; I'd be perfectly
> happy to accept a variant of Jörg's proposal that we should only ever
> unmap exactly the same range that we mapped. Except we should allow the
> unmapping of adjacent regions together; just not a partial unmap of
> something that was mapped in one go.

Well, except if we've just trusted the IOMMU driver to add a device
behind a non-SP capable IOMMU to our domain and convert the page tables,
that partial unmap is no longer partial and now we get different
behavior than before so we can't depend on that adjacent unmapping.

> > > Seriously, just the address and the size. Let the IOMMU code deal with
> > > whether it can *actually* use large pages on the particular set of
> > > IOMMUs that are in use for the devices you've added to the domain so
> > > far.
> > 
> > You're contradicting yourself here.  Just let the iommu deal with it,
> > but now you're raising concerns that Intel may be mixing super page
> > IOMMU hardware with non-super page IOMMU hardware on the same box, so I
> > do need to be concerned, yet there's no interface to discover super page
> > support. 
> 
> No contradiction. Do Not Concern Yourself. Just tell the IOMMU what you
> want mapped, and where. Let *it* worry about whether it can use
> superpages or not. Like it already *does*.

I'd love to be able to do that, but...

> >  What happens if my IOMMU domain makes use of super pages and
> > then I add a new device behind a new IOMMU without hardware super page
> > support? 
> 
> Currently, you end up with the domain happily including superpages, and
> the less capable IOMMU that you added later won't cope.

This is the trouble with trusting the iommu driver. ;)

>  What we probably
> *ought* to do is walk the page tables and convert any pre-existing
> superpages to small pages, at the time we add the non-SP-capable IOMMU.

And then we need to figure out how to handle that in the proposed
interface changes above since it changes the unmap behavior to the naive
user.  There's also the question of whether the IOMMU driver should
re-evaluate super pages when the less capable IOMMU is removed from the
domain.

> FWIW we currently screw up the handling of cache-coherent vs.
> non-coherent page tables too. That one wants a wbinvd somewhere when we
> add a non-coherent IOMMU to the domain.

You're not selling the "trust the IOMMU driver" story very well here.
Can we assume that the IOMMU_CACHE flag (SNP) is ignored appropriately
by non-coherent IOMMUs?  Is there any downside to ignoring it and always
setting SNP in the IOMMU page tables?  AMD IOMMU ignores it, but it's
also always cache coherent.  Thanks,

Alex

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu