Re: [V6 PATCH 6.1/7] pvh dom0: Add and remove foreign pages

["Jan Beulich" <JBeulich@suse.com>]

>>> 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.

> +/* 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)) ?

> +
> +    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?

> +
> +    guest_physmap_remove_page(d, gpfn, mfn, 0);
> +    put_page(mfn_to_page(mfn));
> +    put_gfn(d, gpfn);
> +
> +    return 0;
> +}

Jan