[PATCH 1/9] x86/np2m: refactor p2m_get_nestedp2m()

[PATCH 1/9] x86/np2m: refactor p2m_get_nestedp2m()

[George Dunlap]

From: Sergey Dyasli <sergey.dyasli@citrix.com>

1. Add a helper function assign_np2m()
2. Remove useless volatile
3. Update function's comment in the header
4. Minor style fixes ('\n' and d)

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/mm/p2m.c     | 31 ++++++++++++++++++-------------
 xen/include/asm-x86/p2m.h |  6 +++---
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 0b479105b9..27b90eb815 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1776,14 +1776,24 @@ p2m_flush_nestedp2m(struct domain *d)
         p2m_flush_table(d->arch.nested_p2m[i]);
 }
 
+static void assign_np2m(struct vcpu *v, struct p2m_domain *p2m)
+{
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    struct domain *d = v->domain;
+
+    /* Bring this np2m to the top of the LRU list */
+    p2m_getlru_nestedp2m(d, p2m);
+
+    nv->nv_flushp2m = 0;
+    nv->nv_p2m = p2m;
+    cpumask_set_cpu(v->processor, p2m->dirty_cpumask);
+}
+
 struct p2m_domain *
 p2m_get_nestedp2m(struct vcpu *v, uint64_t np2m_base)
 {
-    /* Use volatile to prevent gcc to cache nv->nv_p2m in a cpu register as
-     * this may change within the loop by an other (v)cpu.
-     */
-    volatile struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-    struct domain *d;
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    struct domain *d = v->domain;
     struct p2m_domain *p2m;
 
     /* Mask out low bits; this avoids collisions with P2M_BASE_EADDR */
@@ -1793,7 +1803,6 @@ p2m_get_nestedp2m(struct vcpu *v, uint64_t np2m_base)
         nv->nv_p2m = NULL;
     }
 
-    d = v->domain;
     nestedp2m_lock(d);
     p2m = nv->nv_p2m;
     if ( p2m ) 
@@ -1801,15 +1810,13 @@ p2m_get_nestedp2m(struct vcpu *v, uint64_t np2m_base)
         p2m_lock(p2m);
         if ( p2m->np2m_base == np2m_base || p2m->np2m_base == P2M_BASE_EADDR )
         {
-            nv->nv_flushp2m = 0;
-            p2m_getlru_nestedp2m(d, p2m);
-            nv->nv_p2m = p2m;
             if ( p2m->np2m_base == P2M_BASE_EADDR )
                 hvm_asid_flush_vcpu(v);
             p2m->np2m_base = np2m_base;
-            cpumask_set_cpu(v->processor, p2m->dirty_cpumask);
+            assign_np2m(v, p2m);
             p2m_unlock(p2m);
             nestedp2m_unlock(d);
+
             return p2m;
         }
         p2m_unlock(p2m);
@@ -1820,11 +1827,9 @@ p2m_get_nestedp2m(struct vcpu *v, uint64_t np2m_base)
     p2m = p2m_getlru_nestedp2m(d, NULL);
     p2m_flush_table(p2m);
     p2m_lock(p2m);
-    nv->nv_p2m = p2m;
     p2m->np2m_base = np2m_base;
-    nv->nv_flushp2m = 0;
     hvm_asid_flush_vcpu(v);
-    cpumask_set_cpu(v->processor, p2m->dirty_cpumask);
+    assign_np2m(v, p2m);
     p2m_unlock(p2m);
     nestedp2m_unlock(d);
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 10cdfc09a9..78f51a6ce6 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -359,9 +359,9 @@ struct p2m_domain {
 /* get host p2m table */
 #define p2m_get_hostp2m(d)      ((d)->arch.p2m)
 
-/* Get p2m table (re)usable for specified np2m base.
- * Automatically destroys and re-initializes a p2m if none found.
- * If np2m_base == 0 then v->arch.hvm_vcpu.guest_cr[3] is used.
+/*
+ * Assigns an np2m with the specified np2m_base to the specified vCPU
+ * and returns that np2m.
  */
 struct p2m_domain *p2m_get_nestedp2m(struct vcpu *v, uint64_t np2m_base);
 
-- 
2.14.1


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

[PATCH 2/9] x86/np2m: Have invept flush all np2m entries with the same base pointer

[George Dunlap]

nvmx_handle_invept() updates current's np2m just to flush it.  This is
not only wasteful, but ineffective: if several L2 vcpus share the same
np2m base pointer, they all need to be flushed (not only the current
one).

Introduce a new function, np2m_flush_base() which will flush all
shadow p2m's that match a given base pointer.

Convert p2m_flush_table() into p2m_flush_table_locked() in order not
to release the p2m_lock after np2m_base check.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
Changes since v1:
- Combine patches 2 and 3 ("x86/np2m: add np2m_flush_base()" and
    "x86/vvmx: use np2m_flush_base() for INVEPT_SINGLE_CONTEXT")
- Reword commit text

CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/vmx/vvmx.c |  7 +------
 xen/arch/x86/mm/p2m.c       | 35 +++++++++++++++++++++++++++++------
 xen/include/asm-x86/p2m.h   |  2 ++
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index cd0ee0a307..d333aa6d78 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1910,12 +1910,7 @@ int nvmx_handle_invept(struct cpu_user_regs *regs)
     {
     case INVEPT_SINGLE_CONTEXT:
     {
-        struct p2m_domain *p2m = p2m_get_nestedp2m(current, eptp);
-        if ( p2m )
-        {
-            p2m_flush(current, p2m);
-            ept_sync_domain(p2m);
-        }
+        np2m_flush_base(current, eptp);
         break;
     }
     case INVEPT_ALL_CONTEXT:
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 27b90eb815..b7588b2ec1 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1711,15 +1711,14 @@ p2m_getlru_nestedp2m(struct domain *d, struct p2m_domain *p2m)
     return p2m;
 }
 
-/* Reset this p2m table to be empty */
 static void
-p2m_flush_table(struct p2m_domain *p2m)
+p2m_flush_table_locked(struct p2m_domain *p2m)
 {
     struct page_info *top, *pg;
     struct domain *d = p2m->domain;
     mfn_t mfn;
 
-    p2m_lock(p2m);
+    ASSERT(p2m_locked_by_me(p2m));
 
     /*
      * "Host" p2m tables can have shared entries &c that need a bit more care
@@ -1732,10 +1731,7 @@ p2m_flush_table(struct p2m_domain *p2m)
 
     /* No need to flush if it's already empty */
     if ( p2m_is_nestedp2m(p2m) && p2m->np2m_base == P2M_BASE_EADDR )
-    {
-        p2m_unlock(p2m);
         return;
-    }
 
     /* This is no longer a valid nested p2m for any address space */
     p2m->np2m_base = P2M_BASE_EADDR;
@@ -1755,7 +1751,14 @@ p2m_flush_table(struct p2m_domain *p2m)
             d->arch.paging.free_page(d, pg);
     }
     page_list_add(top, &p2m->pages);
+}
 
+/* Reset this p2m table to be empty */
+static void
+p2m_flush_table(struct p2m_domain *p2m)
+{
+    p2m_lock(p2m);
+    p2m_flush_table_locked(p2m);
     p2m_unlock(p2m);
 }
 
@@ -1776,6 +1779,26 @@ p2m_flush_nestedp2m(struct domain *d)
         p2m_flush_table(d->arch.nested_p2m[i]);
 }
 
