From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [V5 PATCH 6/7] pvh dom0: Add and remove foreign pages Date: Thu, 05 Dec 2013 13:12:57 +0000 Message-ID: <52A07BD9.7060003@linaro.org> References: <1386209128-5261-1-git-send-email-mukesh.rathor@oracle.com> <1386209128-5261-7-git-send-email-mukesh.rathor@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1386209128-5261-7-git-send-email-mukesh.rathor@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: Mukesh Rathor , Xen-devel@lists.xensource.com Cc: george.dunlap@eu.citrix.com, keir.xen@gmail.com, tim@xen.org, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org On 12/05/2013 02:05 AM, Mukesh Rathor wrote: > In this patch, a new function, xenmem_add_foreign_to_p2m(), is added > to map pages from foreign guest into current dom0 for domU creation. > Such pages are typed p2m_map_foreign. Also, support is added here to > XENMEM_remove_from_physmap to remove such pages. Note, in the remove > path, we must release the refcount that was taken during the map phase. > > Signed-off-by: Mukesh Rathor > --- > xen/arch/x86/mm.c | 88 +++++++++++++++++++++++++++++++++++++++++---- > xen/common/memory.c | 40 ++++++++++++++++++-- > xen/include/asm-arm/p2m.h | 2 + > 3 files changed, 119 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > index e3da479..1a4d564 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; > @@ -4520,9 +4520,75 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p) > return 0; > } > > +/* > + * 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 so it is not lost > + * while mapped here. The refcnt is released in do_memory_op() > + * via XENMEM_remove_from_physmap. > + * Returns: 0 ==> success > + */ > +static int xenmem_add_foreign_to_p2m(struct domain *tdom, unsigned long fgfn, > + unsigned long gpfn, struct domain *fdom) > +{ > + p2m_type_t p2mt, p2mt_prev; > + int rc = 0; > + unsigned long prev_mfn, mfn = 0; > + struct page_info *page = NULL; > + > + 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 = mfn_x(get_gfn(tdom, gpfn, &p2mt_prev)); > + if ( mfn_valid(prev_mfn) ) > + { > + if ( is_xen_heap_mfn(prev_mfn) ) > + /* Xen heap frames are simply unhooked from this phys slot */ > + guest_physmap_remove_page(tdom, gpfn, 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(mfn)) == 0 ) > + { > + gdprintk(XENLOG_WARNING, "set_foreign_p2m_entry failed. " > + "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n", > + gpfn, mfn, fgfn, tdom->domain_id, fdom->domain_id); > + put_page(page); > + rc = -EINVAL; > + } > + > + /* > + * 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; > +} > + > 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 +4647,13 @@ static int xenmem_add_to_physmap_once( > page = mfn_to_page(mfn); > break; > } > + > + case XENMAPSPACE_gmfn_foreign: > + { > + rc = xenmem_add_foreign_to_p2m(d, xatp->idx, xatp->gpfn, fdom); > + return rc; > + } > + > default: > break; > } > @@ -4646,7 +4719,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 +4745,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 +4767,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 +4854,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/common/memory.c b/xen/common/memory.c > index eb7b72b..ae11828 100644 > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -675,9 +675,11 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > > case XENMEM_remove_from_physmap: > { > + unsigned long mfn; > struct xen_remove_from_physmap xrfp; > struct page_info *page; > struct domain *d; > + p2m_type_t p2mt; > > if ( copy_from_guest(&xrfp, arg, 1) ) > return -EFAULT; > @@ -693,11 +695,41 @@ 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 ( page ) > + /* > + * if 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. See also xenmem_add_foreign_to_p2m(). > + */ > + page = get_page_from_gfn(d, xrfp.gpfn, &p2mt, P2M_ALLOC); Actually, on debug build, get_page_from_gfn on ARM will crash Xen. For now we have an ASSERT to check if the type (p2m_type_t *) is NULL. -- Julien Grall