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

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Wed, 11 Dec 2013 18:46:06 -0800
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Wed, 11 Dec 2013 15:29:03 +0100
> Tim Deegan <tim@xen.org> wrote:
..........
> I'll have another version out hopefully tomorrow, with
> get_page* and put_page* in ept path, and p2m_teardown fixed up, and
> all tested. I'm thinking something along the lines of:
> 
> ept_set_entry():
>    ...
>    if (p2mt == foreign)
>    {
>        page = mfn_to_page(mfn);
>        fdom = page_get_owner(page);
>        get_page(page, fdom);
>    }
>    table = map_domain_page(pagetable_get_pfn(p2m_get_pagetable(p2m)));
>    .....
> 
> 
> thanks a lot,
> Mukesh

Ok, this is what I came up with, please lmk. Thanks.
CCing Jun and Eddie for EPT changes.

-Mukesh

------------

In this patch, a new function, p2m_add_foreign(), is added
to map pages from foreign guest into current dom0 for domU creation.
Such pages are typed p2m_map_foreign. Another function
p2m_remove_foreign() is added to remove such pages. Note, it is the
nature of such pages that a refcnt is held during their stay in the p2m.
The refcnt is added and released in the low level ept code for convenience.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 xen/arch/x86/mm.c         |   23 +++++++---
 xen/arch/x86/mm/p2m-ept.c |   30 +++++++++++---
 xen/arch/x86/mm/p2m.c     |   98 +++++++++++++++++++++++++++++++++++++++++++++
 xen/common/memory.c       |   12 +++++-
 xen/include/asm-arm/p2m.h |    8 +++-
 xen/include/asm-x86/p2m.h |    7 +++
 6 files changed, 163 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index e3da479..6c2edc4 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;
@@ -4522,7 +4522,8 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
 
 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 +4582,13 @@ static int xenmem_add_to_physmap_once(
             page = mfn_to_page(mfn);
             break;
         }
+
+        case XENMAPSPACE_gmfn_foreign:
+        {
+            rc = p2m_add_foreign(d, xatp->idx, xatp->gpfn, fdom);
+            return rc;
+        }
+
         default:
             break;
     }
@@ -4646,7 +4654,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 +4680,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 +4702,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 +4789,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/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 08d1d72..3b9f764 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -36,8 +36,6 @@
 
 #define atomic_read_ept_entry(__pepte)                              \
     ( (ept_entry_t) { .epte = read_atomic(&(__pepte)->epte) } )
-#define atomic_write_ept_entry(__pepte, __epte)                     \
-    write_atomic(&(__pepte)->epte, (__epte).epte)
 
 #define is_epte_present(ept_entry)      ((ept_entry)->epte & 0x7)
 #define is_epte_superpage(ept_entry)    ((ept_entry)->sp)
@@ -46,6 +44,26 @@ static inline bool_t is_epte_valid(ept_entry_t *e)
     return (e->epte != 0 && e->sa_p2mt != p2m_invalid);
 }
 
+static inline void write_ept_entry(ept_entry_t *entryptr, ept_entry_t *new)
+{
+    if ( p2m_is_foreign(entryptr->sa_p2mt) )
+        put_page(mfn_to_page(entryptr->mfn));
+
+    if ( p2m_is_foreign(new->sa_p2mt) )
+    {
+        struct page_info *page;
+        struct domain *fdom;
+
+        ASSERT(mfn_valid(new->mfn));
+        ASSERT(new->mfn != entryptr->mfn);
+        page = mfn_to_page(new->mfn);
+        fdom = page_get_owner(page);
+        get_page(page, fdom);
+    }
+    write_atomic(&entryptr->epte, new->epte);
+}
+
+
 static void ept_p2m_type_to_flags(ept_entry_t *entry, p2m_type_t type, p2m_access_t access)
 {
     /* First apply type permissions */
@@ -378,7 +396,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn,
             ept_p2m_type_to_flags(&new_entry, p2mt, p2ma);
         }
 
-        atomic_write_ept_entry(ept_entry, new_entry);
+        write_ept_entry(ept_entry, &new_entry);
     }
     else
     {
@@ -398,7 +416,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn,
 
         /* now install the newly split ept sub-tree */
         /* NB: please make sure domian is paused and no in-fly VT-d DMA. */
-        atomic_write_ept_entry(ept_entry, split_ept_entry);
+        write_ept_entry(ept_entry, &split_ept_entry);
 
         /* then move to the level we want to make real changes */
         for ( ; i > target; i-- )
@@ -428,7 +446,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn,
 
         ept_p2m_type_to_flags(&new_entry, p2mt, p2ma);
 
-        atomic_write_ept_entry(ept_entry, new_entry);
+        write_ept_entry(ept_entry, &new_entry);
     }
 
     /* Track the highest gfn for which we have ever had a valid mapping */
@@ -665,7 +683,7 @@ static void ept_change_entry_type_page(mfn_t ept_page_mfn, int ept_page_level,
 
             e.sa_p2mt = nt;
             ept_p2m_type_to_flags(&e, nt, e.access);
-            atomic_write_ept_entry(&epte[i], e);
+            write_ept_entry(&epte[i], &e);
         }
     }
 
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 0659ef1..29f7b23 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -479,6 +479,8 @@ void p2m_teardown(struct p2m_domain *p2m)
             /* Does not fail with ENOMEM given the DESTROY flag */
             BUG_ON(mem_sharing_unshare_page(d, gfn, MEM_SHARING_DESTROY_GFN));
         }
