From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [RFC PATCH] PVH: cleanup of p2m upon p2m destroy Date: Wed, 18 Dec 2013 18:01:55 -0800 Message-ID: <20131218180155.1dc09717@mantra.us.oracle.com> References: <20131216174728.2ba3ad9a@mantra.us.oracle.com> <52B01C88020000780010E042@nat28.tlf.novell.com> <20131217101957.GB32721@deinos.phlegethon.org> <20131217184412.2372eb45@mantra.us.oracle.com> <20131218100958.GB24792@deinos.phlegethon.org> <20131218165152.GO24792@deinos.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VtSw7-0000x6-TY for xen-devel@lists.xenproject.org; Thu, 19 Dec 2013 02:02:04 +0000 In-Reply-To: <20131218165152.GO24792@deinos.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: xen-devel , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Wed, 18 Dec 2013 17:51:52 +0100 Tim Deegan 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? No, not quite, because I need to know which mfns are foreign and do put_page on them. By changing things around a bit for change ept type, I came up with following: diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 0ba2365..c996aac 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -838,6 +838,80 @@ void setup_ept_dump(void) register_keyhandler('D', &ept_p2m_table); } +typedef int (ept_walk_entry_callback_f)(ept_entry_t *, unsigned long); + +static int ept_change_entry_type(ept_entry_t *entry, unsigned long data) +{ + p2m_type_t new = (p2m_type_t)data; + + entry->sa_p2mt = new; + ept_p2m_type_to_flags(entry, new, entry->access); + return 1; +} + +static int ept_put_foreign_mfn(ept_entry_t *entry, unsigned long data) +{ + put_page(mfn_to_page(entry->mfn)); + return 0; +} + +static void ept_walk_entry_callback_recurse( + mfn_t ept_page_mfn, int ept_page_level, p2m_type_t p2mt, + ept_walk_entry_callback_f *fp, unsigned long data) +{ + ept_entry_t e, *epte = map_domain_page(mfn_x(ept_page_mfn)); + + for ( int i = 0; i < EPT_PAGETABLE_ENTRIES; i++ ) + { + if ( !is_epte_valid(epte + i) ) + continue; + + if ( (ept_page_level > 0) && !is_epte_superpage(epte + i) ) + ept_walk_entry_callback_recurse(_mfn(epte[i].mfn), + ept_page_level - 1, p2mt, fp, data); + else + { + e = atomic_read_ept_entry(&epte[i]); + + if ( e.sa_p2mt == p2mt && fp(&e, data) ) + atomic_write_ept_entry(&epte[i], &e); + } + } + + unmap_domain_page(epte); +} + +static void ept_walk_entry_callback(struct p2m_domain *p2m, p2m_type_t p2mt, + ept_walk_entry_callback_f *fp, + unsigned long data, int do_flush) +{ + struct ept_data *ept = &p2m->ept; + + if ( ept_get_asr(ept) == 0 ) + return; + + ept_walk_entry_callback_recurse(_mfn(ept_get_asr(ept)), ept_get_wl(ept), + p2mt, fp, data); + if ( do_flush ) + ept_sync_domain(p2m); +} + +int ept_release_foreign_pages(struct p2m_domain *p2m) +{ + ept_walk_entry_callback(p2m, p2m_map_foreign, ept_put_foreign_mfn, 0, 0); + return 0; +} + +int ept_change_entry_type_global_mine(struct p2m_domain *p2m, + p2m_type_t ot, p2m_type_t nt) +{ + BUG_ON(p2m_is_grant(ot) || p2m_is_grant(nt)); + BUG_ON(ot != nt && (ot == p2m_mmio_direct || nt == p2m_mmio_direct)); + + ept_walk_entry_callback(p2m, ot, ept_change_entry_type, nt, 1); + return 0; +} + /* * Local variables: * mode: C I can further make this pre-emptible by returning some "handle" that the caller must stash somewhere, say, p2m_domain, so that at ept level, synchronization doesn't need to be concerned about. what do you think? thanks mukesh