+void np2m_flush_base(struct vcpu *v, unsigned long np2m_base)
+{
+    struct domain *d = v->domain;
+    struct p2m_domain *p2m;
+    unsigned int i;
+
+    np2m_base &= ~(0xfffull);
+
+    nestedp2m_lock(d);
+    for ( i = 0; i < MAX_NESTEDP2M; i++ )
+    {
+        p2m = d->arch.nested_p2m[i];
+        p2m_lock(p2m);
+        if ( p2m->np2m_base == np2m_base )
+            p2m_flush_table_locked(p2m);
+        p2m_unlock(p2m);
+    }
+    nestedp2m_unlock(d);
+}
+
 static void assign_np2m(struct vcpu *v, struct p2m_domain *p2m)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 78f51a6ce6..5501ccc455 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -779,6 +779,8 @@ int p2m_pt_handle_deferred_changes(uint64_t gpa);
 void p2m_flush(struct vcpu *v, struct p2m_domain *p2m);
 /* Flushes all nested p2m tables */
 void p2m_flush_nestedp2m(struct domain *d);
+/* Flushes all np2m objects with the specified np2m_base */
+void np2m_flush_base(struct vcpu *v, unsigned long np2m_base);
 
 void nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
     l1_pgentry_t *p, l1_pgentry_t new, unsigned int level);
-- 
2.14.1


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

[PATCH 3/9] x86/np2m: remove np2m_base from p2m_get_nestedp2m()

[George Dunlap]

From: Sergey Dyasli <sergey.dyasli@citrix.com>

Remove np2m_base parameter as it should always match the value of
np2m_base in VMCX12.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c | 6 +++++-
 xen/arch/x86/hvm/vmx/vvmx.c      | 3 +--
 xen/arch/x86/mm/hap/nested_hap.c | 2 +-
 xen/arch/x86/mm/p2m.c            | 8 ++++----
 xen/include/asm-x86/p2m.h        | 5 ++---
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 66a1777298..1de896e456 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -412,7 +412,11 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu *v,
     ASSERT(v != NULL);
     ASSERT(vvmcb != NULL);
     ASSERT(n2vmcb != NULL);
-    p2m = p2m_get_nestedp2m(v, vvmcb->_h_cr3);
+
+    /* This will allow nsvm_vcpu_hostcr3() to return correct np2m_base */
+    vcpu_nestedsvm(v).ns_vmcb_hostcr3 = vvmcb->_h_cr3;
+
+    p2m = p2m_get_nestedp2m(v);
     n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
 }
 
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index d333aa6d78..2f468e6ced 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1109,8 +1109,7 @@ static void load_shadow_guest_state(struct vcpu *v)
 
 uint64_t get_shadow_eptp(struct vcpu *v)
 {
-    uint64_t np2m_base = nvmx_vcpu_eptp_base(v);
-    struct p2m_domain *p2m = p2m_get_nestedp2m(v, np2m_base);
+    struct p2m_domain *p2m = p2m_get_nestedp2m(v);
     struct ept_data *ept = &p2m->ept;
 
     ept->mfn = pagetable_get_pfn(p2m_get_pagetable(p2m));
diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index 162afed46b..ed137fa784 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -212,7 +212,7 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
     uint8_t p2ma_21 = p2m_access_rwx;
 
     p2m = p2m_get_hostp2m(d); /* L0 p2m */
-    nested_p2m = p2m_get_nestedp2m(v, nhvm_vcpu_p2m_base(v));
+    nested_p2m = p2m_get_nestedp2m(v);
 
     /* walk the L1 P2M table */
     rv = nestedhap_walk_L1_p2m(v, *L2_gpa, &L1_gpa, &page_order_21, &p2ma_21,
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index b7588b2ec1..d3e602de22 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1813,11 +1813,12 @@ static void assign_np2m(struct vcpu *v, struct p2m_domain *p2m)
 }
 
 struct p2m_domain *
-p2m_get_nestedp2m(struct vcpu *v, uint64_t np2m_base)
+p2m_get_nestedp2m(struct vcpu *v)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct domain *d = v->domain;
     struct p2m_domain *p2m;
+    uint64_t np2m_base = nhvm_vcpu_p2m_base(v);
 
     /* Mask out low bits; this avoids collisions with P2M_BASE_EADDR */
     np2m_base &= ~(0xfffull);
@@ -1865,7 +1866,7 @@ p2m_get_p2m(struct vcpu *v)
     if (!nestedhvm_is_n2(v))
         return p2m_get_hostp2m(v->domain);
 
-    return p2m_get_nestedp2m(v, nhvm_vcpu_p2m_base(v));
+    return p2m_get_nestedp2m(v);
 }
 
 unsigned long paging_gva_to_gfn(struct vcpu *v,
@@ -1880,13 +1881,12 @@ unsigned long paging_gva_to_gfn(struct vcpu *v,
         unsigned long l2_gfn, l1_gfn;
         struct p2m_domain *p2m;
         const struct paging_mode *mode;
-        uint64_t np2m_base = nhvm_vcpu_p2m_base(v);
         uint8_t l1_p2ma;
         unsigned int l1_page_order;
         int rv;
 
         /* translate l2 guest va into l2 guest gfn */
-        p2m = p2m_get_nestedp2m(v, np2m_base);
+        p2m = p2m_get_nestedp2m(v);
         mode = paging_get_nestedmode(v);
         l2_gfn = mode->gva_to_gfn(v, p2m, va, pfec);
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 5501ccc455..85874ab401 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -360,10 +360,9 @@ struct p2m_domain {
 #define p2m_get_hostp2m(d)      ((d)->arch.p2m)
 
 /*
- * Assigns an np2m with the specified np2m_base to the specified vCPU
- * and returns that np2m.
+ * Updates vCPU's n2pm to match its np2m_base in VMCX12 and returns that np2m.
  */
-struct p2m_domain *p2m_get_nestedp2m(struct vcpu *v, uint64_t np2m_base);
+struct p2m_domain *p2m_get_nestedp2m(struct vcpu *v);
 
 /* If vcpu is in host mode then behaviour matches p2m_get_hostp2m().
  * If vcpu is in guest mode then behaviour matches p2m_get_nestedp2m().
-- 
2.14.1


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

[PATCH 4/9] x86/np2m: Simplify nestedhvm_hap_nested_page_fault

[George Dunlap]

There is a possibility for nested_p2m to became stale between
nestedhvm_hap_nested_page_fault() and nestedhap_fix_p2m().  At the moment
this is handled by detecting such a race inside nestedhap_fix_p2m() and
special-casing it.

Instead, introduce p2m_get_nestedp2m_locked(), which will returned a
still-locked p2m.  This allows us to call nestedhap_fix_p2m() with the
lock held and remove the code detecting the special-case.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2:
- Merged patch 9 and 10 ("x86/np2m: add p2m_get_nestedp2m_locked()"
     and "x86/np2m: improve nestedhvm_hap_nested_page_fault()")
- Updated commit message
- Fix comment style in nestedhap_fix_p2m()

CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/mm/hap/nested_hap.c | 31 +++++++++++++------------------
 xen/arch/x86/mm/p2m.c            | 12 +++++++++---
 xen/include/asm-x86/p2m.h        |  2 ++
 3 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index ed137fa784..844b32f702 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -101,28 +101,23 @@ nestedhap_fix_p2m(struct vcpu *v, struct p2m_domain *p2m,
                   unsigned int page_order, p2m_type_t p2mt, p2m_access_t p2ma)
 {
     int rc = 0;
+    unsigned long gfn, mask;
+    mfn_t mfn;
+
     ASSERT(p2m);
     ASSERT(p2m->set_entry);
+    ASSERT(p2m_locked_by_me(p2m));
 
-    p2m_lock(p2m);
-
-    /* If this p2m table has been flushed or recycled under our feet, 
-     * leave it alone.  We'll pick up the right one as we try to 
-     * vmenter the guest. */
-    if ( p2m->np2m_base == nhvm_vcpu_p2m_base(v) )
-    {
-        unsigned long gfn, mask;
-        mfn_t mfn;
-
-        /* If this is a superpage mapping, round down both addresses
-         * to the start of the superpage. */
-        mask = ~((1UL << page_order) - 1);
+    /* 
+     * If this is a superpage mapping, round down both addresses to
+     * the start of the superpage.
+     */
+    mask = ~((1UL << page_order) - 1);
 
-        gfn = (L2_gpa >> PAGE_SHIFT) & mask;
-        mfn = _mfn((L0_gpa >> PAGE_SHIFT) & mask);
+    gfn = (L2_gpa >> PAGE_SHIFT) & mask;
+    mfn = _mfn((L0_gpa >> PAGE_SHIFT) & mask);
 
-        rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
-    }
+    rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
 
     p2m_unlock(p2m);
 
@@ -212,7 +207,6 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
     uint8_t p2ma_21 = p2m_access_rwx;
 
     p2m = p2m_get_hostp2m(d); /* L0 p2m */
-    nested_p2m = p2m_get_nestedp2m(v);
 
     /* walk the L1 P2M table */
     rv = nestedhap_walk_L1_p2m(v, *L2_gpa, &L1_gpa, &page_order_21, &p2ma_21,
@@ -278,6 +272,7 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
     p2ma_10 &= (p2m_access_t)p2ma_21;
 
     /* fix p2m_get_pagetable(nested_p2m) */
+    nested_p2m = p2m_get_nestedp2m_locked(v);
     nestedhap_fix_p2m(v, nested_p2m, *L2_gpa, L0_gpa, page_order_20,
         p2mt_10, p2ma_10);
 
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index d3e602de22..aa3182dec6 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1813,7 +1813,7 @@ static void assign_np2m(struct vcpu *v, struct p2m_domain *p2m)
 }
 
 struct p2m_domain *
-p2m_get_nestedp2m(struct vcpu *v)
+p2m_get_nestedp2m_locked(struct vcpu *v)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct domain *d = v->domain;
@@ -1838,7 +1838,6 @@ p2m_get_nestedp2m(struct vcpu *v)
                 hvm_asid_flush_vcpu(v);
             p2m->np2m_base = np2m_base;
             assign_np2m(v, p2m);
-            p2m_unlock(p2m);
             nestedp2m_unlock(d);
 
             return p2m;
@@ -1854,12 +1853,19 @@ p2m_get_nestedp2m(struct vcpu *v)
     p2m->np2m_base = np2m_base;
     hvm_asid_flush_vcpu(v);
     assign_np2m(v, p2m);
-    p2m_unlock(p2m);
     nestedp2m_unlock(d);
 
     return p2m;
 }
 
+struct p2m_domain *p2m_get_nestedp2m(struct vcpu *v)
+{
+    struct p2m_domain *p2m = p2m_get_nestedp2m_locked(v);
+    p2m_unlock(p2m);
+
+    return p2m;
+}
+
 struct p2m_domain *
 p2m_get_p2m(struct vcpu *v)
 {
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 85874ab401..4a1c10c130 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -363,6 +363,8 @@ struct p2m_domain {
  * Updates vCPU's n2pm to match its np2m_base in VMCX12 and returns that np2m.
  */
 struct p2m_domain *p2m_get_nestedp2m(struct vcpu *v);
+/* Similar to the above except that returned p2m is still write-locked */
+struct p2m_domain *p2m_get_nestedp2m_locked(struct vcpu *v);
 
 /* If vcpu is in host mode then behaviour matches p2m_get_hostp2m().
  * If vcpu is in guest mode then behaviour matches p2m_get_nestedp2m().
-- 
2.14.1


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

[PATCH 5/9] x86/vvmx: Make updating shadow EPTP value more efficient

[George Dunlap]

At the moment, the shadow EPTP value is written unconditionally in
ept_handle_violation().

Instead, write the value on vmentry to the guest; but only write it if
the value needs updating.

To detect this, add a flag to the nestedvcpu struct, stale_np2m, to
indicate when such an action is necessary.  Set it when the nested p2m
changes or when the np2m is flushed by an IPI, and clear it when we
write the new value.

Since an IPI invalidating the p2m may happen between
nvmx_switch_guest() and vmx_vmenter, but we can't perform the vmwrite
with interrupts disabled, check the flag just before entering the
guest and restart the vmentry if it's set.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2
- Merge patches 6, 7, and 14 ("x86/np2m: add stale_np2m flag",
    "x86/vvmx: restart nested vmentry in case of stale_np2m", and
    "x86/vvmx: remove EPTP write from ept_handle_violation()")
- Write to stale_np2m before the vmwrite to avoid a race

CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/nestedhvm.c   |  2 ++
 xen/arch/x86/hvm/vmx/entry.S   |  6 ++++++
 xen/arch/x86/hvm/vmx/vmx.c     | 14 +++++++-------
 xen/arch/x86/hvm/vmx/vvmx.c    | 20 ++++++++++++++++++++
 xen/arch/x86/mm/p2m.c          | 10 ++++++++--
 xen/include/asm-x86/hvm/vcpu.h |  1 +
 6 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/hvm/nestedhvm.c b/xen/arch/x86/hvm/nestedhvm.c
index f2f7469d86..74a464d162 100644
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -56,6 +56,7 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_vvmcxaddr = INVALID_PADDR;
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
+    nv->stale_np2m = false;
 
     hvm_asid_flush_vcpu_asid(&nv->nv_n2asid);
 
@@ -107,6 +108,7 @@ nestedhvm_flushtlb_ipi(void *info)
      */
     hvm_asid_flush_core();
     vcpu_nestedhvm(v).nv_p2m = NULL;
+    vcpu_nestedhvm(v).stale_np2m = true;
 }
 
 void
diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index 53eedc6363..9fb8f89220 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -79,6 +79,8 @@ UNLIKELY_END(realmode)
 
         mov  %rsp,%rdi
         call vmx_vmenter_helper
+        cmp  $0,%eax
+        jne .Lvmx_vmentry_restart
         mov  VCPU_hvm_guest_cr2(%rbx),%rax
 
         pop  %r15
@@ -117,6 +119,10 @@ ENTRY(vmx_asm_do_vmentry)
         GET_CURRENT(bx)
         jmp  .Lvmx_do_vmentry
 
+.Lvmx_vmentry_restart:
+        sti
+        jmp  .Lvmx_do_vmentry
+
 .Lvmx_goto_emulator:
         sti
         mov  %rsp,%rdi
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9cfa9b6965..c9a4111267 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3249,12 +3249,6 @@ static void ept_handle_violation(ept_qual_t q, paddr_t gpa)
     case 0:         // Unhandled L1 EPT violation
         break;
     case 1:         // This violation is handled completly
-        /*Current nested EPT maybe flushed by other vcpus, so need
-         * to re-set its shadow EPTP pointer.
-         */
-        if ( nestedhvm_vcpu_in_guestmode(current) &&
-                        nestedhvm_paging_mode_hap(current ) )
-            __vmwrite(EPT_POINTER, get_shadow_eptp(current));
         return;
     case -1:        // This vioaltion should be injected to L1 VMM
         vcpu_nestedhvm(current).nv_vmexit_pending = 1;
@@ -4203,13 +4197,17 @@ static void lbr_fixup(void)
         bdw_erratum_bdf14_fixup();
 }
 
-void vmx_vmenter_helper(const struct cpu_user_regs *regs)
+int vmx_vmenter_helper(const struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
     u32 new_asid, old_asid;
     struct hvm_vcpu_asid *p_asid;
     bool_t need_flush;
 
+    /* Shadow EPTP can't be updated here because irqs are disabled */
+     if ( nestedhvm_vcpu_in_guestmode(curr) && vcpu_nestedhvm(curr).stale_np2m )
+         return 1;
+
     if ( curr->domain->arch.hvm_domain.pi_ops.do_resume )
         curr->domain->arch.hvm_domain.pi_ops.do_resume(curr);
 
@@ -4270,6 +4268,8 @@ void vmx_vmenter_helper(const struct cpu_user_regs *regs)
     __vmwrite(GUEST_RIP,    regs->rip);
     __vmwrite(GUEST_RSP,    regs->rsp);
     __vmwrite(GUEST_RFLAGS, regs->rflags | X86_EFLAGS_MBS);
+
+    return 0;
 }
 
 /*
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 2f468e6ced..48e37158af 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1405,12 +1405,32 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
     vmsucceed(regs);
 }
 
+static void nvmx_eptp_update(void)
+{
+    if ( !nestedhvm_vcpu_in_guestmode(current) ||
+          vcpu_nestedhvm(current).nv_vmexit_pending ||
+         !vcpu_nestedhvm(current).stale_np2m ||
+         !nestedhvm_paging_mode_hap(current) )
+        return;
+
+    /*
+     * Interrupts are enabled here, so we need to clear stale_np2m
+     * before we do the vmwrite.  If we do it in the other order, an
+     * and IPI comes in changing the shadow eptp after the vmwrite,
+     * we'll complete the vmenter with a stale eptp value.
+     */
+    vcpu_nestedhvm(current).stale_np2m = false;
+    __vmwrite(EPT_POINTER, get_shadow_eptp(current));
+}
+
 void nvmx_switch_guest(void)
 {
     struct vcpu *v = current;
     struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
     struct cpu_user_regs *regs = guest_cpu_user_regs();
 
+    nvmx_eptp_update();
+
     /*
      * A pending IO emulation may still be not finished. In this case, no
      * virtual vmswitch is allowed. Or else, the following IO emulation will
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index aa3182dec6..fd48a3b9db 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1812,6 +1812,12 @@ static void assign_np2m(struct vcpu *v, struct p2m_domain *p2m)
     cpumask_set_cpu(v->processor, p2m->dirty_cpumask);
 }
 
+static void nvcpu_flush(struct vcpu *v)
+{
+    hvm_asid_flush_vcpu(v);
+    vcpu_nestedhvm(v).stale_np2m = true;
+}
+
 struct p2m_domain *
 p2m_get_nestedp2m_locked(struct vcpu *v)
 {
@@ -1835,7 +1841,7 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
         if ( p2m->np2m_base == np2m_base || p2m->np2m_base == P2M_BASE_EADDR )
         {
             if ( p2m->np2m_base == P2M_BASE_EADDR )
-                hvm_asid_flush_vcpu(v);
+                nvcpu_flush(v);
             p2m->np2m_base = np2m_base;
             assign_np2m(v, p2m);
             nestedp2m_unlock(d);
@@ -1851,7 +1857,7 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
     p2m_flush_table(p2m);
     p2m_lock(p2m);
     p2m->np2m_base = np2m_base;
-    hvm_asid_flush_vcpu(v);
+    nvcpu_flush(v);
     assign_np2m(v, p2m);
     nestedp2m_unlock(d);
 
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index 6c54773f1c..5cfa4b4aa4 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -115,6 +115,7 @@ struct nestedvcpu {
 
     bool_t nv_flushp2m; /* True, when p2m table must be flushed */
     struct p2m_domain *nv_p2m; /* used p2m table for this vcpu */
+    bool stale_np2m; /* True when p2m_base in VMCX02 is no longer valid */
 
     struct hvm_vcpu_asid nv_n2asid;
 
-- 
2.14.1


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

[PATCH 6/9] x86/np2m: Send flush IPIs only when a vcpu is actively using a shadow p2m

[George Dunlap]

Flush IPIs are sent to all cpus in a shadow p2m's dirty_cpumask when
updated.  This mask however is far to broad.  A pcpu's bit is set in
the cpumask when a vcpu runs on that pcpu, but is only cleared when a
flush happens.  This means that the IPI includes the current pcpu of
vcpus that are not currently running, and also includes any pcpu that
has ever had a vcpu use this p2m since the last flush (which in turn
will cause spurious invalidations if a different vcpu is using a
shadow p2m).

Avoid these IPIs by keeping closer track of where a p2m is being used,
and when a vcpu needs to be flushed:

- On schedule-out, clear v->processor in p2m->dirty_cpumask
- Add a 'generation' counter to the p2m and nestedvcpu structs to
  detect changes that would require re-loads on re-entry
- On schedule-in or p2m change:
  - Set v->processor in p2m->dirty_cpumask
  - flush the vcpu's nested p2m pointer (and update nv->generation) if
    the generation changed

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
Changes since v1:
- Combine patches 5 and 8, and the scheduling bits of patch 11
  ("x86/np2m: add np2m_generation", "x86/np2m: add np2m_schedule()",
   and "x86/np2m: implement sharing of np2m between vCPUs")
- Reword commit message

CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/domain.c          |  2 ++
 xen/arch/x86/hvm/nestedhvm.c   |  1 +
 xen/arch/x86/hvm/vmx/vvmx.c    |  3 +++
 xen/arch/x86/mm/p2m.c          | 55 +++++++++++++++++++++++++++++++++++++++++-
 xen/include/asm-x86/hvm/vcpu.h |  1 +
 xen/include/asm-x86/p2m.h      |  6 +++++
 6 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 466a1a2fac..35ea0d2418 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1668,6 +1668,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
     {
         _update_runstate_area(prev);
         vpmu_switch_from(prev);
+        np2m_schedule(NP2M_SCHEDLE_OUT);
     }
 
     if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm_vcpu.tm_list) )
@@ -1716,6 +1717,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
 
         /* Must be done with interrupts enabled */
         vpmu_switch_to(next);
+        np2m_schedule(NP2M_SCHEDLE_IN);
     }
 
     /* Ensure that the vcpu has an up-to-date time base. */
diff --git a/xen/arch/x86/hvm/nestedhvm.c b/xen/arch/x86/hvm/nestedhvm.c
index 74a464d162..ab50b2ab98 100644
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -57,6 +57,7 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
     nv->stale_np2m = false;
+    nv->np2m_generation = 0;
 
     hvm_asid_flush_vcpu_asid(&nv->nv_n2asid);
 
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 48e37158af..a6a558b460 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1367,6 +1367,9 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
          !(v->arch.hvm_vcpu.guest_efer & EFER_LMA) )
         shadow_to_vvmcs_bulk(v, ARRAY_SIZE(gpdpte_fields), gpdpte_fields);
 
+    /* This will clear current pCPU bit in p2m->dirty_cpumask */
+    np2m_schedule(NP2M_SCHEDLE_OUT);
+
     vmx_vmcs_switch(v->arch.hvm_vmx.vmcs_pa, nvcpu->nv_n1vmcx_pa);
 
     nestedhvm_vcpu_exit_guestmode(v);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index fd48a3b9db..3c6c486c00 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -73,6 +73,7 @@ static int p2m_initialise(struct domain *d, struct p2m_domain *p2m)
     p2m->p2m_class = p2m_host;
 
     p2m->np2m_base = P2M_BASE_EADDR;
+    p2m->np2m_generation = 0;
 
     for ( i = 0; i < ARRAY_SIZE(p2m->pod.mrp.list); ++i )
         p2m->pod.mrp.list[i] = gfn_x(INVALID_GFN);
@@ -1735,6 +1736,7 @@ p2m_flush_table_locked(struct p2m_domain *p2m)
 
     /* This is no longer a valid nested p2m for any address space */
     p2m->np2m_base = P2M_BASE_EADDR;
+    p2m->np2m_generation++;
 
     /* Make sure nobody else is using this p2m table */
     nestedhvm_vmcx_flushtlb(p2m);
@@ -1809,6 +1811,7 @@ static void assign_np2m(struct vcpu *v, struct p2m_domain *p2m)
 
     nv->nv_flushp2m = 0;
     nv->nv_p2m = p2m;
+    nv->np2m_generation = p2m->np2m_generation;
     cpumask_set_cpu(v->processor, p2m->dirty_cpumask);
 }
 