+        if ( p2m_is_foreign(t) )
+            put_page(mfn_to_page(mfn));
     }
 
     p2m->phys_table = pagetable_null();
@@ -1741,6 +1743,102 @@ out_p2m_audit:
 #endif /* P2M_AUDIT */
 
 /*
+ * 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 in lower level routines
+ *              so it is not lost while mapped here. The refcnt is released 
+ *              via the p2m_remove_foreign path.
+ * Returns: 0 ==> success
+ */
+int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
+                    unsigned long gpfn, struct domain *fdom)
+{
+    p2m_type_t p2mt, p2mt_prev;
+    mfn_t prev_mfn, mfn;
+    struct page_info *page;
+    int rc = 0;
+
+    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 = get_gfn(tdom, gpfn, &p2mt_prev);
+    if ( mfn_valid(prev_mfn) )
+    {
+        if ( is_xen_heap_mfn(mfn_x(prev_mfn)) )
+            /* Xen heap frames are simply unhooked from this phys slot */
+            guest_physmap_remove_page(tdom, gpfn, mfn_x(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) == 0 )
+    {
+        gdprintk(XENLOG_WARNING, "set_foreign_p2m_entry failed. "
+                 "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
+                 gpfn, mfn_x(mfn), fgfn, tdom->domain_id, fdom->domain_id);
+        rc = -EINVAL;
+    }
+    put_page(page);
+
+    /*
+     * 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;
+}
+
+int p2m_remove_foreign(struct domain *d, unsigned long gpfn)
+{
+    mfn_t mfn;
+    p2m_type_t p2mt;
+    struct domain *foreign_dom;
+
+    ASSERT(is_pvh_domain(d));
+
+    mfn = get_gfn_query(d, gpfn, &p2mt);
+    if ( unlikely(!p2m_is_foreign(p2mt)) )
+    {
+        put_gfn(d, gpfn);
+        gdprintk(XENLOG_WARNING, "Invalid type for gpfn:%lx domid:%d t:%d\n",
+                 gpfn, d->domain_id, p2mt);
+        return -EINVAL;
+    }
+    if ( unlikely(!mfn_valid(mfn)) )
+    {
+        put_gfn(d, gpfn);
+        gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n",
+                 gpfn, d->domain_id);
+        return -EINVAL;
+    }
+
+    foreign_dom = page_get_owner(mfn_to_page(mfn));
+    ASSERT(d != foreign_dom);
+
+    guest_physmap_remove_page(d, gpfn, mfn_x(mfn), 0);
+    put_gfn(d, gpfn);
+    return 0;
+}
+/*
  * Local variables:
  * mode: C
  * c-file-style: "BSD"
diff --git a/xen/common/memory.c b/xen/common/memory.c
index eb7b72b..53f67c1 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -678,6 +678,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         struct xen_remove_from_physmap xrfp;
         struct page_info *page;
         struct domain *d;
+        p2m_type_t p2mt = INT_MAX;
 
         if ( copy_from_guest(&xrfp, arg, 1) )
             return -EFAULT;
@@ -693,12 +694,21 @@ 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 autotranslate guest, (eg 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.
+         */
+        page = get_page_from_gfn(d, xrfp.gpfn, &p2mt, P2M_ALLOC);
         if ( page )
         {
             guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
             put_page(page);
         }
+        else if ( p2m_is_foreign(p2mt) )
+            rc = p2m_remove_foreign(d, xrfp.gpfn);
         else
             rc = -ENOENT;
 
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index c660820..a664950 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -83,7 +83,7 @@ static inline struct page_info *get_page_from_gfn(
     struct page_info *page;
     unsigned long mfn = gmfn_to_mfn(d, gfn);
 
-    ASSERT(t == NULL);
+    ASSERT(!t || *t == INT_MAX);
 
     if (!mfn_valid(mfn))
         return NULL;
@@ -110,6 +110,12 @@ static inline int get_page_and_type(struct page_info *page,
     return rc;
 }
 
+#define p2m_is_foreign(_t) (0 && (_t))
+static inline int p2m_remove_foreign(struct domain *d, unsigned long gpfn)
+{
+    return -ENOSYS;
+}
+
 #endif /* _XEN_P2M_H */
 
 /*
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 6fc71a1..6371705 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -515,6 +515,13 @@ int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn);
 /* Set foreign mfn in the current guest's p2m table. */
 int set_foreign_p2m_entry(struct domain *domp, unsigned long gfn, mfn_t mfn);
 
+/* Add foreign mapping to the guest's p2m table. */
+int p2m_add_foreign(struct domain *tdom, unsigned long fgfn,
+                    unsigned long gpfn, struct domain *fdom);
+
+/* Remove foreign mapping from the guest's p2m table. */
+int p2m_remove_foreign(struct domain *d, unsigned long gpfn);
+
 /* 
  * Populate-on-demand
  */
-- 
1.7.2.3