Subject: Re: [PATCH kernel v4 4/4] powerpc/mm/iommu, vfio/spapr: Put pages on VFIO container shutdown

From: David Gibson <[email-masked]>
Date: Tue, 8 Nov 2016 14:33:12 +1100

On Thu, Nov 03, 2016 at 12:02:28PM +1100, Paul Mackerras wrote:
> On Wed, Nov 02, 2016 at 01:44:03PM +1100, Alexey Kardashevskiy wrote:
> > On 31/10/16 15:23, David Gibson wrote:
> [...]
> > > 
> > > Um.. yeah.. that's not really ok.  Prohibiting overlapping
> > > registrations on the same container is reasonable enough.  Having a
> > > container not be able to register memory because some completely
> > > different container has registered something overlapping is getting
> > > very ugly.
> > 
> > I am lost here. Does this mean the patches cannot go upstream?
> > 
> > Also how would I implement overlapping if we are not teaching KVM about
> > VFIO containers? The mm list has a counter of how many times each memory
> > region was mapped via TCE (and this prevents unregistration), and if we
> > want overlapping regions - a "mapped" counter of which one would I update
> > in real mode (where I only have a user address and a LIOBN)?
> 
> The patches fix a real bug, where we run out of memory to run VMs.
> 
> The patches don't change the interface, and don't introduce the
> constraint that is being discussed here (that the regions being
> registered may not overlap unless they are identical to a previously
> registered region).  That constraint is already present in the
> upstream code.

Ah, good point.  I hadn't thought that through and realized that
limitation was already through.

> They do change the behaviour when you use a container fd from a
> different process from the one which opened the fd, but that is not
> something that worked in any meaningful way before anyway.
> 
> So David, do you still see any reason why the patches should not be
> accepted?

Given the (still hideously ugly) limitation is not new, then no, not
any more.  I'll send an R-b.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson