From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [V6 PATCH 6.1/7] pvh dom0: Add and remove foreign
 pages
Date: Mon, 9 Dec 2013 17:30:18 -0800
Message-ID: <20131209173018.4585dfb7@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>
	<20131206183416.6c78f955@mantra.us.oracle.com>
	<52A5A074020000780010B4B5@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <52A5A074020000780010B4B5@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: george.dunlap@eu.citrix.com, Xen-devel@lists.xensource.com, keir.xen@gmail.com, tim@xen.org, Ian Campbell <Ian.Campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org

On Mon, 09 Dec 2013 09:50:28 +0000
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 07.12.13 at 03:34, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > New version of the patch with xenmem_rem_foreign_from_p2m() created:
> > 
> > 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. Another function
> > xenmem_rem_foreign_from_p2m() is added to remove such pages. Note,
> > in the remove path, we must release the refcount that was taken
> > during the map phase.
> 
> Wouldn't both of the new functions better go into arch/x86/mm/p2m.c
> (as already reflected by the declaration of the one that's currently
> not static being placed in p2m.h)? In any event, the p2m interaction
> here needs Tim's blessing.

ok, i can move them to p2m.c, perhaps rename to p2m_add_foreign...

> > +/* Note, the refcnt released here is taken in
> > xenmem_add_foreign_to_p2m */ +int
> > xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn) +{
> > +    unsigned long mfn;
> > +    p2m_type_t p2mt;
> > +    struct domain *foreign_dom;
> > +
> > +    mfn = mfn_x(get_gfn_query(d, gpfn, &p2mt));
> > +    if ( !mfn_valid(mfn) )
> > +    {
> > +        gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx
> > domid:%d\n",
> > +                 gpfn, d->domain_id);
> > +        return -EINVAL;
> > +    }
> 
> ASSERT(p2m_is_foreign(p2mt)) ?

Called for foreign only, but good idea to check for it in case of
other callers in future.

> > +
> > +    foreign_dom = page_get_owner(mfn_to_page(mfn));
> > +    ASSERT(d != foreign_dom);
> > +    ASSERT(is_pvh_domain(d));
> 
> Wouldn't this better be done first thing in the function?

ok.