@@ -1840,7 +1843,9 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
         p2m_lock(p2m);
         if ( p2m->np2m_base == np2m_base || p2m->np2m_base == P2M_BASE_EADDR )
         {
-            if ( p2m->np2m_base == P2M_BASE_EADDR )
+            /* Check if np2m was flushed just before the lock */
+            if ( p2m->np2m_base == P2M_BASE_EADDR ||
+                 nv->np2m_generation != p2m->np2m_generation )
                 nvcpu_flush(v);
             p2m->np2m_base = np2m_base;
             assign_np2m(v, p2m);
@@ -1848,6 +1853,11 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
 
             return p2m;
         }
+        else
+        {
+            /* vCPU is switching from some other valid np2m */
+            cpumask_clear_cpu(v->processor, p2m->dirty_cpumask);
+        }
         p2m_unlock(p2m);
     }
 
@@ -1881,6 +1891,49 @@ p2m_get_p2m(struct vcpu *v)
     return p2m_get_nestedp2m(v);
 }
 
+void np2m_schedule(int dir)
+{
+    struct nestedvcpu *nv = &vcpu_nestedhvm(current);
+    struct p2m_domain *p2m;
+
+    ASSERT(dir == NP2M_SCHEDLE_IN || dir == NP2M_SCHEDLE_OUT);
+
+    if ( !nestedhvm_enabled(current->domain) ||
+         !nestedhvm_vcpu_in_guestmode(current) ||
+         !nestedhvm_paging_mode_hap(current) )
+        return;
+
+    p2m = nv->nv_p2m;
+    if ( p2m )
+    {
+        bool np2m_valid;
+
+        p2m_lock(p2m);
+        np2m_valid = p2m->np2m_base == nhvm_vcpu_p2m_base(current) &&
+                     nv->np2m_generation == p2m->np2m_generation;
+        if ( dir == NP2M_SCHEDLE_OUT && np2m_valid )
+        {
+            /*
+             * The np2m is up to date but this vCPU will no longer use it,
+             * which means there are no reasons to send a flush IPI.
+             */
+            cpumask_clear_cpu(current->processor, p2m->dirty_cpumask);
+        }
+        else if ( dir == NP2M_SCHEDLE_IN )
+        {
+            if ( !np2m_valid )
+            {
+                /* This vCPU's np2m was flushed while it was not runnable */
+                hvm_asid_flush_core();
+                vcpu_nestedhvm(current).nv_p2m = NULL;
+            }
+            else
+                cpumask_set_cpu(current->processor, p2m->dirty_cpumask);
+        }
+        p2m_unlock(p2m);
+    }
+}
+
 unsigned long paging_gva_to_gfn(struct vcpu *v,
                                 unsigned long va,
                                 uint32_t *pfec)
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index 5cfa4b4aa4..afe5ffc6b3 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -116,6 +116,7 @@ struct nestedvcpu {
     bool_t nv_flushp2m; /* True, when p2m table must be flushed */
     struct p2m_domain *nv_p2m; /* used p2m table for this vcpu */
     bool stale_np2m; /* True when p2m_base in VMCX02 is no longer valid */
+    uint64_t np2m_generation;
 
     struct hvm_vcpu_asid nv_n2asid;
 
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 4a1c10c130..8d4aa8c6bf 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -209,6 +209,7 @@ struct p2m_domain {
      * to set it to any other value. */
 #define P2M_BASE_EADDR     (~0ULL)
     uint64_t           np2m_base;
+    uint64_t           np2m_generation;
 
     /* Nested p2ms: linked list of n2pms allocated to this domain. 
      * The host p2m hasolds the head of the list and the np2ms are 
@@ -371,6 +372,11 @@ struct p2m_domain *p2m_get_nestedp2m_locked(struct vcpu *v);
  */
 struct p2m_domain *p2m_get_p2m(struct vcpu *v);
 
+#define NP2M_SCHEDLE_IN  0
+#define NP2M_SCHEDLE_OUT 1
+
+void np2m_schedule(int dir);
+
 static inline bool_t p2m_is_hostp2m(const struct p2m_domain *p2m)
 {
     return p2m->p2m_class == p2m_host;
-- 
2.14.1


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

[PATCH 7/9] x86/np2m: implement sharing of np2m between vCPUs

[George Dunlap]

From: Sergey Dyasli <sergey.dyasli@citrix.com>

At the moment, nested p2ms are not shared between vcpus even if they
share the same base pointer.

Modify p2m_get_nestedp2m() to allow sharing a np2m between multiple
vcpus with the same np2m_base (L1 np2m_base value in VMCX12).

If the current np2m doesn't match the current base pointer, first look
for another nested p2m in the same domain with the same base pointer,
before reclaiming one from the LRU.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
Changes since v1:
- Moved scheduling changes (generation check and dirty_cpumask clear)
  to a previous patch
- Reword commit message

CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/vmx/vvmx.c |  1 +
 xen/arch/x86/mm/p2m.c       | 26 ++++++++++++++++++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index a6a558b460..2218d4f1ff 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1201,6 +1201,7 @@ static void virtual_vmentry(struct cpu_user_regs *regs)
 
     /* Setup virtual ETP for L2 guest*/
     if ( nestedhvm_paging_mode_hap(v) )
+        /* This will setup the initial np2m for the nested vCPU */
         __vmwrite(EPT_POINTER, get_shadow_eptp(v));
     else
         __vmwrite(EPT_POINTER, get_host_eptp(v));
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 3c6c486c00..4eac61e95b 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1828,6 +1828,7 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
     struct domain *d = v->domain;
     struct p2m_domain *p2m;
     uint64_t np2m_base = nhvm_vcpu_p2m_base(v);
+    unsigned int i;
 
     /* Mask out low bits; this avoids collisions with P2M_BASE_EADDR */
     np2m_base &= ~(0xfffull);
@@ -1841,19 +1842,19 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
     if ( p2m ) 
     {
         p2m_lock(p2m);
-        if ( p2m->np2m_base == np2m_base || p2m->np2m_base == P2M_BASE_EADDR )
+        if ( p2m->np2m_base == np2m_base )
         {
             /* Check if np2m was flushed just before the lock */
-            if ( p2m->np2m_base == P2M_BASE_EADDR ||
-                 nv->np2m_generation != p2m->np2m_generation )
+            if ( nv->np2m_generation != p2m->np2m_generation )
                 nvcpu_flush(v);
+            /* np2m is up-to-date */
             p2m->np2m_base = np2m_base;
             assign_np2m(v, p2m);
             nestedp2m_unlock(d);
 
             return p2m;
         }
-        else
+        else if ( p2m->np2m_base != P2M_BASE_EADDR )
         {
             /* vCPU is switching from some other valid np2m */
             cpumask_clear_cpu(v->processor, p2m->dirty_cpumask);
@@ -1861,6 +1862,23 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
         p2m_unlock(p2m);
     }
 
+    /* Share a np2m if possible */
+    for ( i = 0; i < MAX_NESTEDP2M; i++ )
+    {
+        p2m = d->arch.nested_p2m[i];
+        p2m_lock(p2m);
+        if ( p2m->np2m_base == np2m_base )
+        {
+            nvcpu_flush(v);
+            p2m->np2m_base = np2m_base;
+            assign_np2m(v, p2m);
+            nestedp2m_unlock(d);
+
+            return p2m;
+        }
+        p2m_unlock(p2m);
+    }
+
     /* All p2m's are or were in use. Take the least recent used one,
      * flush it and reuse. */
     p2m = p2m_getlru_nestedp2m(d, NULL);
-- 
2.14.1


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

[PATCH 8/9] x86/np2m: refactor p2m_get_nestedp2m_locked()

[George Dunlap]

From: Sergey Dyasli <sergey.dyasli@citrix.com>

Remove some code duplication.

Suggested-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/mm/p2m.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 4eac61e95b..dee6d7f0f2 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1829,6 +1829,7 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
     struct p2m_domain *p2m;
     uint64_t np2m_base = nhvm_vcpu_p2m_base(v);
     unsigned int i;
+    bool needs_flush = true;
 
     /* Mask out low bits; this avoids collisions with P2M_BASE_EADDR */
     np2m_base &= ~(0xfffull);
@@ -1845,14 +1846,10 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
         if ( p2m->np2m_base == np2m_base )
         {
             /* Check if np2m was flushed just before the lock */
-            if ( nv->np2m_generation != p2m->np2m_generation )
-                nvcpu_flush(v);
+            if ( nv->np2m_generation == p2m->np2m_generation )
+                needs_flush = false;
             /* np2m is up-to-date */
-            p2m->np2m_base = np2m_base;
-            assign_np2m(v, p2m);
-            nestedp2m_unlock(d);
-
-            return p2m;
+            goto found;
         }
         else if ( p2m->np2m_base != P2M_BASE_EADDR )
         {
@@ -1867,15 +1864,10 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
     {
         p2m = d->arch.nested_p2m[i];
         p2m_lock(p2m);
+
         if ( p2m->np2m_base == np2m_base )
-        {
-            nvcpu_flush(v);
-            p2m->np2m_base = np2m_base;
-            assign_np2m(v, p2m);
-            nestedp2m_unlock(d);
+            goto found;
 
-            return p2m;
-        }
         p2m_unlock(p2m);
     }
 
@@ -1884,8 +1876,11 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
     p2m = p2m_getlru_nestedp2m(d, NULL);
     p2m_flush_table(p2m);
     p2m_lock(p2m);
+
+ found:
+    if ( needs_flush )
+        nvcpu_flush(v);
     p2m->np2m_base = np2m_base;
-    nvcpu_flush(v);
     assign_np2m(v, p2m);
     nestedp2m_unlock(d);
 
-- 
2.14.1


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

[PATCH 9/9] x86/np2m: add break to np2m_flush_eptp()

[George Dunlap]

From: Sergey Dyasli <sergey.dyasli@citrix.com>

Now that np2m sharing is implemented, there can be only one np2m object
with the same np2m_base. Break from loop if the required np2m was found
during np2m_flush_eptp().

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/mm/p2m.c     | 4 ++++
 xen/include/asm-x86/p2m.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index dee6d7f0f2..bcde4df93f 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1795,7 +1795,11 @@ void np2m_flush_base(struct vcpu *v, unsigned long np2m_base)
         p2m = d->arch.nested_p2m[i];
         p2m_lock(p2m);
         if ( p2m->np2m_base == np2m_base )
+        {
             p2m_flush_table_locked(p2m);
+            p2m_unlock(p2m);
+            break;
+        }
         p2m_unlock(p2m);
     }
     nestedp2m_unlock(d);
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 8d4aa8c6bf..f28ca5e169 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -786,7 +786,7 @@ int p2m_pt_handle_deferred_changes(uint64_t gpa);
 void p2m_flush(struct vcpu *v, struct p2m_domain *p2m);
 /* Flushes all nested p2m tables */
 void p2m_flush_nestedp2m(struct domain *d);
-/* Flushes all np2m objects with the specified np2m_base */
+/* Flushes the np2m specified by np2m_base (if it exists) */
 void np2m_flush_base(struct vcpu *v, unsigned long np2m_base);
 
 void nestedp2m_write_p2m_entry(struct p2m_domain *p2m, unsigned long gfn,
-- 
2.14.1


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

Re: [PATCH 5/9] x86/vvmx: Make updating shadow EPTP value more efficient

[Andrew Cooper]

On 29/09/17 16:01, George Dunlap wrote:
> @@ -4203,13 +4197,17 @@ static void lbr_fixup(void)
>          bdw_erratum_bdf14_fixup();
>  }
>  
> -void vmx_vmenter_helper(const struct cpu_user_regs *regs)
> +int vmx_vmenter_helper(const struct cpu_user_regs *regs)

What are the semantics of this call?  The result looks boolean, and
indicates that the vmentry should be aborted?

>  {
>      struct vcpu *curr = current;
>      u32 new_asid, old_asid;
>      struct hvm_vcpu_asid *p_asid;
>      bool_t need_flush;
>  
> +    /* Shadow EPTP can't be updated here because irqs are disabled */
> +     if ( nestedhvm_vcpu_in_guestmode(curr) && vcpu_nestedhvm(curr).stale_np2m )
> +         return 1;
> +
>      if ( curr->domain->arch.hvm_domain.pi_ops.do_resume )
>          curr->domain->arch.hvm_domain.pi_ops.do_resume(curr);
>  
> @@ -4270,6 +4268,8 @@ void vmx_vmenter_helper(const struct cpu_user_regs *regs)
>      __vmwrite(GUEST_RIP,    regs->rip);
>      __vmwrite(GUEST_RSP,    regs->rsp);
>      __vmwrite(GUEST_RFLAGS, regs->rflags | X86_EFLAGS_MBS);
> +
> +    return 0;
>  }
>  
>  /*
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 2f468e6ced..48e37158af 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1405,12 +1405,32 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
>      vmsucceed(regs);
>  }
>  
> +static void nvmx_eptp_update(void)
> +{

struct vcpu *curr = current; will most likely half the compiled size of
this function.

> +    if ( !nestedhvm_vcpu_in_guestmode(current) ||
> +          vcpu_nestedhvm(current).nv_vmexit_pending ||
> +         !vcpu_nestedhvm(current).stale_np2m ||
> +         !nestedhvm_paging_mode_hap(current) )
> +        return;
> +
> +    /*
> +     * Interrupts are enabled here, so we need to clear stale_np2m
> +     * before we do the vmwrite.  If we do it in the other order, an
> +     * and IPI comes in changing the shadow eptp after the vmwrite,
> +     * we'll complete the vmenter with a stale eptp value.
> +     */
> +    vcpu_nestedhvm(current).stale_np2m = false;
> +    __vmwrite(EPT_POINTER, get_shadow_eptp(current));
> +}
> +
>  void nvmx_switch_guest(void)
>  {
>      struct vcpu *v = current;
>      struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
>      struct cpu_user_regs *regs = guest_cpu_user_regs();
>  
> +    nvmx_eptp_update();
> +
>      /*
>       * A pending IO emulation may still be not finished. In this case, no
>       * virtual vmswitch is allowed. Or else, the following IO emulation will
> diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
> index 6c54773f1c..5cfa4b4aa4 100644
> --- a/xen/include/asm-x86/hvm/vcpu.h
> +++ b/xen/include/asm-x86/hvm/vcpu.h
> @@ -115,6 +115,7 @@ struct nestedvcpu {
>  
>      bool_t nv_flushp2m; /* True, when p2m table must be flushed */
>      struct p2m_domain *nv_p2m; /* used p2m table for this vcpu */
> +    bool stale_np2m; /* True when p2m_base in VMCX02 is no longer valid */

VMCx02 ? which helps distinguish the two parts of semantic information
encoded there, and to avoid looking like we've gained a third acronym.

~Andrew

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

Re: [PATCH 2/9] x86/np2m: Have invept flush all np2m entries with the same base pointer

[Sergey Dyasli]

On Fri, 2017-09-29 at 16:01 +0100, George Dunlap wrote:
> nvmx_handle_invept() updates current's np2m just to flush it.  This is
> not only wasteful, but ineffective: if several L2 vcpus share the same
> np2m base pointer, they all need to be flushed (not only the current
> one).

I don't follow this completely. L1 will use INVEPT on each vCPU that
shares the same np2m pointer. The main idea here was not to update
current's np2m just to flush it.

> 
> Introduce a new function, np2m_flush_base() which will flush all
> shadow p2m's that match a given base pointer.
> 
> Convert p2m_flush_table() into p2m_flush_table_locked() in order not
> to release the p2m_lock after np2m_base check.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> Changes since v1:
> - Combine patches 2 and 3 ("x86/np2m: add np2m_flush_base()" and
>     "x86/vvmx: use np2m_flush_base() for INVEPT_SINGLE_CONTEXT")
> - Reword commit text
> 
-- 
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 4/9] x86/np2m: Simplify nestedhvm_hap_nested_page_fault

[Sergey Dyasli]

One comment below.

On Fri, 2017-09-29 at 16:01 +0100, George Dunlap wrote:
> There is a possibility for nested_p2m to became stale between
> nestedhvm_hap_nested_page_fault() and nestedhap_fix_p2m().  At the moment
> this is handled by detecting such a race inside nestedhap_fix_p2m() and
> special-casing it.
> 
> Instead, introduce p2m_get_nestedp2m_locked(), which will returned a
> still-locked p2m.  This allows us to call nestedhap_fix_p2m() with the
> lock held and remove the code detecting the special-case.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> ---
> v2:
> - Merged patch 9 and 10 ("x86/np2m: add p2m_get_nestedp2m_locked()"
>      and "x86/np2m: improve nestedhvm_hap_nested_page_fault()")
> - Updated commit message
> - Fix comment style in nestedhap_fix_p2m()
> 
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Jun Nakajima <jun.nakajima@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> ---
>  xen/arch/x86/mm/hap/nested_hap.c | 31 +++++++++++++------------------
>  xen/arch/x86/mm/p2m.c            | 12 +++++++++---
>  xen/include/asm-x86/p2m.h        |  2 ++
>  3 files changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
> index ed137fa784..844b32f702 100644
> --- a/xen/arch/x86/mm/hap/nested_hap.c
> +++ b/xen/arch/x86/mm/hap/nested_hap.c
> @@ -101,28 +101,23 @@ nestedhap_fix_p2m(struct vcpu *v, struct p2m_domain *p2m,
>                    unsigned int page_order, p2m_type_t p2mt, p2m_access_t p2ma)
>  {
>      int rc = 0;
> +    unsigned long gfn, mask;
> +    mfn_t mfn;
> +
>      ASSERT(p2m);
>      ASSERT(p2m->set_entry);
> +    ASSERT(p2m_locked_by_me(p2m));
>  
> -    p2m_lock(p2m);
> -
> -    /* If this p2m table has been flushed or recycled under our feet, 
> -     * leave it alone.  We'll pick up the right one as we try to 
> -     * vmenter the guest. */
> -    if ( p2m->np2m_base == nhvm_vcpu_p2m_base(v) )
> -    {
> -        unsigned long gfn, mask;
> -        mfn_t mfn;
> -
> -        /* If this is a superpage mapping, round down both addresses
> -         * to the start of the superpage. */
> -        mask = ~((1UL << page_order) - 1);
> +    /* 
> +     * If this is a superpage mapping, round down both addresses to
> +     * the start of the superpage.
> +     */
> +    mask = ~((1UL << page_order) - 1);
>  
> -        gfn = (L2_gpa >> PAGE_SHIFT) & mask;
> -        mfn = _mfn((L0_gpa >> PAGE_SHIFT) & mask);
> +    gfn = (L2_gpa >> PAGE_SHIFT) & mask;
> +    mfn = _mfn((L0_gpa >> PAGE_SHIFT) & mask);
>  
> -        rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
> -    }
> +    rc = p2m_set_entry(p2m, gfn, mfn, page_order, p2mt, p2ma);
>  
>      p2m_unlock(p2m);

I have the following fixup: move p2m_unlock() out of nestedhap_fix_p2m()
for balanced lock/unlock.

>  
> @@ -212,7 +207,6 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
>      uint8_t p2ma_21 = p2m_access_rwx;
>  
>      p2m = p2m_get_hostp2m(d); /* L0 p2m */
> -    nested_p2m = p2m_get_nestedp2m(v);
>  
>      /* walk the L1 P2M table */
>      rv = nestedhap_walk_L1_p2m(v, *L2_gpa, &L1_gpa, &page_order_21, &p2ma_21,
> @@ -278,6 +272,7 @@ nestedhvm_hap_nested_page_fault(struct vcpu *v, paddr_t *L2_gpa,
>      p2ma_10 &= (p2m_access_t)p2ma_21;
>  
>      /* fix p2m_get_pagetable(nested_p2m) */
> +    nested_p2m = p2m_get_nestedp2m_locked(v);
>      nestedhap_fix_p2m(v, nested_p2m, *L2_gpa, L0_gpa, page_order_20,
>          p2mt_10, p2ma_10);
>  
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index d3e602de22..aa3182dec6 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1813,7 +1813,7 @@ static void assign_np2m(struct vcpu *v, struct p2m_domain *p2m)
>  }
>  
>  struct p2m_domain *
> -p2m_get_nestedp2m(struct vcpu *v)
> +p2m_get_nestedp2m_locked(struct vcpu *v)
>  {
>      struct nestedvcpu *nv = &vcpu_nestedhvm(v);
>      struct domain *d = v->domain;
> @@ -1838,7 +1838,6 @@ p2m_get_nestedp2m(struct vcpu *v)
>                  hvm_asid_flush_vcpu(v);
>              p2m->np2m_base = np2m_base;
>              assign_np2m(v, p2m);
> -            p2m_unlock(p2m);
>              nestedp2m_unlock(d);
>  
>              return p2m;
> @@ -1854,12 +1853,19 @@ p2m_get_nestedp2m(struct vcpu *v)
>      p2m->np2m_base = np2m_base;
>      hvm_asid_flush_vcpu(v);
>      assign_np2m(v, p2m);
> -    p2m_unlock(p2m);
>      nestedp2m_unlock(d);
>  
>      return p2m;
>  }
>  
> +struct p2m_domain *p2m_get_nestedp2m(struct vcpu *v)
> +{
> +    struct p2m_domain *p2m = p2m_get_nestedp2m_locked(v);
> +    p2m_unlock(p2m);
> +
> +    return p2m;
> +}
> +
>  struct p2m_domain *
>  p2m_get_p2m(struct vcpu *v)
>  {
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index 85874ab401..4a1c10c130 100644
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -363,6 +363,8 @@ struct p2m_domain {
>   * Updates vCPU's n2pm to match its np2m_base in VMCX12 and returns that np2m.
>   */
>  struct p2m_domain *p2m_get_nestedp2m(struct vcpu *v);
> +/* Similar to the above except that returned p2m is still write-locked */
> +struct p2m_domain *p2m_get_nestedp2m_locked(struct vcpu *v);
>  
>  /* If vcpu is in host mode then behaviour matches p2m_get_hostp2m().
>   * If vcpu is in guest mode then behaviour matches p2m_get_nestedp2m().
-- 
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/9] x86/np2m: Have invept flush all np2m entries with the same base pointer

[George Dunlap]

On 10/02/2017 10:37 AM, Sergey Dyasli wrote:
> On Fri, 2017-09-29 at 16:01 +0100, George Dunlap wrote:
>> nvmx_handle_invept() updates current's np2m just to flush it.  This is
>> not only wasteful, but ineffective: if several L2 vcpus share the same
>> np2m base pointer, they all need to be flushed (not only the current
>> one).
> 
> I don't follow this completely. L1 will use INVEPT on each vCPU that
> shares the same np2m pointer. The main idea here was not to update
> current's np2m just to flush it.

Hmm, yes the INVEPT thing is true.  But if that's the case, why do we
need np2m_flush_base() to loop over the whole list and flush all np2ms
with the same pointer?

 -George

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

Re: [PATCH 5/9] x86/vvmx: Make updating shadow EPTP value more efficient

[Sergey Dyasli]

On Fri, 2017-09-29 at 16:56 +0100, Andrew Cooper wrote:
> On 29/09/17 16:01, George Dunlap wrote:
> > @@ -4203,13 +4197,17 @@ static void lbr_fixup(void)
> >          bdw_erratum_bdf14_fixup();
> >  }
> >  
> > -void vmx_vmenter_helper(const struct cpu_user_regs *regs)
> > +int vmx_vmenter_helper(const struct cpu_user_regs *regs)
> 
> What are the semantics of this call?  The result looks boolean, and
> indicates that the vmentry should be aborted?

Currently vmx_vmenter_helper() returns !0 if the vmentry must be
restarted.

> 
> >  {
> >      struct vcpu *curr = current;
> >      u32 new_asid, old_asid;
> >      struct hvm_vcpu_asid *p_asid;
> >      bool_t need_flush;
> >  
> > +    /* Shadow EPTP can't be updated here because irqs are disabled */
> > +     if ( nestedhvm_vcpu_in_guestmode(curr) && vcpu_nestedhvm(curr).stale_np2m )
> > +         return 1;
> > +
> >      if ( curr->domain->arch.hvm_domain.pi_ops.do_resume )
> >          curr->domain->arch.hvm_domain.pi_ops.do_resume(curr);
> >  
> > @@ -4270,6 +4268,8 @@ void vmx_vmenter_helper(const struct cpu_user_regs *regs)
> >      __vmwrite(GUEST_RIP,    regs->rip);
> >      __vmwrite(GUEST_RSP,    regs->rsp);
> >      __vmwrite(GUEST_RFLAGS, regs->rflags | X86_EFLAGS_MBS);
> > +
> > +    return 0;
> >  }
> >  
> >  /*
> > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> > index 2f468e6ced..48e37158af 100644
> > --- a/xen/arch/x86/hvm/vmx/vvmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> > @@ -1405,12 +1405,32 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
> >      vmsucceed(regs);
> >  }
> >  
> > +static void nvmx_eptp_update(void)
> > +{
> 
> struct vcpu *curr = current; will most likely half the compiled size of
> this function.

Yes, passing a struct vcpu *v to nvmx_eptp_update() removes all
the additional:

    mov    %rsp,%rax
    or     $0x7fff,%rax
    
I wasn't aware of such behavior and will correct the usage of current
for all patches in v3.

> 
> > +    if ( !nestedhvm_vcpu_in_guestmode(current) ||
> > +          vcpu_nestedhvm(current).nv_vmexit_pending ||
> > +         !vcpu_nestedhvm(current).stale_np2m ||
> > +         !nestedhvm_paging_mode_hap(current) )
> > +        return;
> > +
> > +    /*
> > +     * Interrupts are enabled here, so we need to clear stale_np2m
> > +     * before we do the vmwrite.  If we do it in the other order, an
> > +     * and IPI comes in changing the shadow eptp after the vmwrite,
> > +     * we'll complete the vmenter with a stale eptp value.
> > +     */
> > +    vcpu_nestedhvm(current).stale_np2m = false;
> > +    __vmwrite(EPT_POINTER, get_shadow_eptp(current));
> > +}
> > +
> >  void nvmx_switch_guest(void)
> >  {
> >      struct vcpu *v = current;
> >      struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> >      struct cpu_user_regs *regs = guest_cpu_user_regs();
> >  
> > +    nvmx_eptp_update();
> > +
> >      /*
> >       * A pending IO emulation may still be not finished. In this case, no
> >       * virtual vmswitch is allowed. Or else, the following IO emulation will
> > diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
> > index 6c54773f1c..5cfa4b4aa4 100644
> > --- a/xen/include/asm-x86/hvm/vcpu.h
> > +++ b/xen/include/asm-x86/hvm/vcpu.h
> > @@ -115,6 +115,7 @@ struct nestedvcpu {
> >  
> >      bool_t nv_flushp2m; /* True, when p2m table must be flushed */
> >      struct p2m_domain *nv_p2m; /* used p2m table for this vcpu */
> > +    bool stale_np2m; /* True when p2m_base in VMCX02 is no longer valid */
> 
> VMCx02 ? which helps distinguish the two parts of semantic information
> encoded there, and to avoid looking like we've gained a third acronym.

I like this suggestion. Will update comments and commit messages for all
patches in v3.

-- 
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/9] x86/np2m: Have invept flush all np2m entries with the same base pointer

[George Dunlap]

On 10/02/2017 10:40 AM, George Dunlap wrote:
> On 10/02/2017 10:37 AM, Sergey Dyasli wrote:
>> On Fri, 2017-09-29 at 16:01 +0100, George Dunlap wrote:
>>> nvmx_handle_invept() updates current's np2m just to flush it.  This is
>>> not only wasteful, but ineffective: if several L2 vcpus share the same
>>> np2m base pointer, they all need to be flushed (not only the current
>>> one).
>>
>> I don't follow this completely. L1 will use INVEPT on each vCPU that
>> shares the same np2m pointer. The main idea here was not to update
>> current's np2m just to flush it.
> 
> Hmm, yes the INVEPT thing is true.  But if that's the case, why do we
> need np2m_flush_base() to loop over the whole list and flush all np2ms
> with the same pointer?

Oh, nevermind -- you don't know which np2m is being used by this vcpu,
so you have to flush all of the np2ms that match that base pointer.

What about this changelog:

---
x86/np2m: Flush p2m rather than switching on nested invept

At the moment, nvmx_handle_invept() updates the current np2m just to
flush it.  Instead introduce a function, np2m_flush_base(), which will
look up the np2m base pointer and call p2m_flush_table() instead.

Unfortunately, since we don't know which p2m a given vcpu is using, we
must flush all p2ms that share that base pointer.

Convert p2m_flush_table() into p2m_flush_table_locked() in order not
to release the p2m_lock after np2m_base check.

Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---

 -George

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

Re: [PATCH 2/9] x86/np2m: Have invept flush all np2m entries with the same base pointer

[Sergey Dyasli]

On Mon, 2017-10-02 at 11:07 +0100, George Dunlap wrote:
> On 10/02/2017 10:40 AM, George Dunlap wrote:
> > On 10/02/2017 10:37 AM, Sergey Dyasli wrote:
> > > On Fri, 2017-09-29 at 16:01 +0100, George Dunlap wrote:
> > > > nvmx_handle_invept() updates current's np2m just to flush it.  This is
> > > > not only wasteful, but ineffective: if several L2 vcpus share the same
> > > > np2m base pointer, they all need to be flushed (not only the current
> > > > one).
> > > 
> > > I don't follow this completely. L1 will use INVEPT on each vCPU that
> > > shares the same np2m pointer. The main idea here was not to update
> > > current's np2m just to flush it.
> > 
> > Hmm, yes the INVEPT thing is true.  But if that's the case, why do we
> > need np2m_flush_base() to loop over the whole list and flush all np2ms
> > with the same pointer?
> 
> Oh, nevermind -- you don't know which np2m is being used by this vcpu,
> so you have to flush all of the np2ms that match that base pointer.
> 
> What about this changelog:
> 
> ---
> x86/np2m: Flush p2m rather than switching on nested invept

It's not entirely clear what "switching" means here. But I fail to
think of any other good alternatives for the patch's subject.

> 
> At the moment, nvmx_handle_invept() updates the current np2m just to
> flush it.  Instead introduce a function, np2m_flush_base(), which will
> look up the np2m base pointer and call p2m_flush_table() instead.
> 
> Unfortunately, since we don't know which p2m a given vcpu is using, we
> must flush all p2ms that share that base pointer.

My reasoning was the same:

INVEPT from L1 happens outside of L02 vCPU's context and currently it's
impossible (because of scheduling) to detect the exact np2m object that
needs to be flushed.

> 
> Convert p2m_flush_table() into p2m_flush_table_locked() in order not
> to release the p2m_lock after np2m_base check.
> 
> Signed-off-by: Sergey Dyasli <sergey.dyasli@citrix.com>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
-- 
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel