From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [hybrid]: code review for function mapping pfn to foreign mfn Date: Mon, 23 Apr 2012 18:37:09 -0700 Message-ID: <20120423183709.5636656f@mantra.us.oracle.com> References: <20120413182952.504e2775@mantra.us.oracle.com> <20120419141527.GB23663@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120419141527.GB23663@ocelot.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: Keir Fraser , "Xen-devel@lists.xensource.com" , Ian Campbell , "stefano.stabellini@eu.citrix.com" List-Id: xen-devel@lists.xenproject.org On Thu, 19 Apr 2012 15:15:27 +0100 Tim Deegan wrote: > At 18:29 -0700 on 13 Apr (1334341792), Mukesh Rathor wrote: > - AFAICT you're using set_mmio_p2m_entry and adding a new unmap > operation just to avoid having the m2p updated. Since you can't > rely on the unmap always happening through the new call (and you don't > enforce it anywhere), it would be better to add a new p2m_type > just for non-grant foreign mappings. Then you can gate the m2p > updates in the existing code on the map being normal RAM, as is > already done for p2m_is_grant(). Hi Tim, The variants of get_page* are confusing me, so wanna double check with you. I should be able to do something like following, right? /* add frames from foreign domain to current domain physmap. Similar to * XENMEM_add_to_physmap but the frame is foreign being mapped into current, * and is not removed from foreign domain. */ static long noinline _add_foreign_to_pmap_batch(struct xen_add_to_physmap *xpmp) { unsigned long rc, prev_mfn, mfn = 0; struct domain *fdom, *currd = current->domain; unsigned long fgmfn = xpmp->idx, gpfn = xpmp->gpfn; p2m_type_t p2mt; if ( (fdom = get_pg_owner(xpmp->foreign_domid)) == NULL ) { return -EPERM; } mfn = mfn_x(gfn_to_mfn_query(p2m_get_hostp2m(fdom), fgmfn, &p2mt)); if ( !p2m_is_valid(p2mt) ) { put_pg_owner(fdom); return -EINVAL; } if ( (rc=get_page_and_type_from_pagenr(mfn, PGT_writable_page,fdom,0,0)) ) { put_pg_owner(fdom); return rc; } /* Remove previously mapped page if it was present. */ prev_mfn = gmfn_to_mfn(currd, gpfn); 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(currd, gpfn, prev_mfn, 0); else /* Normal domain memory is freed, to avoid leaking memory. */ guest_remove_page(currd, gpfn); } if (set_foreign_p2m_entry(p2m_get_hostp2m(currd), gpfn, mfn) == 0) { printk("guest_physmap_add_page failed. gpfn:%lx mfn:%lx fgmfn:%lx\n", gpfn, mfn, fgmfn); rc = -EINVAL; ?????????????? } put_pg_owner(fdom); return rc; } /* Returns: True for success. 0 for failure */ int set_foreign_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn) { int rc = 0; p2m_type_t ot; mfn_t omfn; if ( !paging_mode_translate(p2m->domain) ) return 0; omfn = gfn_to_mfn_query(p2m, gfn, &ot); if (mfn_valid(omfn)) { gdprintk(XENLOG_ERR, "Already mapped mfn %lx at gfn:%lx\n", omfn, gfn); set_gpfn_from_mfn(mfn_x(omfn), INVALID_M2P_ENTRY); } P2M_DEBUG("set foreign %lx %lx\n", gfn, mfn_x(mfn)); p2m_lock(p2m); rc = set_p2m_entry(p2m, gfn, mfn, 0, p2m_map_foreign, p2m->default_access); audit_p2m(p2m, 1); p2m_unlock(p2m); if ( rc == 0 ) gdprintk(XENLOG_ERR, "set_foreign_p2m_entry: set_p2m_entry failed! gfn:%lx mfn=%08lx\n", gfn, mfn_x(gfn_to_mfn(p2m, gfn, &ot))); return rc; } thanks, Mukesh