[PATCH] nestedhvm: ASID emulation

[PATCH] nestedhvm: ASID emulation

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]


Implement ASID emulation.
This allows the l1 guest to run the l2 guest using hw ASID.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_nh_asid.diff --]
[-- Type: text/plain, Size: 11097 bytes --]

diff -r 40c33d1a9c21 -r a93873d200a6 xen/arch/x86/hvm/asid.c
--- a/xen/arch/x86/hvm/asid.c
+++ b/xen/arch/x86/hvm/asid.c
@@ -48,9 +48,9 @@
 
 /* Per-CPU ASID management. */
 struct hvm_asid_data {
-   u64 core_asid_generation;
-   u32 next_asid;
-   u32 max_asid;
+   uint64_t core_asid_generation;
+   uint32_t next_asid;
+   uint32_t max_asid;
    bool_t disabled;
 };
 
@@ -58,7 +58,7 @@ static DEFINE_PER_CPU(struct hvm_asid_da
 
 void hvm_asid_init(int nasids)
 {
-    static s8 g_disabled = -1;
+    static int8_t g_disabled = -1;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
 
     data->max_asid = nasids - 1;
@@ -102,10 +102,14 @@ void hvm_asid_flush_core(void)
     data->disabled = 1;
 }
 
-bool_t hvm_asid_handle_vmenter(void)
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest)
 {
-    struct vcpu *curr = current;
+    int need_flush = 0;
+    struct vcpu *v = current;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
+    struct nestedvcpu *nv;
+
+    nv = &vcpu_nestedhvm(v);
 
     /* On erratum #170 systems we must flush the TLB. 
      * Generation overruns are taken here, too. */
@@ -113,30 +117,72 @@ bool_t hvm_asid_handle_vmenter(void)
         goto disabled;
 
     /* Test if VCPU has valid ASID. */
-    if ( curr->arch.hvm_vcpu.asid_generation == data->core_asid_generation )
-        return 0;
-
-    /* If there are no free ASIDs, need to go to a new generation */
-    if ( unlikely(data->next_asid > data->max_asid) )
-    {
-        hvm_asid_flush_core();
-        data->next_asid = 1;
-        if ( data->disabled )
-            goto disabled;
+    if ( v->arch.hvm_vcpu.asid_generation == data->core_asid_generation ) {
+        if ( run_n2guest ) {
+            if ( !nv->nv_new_vasid && data->next_asid > nv->nv_n2asid ) {
+                /* l1 guest doesn't request a new asid */
+                /* When asid generation changed last time when we were
+                 * were going to run l1 guest then
+                 * next_asid <= nv->nv_n2asid.
+                 */
+                ASSERT(nv->nv_n2asid != 0);
+                ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+                v->arch.hvm_vcpu.asid = nv->nv_n2asid;
+                return 0;
+            }
+        } else if ( data->next_asid > nv->nv_n1asid ) {
+            /* When asid generation changed last time when we were going to
+             * run l2 guest then next_asid <= nv->nv_n1asid.
+             */
+            ASSERT(nv->nv_n1asid != 0);
+            ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+            v->arch.hvm_vcpu.asid = nv->nv_n1asid;
+            return 0;
+        }
     }
 
-    /* Now guaranteed to be a free ASID. */
-    curr->arch.hvm_vcpu.asid = data->next_asid++;
-    curr->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
+    do {
+        /* If there are no free ASIDs, need to go to a new generation */
+        if ( unlikely(data->next_asid > data->max_asid) )
+        {
+            hvm_asid_flush_core();
+            data->next_asid = 1;
+            if ( data->disabled )
+                goto disabled;
+        }
 
-    /*
-     * When we assign ASID 1, flush all TLB entries as we are starting a new
-     * generation, and all old ASID allocations are now stale. 
-     */
-    return (curr->arch.hvm_vcpu.asid == 1);
+        if ( data->next_asid == 1 ) {
+            /* We start a new generation, so all old ASID allocations are
+             * stale now. Ensure we flush the TLB also in case of another
+             * iteration.
+             */
+            need_flush = 1;
+        }
+
+        /* Now guaranteed to be a free ASID. */
+        if ( run_n2guest )
+            /* nv_n1asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n2asid = data->next_asid++;
+        else
+            /* nv_n2asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n1asid = data->next_asid++;
+
+        /* Make sure an asid isn't used twice */
+    } while (nv->nv_n2asid == nv->nv_n1asid);
+
+    ASSERT(nv->nv_n1asid != 0);
+
+    v->arch.hvm_vcpu.asid = (run_n2guest) ? nv->nv_n2asid : nv->nv_n1asid;
+    v->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
+
+    return need_flush;
 
  disabled:
-    curr->arch.hvm_vcpu.asid = 0;
+    v->arch.hvm_vcpu.asid = 0;
     return 0;
 }
 
diff -r 40c33d1a9c21 -r a93873d200a6 xen/arch/x86/hvm/nestedhvm.c
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -61,6 +61,9 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_vvmcxaddr = VMCX_EADDR;
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
+    nv->nv_new_vasid = 0;
+    nv->nv_n1asid = 0;
+    nv->nv_n2asid = 0;
 
     if ( hvm_funcs.nhvm_vcpu_reset )
         hvm_funcs.nhvm_vcpu_reset(v);
diff -r 40c33d1a9c21 -r a93873d200a6 xen/arch/x86/hvm/svm/asid.c
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -22,6 +22,7 @@
 #include <xen/perfc.h>
 #include <asm/hvm/svm/asid.h>
 #include <asm/amd.h>
+#include <asm/hvm/nestedhvm.h>
 
 void svm_asid_init(struct cpuinfo_x86 *c)
 {
@@ -42,7 +43,13 @@ asmlinkage void svm_asid_handle_vmrun(vo
 {
     struct vcpu *curr = current;
     struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
-    bool_t need_flush = hvm_asid_handle_vmenter();
+    bool_t need_flush;
+    bool_t vcpu_guestmode = 0;
+
+    if ( nestedhvm_enabled(curr->domain) && nestedhvm_vcpu_in_guestmode(curr) )
+        vcpu_guestmode = 1;
+
+    need_flush = hvm_asid_handle_vmenter(vcpu_guestmode);
 
     /* ASID 0 indicates that ASIDs are disabled. */
     if ( curr->arch.hvm_vcpu.asid == 0 )
diff -r 40c33d1a9c21 -r a93873d200a6 xen/arch/x86/hvm/svm/nestedsvm.c
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -261,8 +261,6 @@ int nsvm_vcpu_hostrestore(struct vcpu *v
     /* Cleanbits */
     n1vmcb->cleanbits.bytes = 0;
 
-    hvm_asid_flush_vcpu(v);
-
     return 0;
 }
 
@@ -408,9 +406,7 @@ static int nsvm_vmcb_prepare4vmrun(struc
     if (rc)
         return rc;
 
-    /* ASID */
-    hvm_asid_flush_vcpu(v);
-    /* n2vmcb->_guest_asid = ns_vmcb->_guest_asid; */
+    /* ASID - Emulation handled in hvm_asid_handle_vmenter() */
 
     /* TLB control */
     n2vmcb->tlb_control = n1vmcb->tlb_control | ns_vmcb->tlb_control;
@@ -605,8 +601,8 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct
     svm->ns_vmcb_guestcr3 = ns_vmcb->_cr3;
     svm->ns_vmcb_hostcr3 = ns_vmcb->_h_cr3;
 
-    nv->nv_flushp2m = (ns_vmcb->tlb_control
-        || (svm->ns_guest_asid != ns_vmcb->_guest_asid));
+    nv->nv_new_vasid = (svm->ns_guest_asid != ns_vmcb->_guest_asid);
+    nv->nv_flushp2m = (ns_vmcb->tlb_control || nv->nv_new_vasid);
     svm->ns_guest_asid = ns_vmcb->_guest_asid;
 
     /* nested paging for the guest */
diff -r 40c33d1a9c21 -r a93873d200a6 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1580,6 +1580,17 @@ static void svm_vmexit_do_invalidate_cac
     __update_guest_eip(regs, inst_len);
 }
 
+static void svm_invlpga_intercept(struct vcpu *v,
+    unsigned long vaddr, uint32_t asid)
+{
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    if (asid == 0)
+       asid = nv->nv_n1asid; /* remap to l1 guest asid */
+    else
+       asid = nv->nv_n2asid; /* remap to l2 guest asid */
+    svm_invlpga(vaddr, asid);
+}
+
 static void svm_invlpg_intercept(unsigned long vaddr)
 {
     struct vcpu *curr = current;
@@ -1892,10 +1903,12 @@ asmlinkage void svm_vmexit_handler(struc
     case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
     case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
     case VMEXIT_INVLPG:
-    case VMEXIT_INVLPGA:
         if ( !handle_mmio() )
             hvm_inject_exception(TRAP_gp_fault, 0, 0);
         break;
+    case VMEXIT_INVLPGA:
+        svm_invlpga_intercept(v, regs->rax, regs->ecx);
+        break;
 
     case VMEXIT_VMMCALL:
         if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
diff -r 40c33d1a9c21 -r a93873d200a6 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2673,7 +2673,7 @@ asmlinkage void vmx_vmenter_helper(void)
         goto out;
 
     old_asid = curr->arch.hvm_vcpu.asid;
-    need_flush = hvm_asid_handle_vmenter();
+    need_flush = hvm_asid_handle_vmenter(0 /* false */);
     new_asid = curr->arch.hvm_vcpu.asid;
 
     if ( unlikely(new_asid != old_asid) )
diff -r 40c33d1a9c21 -r a93873d200a6 xen/include/asm-x86/hvm/asid.h
--- a/xen/include/asm-x86/hvm/asid.h
+++ b/xen/include/asm-x86/hvm/asid.h
@@ -35,7 +35,7 @@ void hvm_asid_flush_core(void);
 
 /* Called before entry to guest context. Checks ASID allocation, returns a
  * boolean indicating whether all ASIDs must be flushed. */
-bool_t hvm_asid_handle_vmenter(void);
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest);
 
 #endif /* __ASM_X86_HVM_ASID_H__ */
 
diff -r 40c33d1a9c21 -r a93873d200a6 xen/include/asm-x86/hvm/svm/asid.h
--- a/xen/include/asm-x86/hvm/svm/asid.h
+++ b/xen/include/asm-x86/hvm/svm/asid.h
@@ -34,10 +34,7 @@ static inline void svm_asid_g_invlpg(str
 {
 #if 0
     /* Optimization? */
-    asm volatile (".byte 0x0F,0x01,0xDF    \n"
-                  : /* output */
-                  : /* input */
-                  "a" (g_vaddr), "c"(v->arch.hvm_svm.vmcb->guest_asid) );
+    svm_invlpga(g_vaddr, v->arch.hvm_svm.vmcb->guest_asid);
 #endif
 
     /* Safe fallback. Take a new ASID. */
diff -r 40c33d1a9c21 -r a93873d200a6 xen/include/asm-x86/hvm/svm/svm.h
--- a/xen/include/asm-x86/hvm/svm/svm.h
+++ b/xen/include/asm-x86/hvm/svm/svm.h
@@ -60,6 +60,15 @@ static inline void svm_vmsave(void *vmcb
         : : "a" (__pa(vmcb)) : "memory" );
 }
 
+static inline void svm_invlpga(unsigned long vaddr, uint32_t asid)
+{
+    asm volatile (
+        ".byte 0x0f,0x01,0xdf"
+        : /* output */
+        : /* input */
+        "a" (vaddr), "c" (asid));
+}
+
 unsigned long *svm_msrbit(unsigned long *msr_bitmap, uint32_t msr);
 void __update_guest_eip(struct cpu_user_regs *regs, unsigned int inst_len);
 
diff -r 40c33d1a9c21 -r a93873d200a6 xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -49,6 +49,11 @@ struct nestedvcpu {
     uint64_t nv_n1vmcx_pa; /* host physical address of nv_n1vmcx */
     uint64_t nv_n2vmcx_pa; /* host physical address of nv_n2vmcx */
 
+    /* ASID emulation */
+    bool_t nv_new_vasid;   /* true when l1 guest requests new virtual asid */
+    uint32_t nv_n1asid;    /* hw ASID number used to run l1 guest */
+    uint32_t nv_n2asid;    /* hw ASID number used to run l2 guest */
+
     /* SVM/VMX arch specific */
     union {
         struct nestedsvm nsvm;
@@ -100,8 +105,8 @@ struct hvm_vcpu {
     bool_t              hcall_preempted;
     bool_t              hcall_64bit;
 
-    u64                 asid_generation;
-    u32                 asid;
+    uint64_t            asid_generation;
+    uint32_t            asid;
 
     u32                 msr_tsc_aux;
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 13/04/2011 09:57, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 
> Implement ASID emulation.
> This allows the l1 guest to run the l2 guest using hw ASID.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

This patch will be complex enough without mixing in cleanups as well (e.g.,
converting sized type decls to all use [u]intN_t style). Split the cleanups
from the new mechanism.

 -- Keir

[PATCH] nestedhvm: ASID emulation

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]


Implement ASID emulation.
This allows the l1 guest to run the l2 guest using hw ASID.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_nh_asid.diff --]
[-- Type: text/plain, Size: 8937 bytes --]

diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/arch/x86/hvm/asid.c
--- a/xen/arch/x86/hvm/asid.c
+++ b/xen/arch/x86/hvm/asid.c
@@ -102,10 +102,14 @@ void hvm_asid_flush_core(void)
     data->disabled = 1;
 }
 
-bool_t hvm_asid_handle_vmenter(void)
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest)
 {
+    int need_flush = 0;
     struct vcpu *v = current;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
+    struct nestedvcpu *nv;
+
+    nv = &vcpu_nestedhvm(v);
 
     /* On erratum #170 systems we must flush the TLB. 
      * Generation overruns are taken here, too. */
@@ -113,27 +117,69 @@ bool_t hvm_asid_handle_vmenter(void)
         goto disabled;
 
     /* Test if VCPU has valid ASID. */
-    if ( v->arch.hvm_vcpu.asid_generation == data->core_asid_generation )
-        return 0;
-
-    /* If there are no free ASIDs, need to go to a new generation */
-    if ( unlikely(data->next_asid > data->max_asid) )
-    {
-        hvm_asid_flush_core();
-        data->next_asid = 1;
-        if ( data->disabled )
-            goto disabled;
+    if ( v->arch.hvm_vcpu.asid_generation == data->core_asid_generation ) {
+        if ( run_n2guest ) {
+            if ( !nv->nv_new_vasid && data->next_asid > nv->nv_n2asid ) {
+                /* l1 guest doesn't request a new asid */
+                /* When asid generation changed last time when we were
+                 * were going to run l1 guest then
+                 * next_asid <= nv->nv_n2asid.
+                 */
+                ASSERT(nv->nv_n2asid != 0);
+                ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+                v->arch.hvm_vcpu.asid = nv->nv_n2asid;
+                return 0;
+            }
+        } else if ( data->next_asid > nv->nv_n1asid ) {
+            /* When asid generation changed last time when we were going to
+             * run l2 guest then next_asid <= nv->nv_n1asid.
+             */
+            ASSERT(nv->nv_n1asid != 0);
+            ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+            v->arch.hvm_vcpu.asid = nv->nv_n1asid;
+            return 0;
+        }
     }
 
-    /* Now guaranteed to be a free ASID. */
-    v->arch.hvm_vcpu.asid = data->next_asid++;
+    do {
+        /* If there are no free ASIDs, need to go to a new generation */
+        if ( unlikely(data->next_asid > data->max_asid) )
+        {
+            hvm_asid_flush_core();
+            data->next_asid = 1;
+            if ( data->disabled )
+                goto disabled;
+        }
+
+        if ( data->next_asid == 1 ) {
+            /* We start a new generation, so all old ASID allocations are
+             * stale now. Ensure we flush the TLB also in case of another
+             * iteration.
+             */
+            need_flush = 1;
+        }
+
+        /* Now guaranteed to be a free ASID. */
+        if ( run_n2guest )
+            /* nv_n1asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n2asid = data->next_asid++;
+        else
+            /* nv_n2asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n1asid = data->next_asid++;
+
+        /* Make sure an asid isn't used twice */
+    } while (nv->nv_n2asid == nv->nv_n1asid);
+
+    ASSERT(nv->nv_n1asid != 0);
+
+    v->arch.hvm_vcpu.asid = (run_n2guest) ? nv->nv_n2asid : nv->nv_n1asid;
     v->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
 
-    /*
-     * When we assign ASID 1, flush all TLB entries as we are starting a new
-     * generation, and all old ASID allocations are now stale. 
-     */
-    return (v->arch.hvm_vcpu.asid == 1);
+    return need_flush;
 
  disabled:
     v->arch.hvm_vcpu.asid = 0;
diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/arch/x86/hvm/nestedhvm.c
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -61,6 +61,9 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_vvmcxaddr = VMCX_EADDR;
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
+    nv->nv_new_vasid = 0;
+    nv->nv_n1asid = 0;
+    nv->nv_n2asid = 0;
 
     if ( hvm_funcs.nhvm_vcpu_reset )
         hvm_funcs.nhvm_vcpu_reset(v);
diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/arch/x86/hvm/svm/asid.c
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -22,6 +22,7 @@
 #include <xen/perfc.h>
 #include <asm/hvm/svm/asid.h>
 #include <asm/amd.h>
+#include <asm/hvm/nestedhvm.h>
 
 void svm_asid_init(struct cpuinfo_x86 *c)
 {
@@ -42,7 +43,13 @@ asmlinkage void svm_asid_handle_vmrun(vo
 {
     struct vcpu *curr = current;
     struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
-    bool_t need_flush = hvm_asid_handle_vmenter();
+    bool_t need_flush;
+    bool_t vcpu_guestmode = 0;
+
+    if ( nestedhvm_enabled(curr->domain) && nestedhvm_vcpu_in_guestmode(curr) )
+        vcpu_guestmode = 1;
+
+    need_flush = hvm_asid_handle_vmenter(vcpu_guestmode);
 
     /* ASID 0 indicates that ASIDs are disabled. */
     if ( curr->arch.hvm_vcpu.asid == 0 )
diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/arch/x86/hvm/svm/nestedsvm.c
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -261,8 +261,6 @@ int nsvm_vcpu_hostrestore(struct vcpu *v
     /* Cleanbits */
     n1vmcb->cleanbits.bytes = 0;
 
-    hvm_asid_flush_vcpu(v);
-
     return 0;
 }
 
@@ -408,9 +406,7 @@ static int nsvm_vmcb_prepare4vmrun(struc
     if (rc)
         return rc;
 
-    /* ASID */
-    hvm_asid_flush_vcpu(v);
-    /* n2vmcb->_guest_asid = ns_vmcb->_guest_asid; */
+    /* ASID - Emulation handled in hvm_asid_handle_vmenter() */
 
     /* TLB control */
     n2vmcb->tlb_control = n1vmcb->tlb_control | ns_vmcb->tlb_control;
@@ -605,8 +601,8 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct
     svm->ns_vmcb_guestcr3 = ns_vmcb->_cr3;
     svm->ns_vmcb_hostcr3 = ns_vmcb->_h_cr3;
 
-    nv->nv_flushp2m = (ns_vmcb->tlb_control
-        || (svm->ns_guest_asid != ns_vmcb->_guest_asid));
+    nv->nv_new_vasid = (svm->ns_guest_asid != ns_vmcb->_guest_asid);
+    nv->nv_flushp2m = (ns_vmcb->tlb_control || nv->nv_new_vasid);
     svm->ns_guest_asid = ns_vmcb->_guest_asid;
 
     /* nested paging for the guest */
diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1580,6 +1580,17 @@ static void svm_vmexit_do_invalidate_cac
     __update_guest_eip(regs, inst_len);
 }
 
+static void svm_invlpga_intercept(struct vcpu *v,
+    unsigned long vaddr, uint32_t asid)
+{
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    if (asid == 0)
+       asid = nv->nv_n1asid; /* remap to l1 guest asid */
+    else
+       asid = nv->nv_n2asid; /* remap to l2 guest asid */
+    svm_invlpga(vaddr, asid);
+}
+
 static void svm_invlpg_intercept(unsigned long vaddr)
 {
     struct vcpu *curr = current;
@@ -1892,10 +1903,12 @@ asmlinkage void svm_vmexit_handler(struc
     case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
     case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
     case VMEXIT_INVLPG:
-    case VMEXIT_INVLPGA:
         if ( !handle_mmio() )
             hvm_inject_exception(TRAP_gp_fault, 0, 0);
         break;
+    case VMEXIT_INVLPGA:
+        svm_invlpga_intercept(v, regs->rax, regs->ecx);
+        break;
 
     case VMEXIT_VMMCALL:
         if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2673,7 +2673,7 @@ asmlinkage void vmx_vmenter_helper(void)
         goto out;
 
     old_asid = curr->arch.hvm_vcpu.asid;
-    need_flush = hvm_asid_handle_vmenter();
+    need_flush = hvm_asid_handle_vmenter(0 /* false */);
     new_asid = curr->arch.hvm_vcpu.asid;
 
     if ( unlikely(new_asid != old_asid) )
diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/include/asm-x86/hvm/asid.h
--- a/xen/include/asm-x86/hvm/asid.h
+++ b/xen/include/asm-x86/hvm/asid.h
@@ -35,7 +35,7 @@ void hvm_asid_flush_core(void);
 
 /* Called before entry to guest context. Checks ASID allocation, returns a
  * boolean indicating whether all ASIDs must be flushed. */
-bool_t hvm_asid_handle_vmenter(void);
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest);
 
 #endif /* __ASM_X86_HVM_ASID_H__ */
 
diff -r 5cd1060fb63e -r cb2f8bc62a7e xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -49,6 +49,11 @@ struct nestedvcpu {
     uint64_t nv_n1vmcx_pa; /* host physical address of nv_n1vmcx */
     uint64_t nv_n2vmcx_pa; /* host physical address of nv_n2vmcx */
 
+    /* ASID emulation */
+    bool_t nv_new_vasid;   /* true when l1 guest requests new virtual asid */
+    uint32_t nv_n1asid;    /* hw ASID number used to run l1 guest */
+    uint32_t nv_n2asid;    /* hw ASID number used to run l2 guest */
+
     /* SVM/VMX arch specific */
     union {
         struct nestedsvm nsvm;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 13/04/2011 11:37, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> 
> Implement ASID emulation.
> This allows the l1 guest to run the l2 guest using hw ASID.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

First, how much of a win is this compared with what we do currently?

Second, the two-asids-per-vcpu allocation scheme in
hvm_asid_handle_vmenter() looks broken. I mean, consider this comment:
  /* When asid generation changed last time when we were
   * were going to run l1 guest then next_asid <= nv->nv_n2asid. */
I don't see how you can assert this to be true. Arbitrary generations can
have passed, and next_asid incremented to an arbitrary value, since the last
time you allocated nv_n2asid.

I wouldn't bother fixing #2 unless there's a convincing answer for #1.

 -- Keir

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

Rebased due to change in previous patch.

On 04/13/11 12:37, Christoph Egger wrote:
>
> Implement ASID emulation.
> This allows the l1 guest to run the l2 guest using hw ASID.
>
> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>
>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_nh_asid.diff --]
[-- Type: text/plain, Size: 8967 bytes --]

diff -r 5458a9862db2 -r c217f85d7d0a xen/arch/x86/hvm/asid.c
--- a/xen/arch/x86/hvm/asid.c
+++ b/xen/arch/x86/hvm/asid.c
@@ -102,10 +102,14 @@ void hvm_asid_flush_core(void)
     data->disabled = 1;
 }
 
-bool_t hvm_asid_handle_vmenter(void)
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest)
 {
+    int need_flush = 0;
     struct vcpu *curr = current;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
+    struct nestedvcpu *nv;
+
+    nv = &vcpu_nestedhvm(v);
 
     /* On erratum #170 systems we must flush the TLB. 
      * Generation overruns are taken here, too. */
@@ -113,27 +117,69 @@ bool_t hvm_asid_handle_vmenter(void)
         goto disabled;
 
     /* Test if VCPU has valid ASID. */
-    if ( curr->arch.hvm_vcpu.asid_generation == data->core_asid_generation )
-        return 0;
-
-    /* If there are no free ASIDs, need to go to a new generation */
-    if ( unlikely(data->next_asid > data->max_asid) )
-    {
-        hvm_asid_flush_core();
-        data->next_asid = 1;
-        if ( data->disabled )
-            goto disabled;
+    if ( curr->arch.hvm_vcpu.asid_generation == data->core_asid_generation ) {
+        if ( run_n2guest ) {
+            if ( !nv->nv_new_vasid && data->next_asid > nv->nv_n2asid ) {
+                /* l1 guest doesn't request a new asid */
+                /* When asid generation changed last time when we were
+                 * were going to run l1 guest then
+                 * next_asid <= nv->nv_n2asid.
+                 */
+                ASSERT(nv->nv_n2asid != 0);
+                ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+                curr->arch.hvm_vcpu.asid = nv->nv_n2asid;
+                return 0;
+            }
+        } else if ( data->next_asid > nv->nv_n1asid ) {
+            /* When asid generation changed last time when we were going to
+             * run l2 guest then next_asid <= nv->nv_n1asid.
+             */
+            ASSERT(nv->nv_n1asid != 0);
+            ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+            curr->arch.hvm_vcpu.asid = nv->nv_n1asid;
+            return 0;
+        }
     }
 
-    /* Now guaranteed to be a free ASID. */
-    curr->arch.hvm_vcpu.asid = data->next_asid++;
+    do {
+        /* If there are no free ASIDs, need to go to a new generation */
+        if ( unlikely(data->next_asid > data->max_asid) )
+        {
+            hvm_asid_flush_core();
+            data->next_asid = 1;
+            if ( data->disabled )
+                goto disabled;
+        }
+
+        if ( data->next_asid == 1 ) {
+            /* We start a new generation, so all old ASID allocations are
+             * stale now. Ensure we flush the TLB also in case of another
+             * iteration.
+             */
+            need_flush = 1;
+        }
+
+        /* Now guaranteed to be a free ASID. */
+        if ( run_n2guest )
+            /* nv_n1asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n2asid = data->next_asid++;
+        else
+            /* nv_n2asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n1asid = data->next_asid++;
+
+        /* Make sure an asid isn't used twice */
+    } while (nv->nv_n2asid == nv->nv_n1asid);
+
+    ASSERT(nv->nv_n1asid != 0);
+
+    curr->arch.hvm_vcpu.asid = (run_n2guest) ? nv->nv_n2asid : nv->nv_n1asid;
     curr->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
 
-    /*
-     * When we assign ASID 1, flush all TLB entries as we are starting a new
-     * generation, and all old ASID allocations are now stale. 
-     */
-    return (curr->arch.hvm_vcpu.asid == 1);
+    return need_flush;
 
  disabled:
     curr->arch.hvm_vcpu.asid = 0;
diff -r 5458a9862db2 -r c217f85d7d0a xen/arch/x86/hvm/nestedhvm.c
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -61,6 +61,9 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_vvmcxaddr = VMCX_EADDR;
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
+    nv->nv_new_vasid = 0;
+    nv->nv_n1asid = 0;
+    nv->nv_n2asid = 0;
 
     if ( hvm_funcs.nhvm_vcpu_reset )
         hvm_funcs.nhvm_vcpu_reset(v);
diff -r 5458a9862db2 -r c217f85d7d0a xen/arch/x86/hvm/svm/asid.c
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -22,6 +22,7 @@
 #include <xen/perfc.h>
 #include <asm/hvm/svm/asid.h>
 #include <asm/amd.h>
+#include <asm/hvm/nestedhvm.h>
 
 void svm_asid_init(struct cpuinfo_x86 *c)
 {
@@ -42,7 +43,13 @@ asmlinkage void svm_asid_handle_vmrun(vo
 {
     struct vcpu *curr = current;
     struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
-    bool_t need_flush = hvm_asid_handle_vmenter();
+    bool_t need_flush;
+    bool_t vcpu_guestmode = 0;
+
+    if ( nestedhvm_enabled(curr->domain) && nestedhvm_vcpu_in_guestmode(curr) )
+        vcpu_guestmode = 1;
+
+    need_flush = hvm_asid_handle_vmenter(vcpu_guestmode);
 
     /* ASID 0 indicates that ASIDs are disabled. */
     if ( curr->arch.hvm_vcpu.asid == 0 )
diff -r 5458a9862db2 -r c217f85d7d0a xen/arch/x86/hvm/svm/nestedsvm.c
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -261,8 +261,6 @@ int nsvm_vcpu_hostrestore(struct vcpu *v
     /* Cleanbits */
     n1vmcb->cleanbits.bytes = 0;
 
-    hvm_asid_flush_vcpu(v);
-
     return 0;
 }
 
@@ -408,9 +406,7 @@ static int nsvm_vmcb_prepare4vmrun(struc
     if (rc)
         return rc;
 
-    /* ASID */
-    hvm_asid_flush_vcpu(v);
-    /* n2vmcb->_guest_asid = ns_vmcb->_guest_asid; */
+    /* ASID - Emulation handled in hvm_asid_handle_vmenter() */
 
     /* TLB control */
     n2vmcb->tlb_control = n1vmcb->tlb_control | ns_vmcb->tlb_control;
@@ -605,8 +601,8 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct
     svm->ns_vmcb_guestcr3 = ns_vmcb->_cr3;
     svm->ns_vmcb_hostcr3 = ns_vmcb->_h_cr3;
 
-    nv->nv_flushp2m = (ns_vmcb->tlb_control
-        || (svm->ns_guest_asid != ns_vmcb->_guest_asid));
+    nv->nv_new_vasid = (svm->ns_guest_asid != ns_vmcb->_guest_asid);
+    nv->nv_flushp2m = (ns_vmcb->tlb_control || nv->nv_new_vasid);
     svm->ns_guest_asid = ns_vmcb->_guest_asid;
 
     /* nested paging for the guest */
diff -r 5458a9862db2 -r c217f85d7d0a xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1580,6 +1580,17 @@ static void svm_vmexit_do_invalidate_cac
     __update_guest_eip(regs, inst_len);
 }
 
+static void svm_invlpga_intercept(struct vcpu *v,
+    unsigned long vaddr, uint32_t asid)
+{
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    if (asid == 0)
+       asid = nv->nv_n1asid; /* remap to l1 guest asid */
+    else
+       asid = nv->nv_n2asid; /* remap to l2 guest asid */
+    svm_invlpga(vaddr, asid);
+}
+
 static void svm_invlpg_intercept(unsigned long vaddr)
 {
     struct vcpu *curr = current;
@@ -1892,10 +1903,12 @@ asmlinkage void svm_vmexit_handler(struc
     case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
     case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
     case VMEXIT_INVLPG:
-    case VMEXIT_INVLPGA:
         if ( !handle_mmio() )
             hvm_inject_exception(TRAP_gp_fault, 0, 0);
         break;
+    case VMEXIT_INVLPGA:
+        svm_invlpga_intercept(v, regs->rax, regs->ecx);
+        break;
 
     case VMEXIT_VMMCALL:
         if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
diff -r 5458a9862db2 -r c217f85d7d0a xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2673,7 +2673,7 @@ asmlinkage void vmx_vmenter_helper(void)
         goto out;
 
     old_asid = curr->arch.hvm_vcpu.asid;
-    need_flush = hvm_asid_handle_vmenter();
+    need_flush = hvm_asid_handle_vmenter(0 /* false */);
     new_asid = curr->arch.hvm_vcpu.asid;
 
     if ( unlikely(new_asid != old_asid) )
diff -r 5458a9862db2 -r c217f85d7d0a xen/include/asm-x86/hvm/asid.h
--- a/xen/include/asm-x86/hvm/asid.h
+++ b/xen/include/asm-x86/hvm/asid.h
@@ -35,7 +35,7 @@ void hvm_asid_flush_core(void);
 
 /* Called before entry to guest context. Checks ASID allocation, returns a
  * boolean indicating whether all ASIDs must be flushed. */
-bool_t hvm_asid_handle_vmenter(void);
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest);
 
 #endif /* __ASM_X86_HVM_ASID_H__ */
 
diff -r 5458a9862db2 -r c217f85d7d0a xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -49,6 +49,11 @@ struct nestedvcpu {
     uint64_t nv_n1vmcx_pa; /* host physical address of nv_n1vmcx */
     uint64_t nv_n2vmcx_pa; /* host physical address of nv_n2vmcx */
 
+    /* ASID emulation */
+    bool_t nv_new_vasid;   /* true when l1 guest requests new virtual asid */
+    uint32_t nv_n1asid;    /* hw ASID number used to run l1 guest */
+    uint32_t nv_n2asid;    /* hw ASID number used to run l2 guest */
+
     /* SVM/VMX arch specific */
     union {
         struct nestedsvm nsvm;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

On 04/13/11 15:27, Keir Fraser wrote:
> On 13/04/2011 11:37, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>
>> Implement ASID emulation.
>> This allows the l1 guest to run the l2 guest using hw ASID.
>>
>> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>
>
> First, how much of a win is this compared with what we do currently?

We talk about a win of about 1000 cycles per VMRUN and another 1000 
cycles per VMEXIT emulation.

That's a speedup of about 10% for each VMRUN and about 20% for each 
VMEXIT emulation.


> Second, the two-asids-per-vcpu allocation scheme in
> hvm_asid_handle_vmenter() looks broken. I mean, consider this comment:
>    /* When asid generation changed last time when we were
>     * were going to run l1 guest then next_asid<= nv->nv_n2asid. */

Oh, the 2nd 'were' should be removed.

> I don't see how you can assert this to be true. Arbitrary generations can
> have passed, and next_asid incremented to an arbitrary value, since the last
> time you allocated nv_n2asid.

There are different cases to handle:

1. nestedhvm is disabled.

In this case, 'run_n2guest' is always false then the function should
have the old behaviour across multiple VMRUNs.

2. nestedhvm is enabled and we are going to run l1 guest

We run the l2 guest in the last call. The asid generation may have
changed by then. In this case the current nv_n1asid number is stale
and the value of data->next_asid is <= of nv->nv_n1asid.
The the value of nv->nv_n1asid is valid data->next_asid is
larger than nv->nv_n1asid. In this case just reuse the same hw ASID
that has been used from the last VMEXIT emulation.

3. nestedhvm is enabled and we are going to run l1 guest again

The same hw ASID should be reused unless the generation changed because
the nestedp2m got flushed or the vcpu moved to a different physical cpu, 
for example.
But the hw ASID number may never match the hw ASID used to run the l2 guest.

4. nestedhvm is enabled and we are going to run l2 guest

We run the l1 guest in the last call. The asid generation may have
changed by then. In this case the current nv_n2asid number is stale
and the value of data->next_asid is <= of nv->nv_n2asid.
The the value of nv->nv_n2asid is valid if l1 guest doesn't change
the virtual asid (= asid number in the virtual vmcb) and
data->next_asid is larger than nv->nv_n2asid. In this case
just reuse the same hw ASID that has been used from the last
VMRUN emulation.

5. nestedhvm is enabled and we are going to run l2 guest again

The same hw ASID should be reused unless the generation changed because
the nestedp2m got flushed or the vcpu moved to a different physical cpu, 
for example.
But the hw ASID number may never match the hw ASID used to run the l1 guest.


In all cases we have to verify that the

> I wouldn't bother fixing #2 unless there's a convincing answer for #1.
>
>   -- Keir


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

On 04/13/11 15:51, Christoph Egger wrote:
> Rebased due to change in previous patch.
Fixed build fallout from rebasing.

> On 04/13/11 12:37, Christoph Egger wrote:
>>
>> Implement ASID emulation.
>> This allows the l1 guest to run the l2 guest using hw ASID.
>>
>> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_nh_asid.diff --]
[-- Type: text/plain, Size: 9176 bytes --]

# HG changeset patch
# User cegger
# Date 1302705903 -7200
Implement ASID implementation.
This allows the guest to run nested guest using hw ASID.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

diff -r e1895b172170 -r 07ce1a6ecad4 xen/arch/x86/hvm/asid.c
--- a/xen/arch/x86/hvm/asid.c
+++ b/xen/arch/x86/hvm/asid.c
@@ -102,10 +102,14 @@ void hvm_asid_flush_core(void)
     data->disabled = 1;
 }
 
-bool_t hvm_asid_handle_vmenter(void)
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest)
 {
+    int need_flush = 0;
     struct vcpu *curr = current;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
+    struct nestedvcpu *nv;
+
+    nv = &vcpu_nestedhvm(curr);
 
     /* On erratum #170 systems we must flush the TLB. 
      * Generation overruns are taken here, too. */
@@ -113,27 +117,69 @@ bool_t hvm_asid_handle_vmenter(void)
         goto disabled;
 
     /* Test if VCPU has valid ASID. */
-    if ( curr->arch.hvm_vcpu.asid_generation == data->core_asid_generation )
-        return 0;
-
-    /* If there are no free ASIDs, need to go to a new generation */
-    if ( unlikely(data->next_asid > data->max_asid) )
-    {
-        hvm_asid_flush_core();
-        data->next_asid = 1;
-        if ( data->disabled )
-            goto disabled;
+    if ( curr->arch.hvm_vcpu.asid_generation == data->core_asid_generation ) {
+        if ( run_n2guest ) {
+            if ( !nv->nv_new_vasid && data->next_asid > nv->nv_n2asid ) {
+                /* l1 guest doesn't request a new asid */
+                /* When asid generation changed last time when we were
+                 * were going to run l1 guest then
+                 * next_asid <= nv->nv_n2asid.
+                 */
+                ASSERT(nv->nv_n2asid != 0);
+                ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+                curr->arch.hvm_vcpu.asid = nv->nv_n2asid;
+                return 0;
+            }
+        } else if ( data->next_asid > nv->nv_n1asid ) {
+            /* When asid generation changed last time when we were going to
+             * run l2 guest then next_asid <= nv->nv_n1asid.
+             */
+            ASSERT(nv->nv_n1asid != 0);
+            ASSERT(nv->nv_n1asid != nv->nv_n2asid);
+            curr->arch.hvm_vcpu.asid = nv->nv_n1asid;
+            return 0;
+        }
     }
 
-    /* Now guaranteed to be a free ASID. */
-    curr->arch.hvm_vcpu.asid = data->next_asid++;
+    do {
+        /* If there are no free ASIDs, need to go to a new generation */
+        if ( unlikely(data->next_asid > data->max_asid) )
+        {
+            hvm_asid_flush_core();
+            data->next_asid = 1;
+            if ( data->disabled )
+                goto disabled;
+        }
+
+        if ( data->next_asid == 1 ) {
+            /* We start a new generation, so all old ASID allocations are
+             * stale now. Ensure we flush the TLB also in case of another
+             * iteration.
+             */
+            need_flush = 1;
+        }
+
+        /* Now guaranteed to be a free ASID. */
+        if ( run_n2guest )
+            /* nv_n1asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n2asid = data->next_asid++;
+        else
+            /* nv_n2asid might have an asid from an old generation.
+             * We handle this on next vmenter.
+             */
+            nv->nv_n1asid = data->next_asid++;
+
+        /* Make sure an asid isn't used twice */
+    } while (nv->nv_n2asid == nv->nv_n1asid);
+
+    ASSERT(nv->nv_n1asid != 0);
+
+    curr->arch.hvm_vcpu.asid = (run_n2guest) ? nv->nv_n2asid : nv->nv_n1asid;
     curr->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
 
-    /*
-     * When we assign ASID 1, flush all TLB entries as we are starting a new
-     * generation, and all old ASID allocations are now stale. 
-     */
-    return (curr->arch.hvm_vcpu.asid == 1);
+    return need_flush;
 
  disabled:
     curr->arch.hvm_vcpu.asid = 0;
diff -r e1895b172170 -r 07ce1a6ecad4 xen/arch/x86/hvm/nestedhvm.c
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -61,6 +61,9 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_vvmcxaddr = VMCX_EADDR;
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
+    nv->nv_new_vasid = 0;
+    nv->nv_n1asid = 0;
+    nv->nv_n2asid = 0;
 
     if ( hvm_funcs.nhvm_vcpu_reset )
         hvm_funcs.nhvm_vcpu_reset(v);
diff -r e1895b172170 -r 07ce1a6ecad4 xen/arch/x86/hvm/svm/asid.c
--- a/xen/arch/x86/hvm/svm/asid.c
+++ b/xen/arch/x86/hvm/svm/asid.c
@@ -22,6 +22,7 @@
 #include <xen/perfc.h>
 #include <asm/hvm/svm/asid.h>
 #include <asm/amd.h>
+#include <asm/hvm/nestedhvm.h>
 
 void svm_asid_init(struct cpuinfo_x86 *c)
 {
@@ -42,7 +43,13 @@ asmlinkage void svm_asid_handle_vmrun(vo
 {
     struct vcpu *curr = current;
     struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
-    bool_t need_flush = hvm_asid_handle_vmenter();
+    bool_t need_flush;
+    bool_t vcpu_guestmode = 0;
+
+    if ( nestedhvm_enabled(curr->domain) && nestedhvm_vcpu_in_guestmode(curr) )
+        vcpu_guestmode = 1;
+
+    need_flush = hvm_asid_handle_vmenter(vcpu_guestmode);
 
     /* ASID 0 indicates that ASIDs are disabled. */
     if ( curr->arch.hvm_vcpu.asid == 0 )
diff -r e1895b172170 -r 07ce1a6ecad4 xen/arch/x86/hvm/svm/nestedsvm.c
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -261,8 +261,6 @@ int nsvm_vcpu_hostrestore(struct vcpu *v
     /* Cleanbits */
     n1vmcb->cleanbits.bytes = 0;
 
-    hvm_asid_flush_vcpu(v);
-
     return 0;
 }
 
@@ -408,9 +406,7 @@ static int nsvm_vmcb_prepare4vmrun(struc
     if (rc)
         return rc;
 
-    /* ASID */
-    hvm_asid_flush_vcpu(v);
-    /* n2vmcb->_guest_asid = ns_vmcb->_guest_asid; */
+    /* ASID - Emulation handled in hvm_asid_handle_vmenter() */
 
     /* TLB control */
     n2vmcb->tlb_control = n1vmcb->tlb_control | ns_vmcb->tlb_control;
@@ -605,8 +601,8 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct
     svm->ns_vmcb_guestcr3 = ns_vmcb->_cr3;
     svm->ns_vmcb_hostcr3 = ns_vmcb->_h_cr3;
 
-    nv->nv_flushp2m = (ns_vmcb->tlb_control
-        || (svm->ns_guest_asid != ns_vmcb->_guest_asid));
+    nv->nv_new_vasid = (svm->ns_guest_asid != ns_vmcb->_guest_asid);
+    nv->nv_flushp2m = (ns_vmcb->tlb_control || nv->nv_new_vasid);
     svm->ns_guest_asid = ns_vmcb->_guest_asid;
 
     /* nested paging for the guest */
diff -r e1895b172170 -r 07ce1a6ecad4 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1580,6 +1580,17 @@ static void svm_vmexit_do_invalidate_cac
     __update_guest_eip(regs, inst_len);
 }
 
+static void svm_invlpga_intercept(struct vcpu *v,
+    unsigned long vaddr, uint32_t asid)
+{
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    if (asid == 0)
+       asid = nv->nv_n1asid; /* remap to l1 guest asid */
+    else
+       asid = nv->nv_n2asid; /* remap to l2 guest asid */
+    svm_invlpga(vaddr, asid);
+}
+
 static void svm_invlpg_intercept(unsigned long vaddr)
 {
     struct vcpu *curr = current;
@@ -1892,10 +1903,12 @@ asmlinkage void svm_vmexit_handler(struc
     case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
     case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
     case VMEXIT_INVLPG:
-    case VMEXIT_INVLPGA:
         if ( !handle_mmio() )
             hvm_inject_exception(TRAP_gp_fault, 0, 0);
         break;
+    case VMEXIT_INVLPGA:
+        svm_invlpga_intercept(v, regs->rax, regs->ecx);
+        break;
 
     case VMEXIT_VMMCALL:
         if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
diff -r e1895b172170 -r 07ce1a6ecad4 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2673,7 +2673,7 @@ asmlinkage void vmx_vmenter_helper(void)
         goto out;
 
     old_asid = curr->arch.hvm_vcpu.asid;
-    need_flush = hvm_asid_handle_vmenter();
+    need_flush = hvm_asid_handle_vmenter(0 /* false */);
     new_asid = curr->arch.hvm_vcpu.asid;
 
     if ( unlikely(new_asid != old_asid) )
diff -r e1895b172170 -r 07ce1a6ecad4 xen/include/asm-x86/hvm/asid.h
--- a/xen/include/asm-x86/hvm/asid.h
+++ b/xen/include/asm-x86/hvm/asid.h
@@ -35,7 +35,7 @@ void hvm_asid_flush_core(void);
 
 /* Called before entry to guest context. Checks ASID allocation, returns a
  * boolean indicating whether all ASIDs must be flushed. */
-bool_t hvm_asid_handle_vmenter(void);
+bool_t hvm_asid_handle_vmenter(bool_t run_n2guest);
 
 #endif /* __ASM_X86_HVM_ASID_H__ */
 
diff -r e1895b172170 -r 07ce1a6ecad4 xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -49,6 +49,11 @@ struct nestedvcpu {
     uint64_t nv_n1vmcx_pa; /* host physical address of nv_n1vmcx */
     uint64_t nv_n2vmcx_pa; /* host physical address of nv_n2vmcx */
 
+    /* ASID emulation */
+    bool_t nv_new_vasid;   /* true when l1 guest requests new virtual asid */
+    uint32_t nv_n1asid;    /* hw ASID number used to run l1 guest */
+    uint32_t nv_n2asid;    /* hw ASID number used to run l2 guest */
+
     /* SVM/VMX arch specific */
     union {
         struct nestedsvm nsvm;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 13/04/2011 15:26, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On 04/13/11 15:27, Keir Fraser wrote:
>> On 13/04/2011 11:37, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>> 
> We talk about a win of about 1000 cycles per VMRUN and another 1000
> cycles per VMEXIT emulation.
> 
> That's a speedup of about 10% for each VMRUN and about 20% for each
> VMEXIT emulation.

Is this measurable on a macro benchmark?

I mean this looks like a micro-optimisation on a feature that noone is going
to use for serious performance work anyway.

> 4. nestedhvm is enabled and we are going to run l2 guest
> 
> We run the l1 guest in the last call. The asid generation may have
> changed by then. In this case the current nv_n2asid number is stale
> and the value of data->next_asid is <= of nv->nv_n2asid.

How do you know for sure that next_asid will be <= nv_n2asid in this case?
What if other VCPUs have run meanwhile, and next_asid has been incremented
multiple times until it is greather than nv_n2asid?

 -- Keir

> The the value of nv->nv_n2asid is valid if l1 guest doesn't change
> the virtual asid (= asid number in the virtual vmcb) and
> data->next_asid is larger than nv->nv_n2asid. In this case
> just reuse the same hw ASID that has been used from the last
> VMRUN emulation.
> 
> 5. nestedhvm is enabled and we are going to run l2 guest again
> 
> The same hw ASID should be reused unless the generation changed because
> the nestedp2m got flushed or the vcpu moved to a different physical cpu,
> for example.
> But the hw ASID number may never match the hw ASID used to run the l1 guest.
> 
> 
> In all cases we have to verify that the
> 
>> I wouldn't bother fixing #2 unless there's a convincing answer for #1.
>> 
>>   -- Keir
> 

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

On 04/13/11 17:05, Keir Fraser wrote:
> On 13/04/2011 15:26, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>> On 04/13/11 15:27, Keir Fraser wrote:
>>> On 13/04/2011 11:37, "Christoph Egger"<Christoph.Egger@amd.com>   wrote:
>>>
>> We talk about a win of about 1000 cycles per VMRUN and another 1000
>> cycles per VMEXIT emulation.
>>
>> That's a speedup of about 10% for each VMRUN and about 20% for each
>> VMEXIT emulation.
>
> Is this measurable on a macro benchmark?

I measured this with xentrace while l2 guest is booting.

The speedup is noticable on the end-user side just by the feeling on how 
fast the l2 guest reacts on user input.


> I mean this looks like a micro-optimisation on a feature that noone is going
> to use for serious performance work anyway.
>
>> 4. nestedhvm is enabled and we are going to run l2 guest
>>
>> We run the l1 guest in the last call. The asid generation may have
>> changed by then. In this case the current nv_n2asid number is stale
>> and the value of data->next_asid is<= of nv->nv_n2asid.
>
> How do you know for sure that next_asid will be<= nv_n2asid in this case?
> What if other VCPUs have run meanwhile, and next_asid has been incremented
> multiple times until it is greather than nv_n2asid?

In that case  curr->arch.hvm_vcpu.asid_generation is not equal to 
data->core_asid_generation  and a new hw ASID number is assigned
regardless of the values of data->next_asid and nv_n2asid.

Christoph

>
>   -- Keir
>
>> The the value of nv->nv_n2asid is valid if l1 guest doesn't change
>> the virtual asid (= asid number in the virtual vmcb) and
>> data->next_asid is larger than nv->nv_n2asid. In this case
>> just reuse the same hw ASID that has been used from the last
>> VMRUN emulation.
>>
>> 5. nestedhvm is enabled and we are going to run l2 guest again
>>
>> The same hw ASID should be reused unless the generation changed because
>> the nestedp2m got flushed or the vcpu moved to a different physical cpu,
>> for example.
>> But the hw ASID number may never match the hw ASID used to run the l1 guest.
>>
>>
>> In all cases we have to verify that the
>>
>>> I wouldn't bother fixing #2 unless there's a convincing answer for #1.
>>>
>>>    -- Keir
>>
>
>
>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 13/04/2011 16:19, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On 04/13/11 17:05, Keir Fraser wrote:
>> On 13/04/2011 15:26, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>> 
>> Is this measurable on a macro benchmark?
> 
> I measured this with xentrace while l2 guest is booting.
> 
> The speedup is noticable on the end-user side just by the feeling on how
> fast the l2 guest reacts on user input.

Yikes. Does nestedhvm suck really badly then? I can't believe this patch
alone gets you from sucky to good performance. Is it an improvement from
sucky to not-quite-as-sucky?

>> I mean this looks like a micro-optimisation on a feature that noone is going
>> to use for serious performance work anyway.
>> 
>>> 4. nestedhvm is enabled and we are going to run l2 guest
>>> 
>>> We run the l1 guest in the last call. The asid generation may have
>>> changed by then. In this case the current nv_n2asid number is stale
>>> and the value of data->next_asid is<= of nv->nv_n2asid.
>> 
>> How do you know for sure that next_asid will be<= nv_n2asid in this case?
>> What if other VCPUs have run meanwhile, and next_asid has been incremented
>> multiple times until it is greather than nv_n2asid?
> 
> In that case  curr->arch.hvm_vcpu.asid_generation is not equal to
> data->core_asid_generation  and a new hw ASID number is assigned
> regardless of the values of data->next_asid and nv_n2asid.

What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid
was assigned from the current generation? Then you'd have next_asid >
nv_n2asid, but nv_n2asid is stale.

 -- Keir

> Christoph
> 
>> 
>>   -- Keir
>> 
>>> The the value of nv->nv_n2asid is valid if l1 guest doesn't change
>>> the virtual asid (= asid number in the virtual vmcb) and
>>> data->next_asid is larger than nv->nv_n2asid. In this case
>>> just reuse the same hw ASID that has been used from the last
>>> VMRUN emulation.
>>> 
>>> 5. nestedhvm is enabled and we are going to run l2 guest again
>>> 
>>> The same hw ASID should be reused unless the generation changed because
>>> the nestedp2m got flushed or the vcpu moved to a different physical cpu,
>>> for example.
>>> But the hw ASID number may never match the hw ASID used to run the l1 guest.
>>> 
>>> 
>>> In all cases we have to verify that the
>>> 
>>>> I wouldn't bother fixing #2 unless there's a convincing answer for #1.
>>>> 
>>>>    -- Keir
>>> 
>> 
>> 
>> 
> 

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

On 04/13/11 18:22, Keir Fraser wrote:
> On 13/04/2011 16:19, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>> On 04/13/11 17:05, Keir Fraser wrote:
>>> On 13/04/2011 15:26, "Christoph Egger"<Christoph.Egger@amd.com>   wrote:
>>>
>>> Is this measurable on a macro benchmark?
>>
>> I measured this with xentrace while l2 guest is booting.
>>
>> The speedup is noticable on the end-user side just by the feeling on how
>> fast the l2 guest reacts on user input.
>
> Yikes. Does nestedhvm suck really badly then? I can't believe this patch
> alone gets you from sucky to good performance. Is it an improvement from
> sucky to not-quite-as-sucky?

The very first patch series I sent to xen-devel, the cost of a
VMRUN emulation was about 25000 cycles with Xen-on-Xen while hvmloader
was coming up as l2 guest. With every new patch series there were also
performance optimizations done. Now the cost of a VMRUN
emulation is about 11000 cycles and with this ASID emulation patch
the cost of a VMRUN emulation goes down to about 10000 cycles.

There is still room for more performance, I am working on.

A cost of 15000 cycles for VMRUN emulation is fast enough that Solitair
in XP mode of Windows 7 guest recognizes a double click ;-)

Solitair in XP mode runs faster than Solitair in Windows 7 ;-)


>>> I mean this looks like a micro-optimisation on a feature that noone is going
>>> to use for serious performance work anyway.
>>>
>>>> 4. nestedhvm is enabled and we are going to run l2 guest
>>>>
>>>> We run the l1 guest in the last call. The asid generation may have
>>>> changed by then. In this case the current nv_n2asid number is stale
>>>> and the value of data->next_asid is<= of nv->nv_n2asid.
>>>
>>> How do you know for sure that next_asid will be<= nv_n2asid in this case?
>>> What if other VCPUs have run meanwhile, and next_asid has been incremented
>>> multiple times until it is greather than nv_n2asid?
>>
>> In that case  curr->arch.hvm_vcpu.asid_generation is not equal to
>> data->core_asid_generation  and a new hw ASID number is assigned
>> regardless of the values of data->next_asid and nv_n2asid.
>
> What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid
> was assigned from the current generation? Then you'd have next_asid>
> nv_n2asid, but nv_n2asid is stale.

When the generation changes, next_asid is set back to 1 and the hw TLB 
flushed.

If next_asid is larger than nv_n2asid then it is reused and this is ok
since the hw TLB got flushed. It is just important that nv_n1asid never
gets the same hw ASID assigned and that is what the do { } while loop
ensures.

The hardware doesn't enforce an incremental use of the hw ASID numbers.

The point of ASID emulation is to reduce the number of
TLB invalidations by switching forth and back between two
hw ASID numbers when switching between l1 and l2 guest.
This reduces pagetable walks of the hw MMU so much that the
performance win is noticable by the end-user.

Without this patch the ASID is invalidated on every VMRUN and VMEXIT 
emulation.

The Flush-by-ASID feature even let me reuse the same hw ASID number
when the l2 guest wants to flush its TLB.

Christoph


>   -- Keir
>
>> Christoph
>>
>>>
>>>    -- Keir
>>>
>>>> The the value of nv->nv_n2asid is valid if l1 guest doesn't change
>>>> the virtual asid (= asid number in the virtual vmcb) and
>>>> data->next_asid is larger than nv->nv_n2asid. In this case
>>>> just reuse the same hw ASID that has been used from the last
>>>> VMRUN emulation.
>>>>
>>>> 5. nestedhvm is enabled and we are going to run l2 guest again
>>>>
>>>> The same hw ASID should be reused unless the generation changed because
>>>> the nestedp2m got flushed or the vcpu moved to a different physical cpu,
>>>> for example.
>>>> But the hw ASID number may never match the hw ASID used to run the l1 guest.
>>>>
>>>>
>>>> In all cases we have to verify that the
>>>>
>>>>> I wouldn't bother fixing #2 unless there's a convincing answer for #1.



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 14/04/2011 10:26, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid
>> was assigned from the current generation? Then you'd have next_asid>
>> nv_n2asid, but nv_n2asid is stale.
> 
> When the generation changes, next_asid is set back to 1 and the hw TLB
> flushed.
> 
> If next_asid is larger than nv_n2asid then it is reused and this is ok
> since the hw TLB got flushed. It is just important that nv_n1asid never
> gets the same hw ASID assigned and that is what the do { } while loop
> ensures.

What if some other vcpu's nv_n1asid or nv_n2asid got assigned the same HW
asid in this generation as this vcpu's (now stale, as it's from a previous
generation's) nv_n2asid? This PCPU can be interleaving execution of other
HVM VCPUs after all.

 -- Keir

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

On 04/14/11 12:28, Keir Fraser wrote:
> On 14/04/2011 10:26, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>> What if nv_n2asid wast last assigned on a previous generation, but nv_n1asid
>>> was assigned from the current generation? Then you'd have next_asid>
>>> nv_n2asid, but nv_n2asid is stale.
>>
>> When the generation changes, next_asid is set back to 1 and the hw TLB
>> flushed.
>>
>> If next_asid is larger than nv_n2asid then it is reused and this is ok
>> since the hw TLB got flushed. It is just important that nv_n1asid never
>> gets the same hw ASID assigned and that is what the do { } while loop
>> ensures.
>
> What if some other vcpu's nv_n1asid or nv_n2asid got assigned the same HW
> asid in this generation as this vcpu's (now stale, as it's from a previous
> generation's) nv_n2asid? This PCPU can be interleaving execution of other
> HVM VCPUs after all.

I am not sure if I got you right. You mean what if two vcpus run on one
physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu() 
before so that asid_generation and core_asid_generation do not match and 
a new asid is always assigned.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]

On 14/04/2011 15:01, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> What if some other vcpu's nv_n1asid or nv_n2asid got assigned the same HW
>> asid in this generation as this vcpu's (now stale, as it's from a previous
>> generation's) nv_n2asid? This PCPU can be interleaving execution of other
>> HVM VCPUs after all.
> 
> I am not sure if I got you right. You mean what if two vcpus run on one
> physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu()
> before so that asid_generation and core_asid_generation do not match and
> a new asid is always assigned.

No, it only does that if a given VCPU gets scheduled onto a *different* PCPU
than last time it ran.

I've attached a mostly rewritten version of your patch that is about half
the size and I believe has a fighting chance of being correct (however it is
only build tested). Give it a look and a spin.

 -- Keir

> Christoph


[-- Attachment #2: 00-nhvm-asid --]
[-- Type: application/octet-stream, Size: 8611 bytes --]

diff -r b5165fb66b56 xen/arch/x86/hvm/asid.c
--- a/xen/arch/x86/hvm/asid.c	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/arch/x86/hvm/asid.c	Thu Apr 14 15:40:48 2011 +0100
@@ -78,9 +78,15 @@
     data->next_asid = 1;
 }
 
+void hvm_asid_flush_vcpu_asid(struct hvm_vcpu_asid *asid)
+{
+    asid->generation = 0;
+}
+
 void hvm_asid_flush_vcpu(struct vcpu *v)
 {
-    v->arch.hvm_vcpu.asid_generation = 0;
+    hvm_asid_flush_vcpu_asid(&v->arch.hvm_vcpu.n1_asid);
+    hvm_asid_flush_vcpu_asid(&v->arch.hvm_vcpu.n2_asid);
 }
 
 void hvm_asid_flush_core(void)
@@ -102,9 +108,8 @@
     data->disabled = 1;
 }
 
-bool_t hvm_asid_handle_vmenter(void)
+bool_t hvm_asid_handle_vmenter(struct hvm_vcpu_asid *asid)
 {
-    struct vcpu *curr = current;
     struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
 
     /* On erratum #170 systems we must flush the TLB. 
@@ -113,7 +118,7 @@
         goto disabled;
 
     /* Test if VCPU has valid ASID. */
-    if ( curr->arch.hvm_vcpu.asid_generation == data->core_asid_generation )
+    if ( asid->generation == data->core_asid_generation )
         return 0;
 
     /* If there are no free ASIDs, need to go to a new generation */
@@ -126,17 +131,17 @@
     }
 
     /* Now guaranteed to be a free ASID. */
-    curr->arch.hvm_vcpu.asid = data->next_asid++;
-    curr->arch.hvm_vcpu.asid_generation = data->core_asid_generation;
+    asid->asid = data->next_asid++;
+    asid->generation = data->core_asid_generation;
 
     /*
      * When we assign ASID 1, flush all TLB entries as we are starting a new
      * generation, and all old ASID allocations are now stale. 
      */
-    return (curr->arch.hvm_vcpu.asid == 1);
+    return (asid->asid == 1);
 
  disabled:
-    curr->arch.hvm_vcpu.asid = 0;
+    asid->asid = 0;
     return 0;
 }
 
diff -r b5165fb66b56 xen/arch/x86/hvm/svm/asid.c
--- a/xen/arch/x86/hvm/svm/asid.c	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/arch/x86/hvm/svm/asid.c	Thu Apr 14 15:40:48 2011 +0100
@@ -22,6 +22,7 @@
 #include <xen/perfc.h>
 #include <asm/hvm/svm/asid.h>
 #include <asm/amd.h>
+#include <asm/hvm/nestedhvm.h>
 
 void svm_asid_init(struct cpuinfo_x86 *c)
 {
@@ -42,17 +43,20 @@
 {
     struct vcpu *curr = current;
     struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
-    bool_t need_flush = hvm_asid_handle_vmenter();
+    struct hvm_vcpu_asid *curr_asid =
+        nestedhvm_vcpu_in_guestmode(curr)
+        ? &curr->arch.hvm_vcpu.n1_asid : &curr->arch.hvm_vcpu.n2_asid;
+    bool_t need_flush = hvm_asid_handle_vmenter(curr_asid);
 
     /* ASID 0 indicates that ASIDs are disabled. */
-    if ( curr->arch.hvm_vcpu.asid == 0 )
+    if ( curr_asid->asid == 0 )
     {
         vmcb_set_guest_asid(vmcb, 1);
         vmcb->tlb_control = 1;
         return;
     }
 
-    vmcb_set_guest_asid(vmcb, curr->arch.hvm_vcpu.asid);
+    vmcb_set_guest_asid(vmcb, curr_asid->asid);
     vmcb->tlb_control = need_flush;
 }
 
diff -r b5165fb66b56 xen/arch/x86/hvm/svm/nestedsvm.c
--- a/xen/arch/x86/hvm/svm/nestedsvm.c	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c	Thu Apr 14 15:40:48 2011 +0100
@@ -261,8 +261,6 @@
     /* Cleanbits */
     n1vmcb->cleanbits.bytes = 0;
 
-    hvm_asid_flush_vcpu(v);
-
     return 0;
 }
 
@@ -408,9 +406,7 @@
     if (rc)
         return rc;
 
-    /* ASID */
-    hvm_asid_flush_vcpu(v);
-    /* n2vmcb->_guest_asid = ns_vmcb->_guest_asid; */
+    /* ASID - Emulation handled in hvm_asid_handle_vmenter() */
 
     /* TLB control */
     n2vmcb->tlb_control = n1vmcb->tlb_control | ns_vmcb->tlb_control;
@@ -605,9 +601,13 @@
     svm->ns_vmcb_guestcr3 = ns_vmcb->_cr3;
     svm->ns_vmcb_hostcr3 = ns_vmcb->_h_cr3;
 
-    nv->nv_flushp2m = (ns_vmcb->tlb_control
-        || (svm->ns_guest_asid != ns_vmcb->_guest_asid));
-    svm->ns_guest_asid = ns_vmcb->_guest_asid;
+    nv->nv_flushp2m = ns_vmcb->tlb_control;
+    if ( svm->ns_guest_asid != ns_vmcb->_guest_asid )
+    {
+        nv->nv_flushp2m = 1;
+        hvm_asid_flush_vcpu_asid(&v->arch.hvm_vcpu.n2_asid);
+        svm->ns_guest_asid = ns_vmcb->_guest_asid;
+    }
 
     /* nested paging for the guest */
     svm->ns_hap_enabled = (ns_vmcb->_np_enable) ? 1 : 0;
diff -r b5165fb66b56 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c	Thu Apr 14 15:40:48 2011 +0100
@@ -1580,6 +1580,15 @@
     __update_guest_eip(regs, inst_len);
 }
 
+static void svm_invlpga_intercept(
+    struct vcpu *v, unsigned long vaddr, uint32_t asid)
+{
+    svm_invlpga(vaddr,
+                (asid == 0)
+                ? v->arch.hvm_vcpu.n1_asid.asid
+                : v->arch.hvm_vcpu.n2_asid.asid);
+}
+
 static void svm_invlpg_intercept(unsigned long vaddr)
 {
     struct vcpu *curr = current;
@@ -1894,11 +1903,14 @@
     case VMEXIT_CR0_READ ... VMEXIT_CR15_READ:
     case VMEXIT_CR0_WRITE ... VMEXIT_CR15_WRITE:
     case VMEXIT_INVLPG:
-    case VMEXIT_INVLPGA:
         if ( !handle_mmio() )
             hvm_inject_exception(TRAP_gp_fault, 0, 0);
         break;
 
+    case VMEXIT_INVLPGA:
+        svm_invlpga_intercept(v, regs->rax, regs->ecx);
+        break;
+
     case VMEXIT_VMMCALL:
         if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 )
             break;
diff -r b5165fb66b56 xen/arch/x86/hvm/vmx/vmcs.c
--- a/xen/arch/x86/hvm/vmx/vmcs.c	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/arch/x86/hvm/vmx/vmcs.c	Thu Apr 14 15:40:48 2011 +0100
@@ -867,9 +867,6 @@
 #endif
     }
 
-    if ( cpu_has_vmx_vpid )
-        __vmwrite(VIRTUAL_PROCESSOR_ID, v->arch.hvm_vcpu.asid);
-
     if ( cpu_has_vmx_pat && paging_mode_hap(d) )
     {
         u64 host_pat, guest_pat;
diff -r b5165fb66b56 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c	Thu Apr 14 15:40:48 2011 +0100
@@ -2667,14 +2667,16 @@
 {
     struct vcpu *curr = current;
     u32 new_asid, old_asid;
+    struct hvm_vcpu_asid *curr_asid;
     bool_t need_flush;
 
     if ( !cpu_has_vmx_vpid )
         goto out;
 
-    old_asid = curr->arch.hvm_vcpu.asid;
-    need_flush = hvm_asid_handle_vmenter();
-    new_asid = curr->arch.hvm_vcpu.asid;
+    curr_asid = &curr->arch.hvm_vcpu.n1_asid;
+    old_asid = curr_asid->asid;
+    need_flush = hvm_asid_handle_vmenter(curr_asid);
+    new_asid = curr_asid->asid;
 
     if ( unlikely(new_asid != old_asid) )
     {
diff -r b5165fb66b56 xen/include/asm-x86/hvm/asid.h
--- a/xen/include/asm-x86/hvm/asid.h	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/include/asm-x86/hvm/asid.h	Thu Apr 14 15:40:48 2011 +0100
@@ -23,11 +23,15 @@
 #include <xen/config.h>
 
 struct vcpu;
+struct hvm_vcpu_asid;
 
 /* Initialise ASID management for the current physical CPU. */
 void hvm_asid_init(int nasids);
 
-/* Invalidate a VCPU's current ASID allocation: forces re-allocation. */
+/* Invalidate a particular ASID allocation: forces re-allocation. */
+void hvm_asid_flush_vcpu_asid(struct hvm_vcpu_asid *asid);
+
+/* Invalidate all ASID allocations for specified VCPU: forces re-allocation. */
 void hvm_asid_flush_vcpu(struct vcpu *v);
 
 /* Flush all ASIDs on this processor core. */
@@ -35,7 +39,7 @@
 
 /* Called before entry to guest context. Checks ASID allocation, returns a
  * boolean indicating whether all ASIDs must be flushed. */
-bool_t hvm_asid_handle_vmenter(void);
+bool_t hvm_asid_handle_vmenter(struct hvm_vcpu_asid *asid);
 
 #endif /* __ASM_X86_HVM_ASID_H__ */
 
diff -r b5165fb66b56 xen/include/asm-x86/hvm/vcpu.h
--- a/xen/include/asm-x86/hvm/vcpu.h	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/include/asm-x86/hvm/vcpu.h	Thu Apr 14 15:40:48 2011 +0100
@@ -70,6 +70,11 @@
 
 #define vcpu_nestedhvm(v) ((v)->arch.hvm_vcpu.nvcpu)
 
+struct hvm_vcpu_asid {
+    uint64_t generation;
+    uint32_t asid;
+};
+
 struct hvm_vcpu {
     /* Guest control-register and EFER values, just as the guest sees them. */
     unsigned long       guest_cr[5];
@@ -100,8 +105,7 @@
     bool_t              hcall_preempted;
     bool_t              hcall_64bit;
 
-    uint64_t            asid_generation;
-    uint32_t            asid;
+    struct hvm_vcpu_asid n1_asid, n2_asid;
 
     u32                 msr_tsc_aux;
 
diff -r b5165fb66b56 xen/include/asm-x86/hvm/vmx/vmx.h
--- a/xen/include/asm-x86/hvm/vmx/vmx.h	Thu Apr 14 14:57:24 2011 +0100
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h	Thu Apr 14 15:40:48 2011 +0100
@@ -377,7 +377,7 @@
         type = INVVPID_ALL_CONTEXT;
 
 execute_invvpid:
-    __invvpid(type, v->arch.hvm_vcpu.asid, (u64)gva);
+    __invvpid(type, v->arch.hvm_vcpu.n1_asid.asid, (u64)gva);
 }
 
 static inline void vpid_sync_all(void)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

On 04/14/11 16:43, Keir Fraser wrote:
> On 14/04/2011 15:01, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>> What if some other vcpu's nv_n1asid or nv_n2asid got assigned the same HW
>>> asid in this generation as this vcpu's (now stale, as it's from a previous
>>> generation's) nv_n2asid? This PCPU can be interleaving execution of other
>>> HVM VCPUs after all.
>>
>> I am not sure if I got you right. You mean what if two vcpus run on one
>> physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu()
>> before so that asid_generation and core_asid_generation do not match and
>> a new asid is always assigned.
>
> No, it only does that if a given VCPU gets scheduled onto a *different* PCPU
> than last time it ran.
>
> I've attached a mostly rewritten version of your patch that is about half
> the size and I believe has a fighting chance of being correct (however it is
> only build tested). Give it a look and a spin.

Yes, it is correct. I like the idea to maintain a generation per asid.
Please apply it. Thanks for this work.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 15/04/2011 09:20, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On 04/14/11 16:43, Keir Fraser wrote:
>> On 14/04/2011 15:01, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>> 
>>>> What if some other vcpu's nv_n1asid or nv_n2asid got assigned the same HW
>>>> asid in this generation as this vcpu's (now stale, as it's from a previous
>>>> generation's) nv_n2asid? This PCPU can be interleaving execution of other
>>>> HVM VCPUs after all.
>>> 
>>> I am not sure if I got you right. You mean what if two vcpus run on one
>>> physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu()
>>> before so that asid_generation and core_asid_generation do not match and
>>> a new asid is always assigned.
>> 
>> No, it only does that if a given VCPU gets scheduled onto a *different* PCPU
>> than last time it ran.
>> 
>> I've attached a mostly rewritten version of your patch that is about half
>> the size and I believe has a fighting chance of being correct (however it is
>> only build tested). Give it a look and a spin.
> 
> Yes, it is correct. I like the idea to maintain a generation per asid.
> Please apply it. Thanks for this work.

You didn't notice my subtle error in switching n1 and n2 asid selection in
svm_asid_handle_vmrun()? ;-) Actually I bet it would take a lot of testing
to pick up on that error, since the transposition doesn't much matter except
that we are then switched round relative to which ASID gets flushed on
guest-initiated INVLPGA, and also we'd flush the 'wrong' ASID when guest
requests a new L2 guest ASID. Anyway, glad I spotted it before I checked the
patch in! Stale TLB bugs are no fun.

 -- Keir

> Christoph
> 

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

On 04/15/11 11:05, Keir Fraser wrote:
>
>
>
> On 15/04/2011 09:20, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>> On 04/14/11 16:43, Keir Fraser wrote:
>>> On 14/04/2011 15:01, "Christoph Egger"<Christoph.Egger@amd.com>   wrote:
>>>
>>>>> What if some other vcpu's nv_n1asid or nv_n2asid got assigned the same HW
>>>>> asid in this generation as this vcpu's (now stale, as it's from a previous
>>>>> generation's) nv_n2asid? This PCPU can be interleaving execution of other
>>>>> HVM VCPUs after all.
>>>>
>>>> I am not sure if I got you right. You mean what if two vcpus run on one
>>>> physical cpu? In this case svm_do_resume() calls hvm_asid_flush_vcpu()
>>>> before so that asid_generation and core_asid_generation do not match and
>>>> a new asid is always assigned.
>>>
>>> No, it only does that if a given VCPU gets scheduled onto a *different* PCPU
>>> than last time it ran.
>>>
>>> I've attached a mostly rewritten version of your patch that is about half
>>> the size and I believe has a fighting chance of being correct (however it is
>>> only build tested). Give it a look and a spin.
>>
>> Yes, it is correct. I like the idea to maintain a generation per asid.
>> Please apply it. Thanks for this work.
>
> You didn't notice my subtle error in switching n1 and n2 asid selection in
> svm_asid_handle_vmrun()? ;-)

Oh, now that you mention it...

A l2 smp guest actually boots up. Maybe I need to use more vcpus than I 
have physical cpus to hit that bug.

I think, this snippet should be like this:

struct vcpu *curr = current;
struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
struct hvm_vcpu_asid *curr_asid;
bool_t need_flush;
bool_t vcpu_guestmode = 0;

if (nestedhvm_enabled(curr->domain) && nestedhvm_vcpu_in_guestmode(curr))
     vcpu_guestmode = 1;

curr_asid = vcpu_guestmode ?
     &curr->arch.hvm_vcpu.n2_asid : &curr->arch.hvm_vcpu.n1_asid;
need_flush = hvm_asid_handle_vmenter(curr_asid);


Christoph


> Actually I bet it would take a lot of testing to pick up on that error,
 > since the transposition doesn't much matter except
> that we are then switched round relative to which ASID gets flushed on
> guest-initiated INVLPGA, and also we'd flush the 'wrong' ASID when guest
> requests a new L2 guest ASID. Anyway, glad I spotted it before I checked the
> patch in! Stale TLB bugs are no fun.
>
>   -- Keir
>
>> Christoph
>>
>
>
>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 15/04/2011 10:08, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

>> You didn't notice my subtle error in switching n1 and n2 asid selection in
>> svm_asid_handle_vmrun()? ;-)
> 
> Oh, now that you mention it...

Yeah, I fixed it before I checked it in (xen-unstable:23229).

> A l2 smp guest actually boots up. Maybe I need to use more vcpus than I
> have physical cpus to hit that bug.

I think it would have been a source of very subtle quite hard-to-repro bugs.
Not something you'd pick up in a simple does-it-boot smoke test. So a pretty
nasty bug all round!

 -- Keir

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

On 04/15/11 11:24, Keir Fraser wrote:
> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>>> You didn't notice my subtle error in switching n1 and n2 asid selection in
>>> svm_asid_handle_vmrun()? ;-)
>>
>> Oh, now that you mention it...
>
> Yeah, I fixed it before I checked it in (xen-unstable:23229).
>
>> A l2 smp guest actually boots up. Maybe I need to use more vcpus than I
>> have physical cpus to hit that bug.
>
> I think it would have been a source of very subtle quite hard-to-repro bugs.
> Not something you'd pick up in a simple does-it-boot smoke test. So a pretty
> nasty bug all round!

Just found another fallout that got lost from my original patch.

After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER 
after about 30 seconds. When starting XP mode again, Win 7 turns SVM on
again.

Then nv_n2asid can be stale. Attached patch fixes this.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_nh_asid.diff --]
[-- Type: text/plain, Size: 418 bytes --]

diff -r 9a0ed35421da -r ad5960696d68 xen/arch/x86/hvm/nestedhvm.c
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -61,6 +61,7 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_vvmcxaddr = VMCX_EADDR;
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
+    memset(&nv->nv_n2asid, 0, sizeof(struct hvm_vcpu_asid));
 
     if ( hvm_funcs.nhvm_vcpu_reset )
         hvm_funcs.nhvm_vcpu_reset(v);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

On 04/15/11 14:53, Keir Fraser wrote:
> On 15/04/2011 10:57, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>> On 04/15/11 11:24, Keir Fraser wrote:
>>> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com>   wrote:
>>
>> Just found another fallout that got lost from my original patch.
>>
>> After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER
>> after about 30 seconds. When starting XP mode again, Win 7 turns SVM on
>> again.
>>
>> Then nv_n2asid can be stale. Attached patch fixes this.
>
> Should be using hvm_asid_flush_vcpu_asid(), so I'll fix the patch to use
> that and then check it in.

Yes, right. Thank you.

Christoph

>   Thanks,
>   Keir
>
>> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>
>>
>
>
>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] nestedhvm: ASID emulation

[Keir Fraser]

On 15/04/2011 10:57, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On 04/15/11 11:24, Keir Fraser wrote:
>> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
> 
> Just found another fallout that got lost from my original patch.
> 
> After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER
> after about 30 seconds. When starting XP mode again, Win 7 turns SVM on
> again.
> 
> Then nv_n2asid can be stale. Attached patch fixes this.

Should be using hvm_asid_flush_vcpu_asid(), so I'll fix the patch to use
that and then check it in.

 Thanks,
 Keir

> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>
> 

Re: [PATCH] nestedhvm: ASID emulation

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

On 04/15/11 14:53, Keir Fraser wrote:
> On 15/04/2011 10:57, "Christoph Egger"<Christoph.Egger@amd.com>  wrote:
>
>> On 04/15/11 11:24, Keir Fraser wrote:
>>> On 15/04/2011 10:08, "Christoph Egger"<Christoph.Egger@amd.com>   wrote:
>>
>> Just found another fallout that got lost from my original patch.
>>
>> After shutting down XP mode in Windows 7, Win7 turns off SVM in EFER
>> after about 30 seconds. When starting XP mode again, Win 7 turns SVM on
>> again.
>>
>> Then nv_n2asid can be stale. Attached patch fixes this.
>
> Should be using hvm_asid_flush_vcpu_asid(), so I'll fix the patch to use
> that and then check it in.

There are a lot of regressions now... I assume the non-nested 
virtualization case got broken.

Attached patch covers the non-nested case (and contains the above 
discussed hvm_asid_flush_vcpu_asid() as it didn't appear yet upstream).

Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_nh_asid.diff --]
[-- Type: text/plain, Size: 1436 bytes --]

diff -r ad5960696d68 xen/arch/x86/hvm/nestedhvm.c
--- a/xen/arch/x86/hvm/nestedhvm.c	Fri Apr 15 12:00:13 2011 +0200
+++ b/xen/arch/x86/hvm/nestedhvm.c	Fri Apr 15 15:43:15 2011 +0200
@@ -61,7 +61,7 @@ nestedhvm_vcpu_reset(struct vcpu *v)
     nv->nv_vvmcxaddr = VMCX_EADDR;
     nv->nv_flushp2m = 0;
     nv->nv_p2m = NULL;
+    hvm_asid_flush_vcpu_asid(&nv->nv_n2asid);
 
     if ( hvm_funcs.nhvm_vcpu_reset )
         hvm_funcs.nhvm_vcpu_reset(v);
diff -r ad5960696d68 xen/arch/x86/hvm/svm/asid.c
--- a/xen/arch/x86/hvm/svm/asid.c	Fri Apr 15 12:00:13 2011 +0200
+++ b/xen/arch/x86/hvm/svm/asid.c	Fri Apr 15 15:43:15 2011 +0200
@@ -43,10 +43,16 @@ asmlinkage void svm_asid_handle_vmrun(vo
 {
     struct vcpu *curr = current;
     struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
-    struct hvm_vcpu_asid *p_asid =
-        nestedhvm_vcpu_in_guestmode(curr)
-        ? &vcpu_nestedhvm(curr).nv_n2asid : &curr->arch.hvm_vcpu.n1asid;
-    bool_t need_flush = hvm_asid_handle_vmenter(p_asid);
+    struct hvm_vcpu_asid *p_asid;
+    bool_t need_flush;
+    bool_t vcpu_guestmode = 0;
+
+    if (nestedhvm_enabled(curr->domain) && nestedhvm_vcpu_in_guestmode(curr))
+        vcpu_guestmode = 1;
+
+    p_asid = vcpu_guestmode ?
+        &vcpu_nestedhvm(curr).nv_n2asid : &curr->arch.hvm_vcpu.n1asid;
+    need_flush = hvm_asid_handle_vmenter(p_asid);
 
     /* ASID 0 indicates that ASIDs are disabled. */
     if ( p_asid->asid == 0 )

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel