From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [V6 PATCH 6.2/7] pvh dom0: Add and remove foreign pages Date: Thu, 12 Dec 2013 18:44:49 -0800 Message-ID: <20131212184449.77cc02db@mantra.us.oracle.com> References: <1386297524-15483-1-git-send-email-mukesh.rathor@oracle.com> <1386297524-15483-7-git-send-email-mukesh.rathor@oracle.com> <20131210162753.2e402081@mantra.us.oracle.com> <20131210164442.3879f6c0@mantra.us.oracle.com> <52A7C14C.2020504@linaro.org> <20131210174755.05e5550f@mantra.us.oracle.com> <20131211142903.GB6450@deinos.phlegethon.org> <20131211184606.4f0d9366@mantra.us.oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20131211184606.4f0d9366@mantra.us.oracle.com> 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: jun.nakajima@intel.com, Xen-devel@lists.xensource.com, Ian Campbell , george.dunlap@eu.citrix.com, Julien Grall , eddie.dong@intel.com, keir.xen@gmail.com, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On Wed, 11 Dec 2013 18:46:06 -0800 Mukesh Rathor wrote: > On Wed, 11 Dec 2013 15:29:03 +0100 > Tim Deegan wrote: .......... > I'll have another version out hopefully tomorrow, with > get_page* and put_page* in ept path, and p2m_teardown fixed up, and > all tested. I'm thinking something along the lines of: > > ept_set_entry(): > ... > if (p2mt == foreign) > { > page = mfn_to_page(mfn); > fdom = page_get_owner(page); > get_page(page, fdom); > } > table = map_domain_page(pagetable_get_pfn(p2m_get_pagetable(p2m))); > ..... > > > thanks a lot, > Mukesh Ok, this is what I came up with, please lmk. Thanks. CCing Jun and Eddie for EPT changes. -Mukesh ------------ 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. Signed-off-by: Mukesh Rathor --- xen/arch/x86/mm.c | 23 +++++++--- xen/arch/x86/mm/p2m-ept.c | 30 +++++++++++--- xen/arch/x86/mm/p2m.c | 98 +++++++++++++++++++++++++++++++++++++++++++++ xen/common/memory.c | 12 +++++- xen/include/asm-arm/p2m.h | 8 +++- xen/include/asm-x86/p2m.h | 7 +++ 6 files changed, 163 insertions(+), 15 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index e3da479..6c2edc4 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -2810,7 +2810,7 @@ static struct domain *get_pg_owner(domid_t domid) goto out; } - if ( unlikely(paging_mode_translate(curr)) ) + if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) { MEM_LOG("Cannot mix foreign mappings with translated domains"); goto out; @@ -4522,7 +4522,8 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p) static int xenmem_add_to_physmap_once( struct domain *d, - const struct xen_add_to_physmap *xatp) + const struct xen_add_to_physmap *xatp, + struct domain *fdom) { struct page_info *page = NULL; unsigned long gfn = 0; /* gcc ... */ @@ -4581,6 +4582,13 @@ static int xenmem_add_to_physmap_once( page = mfn_to_page(mfn); break; } + + case XENMAPSPACE_gmfn_foreign: + { + rc = p2m_add_foreign(d, xatp->idx, xatp->gpfn, fdom); + return rc; + } + default: break; } @@ -4646,7 +4654,7 @@ static int xenmem_add_to_physmap(struct domain *d, start_xatp = *xatp; while ( xatp->size > 0 ) { - rc = xenmem_add_to_physmap_once(d, xatp); + rc = xenmem_add_to_physmap_once(d, xatp, NULL); if ( rc < 0 ) return rc; @@ -4672,11 +4680,12 @@ static int xenmem_add_to_physmap(struct domain *d, return rc; } - return xenmem_add_to_physmap_once(d, xatp); + return xenmem_add_to_physmap_once(d, xatp, NULL); } static int xenmem_add_to_physmap_range(struct domain *d, - struct xen_add_to_physmap_range *xatpr) + struct xen_add_to_physmap_range *xatpr, + struct domain *fdom) { /* Process entries in reverse order to allow continuations */ while ( xatpr->size > 0 ) @@ -4693,7 +4702,7 @@ static int xenmem_add_to_physmap_range(struct domain *d, xatp.space = xatpr->space; xatp.idx = idx; xatp.gpfn = gpfn; - rc = xenmem_add_to_physmap_once(d, &xatp); + rc = xenmem_add_to_physmap_once(d, &xatp, fdom); if ( copy_to_guest_offset(xatpr->errs, xatpr->size-1, &rc, 1) ) return -EFAULT; @@ -4780,7 +4789,7 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg) } rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d, fd); if ( rc == 0 ) - rc = xenmem_add_to_physmap_range(d, &xatpr); + rc = xenmem_add_to_physmap_range(d, &xatpr, fd); rcu_unlock_domain(d); if ( fd ) 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)); + + if ( p2m_is_foreign(new->sa_p2mt) ) + { + struct page_info *page; + struct domain *fdom; + + ASSERT(mfn_valid(new->mfn)); + ASSERT(new->mfn != entryptr->mfn); + page = mfn_to_page(new->mfn); + fdom = page_get_owner(page); + get_page(page, fdom); + } + write_atomic(&entryptr->epte, new->epte); +} + + static void ept_p2m_type_to_flags(ept_entry_t *entry, p2m_type_t type, p2m_access_t access) { /* First apply type permissions */ @@ -378,7 +396,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, ept_p2m_type_to_flags(&new_entry, p2mt, p2ma); } - atomic_write_ept_entry(ept_entry, new_entry); + write_ept_entry(ept_entry, &new_entry); } else { @@ -398,7 +416,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, /* now install the newly split ept sub-tree */ /* NB: please make sure domian is paused and no in-fly VT-d DMA. */ - atomic_write_ept_entry(ept_entry, split_ept_entry); + write_ept_entry(ept_entry, &split_ept_entry); /* then move to the level we want to make real changes */ for ( ; i > target; i-- ) @@ -428,7 +446,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, ept_p2m_type_to_flags(&new_entry, p2mt, p2ma); - atomic_write_ept_entry(ept_entry, new_entry); + write_ept_entry(ept_entry, &new_entry); } /* Track the highest gfn for which we have ever had a valid mapping */ @@ -665,7 +683,7 @@ static void ept_change_entry_type_page(mfn_t ept_page_mfn, int ept_page_level, e.sa_p2mt = nt; ept_p2m_type_to_flags(&e, nt, e.access); - atomic_write_ept_entry(&epte[i], e); + write_ept_entry(&epte[i], &e); } } 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)); } p2m->phys_table = pagetable_null(); @@ -1741,6 +1743,102 @@ out_p2m_audit: #endif /* P2M_AUDIT */ /* + * Add frames from foreign domain to target domain's physmap. Similar to + * XENMAPSPACE_gmfn but the frame is foreign being mapped into current, + * and is not removed from foreign domain. + * Usage: libxl on pvh dom0 creating a guest and doing privcmd_ioctl_mmap. + * Side Effect: the mfn for fgfn will be refcounted in lower level routines + * so it is not lost while mapped here. The refcnt is released + * via the p2m_remove_foreign path. + * Returns: 0 ==> success + */ +int p2m_add_foreign(struct domain *tdom, unsigned long fgfn, + unsigned long gpfn, struct domain *fdom) +{ + p2m_type_t p2mt, p2mt_prev; + mfn_t prev_mfn, mfn; + struct page_info *page; + int rc = 0; + + if ( tdom == fdom || !tdom || !fdom || !is_pvh_domain(tdom) ) + return -EINVAL; + + /* following will take a refcnt on the mfn */ + page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC); + if ( !page || !p2m_is_valid(p2mt) ) + { + if ( page ) + put_page(page); + return -EINVAL; + } + mfn = page_to_mfn(page); + + /* Remove previously mapped page if it is present. */ + prev_mfn = get_gfn(tdom, gpfn, &p2mt_prev); + if ( mfn_valid(prev_mfn) ) + { + if ( is_xen_heap_mfn(mfn_x(prev_mfn)) ) + /* Xen heap frames are simply unhooked from this phys slot */ + guest_physmap_remove_page(tdom, gpfn, mfn_x(prev_mfn), 0); + else + /* Normal domain memory is freed, to avoid leaking memory. */ + guest_remove_page(tdom, gpfn); + } + /* + * Create the new mapping. Can't use guest_physmap_add_page() because it + * will update the m2p table which will result in mfn -> gpfn of dom0 + * and not fgfn of domU. + */ + if ( set_foreign_p2m_entry(tdom, gpfn, mfn) == 0 ) + { + gdprintk(XENLOG_WARNING, "set_foreign_p2m_entry failed. " + "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n", + gpfn, mfn_x(mfn), fgfn, tdom->domain_id, fdom->domain_id); + rc = -EINVAL; + } + put_page(page); + + /* + * We must do this put_gfn after set_foreign_p2m_entry so another cpu + * doesn't populate the gpfn before us. + */ + put_gfn(tdom, gpfn); + + return rc; +} + +int p2m_remove_foreign(struct domain *d, unsigned long gpfn) +{ + mfn_t mfn; + p2m_type_t p2mt; + struct domain *foreign_dom; + + ASSERT(is_pvh_domain(d)); + + mfn = get_gfn_query(d, gpfn, &p2mt); + if ( unlikely(!p2m_is_foreign(p2mt)) ) + { + put_gfn(d, gpfn); + gdprintk(XENLOG_WARNING, "Invalid type for gpfn:%lx domid:%d t:%d\n", + gpfn, d->domain_id, p2mt); + return -EINVAL; + } + if ( unlikely(!mfn_valid(mfn)) ) + { + put_gfn(d, gpfn); + gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n", + gpfn, d->domain_id); + return -EINVAL; + } + + foreign_dom = page_get_owner(mfn_to_page(mfn)); + ASSERT(d != foreign_dom); + + guest_physmap_remove_page(d, gpfn, mfn_x(mfn), 0); + put_gfn(d, gpfn); + return 0; +} +/* * Local variables: * mode: C * c-file-style: "BSD" diff --git a/xen/common/memory.c b/xen/common/memory.c index eb7b72b..53f67c1 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -678,6 +678,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) struct xen_remove_from_physmap xrfp; struct page_info *page; struct domain *d; + p2m_type_t p2mt = INT_MAX; if ( copy_from_guest(&xrfp, arg, 1) ) return -EFAULT; @@ -693,12 +694,21 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return rc; } - page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC); + /* + * If autotranslate guest, (eg pvh), the gfn could be mapped to a mfn + * from foreign domain by the user space tool during domain creation. + * We need to check for that, free it up from the p2m, and release + * refcnt on it. In such a case, page would be NULL and the following + * call would not have refcnt'd the page. + */ + page = get_page_from_gfn(d, xrfp.gpfn, &p2mt, P2M_ALLOC); if ( page ) { guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0); put_page(page); } + else if ( p2m_is_foreign(p2mt) ) + rc = p2m_remove_foreign(d, xrfp.gpfn); else rc = -ENOENT; diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index c660820..a664950 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -83,7 +83,7 @@ static inline struct page_info *get_page_from_gfn( struct page_info *page; unsigned long mfn = gmfn_to_mfn(d, gfn); - ASSERT(t == NULL); + ASSERT(!t || *t == INT_MAX); if (!mfn_valid(mfn)) return NULL; @@ -110,6 +110,12 @@ static inline int get_page_and_type(struct page_info *page, return rc; } +#define p2m_is_foreign(_t) (0 && (_t)) +static inline int p2m_remove_foreign(struct domain *d, unsigned long gpfn) +{ + return -ENOSYS; +} + #endif /* _XEN_P2M_H */ /* diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index 6fc71a1..6371705 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -515,6 +515,13 @@ int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn); /* Set foreign mfn in the current guest's p2m table. */ int set_foreign_p2m_entry(struct domain *domp, unsigned long gfn, mfn_t mfn); +/* Add foreign mapping to the guest's p2m table. */ +int p2m_add_foreign(struct domain *tdom, unsigned long fgfn, + unsigned long gpfn, struct domain *fdom); + +/* Remove foreign mapping from the guest's p2m table. */ +int p2m_remove_foreign(struct domain *d, unsigned long gpfn); + /* * Populate-on-demand */ -- 1.7.2.3