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

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

On Wed, 18 Dec 2013 17:51:52 +0100
Tim Deegan <tim@xen.org> wrote:

> At 11:09 +0100 on 18 Dec (1387361398), Tim Deegan wrote:
> > > 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 :)...
> > 
> > I think it would be best to walk the p2m trie (i.e. bounded by
> > amount of RAM, rather than max GFN) and do it preemptably.  I'll
> > look into something like that for the mem_sharing loop today, and
> > foreign mapping code can reuse it.
> 
> What I've ended up with is making p2m_change_entry_type_global()
> preemptible (which is a bigger task but will be needed as domains get
> bigger).  Do you think that using that function to switch all mappings
> from p2m_foreign to p2m_invalid, appropriately late in the teardown,
> will be good enough for what you need?
> 
> Cheers,
> 
> Tim.

Finally, coming back to this, the answer is yes. Looks like all I need
to do is:

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 9faa663..268a8a2 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -470,6 +470,10 @@ void p2m_teardown(struct p2m_domain *p2m)
 
     p2m_lock(p2m);
 
+    /* pvh: we must release refcnt on all foreign pages */
+    if ( is_pvh_domain(d) )
+        p2m_change_entry_type_global(d, p2m_map_foreign, p2m_invalid);
+
     /* Try to unshare any remaining shared p2m entries. Safeguard
      * Since relinquish_shared_pages should have done the work. */ 
     for ( gfn=0; gfn < p2m->max_mapped_pfn; gfn++ )


In this call, the new atomic_write_ept_entry() will DTRT:

static inline void atomic_write_ept_entry(ept_entry_t *entryptr,
                                          const ept_entry_t *new)
{
    if ( p2m_is_foreign(new->sa_p2mt) )
    {
        struct page_info *page;
        struct domain *fdom;

        ASSERT(mfn_valid(new->mfn));
        page = mfn_to_page(new->mfn);
        fdom = page_get_owner(page);
        get_page(page, fdom);
    }
    if ( p2m_is_foreign(entryptr->sa_p2mt) )
        put_page(mfn_to_page(entryptr->mfn));

    write_atomic(&entryptr->epte, new->epte);
}


If that sounds ok, I'm ready to submit dom0 PVH patches. Pl lmk.

thanks
Mukesh