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

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:

quoted

On 31/10/16 15:23, David Gibson wrote:

[...]

quoted

quoted

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