[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [V6 PATCH 6.2/7] pvh dom0: Add and remove foreign pages
Hi, At 18:44 -0800 on 12 Dec (1386870289), Mukesh Rathor wrote: > In this patch, a new function, p2m_add_foreign(), is added > to map pages from foreign guest into current dom0 for domU creation. > Such pages are typed p2m_map_foreign. Another function > p2m_remove_foreign() is added to remove such pages. Note, it is the > nature of such pages that a refcnt is held during their stay in the p2m. > The refcnt is added and released in the low level ept code for convenience. This looks much better, thank you. I'm afraid it's still not ready to be committed though: > diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c > index 08d1d72..3b9f764 100644 > --- a/xen/arch/x86/mm/p2m-ept.c > +++ b/xen/arch/x86/mm/p2m-ept.c > @@ -36,8 +36,6 @@ > > #define atomic_read_ept_entry(__pepte) \ > ( (ept_entry_t) { .epte = read_atomic(&(__pepte)->epte) } ) > -#define atomic_write_ept_entry(__pepte, __epte) \ > - write_atomic(&(__pepte)->epte, (__epte).epte) > > #define is_epte_present(ept_entry) ((ept_entry)->epte & 0x7) > #define is_epte_superpage(ept_entry) ((ept_entry)->sp) > @@ -46,6 +44,26 @@ static inline bool_t is_epte_valid(ept_entry_t *e) > return (e->epte != 0 && e->sa_p2mt != p2m_invalid); > } > > +static inline void write_ept_entry(ept_entry_t *entryptr, ept_entry_t *new) > +{ > + if ( p2m_is_foreign(entryptr->sa_p2mt) ) > + put_page(mfn_to_page(entryptr->mfn)); The usual idiom for such things is to take the new ref first, then drop the old one. That way you don't have to worry about temporarily dropping a ref to zero (yes, I know the caller ought to have a ref too but it's better to be safe than sorry)... > + if ( p2m_is_foreign(new->sa_p2mt) ) > + { > + struct page_info *page; > + struct domain *fdom; > + > + ASSERT(mfn_valid(new->mfn)); > + ASSERT(new->mfn != entryptr->mfn); ...and you won't need this assertion. > + page = mfn_to_page(new->mfn); > + fdom = page_get_owner(page); > + get_page(page, fdom); > + } > + write_atomic(&entryptr->epte, new->epte); > +} Also, the non-EPT p2m code needs to have _something_ for this, even if it's just an error return on all p2m_foreign mappings (and a warning to console, and some more PVH fixme comments...) > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index 0659ef1..29f7b23 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -479,6 +479,8 @@ void p2m_teardown(struct p2m_domain *p2m) > /* Does not fail with ENOMEM given the DESTROY flag */ > BUG_ON(mem_sharing_unshare_page(d, gfn, > MEM_SHARING_DESTROY_GFN)); > } > + if ( p2m_is_foreign(t) ) > + put_page(mfn_to_page(mfn)); That's not going to work. Just above here there's an early exit if the domain has no shared pages. This loop should go away anyway, since as it says in the comment above it's a noop after the earlier teardown (and if there is work to do, a foreach(0..max) loop is definitely the wrong thing to do). So I'm afraid you can't hook your teardown operation here. You'll have to walk the actual p2m pages. Also, Jan may have an opinion about whether a teardown operation that has to walk each p2m entry would have to be made preemptible. I'm not sure where we draw the line on such things. Tim. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |