Re: [RFC PATCH] PVH: cleanup of p2m upon p2m destroy

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Tue, 17 Dec 2013 11:19:57 +0100
Tim Deegan <tim@xen.org> wrote:

> At 08:42 +0000 on 17 Dec (1387266152), Jan Beulich wrote:
> > >>> On 17.12.13 at 02:47, Mukesh Rathor <mukesh.rathor@oracle.com>
> > >>> wrote:
> > > When a controlling domain is destroyed, any p2m_is_foreign pages
> > > must release the refcnt gotten when the page was added to the p2m.
> > > 
> > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> > 
......
> > 
> > So it looks like you copied one of the two obvious bugs from
> > relinquish_shared_pages() _and_ deferred the preemption point
> > by quite a bit - 10,000 pages is quite a lot, the 512 used there
> > seems much more reasonable.
> > 
> > As to the copied bug: Should hypercall_preempt_check() return
> > false, you'd never again try to preempt.
> 
> Further, looping from 0 to max_mapped_pfn, even preemptibly, is not a
> good way to do this: the guest can set its own max_mapped_pfn, and we
> don't want Xen to spend its time counting to 2^63.

Ok, some p2m code is setting a bad precedent. 

An alternative might be to just create a link list then and walk it. In
general, foreign mappings should be very small, so the overhead of
16 bytes per page for the link list might not be too bad. I will code
it if there is no disagreement from any maintainer... everyone has 
different ideas :)...

Or better, if I add a count of foreign mappings and hang it off the 
p2m_domain, then this code would only execute in case of control
domain going away... it seems in that case walking the p2m would be
tolerable. That should be acceptable?

> Further further, it occurs to me that the refcounting change might
> interact badly with nested EPT, which creates and destroys p2m tables
> quite regularly.

nested ept is not supported on pvh.

thanks
mukesh