All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5] x86/altp2m: Aggregate get entry and populate into common funcs
@ 2019-04-16  8:45 ` Alexandru Stefan ISAILA
  0 siblings, 0 replies; 36+ messages in thread
From: Alexandru Stefan ISAILA @ 2019-04-16  8:45 UTC (permalink / raw)
  To: xen-devel
  Cc: tamas, wei.liu2, rcojocaru, george.dunlap, andrew.cooper3,
	George Dunlap, jbeulich, Alexandru Stefan ISAILA, roger.pau

The code for getting the entry and then populating was repeated in
p2m_change_altp2m_gfn() and in p2m_set_altp2m_mem_access().

The code is now in one place with a bool param that lets the caller choose
if it populates after get_entry().

If remapping is being done then both the old and new gfn's should be
unshared in the hostp2m for keeping things consistent. The page type
of old_gfn was already checked whether it's p2m_ram_rw and bail if it
wasn't so functionality-wise this just simplifies things as a user
doesn't have to request unsharing manually before remapping.
Now, if the new_gfn is invalid it shouldn't query the hostp2m as
that is effectively a request to remove the entry from the altp2m.
But provided that scenario is used only when removing entries that
were previously remapped/copied to the altp2m, those entries already
went through P2M_ALLOC | P2M_UNSHARE before, so it won't have an
affect so the core function get_altp2m_entry() is calling
__get_gfn_type_access() with P2M_ALLOC | P2M_UNSHARE.

altp2m_get_entry_direct() is also called in p2m_set_suppress_ve()
because on a new altp2m view the function will fail with invalid mfn if
p2m->set_entry() was not called before.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>

---
Changes since V4:
	- Add altp2m to patch name
	- Change func name from get_altp2m_entry() to
altp2m_get_entry().
---
 xen/arch/x86/mm/mem_access.c | 30 ++-----------
 xen/arch/x86/mm/p2m.c        | 84 ++++++++++++++++++++----------------
 xen/include/asm-x86/p2m.h    | 17 ++++++++
 3 files changed, 66 insertions(+), 65 deletions(-)

diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index a144bb0ce4..ddfe0169c0 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -262,35 +262,11 @@ int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
     mfn_t mfn;
     p2m_type_t t;
     p2m_access_t old_a;
-    unsigned int page_order;
-    unsigned long gfn_l = gfn_x(gfn);
     int rc;
 
-    mfn = ap2m->get_entry(ap2m, gfn, &t, &old_a, 0, NULL, NULL);
-
-    /* Check host p2m if no valid entry in alternate */
-    if ( !mfn_valid(mfn) )
-    {
-
-        mfn = __get_gfn_type_access(hp2m, gfn_l, &t, &old_a,
-                                    P2M_ALLOC | P2M_UNSHARE, &page_order, 0);
-
-        rc = -ESRCH;
-        if ( !mfn_valid(mfn) || t != p2m_ram_rw )
-            return rc;
-
-        /* If this is a superpage, copy that first */
-        if ( page_order != PAGE_ORDER_4K )
-        {
-            unsigned long mask = ~((1UL << page_order) - 1);
-            gfn_t gfn2 = _gfn(gfn_l & mask);
-            mfn_t mfn2 = _mfn(mfn_x(mfn) & mask);
-
-            rc = ap2m->set_entry(ap2m, gfn2, mfn2, page_order, t, old_a, 1);
-            if ( rc )
-                return rc;
-        }
-    }
+    rc = altp2m_get_entry_prepopulate(ap2m, gfn, &mfn, &t, &old_a);
+    if ( rc )
+        return rc;
 
     /*
      * Inherit the old suppress #VE bit value if it is already set, or set it
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 9e81a30cc4..7bedfd593b 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -478,6 +478,43 @@ void p2m_unlock_and_tlb_flush(struct p2m_domain *p2m)
         mm_write_unlock(&p2m->lock);
 }
 
+int altp2m_get_entry(struct p2m_domain *ap2m,
+                            gfn_t gfn, mfn_t *mfn, p2m_type_t *t,
+                            p2m_access_t *a, bool prepopulate)
+{
+    *mfn = ap2m->get_entry(ap2m, gfn, t, a, 0, NULL, NULL);
+
+    /* Check host p2m if no valid entry in alternate */
+    if ( !mfn_valid(*mfn) && !p2m_is_hostp2m(ap2m) )
+    {
+        struct p2m_domain *hp2m = p2m_get_hostp2m(ap2m->domain);
+        unsigned int page_order;
+        int rc;
+
+        *mfn = __get_gfn_type_access(hp2m, gfn_x(gfn), t, a,
+                                     P2M_ALLOC | P2M_UNSHARE, &page_order, 0);
+
+        rc = -ESRCH;
+        if ( !mfn_valid(*mfn) || *t != p2m_ram_rw )
+            return rc;
+
+        /* If this is a superpage, copy that first */
+        if ( prepopulate && page_order != PAGE_ORDER_4K )
+        {
+            unsigned long mask = ~((1UL << page_order) - 1);
+            gfn_t gfn_aligned = _gfn(gfn_x(gfn) & mask);
+            mfn_t mfn_aligned = _mfn(mfn_x(*mfn) & mask);
+
+            rc = ap2m->set_entry(ap2m, gfn_aligned, mfn_aligned, page_order, *t, *a, 1);
+            if ( rc )
+                return rc;
+        }
+    }
+
+    return 0;
+}
+
+
 mfn_t __get_gfn_type_access(struct p2m_domain *p2m, unsigned long gfn_l,
                     p2m_type_t *t, p2m_access_t *a, p2m_query_t q,
                     unsigned int *page_order, bool_t locked)
@@ -2618,7 +2655,6 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
     p2m_access_t a;
     p2m_type_t t;
     mfn_t mfn;
-    unsigned int page_order;
     int rc = -EINVAL;
 
     if ( idx >= MAX_ALTP2M || d->arch.altp2m_eptp[idx] == mfn_x(INVALID_MFN) )
@@ -2630,47 +2666,21 @@ int p2m_change_altp2m_gfn(struct domain *d, unsigned int idx,
     p2m_lock(hp2m);
     p2m_lock(ap2m);
 
-    mfn = ap2m->get_entry(ap2m, old_gfn, &t, &a, 0, NULL, NULL);
-
     if ( gfn_eq(new_gfn, INVALID_GFN) )
     {
+        mfn = ap2m->get_entry(ap2m, old_gfn, &t, &a, 0, NULL, NULL);
         if ( mfn_valid(mfn) )
             p2m_remove_page(ap2m, gfn_x(old_gfn), mfn_x(mfn), PAGE_ORDER_4K);
         rc = 0;
         goto out;
     }
 
-    /* Check host p2m if no valid entry in alternate */
-    if ( !mfn_valid(mfn) )
-    {
-        mfn = __get_gfn_type_access(hp2m, gfn_x(old_gfn), &t, &a,
-                                    P2M_ALLOC, &page_order, 0);
-
-        if ( !mfn_valid(mfn) || t != p2m_ram_rw )
-            goto out;
-
-        /* If this is a superpage, copy that first */
-        if ( page_order != PAGE_ORDER_4K )
-        {
-            gfn_t gfn;
-            unsigned long mask;
-
-            mask = ~((1UL << page_order) - 1);
-            gfn = _gfn(gfn_x(old_gfn) & mask);
-            mfn = _mfn(mfn_x(mfn) & mask);
-
-            if ( ap2m->set_entry(ap2m, gfn, mfn, page_order, t, a, 1) )
-                goto out;
-        }
-    }
-
-    mfn = ap2m->get_entry(ap2m, new_gfn, &t, &a, 0, NULL, NULL);
-
-    if ( !mfn_valid(mfn) )
-        mfn = hp2m->get_entry(hp2m, new_gfn, &t, &a, 0, NULL, NULL);
+    rc = altp2m_get_entry_prepopulate(ap2m, old_gfn, &mfn, &t, &a);
+    if ( rc )
+        goto out;
 
-    /* Note: currently it is not safe to remap to a shared entry */
-    if ( !mfn_valid(mfn) || (t != p2m_ram_rw) )
+    rc = altp2m_get_entry_direct(ap2m, new_gfn, &mfn, &t, &a);
+    if ( rc )
         goto out;
 
     if ( !ap2m->set_entry(ap2m, old_gfn, mfn, PAGE_ORDER_4K, t, a,
@@ -3002,12 +3012,10 @@ int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve,
     if ( ap2m )
         p2m_lock(ap2m);
 
-    mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL, NULL);
-    if ( !mfn_valid(mfn) )
-    {
-        rc = -ESRCH;
+    rc = altp2m_get_entry_direct(p2m, gfn, &mfn, &t, &a);
+
+    if ( rc )
         goto out;
-    }
 
     rc = p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, t, a, suppress_ve);
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 2801a8ccca..8dc4353645 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -514,6 +514,23 @@ static inline unsigned long mfn_to_gfn(struct domain *d, mfn_t mfn)
         return mfn_x(mfn);
 }
 
+int altp2m_get_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
+                     p2m_type_t *t, p2m_access_t *a, bool prepopulate);
+
+static inline int altp2m_get_entry_direct(struct p2m_domain *ap2m,
+                                          gfn_t gfn, mfn_t *mfn,
+                                          p2m_type_t *t, p2m_access_t *a)
+{
+    return altp2m_get_entry(ap2m, gfn, mfn, t, a, false);
+}
+
+static inline int altp2m_get_entry_prepopulate(struct p2m_domain *ap2m,
+                                               gfn_t gfn, mfn_t *mfn,
+                                               p2m_type_t *t, p2m_access_t *a)
+{
+    return altp2m_get_entry(ap2m, gfn, mfn, t, a, true);
+}
+
 /* Deadlock-avoidance scheme when calling get_gfn on different gfn's */
 struct two_gfns {
     struct domain *first_domain, *second_domain;
-- 
2.17.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2019-04-23 13:27 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16  8:45 [PATCH v5] x86/altp2m: Aggregate get entry and populate into common funcs Alexandru Stefan ISAILA
2019-04-16  8:45 ` [Xen-devel] " Alexandru Stefan ISAILA
2019-04-16 13:44 ` Tamas K Lengyel
2019-04-16 13:44   ` [Xen-devel] " Tamas K Lengyel
2019-04-16 13:51   ` Andrew Cooper
2019-04-16 13:51     ` [Xen-devel] " Andrew Cooper
2019-04-16 13:58     ` George Dunlap
2019-04-16 13:58       ` [Xen-devel] " George Dunlap
2019-04-16 14:25       ` Tamas K Lengyel
2019-04-16 14:25         ` [Xen-devel] " Tamas K Lengyel
2019-04-16 14:01   ` George Dunlap
2019-04-16 14:01     ` [Xen-devel] " George Dunlap
2019-04-16 14:19     ` Tamas K Lengyel
2019-04-16 14:19       ` [Xen-devel] " Tamas K Lengyel
2019-04-16 15:07       ` George Dunlap
2019-04-16 15:07         ` [Xen-devel] " George Dunlap
2019-04-17  7:15         ` Alexandru Stefan ISAILA
2019-04-17  7:15           ` [Xen-devel] " Alexandru Stefan ISAILA
2019-04-17 18:22           ` Tamas K Lengyel
2019-04-17 18:22             ` [Xen-devel] " Tamas K Lengyel
2019-04-18  9:53             ` George Dunlap
2019-04-18  9:53               ` [Xen-devel] " George Dunlap
2019-04-18 13:59               ` Tamas K Lengyel
2019-04-18 13:59                 ` [Xen-devel] " Tamas K Lengyel
2019-04-18 17:02                 ` George Dunlap
2019-04-18 17:02                   ` [Xen-devel] " George Dunlap
2019-04-18 18:42                   ` Tamas K Lengyel
2019-04-18 18:42                     ` [Xen-devel] " Tamas K Lengyel
2019-04-19  8:32                     ` Alexandru Stefan ISAILA
2019-04-19  8:32                       ` [Xen-devel] " Alexandru Stefan ISAILA
2019-04-23 10:49                       ` Alexandru Stefan ISAILA
2019-04-23 10:49                         ` [Xen-devel] " Alexandru Stefan ISAILA
2019-04-23 11:44                       ` George Dunlap
2019-04-23 11:44                         ` [Xen-devel] " George Dunlap
2019-04-23 13:26                         ` Tamas K Lengyel
2019-04-23 13:26                           ` [Xen-devel] " Tamas K Lengyel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.