Re: [Xen-devel] [V2 PATCH 7/8] pvh dom0: Add and remove foreign pages

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.11.13 at 01:03, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> +static int xenmem_add_foreign_to_pmap(unsigned long fgfn, unsigned long gpfn,
> +                                      domid_t foreign_domid)
> +{
> +    p2m_type_t p2mt, p2mt_prev;
> +    int rc = 0;
> +    unsigned long prev_mfn, mfn = 0;
> +    struct domain *fdom, *currd = current->domain;
> +    struct page_info *page = NULL;
> +
> +    if ( currd->domain_id == foreign_domid || foreign_domid == DOMID_SELF ||
> +         !is_pvh_domain(currd) )
> +        return -EINVAL;
> +
> +    if ( !is_control_domain(currd) ||
> +         (fdom = get_pg_owner(foreign_domid)) == NULL )
> +        return -EPERM;

Is this the right approach (i.e. shouldn't this be an XSM call)? Cc-ing
Daniel...

> +    /* 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);
> +        put_pg_owner(fdom);
> +        return -EINVAL;
> +    }
> +    mfn = page_to_mfn(page);
> +
> +    /* Remove previously mapped page if it is present. */
> +    prev_mfn = mfn_x(get_gfn(currd, 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(currd, gpfn, prev_mfn, 0);
> +        else
> +            /* Normal domain memory is freed, to avoid leaking memory. */
> +            guest_remove_page(currd, 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(currd, gpfn, _mfn(mfn)) == 0 )
> +    {
> +        dprintk(XENLOG_WARNING,
> +                "guest_physmap_add_page failed. gpfn:%lx mfn:%lx fgfn:%lx\n",
> +                gpfn, mfn, fgfn);

Either gdprintk() or printk(XENLOG_G_...). And to be useful this
needs to identify both the mapping domain (i.e. if not using
gdprink()) and the subject one.

> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -208,7 +208,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_mapping_t);
>  #define XENMAPSPACE_gmfn_range   3 /* GMFN range, XENMEM_add_to_physmap 
> only. */
>  #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom,
>                                      * XENMEM_add_to_physmap_range only.
> -                                    */
> +                                    * (PVH x86 only) */

Isn't this something that got introduced for ARM? If so, the way
you alter the comment is wrong.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel