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 17:05 +0100, David Woodhouse wrote:
> On Wed, 2013-09-25 at 17:54 +0200, Joerg Roedel wrote:
> > On Sat, Sep 21, 2013 at 09:59:00PM -0500, David Woodhouse wrote:
> > > I *hate* the bizarre calling convention for iommu_unmap(). Is it
> > > actually clearly documented anywhere? Why on earth is it not just
> > > returning void, and expected to unmap what it was *asked* to unmap?
> > 
> > Yeah, I agree that this should be documented since it is quite
> > non-standard/non-obvious behaviour of a function. The reason the
> > interface was implemented this way is that the caller should not need to
> > know (or keep track of) the page-size which was used to map a given iova.
> 
> 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.

> And why are we not allowed to unmap more than a single page at a time?
> 
> Doing unmaps in small pieces (with a full flush between every one) is
> *horribly* slow.

While we call iommu_unmap with PAGE_SIZE, we certainly expect that
larger chunks will be used if they were mapped that way.

> > So the interface is basically, that you give an iova and a size and the
> > iommu driver unmaps _at_least_ a region of that size. But if you ask for
> > a 4k region which is mapped by a 2M page then the whole 2M are unmapped.
> > The return value tells you how much was actually unmapped.
> > 
> > Not the best interface, I know. We should come up with a better way to
> > handle this.
> 
> 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.  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?  We have the same problem with snoop control already.  There's
no atomic re-mapping interface, so KVM just pretends it can unmap and
remap without anyone noticing.  Thanks,

Alex