[PATCH v1 0/5] x86/nestedsvm: rework nested VMRUN/VMEXIT handling

[PATCH v1 0/5] x86/nestedsvm: rework nested VMRUN/VMEXIT handling

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

This series simplifies nested SVM execution by removing pending and
deferred state from the VMRUN and VMEXIT paths.  Nested transitions are
handled directly by rearranging the state transfer and control flow.

The series also removes obsolete interrupt-window and vCPU-switch handling
associated with `nv_vmentry_pending`, while updating the relevant assembly
offsets and nested-SVM data structures.

Patch 1 rearranges VMRUN handling without pending or deferred flags.
Patch 2 removes code related to `nv_vmentry_pending`.
Patch 3 rearranges VMEXIT handling without pending or deferred flags.
Patch 4 update vmcb fields correctly.
Patch 5 fix issues which cause transition failure.

Chunjie Zhu (5):
  x86/nestedsvm: update vmrun without pending or deferred flag
  x86/nestedsvm: remove nv_vmentry_pending
  x86/nestedsvm: update vmexit without pending or deferred flag
  x86/nestedsvm: update vmcb correctly
  x86/nestedsvm: fix up

 xen/arch/x86/hvm/svm/entry.S             |  48 +++-
 xen/arch/x86/hvm/svm/intr.c              |  22 +-
 xen/arch/x86/hvm/svm/nestedhvm.h         |   6 +-
 xen/arch/x86/hvm/svm/nestedsvm.c         | 337 ++++++++++++-----------
 xen/arch/x86/hvm/svm/svm.c               |  25 +-
 xen/arch/x86/hvm/svm/vmcb.c              |  12 +-
 xen/arch/x86/hvm/svm/vmcb.h              |   6 +
 xen/arch/x86/include/asm/hvm/svm-types.h |   9 +-
 xen/arch/x86/include/asm/msr-index.h     |   1 +
 xen/arch/x86/mm/p2m.c                    |   5 +
 xen/arch/x86/x86_64/asm-offsets.c        |   1 +
 11 files changed, 260 insertions(+), 212 deletions(-)

-- 
2.34.1

[PATCH v1 1/5] x86/nestedsvm: update vmrun without pending or deferred flag

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/hvm/svm/entry.S      | 48 +++++++++++++++++++++++--------
 xen/arch/x86/hvm/svm/nestedhvm.h  |  1 +
 xen/arch/x86/hvm/svm/nestedsvm.c  | 31 +++++++++++++++++++-
 xen/arch/x86/hvm/svm/svm.c        | 11 +++++--
 xen/arch/x86/x86_64/asm-offsets.c |  1 +
 5 files changed, 77 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index d9613a2a8fed..59d865635fd5 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -28,31 +28,51 @@ FUNC(svm_asm_do_resume)
         GET_CURRENT(bx)
 .Lsvm_do_resume:
         call svm_intr_assist
-        call nsvm_vcpu_switch
-        ASSERT_NOT_IN_ATOMIC
 
-        mov  VCPU_processor(%rbx),%eax
-        lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
+        /*
+         * The nested p2m repair below may need to take nestedp2m_lock and
+         * issue synchronous cross-CPU TLB flush IPIs, doing this with
+         * interrupts disabled can deadlock.
+         */
         xor  %ecx,%ecx
-        shl  $IRQSTAT_shift,%eax
-        cli
-        cmp  %ecx,(%rdx,%rax,1)
-        jne  .Lsvm_process_softirqs
-
         cmp  %cl,VCPU_nsvm_hap_enabled(%rbx)
 UNLIKELY_START(ne, nsvm_hap)
+        cmpb $0,VCPU_nhvm_guestmode(%rbx)
+        UNLIKELY_DONE(z, nsvm_hap)
+        cmpb $0,VCPU_nhvm_stale_np2m(%rbx)
+        jne  .Lsvm_update_nestedp2m
         cmp  %rcx,VCPU_nhvm_p2m(%rbx)
         sete %al
         test VCPU_nhvm_guestmode(%rbx),%al
         UNLIKELY_DONE(z, nsvm_hap)
+.Lsvm_update_nestedp2m:
         /*
-         * Someone shot down our nested p2m table; go round again
-         * and nsvm_vcpu_switch() will fix it for us.
+         * The nested p2m table is stale, fix it here.
          */
-        sti
+        mov  %rbx,%rdi
+        call nsvm_vcpu_update_nestedp2m
         jmp  .Lsvm_do_resume
 __UNLIKELY_END(nsvm_hap)
 
+        ASSERT_NOT_IN_ATOMIC
+
+        mov  VCPU_processor(%rbx),%eax
+        lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
+        xor  %ecx,%ecx
+        shl  $IRQSTAT_shift,%eax
+        cli
+        cmp  %ecx,(%rdx,%rax,1)
+        jne  .Lsvm_process_softirqs
+
+        cmp  %cl,VCPU_nsvm_hap_enabled(%rbx)
+        je   .Lsvm_vmenter
+        cmpb $0,VCPU_nhvm_guestmode(%rbx)
+        je   .Lsvm_vmenter
+        cmpb $0,VCPU_nhvm_stale_np2m(%rbx)
+        jne  .Lsvm_retry_nestedp2m
+
+.Lsvm_vmenter:
+
         call svm_vmenter_helper
 
         clgi
@@ -148,4 +168,8 @@ __UNLIKELY_END(nsvm_hap)
         sti
         call do_softirq
         jmp  .Lsvm_do_resume
+
+.Lsvm_retry_nestedp2m:
+        sti
+        jmp  .Lsvm_do_resume
 END(svm_asm_do_resume)
diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
index 9bb04a043430..e22ef259c171 100644
--- a/xen/arch/x86/hvm/svm/nestedhvm.h
+++ b/xen/arch/x86/hvm/svm/nestedhvm.h
@@ -41,6 +41,7 @@ void cf_check nsvm_vcpu_destroy(struct vcpu *v);
 int cf_check nsvm_vcpu_initialise(struct vcpu *v);
 int cf_check nsvm_vcpu_reset(struct vcpu *v);
 int nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs);
+void nsvm_vcpu_update_nestedp2m(struct vcpu *v);
 int cf_check nsvm_vcpu_vmexit_event(struct vcpu *v,
                                     const struct x86_event *event);
 uint64_t cf_check nsvm_vcpu_hostcr3(struct vcpu *v);
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index a8b15d6eae05..171b32be2416 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -361,6 +361,35 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu *v,
     n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
 }
 
+void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
+{
+    struct nestedvcpu *nv;
+    struct nestedsvm *svm;
+    struct p2m_domain *p2m;
+
+    ASSERT(v != NULL);
+
+    nv = &vcpu_nestedhvm(v);
+    svm = &vcpu_nestedsvm(v);
+
+    if ( !nestedhvm_paging_mode_hap(v) )
+        return;
+
+    p2m = p2m_get_nestedp2m(v);
+
+    /*
+     * This may happen if we've handled VMEXIT_NPF at the same time as a
+     * nested flush IPI from another CPU.  We've got a different P2M so the
+     * host CR3 needs updating.
+     */
+    if ( svm->ns_vmcb_hostcr3 != vmcb_get_h_cr3(nv->nv_vvmcx) ||
+         vmcb_get_h_cr3(nv->nv_n2vmcx) !=
+         pagetable_get_paddr(p2m_get_pagetable(p2m)) )
+        nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
+
+    nv->stale_np2m = false;
+}
+
 static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
@@ -516,7 +545,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         /* host nested paging + guest nested paging. */
         vmcb_set_np(n2vmcb, true);
 
-        nestedsvm_vmcb_set_nestedp2m(v, ns_vmcb, n2vmcb);
+        nsvm_vcpu_update_nestedp2m(v);
 
         /* hvm_set_cr3() below sets v->arch.hvm.guest_cr[3] for us. */
         rc = hvm_set_cr3(ns_vmcb->_cr3, false, true);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index e6a0edcf71ab..fc447d24d67e 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2118,6 +2118,8 @@ static void
 svm_vmexit_do_vmrun(struct cpu_user_regs *regs,
                     struct vcpu *v, uint64_t vmcbaddr)
 {
+    int rc;
+
     if ( !nsvm_efer_svm_enabled(v) )
     {
         hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
@@ -2131,8 +2133,13 @@ svm_vmexit_do_vmrun(struct cpu_user_regs *regs,
         return;
     }
 
-    vcpu_nestedhvm(v).nv_vmentry_pending = 1;
-    return;
+    rc = nsvm_vcpu_vmrun(v, regs);
+    if ( rc )
+    {
+        if ( nestedsvm_vcpu_vmexit(v, regs, VMEXIT_INVALID, 0, 0) ==
+             NESTEDHVM_VMEXIT_FATALERROR )
+            domain_crash(v->domain);
+    }
 }
 
 static struct page_info *
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index baf266ab8013..772d1959e5a5 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -129,6 +129,7 @@ void __dummy__(void)
 
     OFFSET(VCPU_nhvm_guestmode, struct vcpu, arch.hvm.nvcpu.nv_guestmode);
     OFFSET(VCPU_nhvm_p2m, struct vcpu, arch.hvm.nvcpu.nv_p2m);
+    OFFSET(VCPU_nhvm_stale_np2m, struct vcpu, arch.hvm.nvcpu.stale_np2m);
     OFFSET(VCPU_nsvm_hap_enabled, struct vcpu, arch.hvm.nvcpu.u.nsvm.ns_hap_enabled);
     BLANK();
 #endif
-- 
2.34.1

[PATCH v1 2/5] x86/nestedsvm: remove nv_vmentry_pending

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/hvm/svm/intr.c      | 18 ------------------
 xen/arch/x86/hvm/svm/nestedsvm.c | 23 -----------------------
 2 files changed, 41 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 4b0debfa9a2e..7f6cafb6db78 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -77,24 +77,6 @@ static void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
 
     ASSERT(intack.source != hvm_intsrc_none);
 
-    if ( nestedhvm_enabled(v->domain) )
-    {
-        struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-
-        if ( nv->nv_vmentry_pending )
-        {
-            struct vmcb_struct *gvmcb = nv->nv_vvmcx;
-
-            /* check if l1 guest injects interrupt into l2 guest via vintr.
-             * return here or l2 guest looses interrupts, otherwise.
-             */
-            ASSERT(gvmcb != NULL);
-            intr = vmcb_get_vintr(gvmcb);
-            if ( intr.fields.irq )
-                return;
-        }
-    }
-
     TRACE(TRC_HVM_INTR_WINDOW, intack.vector, intack.source,
           vmcb->event_inj.v ? vmcb->event_inj.vector : -1);
 
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 171b32be2416..edb3f444756d 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1433,31 +1433,8 @@ void asmlinkage nsvm_vcpu_switch(void)
     vmexit:
         nestedsvm_vcpu_vmexit(v, regs, svm->ns_vmexit.exitcode);
         nv->nv_vmexit_pending = 0;
-        nv->nv_vmentry_pending = 0;
         return;
     }
-
-    if ( nv->nv_vmentry_pending )
-    {
-        int ret;
-        ASSERT(!nv->nv_vmexit_pending);
-        ret = nsvm_vcpu_vmrun(v, regs);
-        if ( ret )
-            goto vmexit;
-
-        ASSERT(nestedhvm_vcpu_in_guestmode(v));
-        nv->nv_vmentry_pending = 0;
-    }
-
-    if ( nestedhvm_vcpu_in_guestmode(v) && nestedhvm_paging_mode_hap(v) )
-    {
-        /* In case left the l2 guest due to a physical interrupt (e.g. IPI)
-         * that is not for the l1 guest then we continue running the l2 guest
-         * but check if the nestedp2m is still valid.
-         */
-        if ( nv->nv_p2m == NULL )
-            nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
-    }
 }
 
 /* Interrupts, Virtual GIF */
-- 
2.34.1

[PATCH v1 3/5] x86/nestedsvm: update vmexit without pending or deferred flag

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/hvm/svm/intr.c              |   4 +-
 xen/arch/x86/hvm/svm/nestedhvm.h         |   5 +-
 xen/arch/x86/hvm/svm/nestedsvm.c         | 168 ++++++++---------------
 xen/arch/x86/hvm/svm/svm.c               |  10 +-
 xen/arch/x86/include/asm/hvm/svm-types.h |   7 -
 5 files changed, 72 insertions(+), 122 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 7f6cafb6db78..f9b59a0d6a46 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -55,7 +55,7 @@ static void svm_inject_nmi(struct vcpu *v)
         vmcb, general1_intercepts | GENERAL1_INTERCEPT_IRET);
 }
 
-static void svm_inject_intr(struct vcpu *v, int vector)
+void svm_inject_intr(struct vcpu *v, int vector)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     intinfo_t event;
@@ -69,7 +69,7 @@ static void svm_inject_intr(struct vcpu *v, int vector)
     vmcb->event_inj = event;
 }
 
-static void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
+void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     uint32_t general1_intercepts = vmcb_get_general1_intercepts(vmcb);
diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
index e22ef259c171..5876a948f39a 100644
--- a/xen/arch/x86/hvm/svm/nestedhvm.h
+++ b/xen/arch/x86/hvm/svm/nestedhvm.h
@@ -27,7 +27,8 @@
     (!!((v)->arch.hvm.guest_efer & EFER_SVME))
 
 int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr);
-void nestedsvm_vmexit_defer(struct vcpu *v,
+enum nestedhvm_vmexits
+nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
     uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2);
 enum nestedhvm_vmexits
 nestedsvm_vmexit_n2n1(struct vcpu *v, struct cpu_user_regs *regs);
@@ -65,6 +66,8 @@ int cf_check nsvm_hap_walk_L1_p2m(
 #define NSVM_INTR_MASKED         0
 
 int nestedsvm_vcpu_interrupt(struct vcpu *v, const struct hvm_intack intack);
+void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack);
+void svm_inject_intr(struct vcpu *v, int vector);
 
 #endif /* __X86_HVM_SVM_NESTEDHVM_PRIV_H__ */
 
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index edb3f444756d..9e5e804a5b00 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -152,8 +152,6 @@ int cf_check nsvm_vcpu_reset(struct vcpu *v)
     svm->ns_vmcb_hostcr3 = 0;
     svm->ns_asid = 0;
     svm->ns_hostflags.bytes = 0;
-    svm->ns_vmexit.exitinfo1 = 0;
-    svm->ns_vmexit.exitinfo2 = 0;
 
     svm->ns_iomap = NULL;
 
@@ -712,14 +710,10 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs)
     int ret;
     unsigned int inst_len;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-    struct nestedsvm *svm = &vcpu_nestedsvm(v);
 
     inst_len = svm_get_insn_len(v, INSTR_VMRUN);
     if ( inst_len == 0 )
-    {
-        svm->ns_vmexit.exitcode = VMEXIT_SHUTDOWN;
         return -1;
-    }
 
     nv->nv_vmswitch_in_progress = 1;
     ASSERT(nv->nv_vvmcx != NULL);
@@ -738,15 +732,10 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         break;
     case NSVM_ERROR_VVMCB:
         gdprintk(XENLOG_ERR, "inject VMEXIT(INVALID)\n");
-        svm->ns_vmexit.exitcode = VMEXIT_INVALID;
         return -1;
     case NSVM_ERROR_VMENTRY:
     default:
-        gdprintk(XENLOG_ERR,
-            "nsvm_vcpu_vmentry failed, injecting #UD\n");
-        hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
-        /* Must happen after hvm_inject_hw_exception or it doesn't work right. */
-        nv->nv_vmswitch_in_progress = 0;
+        gdprintk(XENLOG_ERR, "nsvm_vcpu_vmentry failed\n");
         return 1;
     }
 
@@ -760,47 +749,55 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs)
 
 static int
 nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
-    uint64_t exitcode)
+    uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-    struct nestedsvm *svm = &vcpu_nestedsvm(v);
     struct vmcb_struct *ns_vmcb;
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
 
-    if ( vmcb->_vintr.fields.vgif_enable )
+    ASSERT(vmcb->_vintr.fields.vgif_enable);
         vmcb->_vintr.fields.vgif = 0;
-    else
-        svm->ns_gif = 0;
 
     ns_vmcb = nv->nv_vvmcx;
 
-    if ( nv->nv_vmexit_pending )
+    switch ( exitcode )
+    {
+    case VMEXIT_INTR:
     {
-        switch ( exitcode )
+        struct hvm_intack intack = {
+            .source = exitinfo1,
+            .vector = exitinfo2
+        };
+
+        /* See the comment in svm_intr_assist() for why this is necessary */
+        if ( unlikely(vmcb->event_inj.v) ||
+             hvm_interrupt_blocked(v, intack) )
         {
-        case VMEXIT_INTR:
-            if ( unlikely(ns_vmcb->event_inj.v) && nv->nv_vmentry_pending &&
-                 hvm_event_needs_reinjection(ns_vmcb->event_inj.type,
-                                             ns_vmcb->event_inj.vector) )
-                ns_vmcb->exit_int_info = ns_vmcb->event_inj;
-            break;
-        case VMEXIT_EXCEPTION_PF:
-            ns_vmcb->_cr2 = ns_vmcb->ei.exc.cr2;
-            fallthrough;
-        case VMEXIT_NPF:
-            ns_vmcb->exitinfo2 = svm->ns_vmexit.exitinfo2;
-            fallthrough;
-        case VMEXIT_EXCEPTION_NP:
-        case VMEXIT_EXCEPTION_SS:
-        case VMEXIT_EXCEPTION_GP:
-        case VMEXIT_EXCEPTION_15:
-        case VMEXIT_EXCEPTION_MF:
-        case VMEXIT_EXCEPTION_AC:
-            ns_vmcb->exitinfo1 = svm->ns_vmexit.exitinfo1;
-            break;
-        default:
+            svm_enable_intr_window(v, intack);
             break;
         }
+
+        svm_inject_intr(v, intack.vector);
+        pt_intr_post(v, intack);
+        break;
+    }
+
+    case VMEXIT_EXCEPTION_PF:
+        ns_vmcb->_cr2 = exitinfo2;
+        fallthrough;
+    case VMEXIT_NPF:
+        ns_vmcb->exitinfo2 = exitinfo2;
+        fallthrough;
+    case VMEXIT_EXCEPTION_NP:
+    case VMEXIT_EXCEPTION_SS:
+    case VMEXIT_EXCEPTION_GP:
+    case VMEXIT_EXCEPTION_15:
+    case VMEXIT_EXCEPTION_MF:
+    case VMEXIT_EXCEPTION_AC:
+        ns_vmcb->exitinfo1 = exitinfo1;
+        break;
+    default:
+        break;
     }
 
     ns_vmcb->exitcode = exitcode;
@@ -811,10 +808,16 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
 int cf_check nsvm_vcpu_vmexit_event(
     struct vcpu *v, const struct x86_event *event)
 {
+    enum nestedhvm_vmexits ret;
+
     ASSERT(vcpu_nestedhvm(v).nv_vvmcx != NULL);
 
-    nestedsvm_vmexit_defer(v, VMEXIT_EXCEPTION_DE + event->vector,
-                           event->error_code, event->data);
+    ret = nestedsvm_vcpu_vmexit(v, guest_cpu_user_regs(),
+                                VMEXIT_EXCEPTION_DE + event->vector,
+                                event->error_code, event->data);
+    if ( ret == NESTEDHVM_VMEXIT_FATALERROR )
+        domain_crash(v->domain);
+
     return NESTEDHVM_VMEXIT_DONE;
 }
 
@@ -1219,7 +1222,7 @@ enum hvm_intblk cf_check nsvm_intr_blocked(struct vcpu *v)
         if ( v->io.req.state != STATE_IOREQ_NONE )
             return hvm_intblk_shadow;
 
-        if ( !nv->nv_vmexit_pending && n2vmcb->exit_int_info.v )
+        if ( n2vmcb->exit_int_info.v )
         {
             /* Give the l2 guest a chance to finish the delivery of
              * the last injected interrupt or exception before we
@@ -1228,42 +1231,15 @@ enum hvm_intblk cf_check nsvm_intr_blocked(struct vcpu *v)
             return hvm_intblk_shadow;
         }
     }
-
-    if ( nv->nv_vmexit_pending )
-        /* hvm_inject_hw_exception() must have run before.
-         * exceptions have higher priority than interrupts.
-         */
-        return hvm_intblk_rflags_ie;
-
     return hvm_intblk_none;
 }
 
-/* VMEXIT emulation */
-void
-nestedsvm_vmexit_defer(struct vcpu *v,
-    uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2)
-{
-    struct nestedsvm *svm = &vcpu_nestedsvm(v);
-    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
-
-    if ( vmcb->_vintr.fields.vgif_enable )
-        vmcb->_vintr.fields.vgif = 0;
-    else
-        svm->ns_gif = 0;
-
-    svm->ns_vmexit.exitcode = exitcode;
-    svm->ns_vmexit.exitinfo1 = exitinfo1;
-    svm->ns_vmexit.exitinfo2 = exitinfo2;
-    vcpu_nestedhvm(v).nv_vmexit_pending = 1;
-}
-
 enum nestedhvm_vmexits
 nestedsvm_check_intercepts(struct vcpu *v, struct cpu_user_regs *regs,
     uint64_t exitcode)
 {
     bool is_intercepted;
 
-    ASSERT(vcpu_nestedhvm(v).nv_vmexit_pending == 0);
     is_intercepted = nsvm_vmcb_guest_intercepts_exitcode(v, regs, exitcode);
 
     /* 
@@ -1352,11 +1328,12 @@ nestedsvm_vmexit_n2n1(struct vcpu *v, struct cpu_user_regs *regs)
 /* The exitcode is in native SVM/VMX format. The forced exitcode
  * is in generic format.
  */
-static enum nestedhvm_vmexits
+enum nestedhvm_vmexits
 nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
-    uint64_t exitcode)
+    uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2)
 {
     int rc;
+    enum nestedhvm_vmexits ret = NESTEDHVM_VMEXIT_DONE;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
 
     nv->nv_vmswitch_in_progress = 1;
@@ -1368,14 +1345,12 @@ nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
      */
     if ( nestedhvm_vcpu_in_guestmode(v) )
     {
-        enum nestedhvm_vmexits ret;
-
         ret = nestedsvm_vmexit_n2n1(v, regs);
         switch ( ret )
         {
         case NESTEDHVM_VMEXIT_FATALERROR:
             gdprintk(XENLOG_ERR, "VMEXIT: fatal error\n");
-            return ret;
+            goto out;
         case NESTEDHVM_VMEXIT_HOST:
             BUG();
 
@@ -1395,46 +1370,18 @@ nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
     /* Prepare for running the l1 guest. Make the actual
      * modifications to the virtual VMCB/VMCS.
      */
-    rc = nsvm_vcpu_vmexit_inject(v, regs, exitcode);
+    rc = nsvm_vcpu_vmexit_inject(v, regs, exitcode, exitinfo1, exitinfo2);
 
     /* If l1 guest uses shadow paging, update the paging mode. */
     if ( !nestedhvm_paging_mode_hap(v) )
         paging_update_paging_modes(v);
 
-    nv->nv_vmswitch_in_progress = 0;
-
     if ( rc )
-        return NESTEDHVM_VMEXIT_FATALERROR;
-
-    return NESTEDHVM_VMEXIT_DONE;
-}
-
-/* VCPU switch */
-void asmlinkage nsvm_vcpu_switch(void)
-{
-    struct cpu_user_regs *regs = guest_cpu_user_regs();
-    struct vcpu *v = current;
-    struct nestedvcpu *nv;
-    struct nestedsvm *svm;
-
-    if ( !nestedhvm_enabled(v->domain) )
-        return;
-
-    nv = &vcpu_nestedhvm(v);
-    svm = &vcpu_nestedsvm(v);
-    ASSERT(v->arch.hvm.svm.vmcb != NULL);
-    ASSERT(nv->nv_n1vmcx != NULL);
-    ASSERT(nv->nv_n2vmcx != NULL);
-    ASSERT(nv->nv_n1vmcx_pa != INVALID_PADDR);
-    ASSERT(nv->nv_n2vmcx_pa != INVALID_PADDR);
+        ret = NESTEDHVM_VMEXIT_FATALERROR;
 
-    if ( nv->nv_vmexit_pending )
-    {
-    vmexit:
-        nestedsvm_vcpu_vmexit(v, regs, svm->ns_vmexit.exitcode);
-        nv->nv_vmexit_pending = 0;
-        return;
-    }
+out:
+    nv->nv_vmswitch_in_progress = 0;
+    return ret;
 }
 
 /* Interrupts, Virtual GIF */
@@ -1477,7 +1424,10 @@ nestedsvm_vcpu_interrupt(struct vcpu *v, const struct hvm_intack intack)
                                      guest_cpu_user_regs(), exitcode);
     if ( ret )
     {
-        nestedsvm_vmexit_defer(v, exitcode, intack.source, exitinfo2);
+        ret = nestedsvm_vcpu_vmexit(v, guest_cpu_user_regs(), exitcode,
+                                    intack.source, exitinfo2);
+        if ( ret == NESTEDHVM_VMEXIT_FATALERROR )
+            domain_crash(v->domain);
         return NSVM_INTR_FORCEVMEXIT;
     }
 
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index fc447d24d67e..a8d258aa3980 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1619,7 +1619,9 @@ static void svm_do_nested_pgfault(struct vcpu *v,
     case -1:
         ASSERT(nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v));
         /* inject #VMEXIT(NPF) into guest. */
-        nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa);
+        if ( nestedsvm_vcpu_vmexit(v, regs, VMEXIT_NPF, pfec, gpa) ==
+             NESTEDHVM_VMEXIT_FATALERROR )
+            domain_crash(v->domain);
         return;
     }
 
@@ -2595,8 +2597,10 @@ void asmlinkage svm_vmexit_handler(void)
             switch ( nsret )
             {
             case NESTEDHVM_VMEXIT_DONE:
-                /* defer VMEXIT injection */
-                nestedsvm_vmexit_defer(v, exit_reason, exitinfo1, exitinfo2);
+                nsret = nestedsvm_vcpu_vmexit(v, regs, exit_reason,
+                                              exitinfo1, exitinfo2);
+                if ( nsret == NESTEDHVM_VMEXIT_FATALERROR )
+                    domain_crash(v->domain);
                 goto out;
             case NESTEDHVM_VMEXIT_FATALERROR:
                 gdprintk(XENLOG_ERR, "unexpected nestedsvm_vmexit() error\n");
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index beab9a3af203..256bf5b91402 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -69,13 +69,6 @@ struct nestedsvm {
     bool ns_gif;
     bool ns_hap_enabled;
 
-    /* Only meaningful when vmexit_pending flag is set */
-    struct {
-        uint64_t exitcode;  /* native exitcode to inject into l1 guest */
-        uint64_t exitinfo1; /* additional information to the exitcode */
-        uint64_t exitinfo2; /* additional information to the exitcode */
-    } ns_vmexit;
-
     union {
         uint32_t bytes;
         struct {
-- 
2.34.1

[PATCH v1 4/5] x86/nestedsvm: update vmcb correctly

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c         | 100 ++++++++++++++++-------
 xen/arch/x86/hvm/svm/svm.c               |   4 +
 xen/arch/x86/hvm/svm/vmcb.c              |   3 +
 xen/arch/x86/hvm/svm/vmcb.h              |   6 ++
 xen/arch/x86/include/asm/hvm/svm-types.h |   2 +-
 xen/arch/x86/include/asm/msr-index.h     |   1 +
 6 files changed, 87 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 9e5e804a5b00..82e01e513e69 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -146,16 +146,15 @@ int cf_check nsvm_vcpu_reset(struct vcpu *v)
     svm->ns_exception_intercepts = 0;
     svm->ns_general1_intercepts = 0;
     svm->ns_general2_intercepts = 0;
+    svm->ns_general3_intercepts = 0;
 
     svm->ns_hap_enabled = 0;
     svm->ns_vmcb_guestcr3 = 0;
     svm->ns_vmcb_hostcr3 = 0;
     svm->ns_asid = 0;
     svm->ns_hostflags.bytes = 0;
-
     svm->ns_iomap = NULL;
 
-    svm->ns_gif = 1;
     return 0;
 }
 
@@ -416,6 +415,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         svm->ns_exception_intercepts = ns_vmcb->_exception_intercepts;
         svm->ns_general1_intercepts = ns_vmcb->_general1_intercepts;
         svm->ns_general2_intercepts = ns_vmcb->_general2_intercepts;
+        svm->ns_general3_intercepts = ns_vmcb->_general3_intercepts;
     }
 
     /* We could track the cleanbits of the n1vmcb from
@@ -444,6 +444,8 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         n1vmcb->_general1_intercepts | ns_vmcb->_general1_intercepts;
     n2vmcb->_general2_intercepts =
         n1vmcb->_general2_intercepts | ns_vmcb->_general2_intercepts;
+    n2vmcb->_general3_intercepts =
+        n1vmcb->_general3_intercepts | ns_vmcb->_general3_intercepts;
 
     /* Nested Pause Filter */
     if ( ns_vmcb->_general1_intercepts & GENERAL1_INTERCEPT_PAUSE )
@@ -472,6 +474,17 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         n2vmcb->_vintr.fields.intr_masking = 1;
     }
 
+    /*
+     * If L1 granted its L2 vGIF, the copy above already carried the enable
+     * and the GIF through.  Otherwise seed it set, as a VMRUN of a guest
+     * without vGIF starts with GIF set (APM vol.2 15.5).
+     */
+    if ( !ns_vmcb->_vintr.fields.vgif_enable )
+    {
+        n2vmcb->_vintr.fields.vgif_enable = 1;
+        n2vmcb->_vintr.fields.vgif = 1;
+    }
+
     /* Interrupt state */
     n2vmcb->int_stat = ns_vmcb->int_stat;
 
@@ -487,7 +500,8 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
     n2vmcb->virt_ext.bytes =
         n1vmcb->virt_ext.bytes | ns_vmcb->virt_ext.bytes;
 
-    /* NextRIP - only evaluated on #VMEXIT. */
+    /* NextRIP is consumed by VMRUN as the return address for injected soft events. */
+    n2vmcb->nextrip = ns_vmcb->nextrip;
 
     /*
      * VMCB Save State Area
@@ -652,10 +666,12 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct cpu_user_regs *regs,
     int ret;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
-    struct vmcb_struct *ns_vmcb;
+    struct vmcb_struct *ns_vmcb, *n1vmcb;
 
     ns_vmcb = nv->nv_vvmcx;
+    n1vmcb = nv->nv_n1vmcx;
     ASSERT(ns_vmcb != NULL);
+    ASSERT(n1vmcb != NULL);
     ASSERT(nv->nv_n2vmcx != NULL);
     ASSERT(nv->nv_n2vmcx_pa != INVALID_PADDR);
 
@@ -700,7 +716,8 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct cpu_user_regs *regs,
         return ret;
     }
 
-    svm->ns_gif = 1;
+    ASSERT(n1vmcb->_vintr.fields.vgif_enable);
+    n1vmcb->_vintr.fields.vgif = 1;
     return 0;
 }
 
@@ -756,7 +773,7 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
 
     ASSERT(vmcb->_vintr.fields.vgif_enable);
-        vmcb->_vintr.fields.vgif = 0;
+    vmcb->_vintr.fields.vgif = 0;
 
     ns_vmcb = nv->nv_vvmcx;
 
@@ -939,12 +956,18 @@ nsvm_vmcb_guest_intercepts_exitcode(struct vcpu *v,
             break;
         return 0;
 
-    case VMEXIT_VMRUN ... VMEXIT_XSETBV:
+    case VMEXIT_VMRUN ... VMEXIT_RDPRU:
         exit_bits = 1ULL << (exitcode - VMEXIT_VMRUN);
         if ( svm->ns_general2_intercepts & exit_bits )
             break;
         return 0;
 
+    case VMEXIT_INVLPGB ... VMEXIT_IDLE_HLT:
+        exit_bits = 1ULL << (exitcode - VMEXIT_INVLPGB);
+        if ( svm->ns_general3_intercepts & exit_bits )
+            break;
+        return 0;
+
     case VMEXIT_NPF:
         if ( nestedhvm_paging_mode_hap(v) )
             break;
@@ -1026,10 +1049,15 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
     /* TLB control */
     ns_vmcb->tlb_control = 0;
 
-    /* Virtual Interrupts */
-    ns_vmcb->_vintr = n2vmcb->_vintr;
-    if ( !svm->ns_hostflags.fields.vintrmask )
-        ns_vmcb->_vintr.fields.intr_masking = 0;
+    /*
+     * Virtual Interrupts.  #VMEXIT writes back V_TPR and V_IRQ (APM vol.2
+     * 15.6); the remaining fields do not change.  Copy vGIF back only if
+     * enabled in L1.
+     */
+    ns_vmcb->_vintr.fields.tpr = n2vmcb->_vintr.fields.tpr;
+    ns_vmcb->_vintr.fields.irq = n2vmcb->_vintr.fields.irq;
+    if ( ns_vmcb->_vintr.fields.vgif_enable )
+        ns_vmcb->_vintr.fields.vgif = n2vmcb->_vintr.fields.vgif;
 
     /* Interrupt state */
     ns_vmcb->int_stat = n2vmcb->int_stat;
@@ -1437,19 +1465,30 @@ nestedsvm_vcpu_interrupt(struct vcpu *v, const struct hvm_intack intack)
 bool
 nestedsvm_gif_isset(struct vcpu *v)
 {
-    struct nestedsvm *svm = &vcpu_nestedsvm(v);
-    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    struct vmcb_struct *n1vmcb = nv->nv_n1vmcx;
+    struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
+    struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
 
-    /* get the vmcb gif value if using vgif */
-    if ( vmcb->_vintr.fields.vgif_enable )
-        return vmcb->_vintr.fields.vgif;
-    else
-        return svm->ns_gif;
+    /*
+     * An L2 with no vGIF of its own shares L1's GIF.  L0 keeps that shared
+     * GIF in the L2 shadow, to keep an L2 CLGI off the host's.
+     */
+    if ( nestedhvm_vcpu_in_guestmode(v) &&
+         !ns_vmcb->_vintr.fields.vgif_enable )
+        return n2vmcb->_vintr.fields.vgif;
+
+    /*
+     * The VMCB virtual GIF is authoritative.
+     */
+    return n1vmcb->_vintr.fields.vgif;
 }
 
 void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v)
 {
+    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     unsigned int inst_len;
+    vintr_t intr;
 
     /*
      * STGI doesn't require SVME to be set to be used.  See AMD APM vol
@@ -1464,7 +1503,9 @@ void svm_vmexit_do_stgi(struct cpu_user_regs *regs, struct vcpu *v)
     if ( (inst_len = svm_get_insn_len(v, INSTR_STGI)) == 0 )
         return;
 
-    vcpu_nestedsvm(v).ns_gif = 1;
+    intr = vmcb_get_vintr(vmcb);
+    intr.fields.vgif = 1;
+    vmcb_set_vintr(vmcb, intr);
 
     __update_guest_eip(regs, inst_len);
 }
@@ -1485,10 +1526,9 @@ void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v)
     if ( (inst_len = svm_get_insn_len(v, INSTR_CLGI)) == 0 )
         return;
 
-    vcpu_nestedsvm(v).ns_gif = 0;
-
     /* After a CLGI no interrupts should come */
     intr = vmcb_get_vintr(vmcb);
+    intr.fields.vgif = 0;
     intr.fields.irq = 0;
     general1_intercepts &= ~GENERAL1_INTERCEPT_VINTR;
     vmcb_set_vintr(vmcb, intr);
@@ -1504,10 +1544,17 @@ void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v)
 void svm_nested_features_on_efer_update(struct vcpu *v)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
-    struct nestedsvm *svm = &vcpu_nestedsvm(v);
     u32 general2_intercepts;
     vintr_t vintr;
 
+    /*
+     * These accelerate the L1 VMCB.  A world switch loads an EFER rather
+     * than writing one, and an L2 EFER write says nothing about L1; the L2
+     * shadow is configured in nsvm_vmcb_prepare4vmrun().
+     */
+    if ( nestedhvm_vmswitch_in_progress(v) || nestedhvm_vcpu_in_guestmode(v) )
+        return;
+
     /*
      * Need state for transfering the nested gif status so only write on
      * the hvm_vcpu EFER.SVME changing.
@@ -1515,7 +1562,6 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
     if ( nsvm_efer_svm_enabled(v) )
     {
         if ( !vmcb->virt_ext.fields.vloadsave_enable &&
-             paging_mode_hap(v->domain) &&
              cpu_has_svm_vloadsave )
         {
             vmcb->virt_ext.fields.vloadsave_enable = 1;
@@ -1525,11 +1571,9 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
             vmcb_set_general2_intercepts(vmcb, general2_intercepts);
         }
 
-        if ( !vmcb->_vintr.fields.vgif_enable &&
-             cpu_has_svm_vgif )
+        if ( !vmcb->_vintr.fields.vgif_enable )
         {
             vintr = vmcb_get_vintr(vmcb);
-            vintr.fields.vgif = svm->ns_gif;
             vintr.fields.vgif_enable = 1;
             vmcb_set_vintr(vmcb, vintr);
             general2_intercepts  = vmcb_get_general2_intercepts(vmcb);
@@ -1552,7 +1596,6 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
         if ( vmcb->_vintr.fields.vgif_enable )
         {
             vintr = vmcb_get_vintr(vmcb);
-            svm->ns_gif = vintr.fields.vgif;
             vintr.fields.vgif_enable = 0;
             vmcb_set_vintr(vmcb, vintr);
             general2_intercepts  = vmcb_get_general2_intercepts(vmcb);
@@ -1574,5 +1617,6 @@ void __init start_nested_svm(struct hvm_function_table *hvm_function_table)
         cpu_has_svm_lbrv &&
         cpu_has_svm_nrips &&
         cpu_has_svm_flushbyasid &&
-        cpu_has_svm_decode;
+        cpu_has_svm_decode &&
+        cpu_has_svm_vgif;
 }
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index a8d258aa3980..074f09a0e0e9 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1660,6 +1660,8 @@ static void svm_dr_access(struct vcpu *v, struct cpu_user_regs *regs)
 
     TRACE(TRC_HVM_DR_WRITE);
     __restore_debug_registers(vmcb, v);
+    if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) )
+        vmcb_set_dr_intercepts(v->arch.hvm.svm.vmcb, 0);
 }
 
 static int cf_check svm_msr_read_intercept(
@@ -1990,6 +1992,8 @@ static int cf_check svm_msr_write_intercept(
     case MSR_K8_SYSCFG:
     case MSR_K8_VM_CR:
     case MSR_AMD64_EX_CFG:
+    case MSR_K8_HWCR:
+    case MSR_K8_VM_IGNNE:
         /* ignore write. handle all bits as read-only. */
         break;
 
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index d069280a4da8..5354c4f1b85f 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -194,6 +194,9 @@ static int construct_vmcb(struct vcpu *v)
 
     vmcb->_vintr.fields.vnmi_enable = cpu_has_svm_vnmi;
 
+    /* GIF is set out of reset; with vGIF its value lives in the VMCB. */
+    vmcb->_vintr.fields.vgif = 1;
+
     return 0;
 }
 
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index 3760f71a8625..d098bf1d1355 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -288,7 +288,13 @@ enum VMEXIT_EXITCODE
     VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */
     VMEXIT_XSETBV           = 141, /* 0x8d */
     VMEXIT_RDPRU            = 142, /* 0x8e */
+    VMEXIT_INVLPGB          = 160, /* 0xa0 */
+    VMEXIT_INVLPGB_ILLEGAL  = 161, /* 0xa1 */
+    VMEXIT_INVPCID          = 162, /* 0xa2 */
+    VMEXIT_MCOMMIT          = 163, /* 0xa3 */
+    VMEXIT_TLBSYNC          = 164, /* 0xa4 */
     VMEXIT_BUS_LOCK         = 165, /* 0xa5 */
+    VMEXIT_IDLE_HLT         = 166, /* 0xa6 */
     /* Remember to also update VMEXIT_NPF_PERFC! */
     VMEXIT_NPF              = 1024, /* 0x400, nested paging fault */
     /* Remember to also update SVM_PERF_EXIT_REASON_SIZE! */
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index 256bf5b91402..24363a58ffcb 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -44,6 +44,7 @@ struct nestedsvm {
     uint32_t ns_exception_intercepts;
     uint32_t ns_general1_intercepts;
     uint32_t ns_general2_intercepts;
+    uint32_t ns_general3_intercepts;
 
     /* Cached real MSR permission bitmaps of the l2 guest */
     unsigned long *ns_cached_msrpm;
@@ -66,7 +67,6 @@ struct nestedsvm {
     uint64_t ns_vmcb_guestcr3, ns_vmcb_hostcr3;
     uint32_t ns_asid;
 
-    bool ns_gif;
     bool ns_hap_enabled;
 
     union {
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index ad1c6c97f8f7..beb858bf99bb 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -250,6 +250,7 @@
 #define MSR_K8_VM_CR                        _AC(0xc0010114, U)
 #define  VM_CR_INIT_REDIRECTION             (_AC(1, ULL) <<  1)
 #define  VM_CR_SVM_DISABLE                  (_AC(1, ULL) <<  4)
+#define MSR_K8_VM_IGNNE                     _AC(0xc0010115, U)
 
 #define MSR_VIRT_SPEC_CTRL                  _AC(0xc001011f, U) /* Layout matches MSR_SPEC_CTRL */
 
-- 
2.34.1

[PATCH v1 5/5] x86/nestedsvm: fix up

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c | 19 ++++++++++++++++---
 xen/arch/x86/hvm/svm/vmcb.c      |  9 +++++----
 xen/arch/x86/mm/p2m.c            |  5 +++++
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 82e01e513e69..91872af8aa7b 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -355,7 +355,7 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu *v,
     vcpu_nestedsvm(v).ns_vmcb_hostcr3 = vvmcb->_h_cr3;
 
     p2m = p2m_get_nestedp2m(v);
-    n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
+    vmcb_set_h_cr3(n2vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m)));
 }
 
 void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
@@ -373,6 +373,7 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
         return;
 
     p2m = p2m_get_nestedp2m(v);
+    nv->stale_np2m = false;
 
     /*
      * This may happen if we've handled VMEXIT_NPF at the same time as a
@@ -383,8 +384,6 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
          vmcb_get_h_cr3(nv->nv_n2vmcx) !=
          pagetable_get_paddr(p2m_get_pagetable(p2m)) )
         nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
-
-    nv->stale_np2m = false;
 }
 
 static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
@@ -1024,6 +1023,20 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
     struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
     struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
 
+    ASSERT(v == current);
+
+    /*
+     * The physical VMCB fields covered by VMSAVE/VMLOAD may not be in sync
+     * with v's vmcb if a context switch happened since the last VMLOAD.
+     * VMSAVE below would otherwise capture stale/foreign register state
+     * into the L1 shadow VMCB.
+     */
+    if ( v->arch.hvm.svm.vmcb_sync_state == vmcb_needs_vmload )
+    {
+        svm_vmload_pa(v->arch.hvm.svm.vmcb_pa);
+        v->arch.hvm.svm.vmcb_sync_state = vmcb_in_sync;
+    }
+
     svm_vmsave_pa(nv->nv_n1vmcx_pa);
 
     /* Cache guest physical address of virtual vmcb
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 5354c4f1b85f..2f7053ed7eec 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -357,10 +357,11 @@ bool svm_vmcb_isvalid(
         PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n", cr0);
 
     if ( (cr0 & X86_CR0_PG) &&
-         ((cr3 & 7) ||
-          ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
-          ((efer & EFER_LMA) &&
-           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
+         ((!(cr4 & X86_CR4_PCIDE) &&
+           ((cr3 & 7) ||
+            ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) &&
+             (cr3 & 0xfe0)))) ||
+          (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr)) )
         PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
 
     valid = hvm_cr4_guest_valid_bits(v->domain);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 027b9ae69be3..d1fafc479c78 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1517,7 +1517,12 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
     np2m_base &= ~(0xfffULL);
 
     if ( nv->nv_flushp2m && nv->nv_p2m )
+    {
+        p2m = nv->nv_p2m;
+        if ( p2m )
+            p2m_flush_table(p2m);
         nv->nv_p2m = NULL;
+    }
 
     nestedp2m_lock(d);
     p2m = nv->nv_p2m;
-- 
2.34.1

Re: [PATCH v1 0/5] x86/nestedsvm: rework nested VMRUN/VMEXIT handling

[Jan Beulich]

On 23.09.2026 11:20, chunjie.zhu@citrix.com wrote:
> From: Chunjie Zhu <chunjie.zhu@citrix.com>
> 
> This series simplifies nested SVM execution by removing pending and
> deferred state from the VMRUN and VMEXIT paths.  Nested transitions are
> handled directly by rearranging the state transfer and control flow.
> 
> The series also removes obsolete interrupt-window and vCPU-switch handling
> associated with `nv_vmentry_pending`, while updating the relevant assembly
> offsets and nested-SVM data structures.
> 
> Patch 1 rearranges VMRUN handling without pending or deferred flags.
> Patch 2 removes code related to `nv_vmentry_pending`.
> Patch 3 rearranges VMEXIT handling without pending or deferred flags.
> Patch 4 update vmcb fields correctly.
> Patch 5 fix issues which cause transition failure.
> 
> Chunjie Zhu (5):
>   x86/nestedsvm: update vmrun without pending or deferred flag
>   x86/nestedsvm: remove nv_vmentry_pending
>   x86/nestedsvm: update vmexit without pending or deferred flag
>   x86/nestedsvm: update vmcb correctly
>   x86/nestedsvm: fix up

This last patch, unless it was meant as an RFC without being marked so,
surely wants a better title and a non-empty description.

Jan

Re: [PATCH v1 5/5] x86/nestedsvm: fix up

[Ross Lagerwall]

On 9/23/26 10:20 AM, chunjie.zhu@citrix.com wrote:
> From: Chunjie Zhu <chunjie.zhu@citrix.com>
> 
> Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
> ---
>   xen/arch/x86/hvm/svm/nestedsvm.c | 19 ++++++++++++++++---
>   xen/arch/x86/hvm/svm/vmcb.c      |  9 +++++----
>   xen/arch/x86/mm/p2m.c            |  5 +++++
>   3 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
> index 82e01e513e69..91872af8aa7b 100644
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -355,7 +355,7 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu *v,
>       vcpu_nestedsvm(v).ns_vmcb_hostcr3 = vvmcb->_h_cr3;
>   
>       p2m = p2m_get_nestedp2m(v);
> -    n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
> +    vmcb_set_h_cr3(n2vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m)));
>   }
>   
>   void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
> @@ -373,6 +373,7 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
>           return;
>   
>       p2m = p2m_get_nestedp2m(v);
> +    nv->stale_np2m = false;
>   
>       /*
>        * This may happen if we've handled VMEXIT_NPF at the same time as a
> @@ -383,8 +384,6 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
>            vmcb_get_h_cr3(nv->nv_n2vmcx) !=
>            pagetable_get_paddr(p2m_get_pagetable(p2m)) )
>           nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
> -
> -    nv->stale_np2m = false;
>   }
>   
>   static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
> @@ -1024,6 +1023,20 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
>       struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
>       struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
>   
> +    ASSERT(v == current);
> +
> +    /*
> +     * The physical VMCB fields covered by VMSAVE/VMLOAD may not be in sync
> +     * with v's vmcb if a context switch happened since the last VMLOAD.
> +     * VMSAVE below would otherwise capture stale/foreign register state
> +     * into the L1 shadow VMCB.
> +     */
> +    if ( v->arch.hvm.svm.vmcb_sync_state == vmcb_needs_vmload )
> +    {
> +        svm_vmload_pa(v->arch.hvm.svm.vmcb_pa);
> +        v->arch.hvm.svm.vmcb_sync_state = vmcb_in_sync;
> +    }
> +
>       svm_vmsave_pa(nv->nv_n1vmcx_pa);
>   
>       /* Cache guest physical address of virtual vmcb
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index 5354c4f1b85f..2f7053ed7eec 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -357,10 +357,11 @@ bool svm_vmcb_isvalid(
>           PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n", cr0);
>   
>       if ( (cr0 & X86_CR0_PG) &&
> -         ((cr3 & 7) ||
> -          ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
> -          ((efer & EFER_LMA) &&
> -           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
> +         ((!(cr4 & X86_CR4_PCIDE) &&
> +           ((cr3 & 7) ||
> +            ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) &&
> +             (cr3 & 0xfe0)))) ||
> +          (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr)) )
>           PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
>   
>       valid = hvm_cr4_guest_valid_bits(v->domain);
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 027b9ae69be3..d1fafc479c78 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1517,7 +1517,12 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
>       np2m_base &= ~(0xfffULL);
>   
>       if ( nv->nv_flushp2m && nv->nv_p2m )
> +    {
> +        p2m = nv->nv_p2m;
> +        if ( p2m )
> +            p2m_flush_table(p2m);
>           nv->nv_p2m = NULL;
> +    }
>   
>       nestedp2m_lock(d);
>       p2m = nv->nv_p2m;

This seems to have combined several bug fixes from our internal patchqueue (I
know because I wrote some of them...), some of which have already been posted
to the list.

It's not clear why they were submitted as part of this series.

Ross

Re: [PATCH v1 5/5] x86/nestedsvm: fix up

[Ross Lagerwall]

On 9/23/26 10:42 AM, Ross Lagerwall wrote:
> On 9/23/26 10:20 AM, chunjie.zhu@citrix.com wrote:
>> From: Chunjie Zhu <chunjie.zhu@citrix.com>
>>
>> Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
>> ---
>>   xen/arch/x86/hvm/svm/nestedsvm.c | 19 ++++++++++++++++---
>>   xen/arch/x86/hvm/svm/vmcb.c      |  9 +++++----
>>   xen/arch/x86/mm/p2m.c            |  5 +++++
>>   3 files changed, 26 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
>> index 82e01e513e69..91872af8aa7b 100644
>> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
>> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
>> @@ -355,7 +355,7 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu *v,
>>       vcpu_nestedsvm(v).ns_vmcb_hostcr3 = vvmcb->_h_cr3;
>>       p2m = p2m_get_nestedp2m(v);
>> -    n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
>> +    vmcb_set_h_cr3(n2vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m)));
>>   }
>>   void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
>> @@ -373,6 +373,7 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
>>           return;
>>       p2m = p2m_get_nestedp2m(v);
>> +    nv->stale_np2m = false;
>>       /*
>>        * This may happen if we've handled VMEXIT_NPF at the same time as a
>> @@ -383,8 +384,6 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
>>            vmcb_get_h_cr3(nv->nv_n2vmcx) !=
>>            pagetable_get_paddr(p2m_get_pagetable(p2m)) )
>>           nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
>> -
>> -    nv->stale_np2m = false;
>>   }
>>   static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
>> @@ -1024,6 +1023,20 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
>>       struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
>>       struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
>> +    ASSERT(v == current);
>> +
>> +    /*
>> +     * The physical VMCB fields covered by VMSAVE/VMLOAD may not be in sync
>> +     * with v's vmcb if a context switch happened since the last VMLOAD.
>> +     * VMSAVE below would otherwise capture stale/foreign register state
>> +     * into the L1 shadow VMCB.
>> +     */
>> +    if ( v->arch.hvm.svm.vmcb_sync_state == vmcb_needs_vmload )
>> +    {
>> +        svm_vmload_pa(v->arch.hvm.svm.vmcb_pa);
>> +        v->arch.hvm.svm.vmcb_sync_state = vmcb_in_sync;
>> +    }
>> +
>>       svm_vmsave_pa(nv->nv_n1vmcx_pa);
>>       /* Cache guest physical address of virtual vmcb
>> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
>> index 5354c4f1b85f..2f7053ed7eec 100644
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -357,10 +357,11 @@ bool svm_vmcb_isvalid(
>>           PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n", cr0);
>>       if ( (cr0 & X86_CR0_PG) &&
>> -         ((cr3 & 7) ||
>> -          ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
>> -          ((efer & EFER_LMA) &&
>> -           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
>> +         ((!(cr4 & X86_CR4_PCIDE) &&
>> +           ((cr3 & 7) ||
>> +            ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) &&
>> +             (cr3 & 0xfe0)))) ||
>> +          (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr)) )
>>           PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
>>       valid = hvm_cr4_guest_valid_bits(v->domain);
>> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
>> index 027b9ae69be3..d1fafc479c78 100644
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -1517,7 +1517,12 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
>>       np2m_base &= ~(0xfffULL);
>>       if ( nv->nv_flushp2m && nv->nv_p2m )
>> +    {
>> +        p2m = nv->nv_p2m;
>> +        if ( p2m )
>> +            p2m_flush_table(p2m);
>>           nv->nv_p2m = NULL;
>> +    }
>>       nestedp2m_lock(d);
>>       p2m = nv->nv_p2m;
> 
> This seems to have combined several bug fixes from our internal patchqueue (I
> know because I wrote some of them...), some of which have already been posted
> to the list.
> 
> It's not clear why they were submitted as part of this series.
> 

Looking further, patches 1, 3, and 4 also contain seemingly unrelated patches
from our internal patchqueue. Can you resubmit this series without these
patches mixed in? Or if they really are required dependencies, (where they
haven't already been posted to xen-devel previously) submit them as separate
patches at the start of this series, maintaining authorship information.

Ross

Re: [PATCH v1 5/5] x86/nestedsvm: fix up

[chunjie.zhu]

On Wed, Sep 23, 2026 at 10:55:41 AM, Ross Lagerwall <ross.lagerwall@citrix.com> wrote:

> On 9/23/26 10:42 AM, Ross Lagerwall wrote:
> > On 9/23/26 10:20 AM, chunjie.zhu@citrix.com wrote:
> >> From: Chunjie Zhu <chunjie.zhu@citrix.com>
> >>
> >> Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
> >> ---
> >>   xen/arch/x86/hvm/svm/nestedsvm.c | 19 ++++++++++++++++---
> >>   xen/arch/x86/hvm/svm/vmcb.c      |  9 +++++----
> >>   xen/arch/x86/mm/p2m.c            |  5 +++++
> >>   3 files changed, 26 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
> >> index 82e01e513e69..91872af8aa7b 100644
> >> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> >> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> >> @@ -355,7 +355,7 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu *v,
> >>       vcpu_nestedsvm(v).ns_vmcb_hostcr3 = vvmcb->_h_cr3;
> >>       p2m = p2m_get_nestedp2m(v);
> >> -    n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
> >> +    vmcb_set_h_cr3(n2vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m)));
> >>   }
> >>   void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
> >> @@ -373,6 +373,7 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
> >>           return;
> >>       p2m = p2m_get_nestedp2m(v);
> >> +    nv->stale_np2m = false;
> >>       /*
> >>        * This may happen if we've handled VMEXIT_NPF at the same time as a
> >> @@ -383,8 +384,6 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
> >>            vmcb_get_h_cr3(nv->nv_n2vmcx) !=
> >>            pagetable_get_paddr(p2m_get_pagetable(p2m)) )
> >>           nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
> >> -
> >> -    nv->stale_np2m = false;
> >>   }
> >>   static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
> >> @@ -1024,6 +1023,20 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
> >>       struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
> >>       struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
> >> +    ASSERT(v == current);
> >> +
> >> +    /*
> >> +     * The physical VMCB fields covered by VMSAVE/VMLOAD may not be in sync
> >> +     * with v's vmcb if a context switch happened since the last VMLOAD.
> >> +     * VMSAVE below would otherwise capture stale/foreign register state
> >> +     * into the L1 shadow VMCB.
> >> +     */
> >> +    if ( v->arch.hvm.svm.vmcb_sync_state == vmcb_needs_vmload )
> >> +    {
> >> +        svm_vmload_pa(v->arch.hvm.svm.vmcb_pa);
> >> +        v->arch.hvm.svm.vmcb_sync_state = vmcb_in_sync;
> >> +    }
> >> +
> >>       svm_vmsave_pa(nv->nv_n1vmcx_pa);
> >>       /* Cache guest physical address of virtual vmcb
> >> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> >> index 5354c4f1b85f..2f7053ed7eec 100644
> >> --- a/xen/arch/x86/hvm/svm/vmcb.c
> >> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> >> @@ -357,10 +357,11 @@ bool svm_vmcb_isvalid(
> >>           PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n", cr0);
> >>       if ( (cr0 & X86_CR0_PG) &&
> >> -         ((cr3 & 7) ||
> >> -          ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
> >> -          ((efer & EFER_LMA) &&
> >> -           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
> >> +         ((!(cr4 & X86_CR4_PCIDE) &&
> >> +           ((cr3 & 7) ||
> >> +            ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) &&
> >> +             (cr3 & 0xfe0)))) ||
> >> +          (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr)) )
> >>           PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
> >>       valid = hvm_cr4_guest_valid_bits(v->domain);
> >> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> >> index 027b9ae69be3..d1fafc479c78 100644
> >> --- a/xen/arch/x86/mm/p2m.c
> >> +++ b/xen/arch/x86/mm/p2m.c
> >> @@ -1517,7 +1517,12 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
> >>       np2m_base &= ~(0xfffULL);
> >>       if ( nv->nv_flushp2m && nv->nv_p2m )
> >> +    {
> >> +        p2m = nv->nv_p2m;
> >> +        if ( p2m )
> >> +            p2m_flush_table(p2m);
> >>           nv->nv_p2m = NULL;
> >> +    }
> >>       nestedp2m_lock(d);
> >>       p2m = nv->nv_p2m;
> > 
> > This seems to have combined several bug fixes from our internal patchqueue (I
> > know because I wrote some of them...), some of which have already been posted
> > to the list.
> > 
> > It's not clear why they were submitted as part of this series.
> > 
>
> Looking further, patches 1, 3, and 4 also contain seemingly unrelated patches
> from our internal patchqueue. Can you resubmit this series without these
> patches mixed in? Or if they really are required dependencies, (where they
> haven't already been posted to xen-devel previously) submit them as separate
> patches at the start of this series, maintaining authorship information.
>
> Ross

Sure. Will send the v2 series.

>
>

[PATCH v2 00/13] x86/nestedsvm: rework nested VMRUN/VMEXIT handling

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

This series simplifies nested SVM execution by removing pending and
deferred state from the VMRUN and VMEXIT paths, and includes the nested
SVM / Viridian fixes the rework depends on.

Changes in v2 (following Ross Lagerwall's review of v1):
 - Dropped the v1 "x86/nestedsvm: update vmcb correctly" and
   "x86/nestedsvm: fix up" patches, which mixed in unrelated bug fixes
   from our internal patchqueue.
 - Those fixes are now submitted as separate patches (1-10) at the start
   of the series, with the original authorship preserved.  The p2m.c
   change from v1 5/5 is not included.
 - The three rework patches from v1 (1-3/5) are now patches 11-13.  The
   stale_np2m handling from v1 5/5 is folded into patch 11; the rest is
   rebased only.  See per-patch notes below the "---" line.

Contributions by author:
 Ross Lagerwall (patches 1-6, 8-9):
   - Fix CR3 MBZ check
   - Adjust L2's DR intercept when adjusting L1
   - Use the correct VMCB for vGIF
   - Set GIF during VMRUN if vGIF is enabled
   - Viridian stimer direct mode support in libxl
   - Clamp Viridian features to known set
   - Don't set VMCB(1-2)'s NP_ENABLE and N_CR3 during VMEXIT to L1
   - Fix VMLOAD/VMSAVE state handling when using nested virt
 Stephen Cheng (patch 7):
   - Require VMSAVEvirt for nested virt
 Lin Liu (patch 10):
   - Don't clear TLB_CONTROL on #VMEXIT
 Chunjie Zhu (patches 11-13):
   - Rework of nested VMRUN/VMEXIT without pending or deferred flags,
     and removal of nv_vmentry_pending

Chunjie Zhu (3):
  x86/nestedsvm: update vmrun without pending or deferred
  x86/nestedsvm: remove nv_vmentry_pending
  x86/nestedsvm: update vmexit without pending or deferred flag

Lin Liu (1):
  x86/nSVM: Don't clear TLB_CONTROL on #VMEXIT

Ross Lagerwall (8):
  nestedsvm: Fix CR3 MBZ check
  nestedsvm: Adjust L2's DR intercept when adjusting L1
  nestedsvm: Use the correct VMCB for vGIF
  nestedsvm: Set GIF during VMRUN if vGIF is enabled
  tools/libxl: Add support for Viridian stimer direct mode
  x86/hvm: Clamp Viridian features to known set
  nestedsvm: Don't set VMCB(1-2)'s NP_ENABLE and N_CR3 during VMEXIT to
    L1
  x86/svm: Fix VMLOAD/VMSAVE state handling when using nested virt

Stephen Cheng (1):
  x86/svm: require VMSAVEvirt for nested virt

 docs/designs/nested-svm-cpu-features.md  |  11 +
 docs/man/xl.cfg.5.pod.in                 |   5 +
 tools/include/libxl.h                    |   6 +
 tools/libs/light/libxl_types.idl         |   1 +
 tools/libs/light/libxl_x86.c             |   5 +
 xen/arch/x86/cpu-policy.c                |   3 +-
 xen/arch/x86/hvm/hvm.c                   |   4 +-
 xen/arch/x86/hvm/svm/entry.S             |  48 +++-
 xen/arch/x86/hvm/svm/intr.c              |  22 +-
 xen/arch/x86/hvm/svm/nestedhvm.h         |   6 +-
 xen/arch/x86/hvm/svm/nestedsvm.c         | 305 +++++++++--------------
 xen/arch/x86/hvm/svm/svm.c               | 226 +++++++----------
 xen/arch/x86/hvm/svm/svm.h               |   2 -
 xen/arch/x86/hvm/svm/vmcb.c              |  32 +--
 xen/arch/x86/hvm/svm/vmcb.h              |   3 +-
 xen/arch/x86/include/asm/hvm/svm-types.h |   7 -
 xen/arch/x86/x86_64/asm-offsets.c        |   1 +
 17 files changed, 302 insertions(+), 385 deletions(-)

-- 
2.34.1

[PATCH v2 01/13] nestedsvm: Fix CR3 MBZ check

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

The existing code checks for any reserved bit set while the APM only
considers it invalid if an MBZ bit is set. Relax the check to match the
APM and hardware.

Some of the reserved bits were observed to be set running Rocky Linux
10.1 on Xen on Xen.

Fixes: 9a779e4fc161 ("Implement SVM specific part for Nested Virtualization")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/vmcb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index a6f09672a7c5..39b9e6bf2068 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -354,10 +354,8 @@ bool svm_vmcb_isvalid(
         PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n", cr0);
 
     if ( (cr0 & X86_CR0_PG) &&
-         ((cr3 & 7) ||
-          ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
-          ((efer & EFER_LMA) &&
-           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
+         ((efer & EFER_LMA) &&
+           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr)) )
         PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
 
     valid = hvm_cr4_guest_valid_bits(v->domain);
-- 
2.34.1

[PATCH v2 02/13] nestedsvm: Adjust L2's DR intercept when adjusting L1

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

If L2 accesses a debug register (like reading DR7) without L1 intercepting
it, it locks up the vCPU. L0 intercepts VMEXIT_DR7_READ, which disables
the intercept for L1 and then restarts L2 which re-executes the
instruction and then this repeats indefinitely.

Disable the intercept for the current VMCB if in guest mode to reflect
what would happen if the VMCB were recreated via
nsvm_vmcb_prepare4vmrun().

Fixes: a59a7be91b61 ("nestedsvm: fix DRn handling")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/svm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 71e48351f22c..8f97f6c31716 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1657,6 +1657,8 @@ static void svm_dr_access(struct vcpu *v, struct cpu_user_regs *regs)
 
     TRACE(TRC_HVM_DR_WRITE);
     __restore_debug_registers(vmcb, v);
+    if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) )
+        vmcb_set_dr_intercepts(v->arch.hvm.svm.vmcb, 0);
 }
 
 static int cf_check svm_msr_read_intercept(
-- 
2.34.1

[PATCH v2 03/13] nestedsvm: Use the correct VMCB for vGIF

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

In some cases, Xen uses the current VMCB to check/change the state of
vGIF but the current VMCB might be VMCB(0-2). Adjust the cases to use
VMCB(1) instead.

L0 may use vGIF to speed up L1 but whether L2 uses vGIF is the L1
hypervisor's choice and L0 should never need to check/change it.

Fixes: 4cd0fad64590 ("x86/svm: Add virtual GIF support")
Fixes: 05bb1116b8c1 ("x86/svm: update VGIF support")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 2e3b70345b68..8943d61e2b25 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1215,10 +1215,11 @@ nestedsvm_vmexit_defer(struct vcpu *v,
     uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2)
 {
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
-    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    struct vmcb_struct *n1vmcb = nv->nv_n1vmcx;
 
-    if ( vmcb->_vintr.fields.vgif_enable )
-        vmcb->_vintr.fields.vgif = 0;
+    if ( n1vmcb->_vintr.fields.vgif_enable )
+        n1vmcb->_vintr.fields.vgif = 0;
     else
         svm->ns_gif = 0;
 
@@ -1482,11 +1483,12 @@ bool
 nestedsvm_gif_isset(struct vcpu *v)
 {
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
-    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
+    struct vmcb_struct *n1vmcb = nv->nv_n1vmcx;
 
     /* get the vmcb gif value if using vgif */
-    if ( vmcb->_vintr.fields.vgif_enable )
-        return vmcb->_vintr.fields.vgif;
+    if ( n1vmcb->_vintr.fields.vgif_enable )
+        return n1vmcb->_vintr.fields.vgif;
     else
         return svm->ns_gif;
 }
-- 
2.34.1

[PATCH v2 04/13] nestedsvm: Set GIF during VMRUN if vGIF is enabled

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

During a VMRUN, the GIF is set by the processor so when Xen emulates
VMRUN, it should set the flag too. This was already handled for !vGIF so
handle it for the vGIF case too.

Fixes: 4cd0fad64590 ("x86/svm: Add virtual GIF support")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 8943d61e2b25..a76cec1fecf1 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -625,10 +625,12 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct cpu_user_regs *regs,
     int ret;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
-    struct vmcb_struct *ns_vmcb;
+    struct vmcb_struct *ns_vmcb, *n1vmcb;
 
     ns_vmcb = nv->nv_vvmcx;
+    n1vmcb = nv->nv_n1vmcx;
     ASSERT(ns_vmcb != NULL);
+    ASSERT(n1vmcb != NULL);
     ASSERT(nv->nv_n2vmcx != NULL);
     ASSERT(nv->nv_n2vmcx_pa != INVALID_PADDR);
 
@@ -673,7 +675,10 @@ nsvm_vcpu_vmentry(struct vcpu *v, struct cpu_user_regs *regs,
         return ret;
     }
 
-    svm->ns_gif = 1;
+    if ( n1vmcb->_vintr.fields.vgif_enable )
+        n1vmcb->_vintr.fields.vgif = 1;
+    else
+        svm->ns_gif = 1;
     return 0;
 }
 
-- 
2.34.1

[PATCH v2 05/13] tools/libxl: Add support for Viridian stimer direct mode

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

Add support to libxl for using the stimer enlightenments in direct mode.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 docs/man/xl.cfg.5.pod.in         | 5 +++++
 tools/include/libxl.h            | 6 ++++++
 tools/libs/light/libxl_types.idl | 1 +
 tools/libs/light/libxl_x86.c     | 5 +++++
 4 files changed, 17 insertions(+)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index d34951edb98d..30c401588fef 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2496,6 +2496,11 @@ ticks and hence enabling this group will ensure that ticks will be
 consistent with use of an enlightened time source (B<time_ref_count> or
 B<reference_tsc>).
 
+=item B<stimer_direct>
+
+This set is the same as the B<stimer> set but additionally supports
+using synthetic timers in direct mode.
+
 =item B<hcall_ipi>
 
 This set incorporates use of a hypercall for interprocessor interrupts.
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 7c098edab663..5475530a5002 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -381,6 +381,12 @@
  */
 #define LIBXL_HAVE_VIRIDIAN_HCALL_IPI 1
 
+/*
+ * LIBXL_HAVE_VIRIDIAN_STIMER_DIRECT indicates that the 'stimer_direct' value
+ * is present in the viridian enlightenment enumeration.
+ */
+#define LIBXL_HAVE_VIRIDIAN_STIMER_DIRECT 1
+
 /*
  * LIBXL_HAVE_BUILDINFO_HVM_ACPI_LAPTOP_SLATE indicates that
  * libxl_domain_build_info has the u.hvm.acpi_laptop_slate field.
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index a7893460f013..95075f04fe45 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -260,6 +260,7 @@ libxl_viridian_enlightenment = Enumeration("viridian_enlightenment", [
     (10, "ex_processor_masks"),
     (11, "no_vp_limit"),
     (12, "cpu_hotplug"),
+    (13, "stimer_direct"),
     ])
 
 libxl_hdtype = Enumeration("hdtype", [
diff --git a/tools/libs/light/libxl_x86.c b/tools/libs/light/libxl_x86.c
index 60d4e8661c93..9e8b48372d63 100644
--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -389,6 +389,11 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid,
     if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_CPU_HOTPLUG))
         mask |= HVMPV_cpu_hotplug;
 
+    if (libxl_bitmap_test(&enlightenments,
+                          LIBXL_VIRIDIAN_ENLIGHTENMENT_STIMER_DIRECT))
+        mask |= HVMPV_time_ref_count | HVMPV_synic | HVMPV_stimer |
+                HVMPV_stimer_direct;
+
     if (mask != 0 &&
         xc_hvm_param_set(CTX->xch,
                          domid,
-- 
2.34.1

[PATCH v2 06/13] x86/hvm: Clamp Viridian features to known set

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

Instead of returning EINVAL on an unknown feature bit, clamp to the
known feature set. This simplifies upgrades since when the hypervisor
and toolstack are updated without an immediate reboot, the toolstack may
set unknown feature bits and the existing behaviour leaves the VM
without any Virdian features enabled.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/hvm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ef9c7cd05777..e2c31d87afc9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4235,8 +4235,10 @@ static int hvm_set_param(struct domain *d, uint32_t index, uint64_t value)
     case HVM_PARAM_VIRIDIAN:
         if ( !IS_ENABLED(CONFIG_VIRIDIAN) )
             rc = -ENODEV;
-        else if ( (value & ~HVMPV_feature_mask) || !(value & HVMPV_base_freq) )
+        else if ( !(value & HVMPV_base_freq) )
             rc = -EINVAL;
+
+        value &= HVMPV_feature_mask;
         break;
     case HVM_PARAM_IDENT_PT:
         /*
-- 
2.34.1

[PATCH v2 07/13] x86/svm: require VMSAVEvirt for nested virt

[chunjie.zhu]

From: Stephen Cheng <stephen.cheng@citrix.com>

Virtual VMLOAD/VMSAVE lets an L1 guest execute VMLOAD and VMSAVE
without intercepts.  Without it, Xen has to map the L1-provided VMCB
and re-execute each instruction in L0, complicating security-sensitive
state handling.

Make VMSAVEvirt a hard requirement for nested SVM.  Advertise it to
L1, enable it when L1 enables SVM, and remove the nested VMLOAD/VMSAVE
emulation path.  Any remaining intercept therefore comes from a guest
which has not enabled SVM and receives #UD.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Stephen Cheng <stephen.cheng@citrix.com>
---
 docs/designs/nested-svm-cpu-features.md |  11 +++
 xen/arch/x86/cpu-policy.c               |   3 +-
 xen/arch/x86/hvm/svm/nestedsvm.c        |  24 ++++--
 xen/arch/x86/hvm/svm/svm.c              | 100 +++---------------------
 xen/arch/x86/hvm/svm/svm.h              |   2 -
 5 files changed, 41 insertions(+), 99 deletions(-)

diff --git a/docs/designs/nested-svm-cpu-features.md b/docs/designs/nested-svm-cpu-features.md
index ce168e68e1b3..798480b259c8 100644
--- a/docs/designs/nested-svm-cpu-features.md
+++ b/docs/designs/nested-svm-cpu-features.md
@@ -109,3 +109,14 @@ leaf 8000000A:edx
   Using it in L0 reduces the chance that we'll make some sort of error
   in the decode path.  And if hardware supports it, it's easy enough
   to provide to the L1.
+
+- 15 `VLoadSave` *Virtual VMLOAD/VMSAVE*: Require for L0, provide to L1
+
+  Without this feature Xen has to intercept the L1 hypervisor's VMLOAD
+  and VMSAVE instructions and emulate them by re-executing the real
+  instruction on a mapped copy of the L1-supplied VMCB.  That path
+  handles a complex, security-sensitive subset of state (the hidden
+  segment descriptors for FS/GS/TR/LDTR plus the SYSCALL/SYSENTER
+  MSRs), so on faithfulness grounds we'd much rather let the hardware
+  do it.  When present, the instructions execute natively in the guest
+  without a #VMEXIT, which is both simpler and faster.
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 48a3185eed8d..a350270f2229 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -845,7 +845,8 @@ static void __init calculate_hvm_max_policy(void)
                                (1u << SVM_FEATURE_NRIPS) |
                                (1u << SVM_FEATURE_FLUSHBYASID) |
                                (1u << SVM_FEATURE_PAUSEFILTER) |
-                               (1u << SVM_FEATURE_DECODEASSISTS));
+                               (1u << SVM_FEATURE_DECODEASSISTS) |
+                               (1u << SVM_FEATURE_VLOADSAVE));
         /* Enable features which are always emulated. */
         p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
     }
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index a76cec1fecf1..79242bdd9117 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -573,7 +573,10 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
 
     /* Keep the host values of the fs, gs, ldtr, tr, kerngsbase,
      * star, lstar, cstar, sfmask, sysenter_cs, sysenter_esp,
-     * sysenter_eip. These are handled via VMSAVE/VMLOAD emulation.
+     * sysenter_eip. These are not transferred by VMRUN/#VMEXIT; they
+     * are moved directly to/from the L1-provided VMCB by the guest's
+     * own VMSAVE/VMLOAD, which run natively (VMSAVEvirt is required
+     * for nested virt).
      */
 
     /* PAT */
@@ -1113,7 +1116,10 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
 
     /* Keep the l2 guest values of the fs, gs, ldtr, tr, kerngsbase,
      * star, lstar, cstar, sfmask, sysenter_cs, sysenter_esp,
-     * sysenter_eip. These are handled via VMSAVE/VMLOAD emulation.
+     * sysenter_eip. These are not transferred by VMRUN/#VMEXIT; they
+     * are moved directly to/from the L1-provided VMCB by the guest's
+     * own VMSAVE/VMLOAD, which run natively (VMSAVEvirt is required
+     * for nested virt).
      */
 
     /* CR2 */
@@ -1565,9 +1571,14 @@ void svm_nested_features_on_efer_update(struct vcpu *v)
      */
     if ( nsvm_efer_svm_enabled(v) )
     {
+        /*
+         * VMSAVEvirt is a hard requirement for nested virt (see
+         * start_nested_svm()), so it is guaranteed present here.
+         */
+        ASSERT(cpu_has_svm_vloadsave);
+
         if ( !vmcb->virt_ext.fields.vloadsave_enable &&
-             !paging_mode_shadow(v->domain) &&
-             cpu_has_svm_vloadsave )
+             !paging_mode_shadow(v->domain) )
         {
             vmcb->virt_ext.fields.vloadsave_enable = 1;
             general2_intercepts  = vmcb_get_general2_intercepts(vmcb);
@@ -1621,9 +1632,10 @@ void __init start_nested_svm(struct hvm_function_table *hvm_function_table)
      * docs/designs/nested-svm-cpu-features.md for rationale.
      */
     hvm_function_table->caps.nested_virt =
-        hvm_function_table->caps.hap && 
+        hvm_function_table->caps.hap &&
         cpu_has_svm_lbrv &&
         cpu_has_svm_nrips &&
         cpu_has_svm_flushbyasid &&
-        cpu_has_svm_decode;
+        cpu_has_svm_decode &&
+        cpu_has_svm_vloadsave;
 }
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 8f97f6c31716..ea32dcead218 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2136,95 +2136,17 @@ svm_vmexit_do_vmrun(struct cpu_user_regs *regs,
     return;
 }
 
-static struct page_info *
-nsvm_get_nvmcb_page(struct vcpu *v, uint64_t vmcbaddr)
-{
-    p2m_type_t p2mt;
-    struct page_info *page;
-    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-
-    if ( !nestedsvm_vmcb_map(v, vmcbaddr) )
-        return NULL;
-
-    /* Need to translate L1-GPA to MPA */
-    page = get_page_from_gfn(v->domain, nv->nv_vvmcxaddr >> PAGE_SHIFT,
-                             &p2mt, P2M_ALLOC | P2M_UNSHARE);
-    if ( !page )
-        return NULL;
-
-    if ( !p2m_is_ram(p2mt) || p2m_is_readonly(p2mt) )
-    {
-        put_page(page);
-        return NULL;
-    }
-
-    return  page;
-}
-
-static void
-svm_vmexit_do_vmload(struct vmcb_struct *vmcb,
-                     struct cpu_user_regs *regs,
-                     struct vcpu *v, uint64_t vmcbaddr)
-{
-    unsigned int inst_len;
-    struct page_info *page;
-
-    if ( (inst_len = svm_get_insn_len(v, INSTR_VMLOAD)) == 0 )
-        return;
-
-    if ( !nsvm_efer_svm_enabled(v) )
-    {
-        hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
-        return;
-    }
-
-    page = nsvm_get_nvmcb_page(v, vmcbaddr);
-    if ( !page )
-    {
-        gdprintk(XENLOG_ERR,
-            "VMLOAD: mapping failed, injecting #GP\n");
-        hvm_inject_hw_exception(X86_EXC_GP, 0);
-        return;
-    }
-
-    svm_vmload_pa(page_to_maddr(page));
-    put_page(page);
-
-    /* State in L1 VMCB is stale now */
-    v->arch.hvm.svm.vmcb_sync_state = vmcb_needs_vmsave;
-
-    __update_guest_eip(regs, inst_len);
-}
-
 static void
-svm_vmexit_do_vmsave(struct vmcb_struct *vmcb,
-                     struct cpu_user_regs *regs,
-                     struct vcpu *v, uint64_t vmcbaddr)
+svm_vmexit_do_vmload_vmsave(struct vcpu *v)
 {
-    unsigned int inst_len;
-    struct page_info *page;
-
-    if ( (inst_len = svm_get_insn_len(v, INSTR_VMSAVE)) == 0 )
-        return;
-
-    if ( !nsvm_efer_svm_enabled(v) )
-    {
-        hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
-        return;
-    }
-
-    page = nsvm_get_nvmcb_page(v, vmcbaddr);
-    if ( !page )
-    {
-        gdprintk(XENLOG_ERR,
-            "VMSAVE: mapping vmcb failed, injecting #GP\n");
-        hvm_inject_hw_exception(X86_EXC_GP, 0);
-        return;
-    }
-
-    svm_vmsave_pa(page_to_maddr(page));
-    put_page(page);
-    __update_guest_eip(regs, inst_len);
+    /*
+     * VMSAVEvirt is required for nested virt, so once a guest enables SVM its
+     * VMLOAD/VMSAVE execute natively and are no longer intercepted.  An
+     * intercepted VMLOAD/VMSAVE therefore means the guest has not enabled SVM:
+     * inject #UD.
+     */
+    ASSERT(!nsvm_efer_svm_enabled(v));
+    hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
 }
 
 static int svm_is_erratum_383(struct cpu_user_regs *regs)
@@ -2956,10 +2878,8 @@ void asmlinkage svm_vmexit_handler(void)
         svm_vmexit_do_vmrun(regs, v, regs->rax);
         break;
     case VMEXIT_VMLOAD:
-        svm_vmexit_do_vmload(vmcb, regs, v, regs->rax);
-        break;
     case VMEXIT_VMSAVE:
-        svm_vmexit_do_vmsave(vmcb, regs, v, regs->rax);
+        svm_vmexit_do_vmload_vmsave(v);
         break;
     case VMEXIT_STGI:
         svm_vmexit_do_stgi(regs, v);
diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
index cfa411ad5ae1..56a1a4840b37 100644
--- a/xen/arch/x86/hvm/svm/svm.h
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -52,8 +52,6 @@ static inline void svm_invlpga(unsigned long linear, uint32_t asid)
 #define INSTR_XSETBV     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0321) /* octal-ok */
 #define INSTR_VMRUN      INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0330) /* octal-ok */
 #define INSTR_VMCALL     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0331) /* octal-ok */
-#define INSTR_VMLOAD     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0332) /* octal-ok */
-#define INSTR_VMSAVE     INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0333) /* octal-ok */
 #define INSTR_STGI       INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0334) /* octal-ok */
 #define INSTR_CLGI       INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0335) /* octal-ok */
 #define INSTR_INVLPGA    INSTR_ENC(X86EMUL_OPC(0x0f, 0x01), 0337) /* octal-ok */
-- 
2.34.1

[PATCH v2 08/13] nestedsvm: Don't set VMCB(1-2)'s NP_ENABLE and N_CR3 during VMEXIT to L1

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

As per the VMRUN pseudocode in APM Vol 3 3.38, the VMCB's NP_ENABLE and
N_CR3 fields are not set during a VMEXIT so don't do this when updating
VMCB(1-2). At the same time, cleanup the somewhat bogus and irrelevant
comments. Not clearing N_CR3 does not introduce a security hole as
stated since L1 can set it regardless and it is never used directly when
running L2.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c | 36 +++-----------------------------
 1 file changed, 3 insertions(+), 33 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 79242bdd9117..d47e57c8a1d7 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1031,37 +1031,6 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
 
     ns_vmcb->event_inj.raw = 0;
 
-    /* Nested paging mode */
-    if ( nestedhvm_paging_mode_hap(v) )
-    {
-        /* host nested paging + guest nested paging. */
-        vmcb_set_np(ns_vmcb, vmcb_get_np(n2vmcb));
-        ns_vmcb->_cr3 = n2vmcb->_cr3;
-        /* The vmcb->h_cr3 is the shadowed h_cr3. The original
-         * unshadowed guest h_cr3 is kept in ns_vmcb->h_cr3,
-         * hence we keep the ns_vmcb->h_cr3 value. */
-    }
-    else if ( !paging_mode_shadow(v->domain) )
-    {
-        /* host nested paging + guest shadow paging. */
-        vmcb_set_np(ns_vmcb, false);
-        /* Throw h_cr3 away. Guest is not allowed to set it or
-         * it can break out, otherwise (security hole!) */
-        ns_vmcb->_h_cr3 = 0x0;
-        /* Stop intercepting #PF (already done above
-         * by restoring cached intercepts). */
-        ns_vmcb->_cr3 = n2vmcb->_cr3;
-    }
-    else
-    {
-        /* host shadow paging + guest shadow paging. */
-        vmcb_set_np(ns_vmcb, false);
-        ns_vmcb->_h_cr3 = 0x0;
-        /* The vmcb->_cr3 is the shadowed cr3. The original
-         * unshadowed guest cr3 is kept in ns_vmcb->_cr3,
-         * hence we keep the ns_vmcb->_cr3 value. */
-    }
-
     /* LBR virtualization - keep lbr control as is */
 
     /* NextRIP */
@@ -1090,8 +1059,9 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
     ns_vmcb->_efer = v->arch.hvm.guest_efer;
 
     /* CRn */
-    ns_vmcb->_cr4 = v->arch.hvm.guest_cr[4];
-    ns_vmcb->_cr0 = v->arch.hvm.guest_cr[0];
+    ns_vmcb->_cr4 = n2vmcb->_cr4;
+    ns_vmcb->_cr3 = n2vmcb->_cr3;
+    ns_vmcb->_cr0 = n2vmcb->_cr0;
 
     /* DRn */
     ns_vmcb->_dr7 = n2vmcb->_dr7;
-- 
2.34.1

[PATCH v2 09/13] x86/svm: Fix VMLOAD/VMSAVE state handling when using nested virt

[chunjie.zhu]

From: Ross Lagerwall <ross.lagerwall@citrix.com>

Currently, the SVM VMLOAD/VMSAVE state handling always uses the vCPU's
active VMCB to load/store the state. The active VMCB changes depending
on whether or not the vCPU is in guest mode yet the state is not
properly copied between VMCB(0-1) and VMCB(0-2) nor is the
vmcb_sync_state updated when switching active VMCB.

The most common way this fails is when context switching to a vCPU in
guest mode and immediately taking a VMEXIT (e.g. due to an interrupt for
L1 having arrived in the meantime). The code issues an unconditional
VMSAVE into VMCB(0-1) but the state has not yet been loaded since
context switching which results in garbage in VMCB(0-1). The garbage
state is then (re-)loaded from VMCB(0-1) shortly before VMRUN. This
results in frequent, random crashes in L2.

Since the state covered by VMLOAD/VMSAVE is a property of the vCPU and
is not affected by switches to and from guest mode, always use VMCB(0-1)
to store and retrieve it. This avoids the need to synchronize state
between VMCBs and fixes the random crashes. L1 itself is responsible for
using VMLOAD/VMSAVE to load/save the state from hardware into its own
VMCB(1-2) and this change does not affect that.

Fixes: 9a779e4fc161 ("Implement SVM specific part for Nested Virtualization")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c |   7 --
 xen/arch/x86/hvm/svm/svm.c       | 107 +++++++++++++++++--------------
 xen/arch/x86/hvm/svm/vmcb.c      |  26 ++++----
 xen/arch/x86/hvm/svm/vmcb.h      |   3 +-
 4 files changed, 75 insertions(+), 68 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index d47e57c8a1d7..e1cb10e1de11 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -198,11 +198,6 @@ static int nsvm_vcpu_hostrestore(struct vcpu *v, struct cpu_user_regs *regs)
     ASSERT(n1vmcb != NULL);
     ASSERT(n2vmcb != NULL);
 
-    /*
-     * nsvm_vmcb_prepare4vmexit() already saved register values
-     * handled by VMSAVE/VMLOAD into n1vmcb directly.
-     */
-
     /* switch vmcb to l1 guest's vmcb */
     v->arch.hvm.svm.vmcb = n1vmcb;
     v->arch.hvm.svm.vmcb_pa = nv->nv_n1vmcx_pa;
@@ -977,8 +972,6 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
     struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
     struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
 
-    svm_vmsave_pa(nv->nv_n1vmcx_pa);
-
     /* Cache guest physical address of virtual vmcb
      * for VMCB Cleanbit emulation.
      */
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index ea32dcead218..5aeffd5e416f 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -443,30 +443,30 @@ static int svm_vmcb_restore(struct vcpu *v, struct hvm_hw_cpu *c)
 
 static void svm_save_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
 {
-    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
 
-    data->sysenter_cs      = vmcb->sysenter_cs;
-    data->sysenter_esp     = vmcb->sysenter_esp;
-    data->sysenter_eip     = vmcb->sysenter_eip;
-    data->shadow_gs        = vmcb->kerngsbase;
-    data->msr_lstar        = vmcb->lstar;
-    data->msr_star         = vmcb->star;
-    data->msr_cstar        = vmcb->cstar;
-    data->msr_syscall_mask = vmcb->sfmask;
+    data->sysenter_cs      = n1_vmcb->sysenter_cs;
+    data->sysenter_esp     = n1_vmcb->sysenter_esp;
+    data->sysenter_eip     = n1_vmcb->sysenter_eip;
+    data->shadow_gs        = n1_vmcb->kerngsbase;
+    data->msr_lstar        = n1_vmcb->lstar;
+    data->msr_star         = n1_vmcb->star;
+    data->msr_cstar        = n1_vmcb->cstar;
+    data->msr_syscall_mask = n1_vmcb->sfmask;
 }
 
 static void svm_load_cpu_state(struct vcpu *v, struct hvm_hw_cpu *data)
 {
-    struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
 
-    vmcb->lstar        = data->msr_lstar;
-    vmcb->star         = data->msr_star;
-    vmcb->cstar        = data->msr_cstar;
-    vmcb->sfmask       = data->msr_syscall_mask;
-    vmcb->kerngsbase   = data->shadow_gs;
-    vmcb->sysenter_cs  = data->sysenter_cs;
-    vmcb->sysenter_esp = data->sysenter_esp;
-    vmcb->sysenter_eip = data->sysenter_eip;
+    n1_vmcb->lstar        = data->msr_lstar;
+    n1_vmcb->star         = data->msr_star;
+    n1_vmcb->cstar        = data->msr_cstar;
+    n1_vmcb->sfmask       = data->msr_syscall_mask;
+    n1_vmcb->kerngsbase   = data->shadow_gs;
+    n1_vmcb->sysenter_cs  = data->sysenter_cs;
+    n1_vmcb->sysenter_esp = data->sysenter_esp;
+    n1_vmcb->sysenter_eip = data->sysenter_eip;
     v->arch.hvm.guest_efer = data->msr_efer;
     svm_update_guest_efer(v);
 }
@@ -578,18 +578,19 @@ static void cf_check svm_cpuid_policy_changed(struct vcpu *v)
 void svm_sync_vmcb(struct vcpu *v, enum vmcb_sync_state new_state)
 {
     struct svm_vcpu *svm = &v->arch.hvm.svm;
+    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
 
     if ( new_state == vmcb_needs_vmsave )
     {
         if ( svm->vmcb_sync_state == vmcb_needs_vmload )
-            svm_vmload_pa(svm->vmcb_pa);
+            svm_vmload_pa(nv->nv_n1vmcx_pa);
 
         svm->vmcb_sync_state = new_state;
     }
     else
     {
         if ( svm->vmcb_sync_state == vmcb_needs_vmsave )
-            svm_vmsave_pa(svm->vmcb_pa);
+            svm_vmsave_pa(nv->nv_n1vmcx_pa);
 
         if ( svm->vmcb_sync_state != vmcb_needs_vmload )
             svm->vmcb_sync_state = new_state;
@@ -605,6 +606,7 @@ static void cf_check svm_get_segment_register(
     struct vcpu *v, enum x86_segment seg, struct segment_register *reg)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
 
     ASSERT((v == current) || !vcpu_runnable(v));
 
@@ -612,8 +614,9 @@ static void cf_check svm_get_segment_register(
     {
     case x86_seg_fs ... x86_seg_gs:
         svm_sync_vmcb(v, vmcb_in_sync);
+        *reg = n1_vmcb->sreg[seg];
+        break;
 
-        /* Fallthrough. */
     case x86_seg_es ... x86_seg_ds:
         *reg = vmcb->sreg[seg];
 
@@ -623,7 +626,7 @@ static void cf_check svm_get_segment_register(
 
     case x86_seg_tss:
         svm_sync_vmcb(v, vmcb_in_sync);
-        *reg = vmcb->tr;
+        *reg = n1_vmcb->tr;
         break;
 
     case x86_seg_gdt:
@@ -636,7 +639,7 @@ static void cf_check svm_get_segment_register(
 
     case x86_seg_ldt:
         svm_sync_vmcb(v, vmcb_in_sync);
-        *reg = vmcb->ldtr;
+        *reg = n1_vmcb->ldtr;
         break;
 
     default:
@@ -651,6 +654,7 @@ static void cf_check svm_set_segment_register(
     struct vcpu *v, enum x86_segment seg, struct segment_register *reg)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
 
     ASSERT((v == current) || !vcpu_runnable(v));
 
@@ -689,12 +693,16 @@ static void cf_check svm_set_segment_register(
 
         /* Fallthrough */
     case x86_seg_es ... x86_seg_cs:
-    case x86_seg_ds ... x86_seg_gs:
+    case x86_seg_ds:
         vmcb->sreg[seg] = *reg;
         break;
 
+    case x86_seg_fs ... x86_seg_gs:
+        n1_vmcb->sreg[seg] = *reg;
+        break;
+
     case x86_seg_tss:
-        vmcb->tr = *reg;
+        n1_vmcb->tr = *reg;
         break;
 
     case x86_seg_gdt:
@@ -708,7 +716,7 @@ static void cf_check svm_set_segment_register(
         break;
 
     case x86_seg_ldt:
-        vmcb->ldtr = *reg;
+        n1_vmcb->ldtr = *reg;
         break;
 
     case x86_seg_sys:
@@ -1667,6 +1675,7 @@ static int cf_check svm_msr_read_intercept(
     struct vcpu *v = current;
     const struct domain *d = v->domain;
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
     const struct nestedsvm *nsvm = &vcpu_nestedsvm(v);
     uint64_t tmp;
 
@@ -1689,43 +1698,43 @@ static int cf_check svm_msr_read_intercept(
     switch ( msr )
     {
     case MSR_IA32_SYSENTER_CS:
-        *msr_content = vmcb->sysenter_cs;
+        *msr_content = n1_vmcb->sysenter_cs;
         break;
 
     case MSR_IA32_SYSENTER_ESP:
-        *msr_content = vmcb->sysenter_esp;
+        *msr_content = n1_vmcb->sysenter_esp;
         break;
 
     case MSR_IA32_SYSENTER_EIP:
-        *msr_content = vmcb->sysenter_eip;
+        *msr_content = n1_vmcb->sysenter_eip;
         break;
 
     case MSR_STAR:
-        *msr_content = vmcb->star;
+        *msr_content = n1_vmcb->star;
         break;
 
     case MSR_LSTAR:
-        *msr_content = vmcb->lstar;
+        *msr_content = n1_vmcb->lstar;
         break;
 
     case MSR_CSTAR:
-        *msr_content = vmcb->cstar;
+        *msr_content = n1_vmcb->cstar;
         break;
 
     case MSR_SYSCALL_MASK:
-        *msr_content = vmcb->sfmask;
+        *msr_content = n1_vmcb->sfmask;
         break;
 
     case MSR_FS_BASE:
-        *msr_content = vmcb->fs.base;
+        *msr_content = n1_vmcb->fs.base;
         break;
 
     case MSR_GS_BASE:
-        *msr_content = vmcb->gs.base;
+        *msr_content = n1_vmcb->gs.base;
         break;
 
     case MSR_SHADOW_GS_BASE:
-        *msr_content = vmcb->kerngsbase;
+        *msr_content = n1_vmcb->kerngsbase;
         break;
 
     case MSR_IA32_MCx_MISC(4): /* Threshold register */
@@ -1858,6 +1867,7 @@ static int cf_check svm_msr_write_intercept(
     struct vcpu *v = current;
     struct domain *d = v->domain;
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
     struct nestedsvm *nsvm = &vcpu_nestedsvm(v);
 
     switch ( msr )
@@ -1891,45 +1901,45 @@ static int cf_check svm_msr_write_intercept(
         switch ( msr )
         {
         case MSR_IA32_SYSENTER_ESP:
-            vmcb->sysenter_esp = msr_content;
+            n1_vmcb->sysenter_esp = msr_content;
             break;
 
         case MSR_IA32_SYSENTER_EIP:
-            vmcb->sysenter_eip = msr_content;
+            n1_vmcb->sysenter_eip = msr_content;
             break;
 
         case MSR_LSTAR:
-            vmcb->lstar = msr_content;
+            n1_vmcb->lstar = msr_content;
             break;
 
         case MSR_CSTAR:
-            vmcb->cstar = msr_content;
+            n1_vmcb->cstar = msr_content;
             break;
 
         case MSR_FS_BASE:
-            vmcb->fs.base = msr_content;
+            n1_vmcb->fs.base = msr_content;
             break;
 
         case MSR_GS_BASE:
-            vmcb->gs.base = msr_content;
+            n1_vmcb->gs.base = msr_content;
             break;
 
         case MSR_SHADOW_GS_BASE:
-            vmcb->kerngsbase = msr_content;
+            n1_vmcb->kerngsbase = msr_content;
             break;
         }
         break;
 
     case MSR_IA32_SYSENTER_CS:
-        vmcb->sysenter_cs = msr_content;
+        n1_vmcb->sysenter_cs = msr_content;
         break;
 
     case MSR_STAR:
-        vmcb->star = msr_content;
+        n1_vmcb->star = msr_content;
         break;
 
     case MSR_SYSCALL_MASK:
-        vmcb->sfmask = msr_content;
+        n1_vmcb->sfmask = msr_content;
         break;
 
     case MSR_IA32_DEBUGCTLMSR:
@@ -2257,6 +2267,7 @@ static bool cf_check svm_get_pending_event(
 static uint64_t cf_check svm_get_reg(struct vcpu *v, unsigned int reg)
 {
     struct vcpu *curr = current;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
     const struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     struct domain *d = v->domain;
 
@@ -2268,7 +2279,7 @@ static uint64_t cf_check svm_get_reg(struct vcpu *v, unsigned int reg)
     case MSR_SHADOW_GS_BASE:
         if ( v == curr )
             svm_sync_vmcb(v, vmcb_in_sync);
-        return vmcb->kerngsbase;
+        return n1_vmcb->kerngsbase;
 
     default:
         printk(XENLOG_G_ERR "%s(%pv, 0x%08x) Bad register\n",
@@ -2544,7 +2555,7 @@ void asmlinkage svm_vmexit_handler(void)
     if ( unlikely(exit_reason == VMEXIT_INVALID) )
     {
         gdprintk(XENLOG_ERR, "invalid VMCB state:\n");
-        svm_vmcb_dump(__func__, vmcb);
+        svm_vmcb_dump(__func__, v, vmcb);
         domain_crash(v->domain);
         goto out;
     }
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 39b9e6bf2068..fc3ba7c44815 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -250,16 +250,17 @@ static void svm_dump_sel(const char *name, const struct segment_register *s)
            name, s->sel, s->attr, s->limit, s->base);
 }
 
-void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb)
+void svm_vmcb_dump(const char *from, struct vcpu *v,
+                   const struct vmcb_struct *vmcb)
 {
-    struct vcpu *curr = current;
+    struct vmcb_struct *n1_vmcb = vcpu_nestedhvm(v).nv_n1vmcx;
 
     /*
      * If we are dumping the VMCB currently in context, some guest state may
      * still be cached in hardware.  Retrieve it.
      */
-    if ( vmcb == curr->arch.hvm.svm.vmcb )
-        svm_sync_vmcb(curr, vmcb_in_sync);
+    if ( v == current )
+        svm_sync_vmcb(v, vmcb_in_sync);
 
     printk("Dumping guest's current state at %s...\n", from);
     printk("Size of VMCB = %zu, paddr = %"PRIpaddr", vaddr = %p\n",
@@ -293,7 +294,8 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb)
     printk("virtual vmload/vmsave = %d, virt_ext = %#"PRIx64"\n",
            vmcb->virt_ext.fields.vloadsave_enable, vmcb->virt_ext.bytes);
     printk("cpl = %d efer = %#"PRIx64" star = %#"PRIx64" lstar = %#"PRIx64"\n",
-           vmcb_get_cpl(vmcb), vmcb_get_efer(vmcb), vmcb->star, vmcb->lstar);
+           vmcb_get_cpl(vmcb), vmcb_get_efer(vmcb), n1_vmcb->star,
+           n1_vmcb->lstar);
     printk("CR0 = 0x%016"PRIx64" CR2 = 0x%016"PRIx64"\n",
            vmcb_get_cr0(vmcb), vmcb_get_cr2(vmcb));
     printk("CR3 = 0x%016"PRIx64" CR4 = 0x%016"PRIx64"\n",
@@ -305,9 +307,9 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb)
     printk("DR6 = 0x%016"PRIx64", DR7 = 0x%016"PRIx64"\n",
            vmcb_get_dr6(vmcb), vmcb_get_dr7(vmcb));
     printk("CSTAR = 0x%016"PRIx64" SFMask = 0x%016"PRIx64"\n",
-           vmcb->cstar, vmcb->sfmask);
+           n1_vmcb->cstar, n1_vmcb->sfmask);
     printk("KernGSBase = 0x%016"PRIx64" PAT = 0x%016"PRIx64"\n",
-           vmcb->kerngsbase, vmcb_get_g_pat(vmcb));
+           n1_vmcb->kerngsbase, vmcb_get_g_pat(vmcb));
     printk("SSP = 0x%016"PRIx64" S_CET = 0x%016"PRIx64" ISST = 0x%016"PRIx64"\n",
            vmcb->_ssp, vmcb->_msr_s_cet, vmcb->_msr_isst);
     printk("H_CR3 = 0x%016"PRIx64" CleanBits = %#x\n",
@@ -319,12 +321,12 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb)
     svm_dump_sel("  DS", &vmcb->ds);
     svm_dump_sel("  SS", &vmcb->ss);
     svm_dump_sel("  ES", &vmcb->es);
-    svm_dump_sel("  FS", &vmcb->fs);
-    svm_dump_sel("  GS", &vmcb->gs);
+    svm_dump_sel("  FS", &n1_vmcb->fs);
+    svm_dump_sel("  GS", &n1_vmcb->gs);
     svm_dump_sel("GDTR", &vmcb->gdtr);
-    svm_dump_sel("LDTR", &vmcb->ldtr);
+    svm_dump_sel("LDTR", &n1_vmcb->ldtr);
     svm_dump_sel("IDTR", &vmcb->idtr);
-    svm_dump_sel("  TR", &vmcb->tr);
+    svm_dump_sel("  TR", &n1_vmcb->tr);
 }
 
 bool svm_vmcb_isvalid(
@@ -423,7 +425,7 @@ static void cf_check vmcb_dump(unsigned char ch)
                 continue;
             }
             printk("\tVCPU %d\n", v->vcpu_id);
-            svm_vmcb_dump("key_handler", v->arch.hvm.svm.vmcb);
+            svm_vmcb_dump("key_handler", v, v->arch.hvm.svm.vmcb);
 
             process_pending_softirqs();
         }
diff --git a/xen/arch/x86/hvm/svm/vmcb.h b/xen/arch/x86/hvm/svm/vmcb.h
index 28dae325ced0..650465b090cf 100644
--- a/xen/arch/x86/hvm/svm/vmcb.h
+++ b/xen/arch/x86/hvm/svm/vmcb.h
@@ -563,7 +563,8 @@ int  svm_create_vmcb(struct vcpu *v);
 void svm_destroy_vmcb(struct vcpu *v);
 
 void setup_vmcb_dump(void);
-void svm_vmcb_dump(const char *from, const struct vmcb_struct *vmcb);
+void svm_vmcb_dump(const char *from, struct vcpu *v,
+                   const struct vmcb_struct *vmcb);
 bool svm_vmcb_isvalid(const char *from, const struct vmcb_struct *vmcb,
                       const struct vcpu *v, bool verbose);
 
-- 
2.34.1

[PATCH v2 10/13] x86/nSVM: Don't clear TLB_CONTROL on #VMEXIT

[chunjie.zhu]

From: Lin Liu <lin.liu01@citrix.com>

nsvm_vmcb_prepare4vmexit() zeroes TLB_CONTROL in the L1-provided VMCB.
That VMCB is mapped writable, so the store lands in L1's memory and L1
sees a field it wrote come back as 0.

APM vol 2 rev 3.45 15.16.1 (p.550): "The VMRUN instruction reads, but
does not change, the value of the TLB_CONTROL field."

Fixes: 9a779e4fc161 ("Implement SVM specific part for Nested Virtualization")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Lin Liu <lin.liu01@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index e1cb10e1de11..3b0afb828c91 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -992,9 +992,6 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
     /* ASID */
     /* vmcb_set_asid(ns_vmcb, vmcb_get_asid(n2vmcb)); */
 
-    /* TLB control */
-    ns_vmcb->tlb_control = 0;
-
     /* Virtual Interrupts */
     ns_vmcb->_vintr = n2vmcb->_vintr;
     if ( !svm->ns_hostflags.fields.vintrmask )
-- 
2.34.1

[PATCH v2 11/13] x86/nestedsvm: update vmrun without pending or deferred

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
Changes in v2:
 - Fold the nested p2m related part of the v1 "fix up" patch in here:
   nsvm_vcpu_update_nestedp2m() now clears nv->stale_np2m before checking
   whether the host CR3 needs updating, so a concurrent flush that sets it
   again is not lost.
 - Rebased on top of the preceding patches; no other functional change.

 xen/arch/x86/hvm/svm/entry.S      | 48 +++++++++++++++++++++++--------
 xen/arch/x86/hvm/svm/nestedhvm.h  |  1 +
 xen/arch/x86/hvm/svm/nestedsvm.c  | 33 ++++++++++++++++++++-
 xen/arch/x86/hvm/svm/svm.c        | 11 +++++--
 xen/arch/x86/x86_64/asm-offsets.c |  1 +
 5 files changed, 79 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index d9613a2a8fed..59d865635fd5 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -28,31 +28,51 @@ FUNC(svm_asm_do_resume)
         GET_CURRENT(bx)
 .Lsvm_do_resume:
         call svm_intr_assist
-        call nsvm_vcpu_switch
-        ASSERT_NOT_IN_ATOMIC
 
-        mov  VCPU_processor(%rbx),%eax
-        lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
+        /*
+         * The nested p2m repair below may need to take nestedp2m_lock and
+         * issue synchronous cross-CPU TLB flush IPIs, doing this with
+         * interrupts disabled can deadlock.
+         */
         xor  %ecx,%ecx
-        shl  $IRQSTAT_shift,%eax
-        cli
-        cmp  %ecx,(%rdx,%rax,1)
-        jne  .Lsvm_process_softirqs
-
         cmp  %cl,VCPU_nsvm_hap_enabled(%rbx)
 UNLIKELY_START(ne, nsvm_hap)
+        cmpb $0,VCPU_nhvm_guestmode(%rbx)
+        UNLIKELY_DONE(z, nsvm_hap)
+        cmpb $0,VCPU_nhvm_stale_np2m(%rbx)
+        jne  .Lsvm_update_nestedp2m
         cmp  %rcx,VCPU_nhvm_p2m(%rbx)
         sete %al
         test VCPU_nhvm_guestmode(%rbx),%al
         UNLIKELY_DONE(z, nsvm_hap)
+.Lsvm_update_nestedp2m:
         /*
-         * Someone shot down our nested p2m table; go round again
-         * and nsvm_vcpu_switch() will fix it for us.
+         * The nested p2m table is stale, fix it here.
          */
-        sti
+        mov  %rbx,%rdi
+        call nsvm_vcpu_update_nestedp2m
         jmp  .Lsvm_do_resume
 __UNLIKELY_END(nsvm_hap)
 
+        ASSERT_NOT_IN_ATOMIC
+
+        mov  VCPU_processor(%rbx),%eax
+        lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
+        xor  %ecx,%ecx
+        shl  $IRQSTAT_shift,%eax
+        cli
+        cmp  %ecx,(%rdx,%rax,1)
+        jne  .Lsvm_process_softirqs
+
+        cmp  %cl,VCPU_nsvm_hap_enabled(%rbx)
+        je   .Lsvm_vmenter
+        cmpb $0,VCPU_nhvm_guestmode(%rbx)
+        je   .Lsvm_vmenter
+        cmpb $0,VCPU_nhvm_stale_np2m(%rbx)
+        jne  .Lsvm_retry_nestedp2m
+
+.Lsvm_vmenter:
+
         call svm_vmenter_helper
 
         clgi
@@ -148,4 +168,8 @@ __UNLIKELY_END(nsvm_hap)
         sti
         call do_softirq
         jmp  .Lsvm_do_resume
+
+.Lsvm_retry_nestedp2m:
+        sti
+        jmp  .Lsvm_do_resume
 END(svm_asm_do_resume)
diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
index 9bb04a043430..e22ef259c171 100644
--- a/xen/arch/x86/hvm/svm/nestedhvm.h
+++ b/xen/arch/x86/hvm/svm/nestedhvm.h
@@ -41,6 +41,7 @@ void cf_check nsvm_vcpu_destroy(struct vcpu *v);
 int cf_check nsvm_vcpu_initialise(struct vcpu *v);
 int cf_check nsvm_vcpu_reset(struct vcpu *v);
 int nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs);
+void nsvm_vcpu_update_nestedp2m(struct vcpu *v);
 int cf_check nsvm_vcpu_vmexit_event(struct vcpu *v,
                                     const struct x86_event *event);
 uint64_t cf_check nsvm_vcpu_hostcr3(struct vcpu *v);
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 3b0afb828c91..09a2187ac7e0 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -356,6 +356,37 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu *v,
     n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
 }
 
+void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
+{
+    struct nestedvcpu *nv;
+    struct nestedsvm *svm;
+    struct vmcb_struct *vvmcb, *n2vmcb;
+    struct p2m_domain *p2m;
+
+    ASSERT(v != NULL);
+
+    nv = &vcpu_nestedhvm(v);
+    svm = &vcpu_nestedsvm(v);
+    vvmcb = nv->nv_vvmcx;
+    n2vmcb = nv->nv_n2vmcx;
+
+    if ( !nestedhvm_paging_mode_hap(v) )
+        return;
+
+    p2m = p2m_get_nestedp2m(v);
+    nv->stale_np2m = false;
+
+    /*
+     * This may happen if we've handled VMEXIT_NPF at the same time as a
+     * nested flush IPI from another CPU.  We've got a different P2M so the
+     * host CR3 needs updating.
+     */
+    if ( svm->ns_vmcb_hostcr3 != vvmcb->_h_cr3 ||
+         n2vmcb->_h_cr3 !=
+         pagetable_get_paddr(p2m_get_pagetable(p2m)) )
+        nestedsvm_vmcb_set_nestedp2m(v, vvmcb, n2vmcb);
+}
+
 static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
@@ -511,7 +542,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         /* host nested paging + guest nested paging. */
         vmcb_set_np(n2vmcb, true);
 
-        nestedsvm_vmcb_set_nestedp2m(v, ns_vmcb, n2vmcb);
+        nsvm_vcpu_update_nestedp2m(v);
 
         /* hvm_set_cr3() below sets v->arch.hvm.guest_cr[3] for us. */
         rc = hvm_set_cr3(ns_vmcb->_cr3, false, true);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 5aeffd5e416f..cb09e924dcd3 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2129,6 +2129,8 @@ static void
 svm_vmexit_do_vmrun(struct cpu_user_regs *regs,
                     struct vcpu *v, uint64_t vmcbaddr)
 {
+    int rc;
+
     if ( !nsvm_efer_svm_enabled(v) )
     {
         hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
@@ -2142,8 +2144,13 @@ svm_vmexit_do_vmrun(struct cpu_user_regs *regs,
         return;
     }
 
-    vcpu_nestedhvm(v).nv_vmentry_pending = 1;
-    return;
+    rc = nsvm_vcpu_vmrun(v, regs);
+    if ( rc )
+    {
+        if ( nestedsvm_vcpu_vmexit(v, regs, VMEXIT_INVALID, 0, 0) ==
+             NESTEDHVM_VMEXIT_FATALERROR )
+            domain_crash(v->domain);
+    }
 }
 
 static void
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index baf266ab8013..772d1959e5a5 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -129,6 +129,7 @@ void __dummy__(void)
 
     OFFSET(VCPU_nhvm_guestmode, struct vcpu, arch.hvm.nvcpu.nv_guestmode);
     OFFSET(VCPU_nhvm_p2m, struct vcpu, arch.hvm.nvcpu.nv_p2m);
+    OFFSET(VCPU_nhvm_stale_np2m, struct vcpu, arch.hvm.nvcpu.stale_np2m);
     OFFSET(VCPU_nsvm_hap_enabled, struct vcpu, arch.hvm.nvcpu.u.nsvm.ns_hap_enabled);
     BLANK();
 #endif
-- 
2.34.1

[PATCH v2 12/13] x86/nestedsvm: remove nv_vmentry_pending

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
Changes in v2:
 - No functional change, rebased only.

 xen/arch/x86/hvm/svm/intr.c      | 18 ------------------
 xen/arch/x86/hvm/svm/nestedsvm.c | 23 -----------------------
 2 files changed, 41 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 4b0debfa9a2e..7f6cafb6db78 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -77,24 +77,6 @@ static void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
 
     ASSERT(intack.source != hvm_intsrc_none);
 
-    if ( nestedhvm_enabled(v->domain) )
-    {
-        struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-
-        if ( nv->nv_vmentry_pending )
-        {
-            struct vmcb_struct *gvmcb = nv->nv_vvmcx;
-
-            /* check if l1 guest injects interrupt into l2 guest via vintr.
-             * return here or l2 guest looses interrupts, otherwise.
-             */
-            ASSERT(gvmcb != NULL);
-            intr = vmcb_get_vintr(gvmcb);
-            if ( intr.fields.irq )
-                return;
-        }
-    }
-
     TRACE(TRC_HVM_INTR_WINDOW, intack.vector, intack.source,
           vmcb->event_inj.v ? vmcb->event_inj.vector : -1);
 
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 09a2187ac7e0..da5cd4848bb6 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1407,31 +1407,8 @@ void asmlinkage nsvm_vcpu_switch(void)
     vmexit:
         nestedsvm_vcpu_vmexit(v, regs, svm->ns_vmexit.exitcode);
         nv->nv_vmexit_pending = 0;
-        nv->nv_vmentry_pending = 0;
         return;
     }
-
-    if ( nv->nv_vmentry_pending )
-    {
-        int ret;
-        ASSERT(!nv->nv_vmexit_pending);
-        ret = nsvm_vcpu_vmrun(v, regs);
-        if ( ret )
-            goto vmexit;
-
-        ASSERT(nestedhvm_vcpu_in_guestmode(v));
-        nv->nv_vmentry_pending = 0;
-    }
-
-    if ( nestedhvm_vcpu_in_guestmode(v) && nestedhvm_paging_mode_hap(v) )
-    {
-        /* In case left the l2 guest due to a physical interrupt (e.g. IPI)
-         * that is not for the l1 guest then we continue running the l2 guest
-         * but check if the nestedp2m is still valid.
-         */
-        if ( nv->nv_p2m == NULL )
-            nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
-    }
 }
 
 /* Interrupts, Virtual GIF */
-- 
2.34.1

[PATCH v2 13/13] x86/nestedsvm: update vmexit without pending or deferred flag

[chunjie.zhu]

From: Chunjie Zhu <chunjie.zhu@citrix.com>

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
Changes in v2:
 - Rebased on top of the preceding patches (nestedsvm_vmexit_defer() now
   removed in its updated form that uses the correct VMCB for vGIF); no
   other functional change.

 xen/arch/x86/hvm/svm/intr.c              |   4 +-
 xen/arch/x86/hvm/svm/nestedhvm.h         |   5 +-
 xen/arch/x86/hvm/svm/nestedsvm.c         | 164 ++++++++---------------
 xen/arch/x86/hvm/svm/svm.c               |  10 +-
 xen/arch/x86/include/asm/hvm/svm-types.h |   7 -
 5 files changed, 71 insertions(+), 119 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index 7f6cafb6db78..f9b59a0d6a46 100644
--- a/xen/arch/x86/hvm/svm/intr.c
+++ b/xen/arch/x86/hvm/svm/intr.c
@@ -55,7 +55,7 @@ static void svm_inject_nmi(struct vcpu *v)
         vmcb, general1_intercepts | GENERAL1_INTERCEPT_IRET);
 }
 
-static void svm_inject_intr(struct vcpu *v, int vector)
+void svm_inject_intr(struct vcpu *v, int vector)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     intinfo_t event;
@@ -69,7 +69,7 @@ static void svm_inject_intr(struct vcpu *v, int vector)
     vmcb->event_inj = event;
 }
 
-static void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
+void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack)
 {
     struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
     uint32_t general1_intercepts = vmcb_get_general1_intercepts(vmcb);
diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
index e22ef259c171..5876a948f39a 100644
--- a/xen/arch/x86/hvm/svm/nestedhvm.h
+++ b/xen/arch/x86/hvm/svm/nestedhvm.h
@@ -27,7 +27,8 @@
     (!!((v)->arch.hvm.guest_efer & EFER_SVME))
 
 int nestedsvm_vmcb_map(struct vcpu *v, uint64_t vmcbaddr);
-void nestedsvm_vmexit_defer(struct vcpu *v,
+enum nestedhvm_vmexits
+nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
     uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2);
 enum nestedhvm_vmexits
 nestedsvm_vmexit_n2n1(struct vcpu *v, struct cpu_user_regs *regs);
@@ -65,6 +66,8 @@ int cf_check nsvm_hap_walk_L1_p2m(
 #define NSVM_INTR_MASKED         0
 
 int nestedsvm_vcpu_interrupt(struct vcpu *v, const struct hvm_intack intack);
+void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack);
+void svm_inject_intr(struct vcpu *v, int vector);
 
 #endif /* __X86_HVM_SVM_NESTEDHVM_PRIV_H__ */
 
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index da5cd4848bb6..8c673bcea9f6 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -152,8 +152,6 @@ int cf_check nsvm_vcpu_reset(struct vcpu *v)
     svm->ns_vmcb_hostcr3 = 0;
     svm->ns_asid = 0;
     svm->ns_hostflags.bytes = 0;
-    svm->ns_vmexit.exitinfo1 = 0;
-    svm->ns_vmexit.exitinfo2 = 0;
 
     svm->ns_iomap = NULL;
 
@@ -717,14 +715,10 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs)
     int ret;
     unsigned int inst_len;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-    struct nestedsvm *svm = &vcpu_nestedsvm(v);
 
     inst_len = svm_get_insn_len(v, INSTR_VMRUN);
     if ( inst_len == 0 )
-    {
-        svm->ns_vmexit.exitcode = VMEXIT_SHUTDOWN;
         return -1;
-    }
 
     nv->nv_vmswitch_in_progress = 1;
     ASSERT(nv->nv_vvmcx != NULL);
@@ -743,15 +737,10 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs)
         break;
     case NSVM_ERROR_VVMCB:
         gdprintk(XENLOG_ERR, "inject VMEXIT(INVALID)\n");
-        svm->ns_vmexit.exitcode = VMEXIT_INVALID;
         return -1;
     case NSVM_ERROR_VMENTRY:
     default:
-        gdprintk(XENLOG_ERR,
-            "nsvm_vcpu_vmentry failed, injecting #UD\n");
-        hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
-        /* Must happen after hvm_inject_hw_exception or it doesn't work right. */
-        nv->nv_vmswitch_in_progress = 0;
+        gdprintk(XENLOG_ERR, "nsvm_vcpu_vmentry failed\n");
         return 1;
     }
 
@@ -765,7 +754,7 @@ nsvm_vcpu_vmrun(struct vcpu *v, struct cpu_user_regs *regs)
 
 static int
 nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
-    uint64_t exitcode)
+    uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2)
 {
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
@@ -779,33 +768,44 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
 
     ns_vmcb = nv->nv_vvmcx;
 
-    if ( nv->nv_vmexit_pending )
+    switch ( exitcode )
+    {
+    case VMEXIT_INTR:
     {
-        switch ( exitcode )
+        struct hvm_intack intack = {
+            .source = exitinfo1,
+            .vector = exitinfo2
+        };
+
+        /* See the comment in svm_intr_assist() for why this is necessary */
+        if ( unlikely(vmcb->event_inj.v) ||
+             hvm_interrupt_blocked(v, intack) )
         {
-        case VMEXIT_INTR:
-            if ( unlikely(ns_vmcb->event_inj.v) && nv->nv_vmentry_pending &&
-                 hvm_event_needs_reinjection(ns_vmcb->event_inj.type,
-                                             ns_vmcb->event_inj.vector) )
-                ns_vmcb->exit_int_info = ns_vmcb->event_inj;
-            break;
-        case VMEXIT_EXCEPTION_PF:
-            ns_vmcb->_cr2 = ns_vmcb->ei.exc.cr2;
-            fallthrough;
-        case VMEXIT_NPF:
-            ns_vmcb->exitinfo2 = svm->ns_vmexit.exitinfo2;
-            fallthrough;
-        case VMEXIT_EXCEPTION_NP:
-        case VMEXIT_EXCEPTION_SS:
-        case VMEXIT_EXCEPTION_GP:
-        case VMEXIT_EXCEPTION_15:
-        case VMEXIT_EXCEPTION_MF:
-        case VMEXIT_EXCEPTION_AC:
-            ns_vmcb->exitinfo1 = svm->ns_vmexit.exitinfo1;
-            break;
-        default:
+            svm_enable_intr_window(v, intack);
             break;
         }
+
+        svm_inject_intr(v, intack.vector);
+        pt_intr_post(v, intack);
+        break;
+    }
+
+    case VMEXIT_EXCEPTION_PF:
+        ns_vmcb->_cr2 = exitinfo2;
+        fallthrough;
+    case VMEXIT_NPF:
+        ns_vmcb->exitinfo2 = exitinfo2;
+        fallthrough;
+    case VMEXIT_EXCEPTION_NP:
+    case VMEXIT_EXCEPTION_SS:
+    case VMEXIT_EXCEPTION_GP:
+    case VMEXIT_EXCEPTION_15:
+    case VMEXIT_EXCEPTION_MF:
+    case VMEXIT_EXCEPTION_AC:
+        ns_vmcb->exitinfo1 = exitinfo1;
+        break;
+    default:
+        break;
     }
 
     ns_vmcb->exitcode = exitcode;
@@ -816,10 +816,16 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
 int cf_check nsvm_vcpu_vmexit_event(
     struct vcpu *v, const struct x86_event *event)
 {
+    enum nestedhvm_vmexits ret;
+
     ASSERT(vcpu_nestedhvm(v).nv_vvmcx != NULL);
 
-    nestedsvm_vmexit_defer(v, VMEXIT_EXCEPTION_DE + event->vector,
-                           event->error_code, event->data);
+    ret = nestedsvm_vcpu_vmexit(v, guest_cpu_user_regs(),
+                                VMEXIT_EXCEPTION_DE + event->vector,
+                                event->error_code, event->data);
+    if ( ret == NESTEDHVM_VMEXIT_FATALERROR )
+        domain_crash(v->domain);
+
     return NESTEDHVM_VMEXIT_DONE;
 }
 
@@ -1192,7 +1198,7 @@ enum hvm_intblk cf_check nsvm_intr_blocked(struct vcpu *v)
         if ( v->io.req.state != STATE_IOREQ_NONE )
             return hvm_intblk_shadow;
 
-        if ( !nv->nv_vmexit_pending && n2vmcb->exit_int_info.v )
+        if ( n2vmcb->exit_int_info.v )
         {
             /* Give the l2 guest a chance to finish the delivery of
              * the last injected interrupt or exception before we
@@ -1201,43 +1207,15 @@ enum hvm_intblk cf_check nsvm_intr_blocked(struct vcpu *v)
             return hvm_intblk_shadow;
         }
     }
-
-    if ( nv->nv_vmexit_pending )
-        /* hvm_inject_hw_exception() must have run before.
-         * exceptions have higher priority than interrupts.
-         */
-        return hvm_intblk_rflags_ie;
-
     return hvm_intblk_none;
 }
 
-/* VMEXIT emulation */
-void
-nestedsvm_vmexit_defer(struct vcpu *v,
-    uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2)
-{
-    struct nestedsvm *svm = &vcpu_nestedsvm(v);
-    struct nestedvcpu *nv = &vcpu_nestedhvm(v);
-    struct vmcb_struct *n1vmcb = nv->nv_n1vmcx;
-
-    if ( n1vmcb->_vintr.fields.vgif_enable )
-        n1vmcb->_vintr.fields.vgif = 0;
-    else
-        svm->ns_gif = 0;
-
-    svm->ns_vmexit.exitcode = exitcode;
-    svm->ns_vmexit.exitinfo1 = exitinfo1;
-    svm->ns_vmexit.exitinfo2 = exitinfo2;
-    vcpu_nestedhvm(v).nv_vmexit_pending = 1;
-}
-
 enum nestedhvm_vmexits
 nestedsvm_check_intercepts(struct vcpu *v, struct cpu_user_regs *regs,
     uint64_t exitcode)
 {
     bool is_intercepted;
 
-    ASSERT(vcpu_nestedhvm(v).nv_vmexit_pending == 0);
     is_intercepted = nsvm_vmcb_guest_intercepts_exitcode(v, regs, exitcode);
 
     /* 
@@ -1326,11 +1304,12 @@ nestedsvm_vmexit_n2n1(struct vcpu *v, struct cpu_user_regs *regs)
 /* The exitcode is in native SVM/VMX format. The forced exitcode
  * is in generic format.
  */
-static enum nestedhvm_vmexits
+enum nestedhvm_vmexits
 nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
-    uint64_t exitcode)
+    uint64_t exitcode, uint64_t exitinfo1, uint64_t exitinfo2)
 {
     int rc;
+    enum nestedhvm_vmexits ret = NESTEDHVM_VMEXIT_DONE;
     struct nestedvcpu *nv = &vcpu_nestedhvm(v);
 
     nv->nv_vmswitch_in_progress = 1;
@@ -1342,14 +1321,12 @@ nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
      */
     if ( nestedhvm_vcpu_in_guestmode(v) )
     {
-        enum nestedhvm_vmexits ret;
-
         ret = nestedsvm_vmexit_n2n1(v, regs);
         switch ( ret )
         {
         case NESTEDHVM_VMEXIT_FATALERROR:
             gdprintk(XENLOG_ERR, "VMEXIT: fatal error\n");
-            return ret;
+            goto out;
         case NESTEDHVM_VMEXIT_HOST:
             BUG();
 
@@ -1369,46 +1346,18 @@ nestedsvm_vcpu_vmexit(struct vcpu *v, struct cpu_user_regs *regs,
     /* Prepare for running the l1 guest. Make the actual
      * modifications to the virtual VMCB/VMCS.
      */
-    rc = nsvm_vcpu_vmexit_inject(v, regs, exitcode);
+    rc = nsvm_vcpu_vmexit_inject(v, regs, exitcode, exitinfo1, exitinfo2);
 
     /* If l1 guest uses shadow paging, update the paging mode. */
     if ( !nestedhvm_paging_mode_hap(v) )
         paging_update_paging_modes(v);
 
-    nv->nv_vmswitch_in_progress = 0;
-
     if ( rc )
-        return NESTEDHVM_VMEXIT_FATALERROR;
-
-    return NESTEDHVM_VMEXIT_DONE;
-}
-
-/* VCPU switch */
-void asmlinkage nsvm_vcpu_switch(void)
-{
-    struct cpu_user_regs *regs = guest_cpu_user_regs();
-    struct vcpu *v = current;
-    struct nestedvcpu *nv;
-    struct nestedsvm *svm;
-
-    if ( !nestedhvm_enabled(v->domain) )
-        return;
-
-    nv = &vcpu_nestedhvm(v);
-    svm = &vcpu_nestedsvm(v);
-    ASSERT(v->arch.hvm.svm.vmcb != NULL);
-    ASSERT(nv->nv_n1vmcx != NULL);
-    ASSERT(nv->nv_n2vmcx != NULL);
-    ASSERT(nv->nv_n1vmcx_pa != INVALID_PADDR);
-    ASSERT(nv->nv_n2vmcx_pa != INVALID_PADDR);
+        ret = NESTEDHVM_VMEXIT_FATALERROR;
 
-    if ( nv->nv_vmexit_pending )
-    {
-    vmexit:
-        nestedsvm_vcpu_vmexit(v, regs, svm->ns_vmexit.exitcode);
-        nv->nv_vmexit_pending = 0;
-        return;
-    }
+out:
+    nv->nv_vmswitch_in_progress = 0;
+    return ret;
 }
 
 /* Interrupts, Virtual GIF */
@@ -1451,7 +1400,10 @@ nestedsvm_vcpu_interrupt(struct vcpu *v, const struct hvm_intack intack)
                                      guest_cpu_user_regs(), exitcode);
     if ( ret )
     {
-        nestedsvm_vmexit_defer(v, exitcode, intack.source, exitinfo2);
+        ret = nestedsvm_vcpu_vmexit(v, guest_cpu_user_regs(), exitcode,
+                                    intack.source, exitinfo2);
+        if ( ret == NESTEDHVM_VMEXIT_FATALERROR )
+            domain_crash(v->domain);
         return NSVM_INTR_FORCEVMEXIT;
     }
 
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index cb09e924dcd3..d955077a484a 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1626,7 +1626,9 @@ static void svm_do_nested_pgfault(struct vcpu *v,
     case -1:
         ASSERT(nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v));
         /* inject #VMEXIT(NPF) into guest. */
-        nestedsvm_vmexit_defer(v, VMEXIT_NPF, pfec, gpa);
+        if ( nestedsvm_vcpu_vmexit(v, regs, VMEXIT_NPF, pfec, gpa) ==
+             NESTEDHVM_VMEXIT_FATALERROR )
+            domain_crash(v->domain);
         return;
     }
 
@@ -2529,8 +2531,10 @@ void asmlinkage svm_vmexit_handler(void)
             switch ( nsret )
             {
             case NESTEDHVM_VMEXIT_DONE:
-                /* defer VMEXIT injection */
-                nestedsvm_vmexit_defer(v, exit_reason, exitinfo1, exitinfo2);
+                nsret = nestedsvm_vcpu_vmexit(v, regs, exit_reason,
+                                              exitinfo1, exitinfo2);
+                if ( nsret == NESTEDHVM_VMEXIT_FATALERROR )
+                    domain_crash(v->domain);
                 goto out;
             case NESTEDHVM_VMEXIT_FATALERROR:
                 gdprintk(XENLOG_ERR, "unexpected nestedsvm_vmexit() error\n");
diff --git a/xen/arch/x86/include/asm/hvm/svm-types.h b/xen/arch/x86/include/asm/hvm/svm-types.h
index beab9a3af203..256bf5b91402 100644
--- a/xen/arch/x86/include/asm/hvm/svm-types.h
+++ b/xen/arch/x86/include/asm/hvm/svm-types.h
@@ -69,13 +69,6 @@ struct nestedsvm {
     bool ns_gif;
     bool ns_hap_enabled;
 
-    /* Only meaningful when vmexit_pending flag is set */
-    struct {
-        uint64_t exitcode;  /* native exitcode to inject into l1 guest */
-        uint64_t exitinfo1; /* additional information to the exitcode */
-        uint64_t exitinfo2; /* additional information to the exitcode */
-    } ns_vmexit;
-
     union {
         uint32_t bytes;
         struct {
-- 
2.34.1

Re: [PATCH v2 05/13] tools/libxl: Add support for Viridian stimer direct mode

[Jan Beulich]

On 10.10.2026 12:13, chunjie.zhu@citrix.com wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> Add support to libxl for using the stimer enlightenments in direct mode.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  docs/man/xl.cfg.5.pod.in         | 5 +++++
>  tools/include/libxl.h            | 6 ++++++
>  tools/libs/light/libxl_types.idl | 1 +
>  tools/libs/light/libxl_x86.c     | 5 +++++
>  4 files changed, 17 insertions(+)

Was it an accident that this patch was included in this series? You didn't
even Cc the toolstack maintainer. And your S-o-b is also missing.

Jan

Re: [PATCH v2 06/13] x86/hvm: Clamp Viridian features to known set

[Jan Beulich]

On 10.10.2026 12:13, chunjie.zhu@citrix.com wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> Instead of returning EINVAL on an unknown feature bit, clamp to the
> known feature set. This simplifies upgrades since when the hypervisor
> and toolstack are updated without an immediate reboot, the toolstack may
> set unknown feature bits and the existing behaviour leaves the VM
> without any Virdian features enabled.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Before submitting this v2, did you check prior communication? As per [1]
this patch was withdrawn.

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2026-09/msg01511.html

Re: [PATCH v2 08/13] nestedsvm: Don't set VMCB(1-2)'s NP_ENABLE and N_CR3 during VMEXIT to L1

[Jan Beulich]

On 10.10.2026 12:13, chunjie.zhu@citrix.com wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> As per the VMRUN pseudocode in APM Vol 3 3.38, the VMCB's NP_ENABLE and
> N_CR3 fields are not set during a VMEXIT so don't do this when updating
> VMCB(1-2). At the same time, cleanup the somewhat bogus and irrelevant
> comments. Not clearing N_CR3 does not introduce a security hole as
> stated since L1 can set it regardless and it is never used directly when
> running L2.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Where did Teddy's R-b go? 

Re: [PATCH v2 00/13] x86/nestedsvm: rework nested VMRUN/VMEXIT handling

[Jan Beulich]

On 10.10.2026 12:13, chunjie.zhu@citrix.com wrote:
> From: Chunjie Zhu <chunjie.zhu@citrix.com>
> 
> This series simplifies nested SVM execution by removing pending and
> deferred state from the VMRUN and VMEXIT paths, and includes the nested
> SVM / Viridian fixes the rework depends on.

After having spotted the 3rd questionable patch in this series now (see
below) I have to ask: What is this series aiming at? Mixing random
changes and adding confusion by improper versioning, due to inclusion
of patches which were previously submitted standalone, doesn't help.

> Stephen Cheng (1):
>   x86/svm: require VMSAVEvirt for nested virt

This patch was at v3 already. Now you're moving it back to v2.

Before I go look further, may I please ask that the state of things be
properly clarified here? And - meanwhile I have to assume almost the
entire series is affected by this - where's your own S-o-b?

Jan

Re: [PATCH v2 01/13] nestedsvm: Fix CR3 MBZ check

[Jan Beulich]

On 10.10.2026 12:13, chunjie.zhu@citrix.com wrote:
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -354,10 +354,8 @@ bool svm_vmcb_isvalid(
>          PRINTF("CR0: bits [63:32] are not zero (%#"PRIx64")\n", cr0);
>  
>      if ( (cr0 & X86_CR0_PG) &&
> -         ((cr3 & 7) ||
> -          ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) && (cr3 & 0xfe0)) ||
> -          ((efer & EFER_LMA) &&
> -           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr))) )
> +         ((efer & EFER_LMA) &&
> +           (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr)) )
>          PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
>  
>      valid = hvm_cr4_guest_valid_bits(v->domain);

While Ross replied to Andrew's question [1], imo that should have led to
commentary being added here.

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2026-05/msg01369.html

Re: [PATCH v2 02/13] nestedsvm: Adjust L2's DR intercept when adjusting L1

[Jan Beulich]

On 10.10.2026 12:13, chunjie.zhu@citrix.com wrote:
> From: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> If L2 accesses a debug register (like reading DR7) without L1 intercepting
> it, it locks up the vCPU. L0 intercepts VMEXIT_DR7_READ, which disables
> the intercept for L1 and then restarts L2 which re-executes the
> instruction and then this repeats indefinitely.
> 
> Disable the intercept for the current VMCB if in guest mode to reflect
> what would happen if the VMCB were recreated via
> nsvm_vmcb_prepare4vmrun().
> 
> Fixes: a59a7be91b61 ("nestedsvm: fix DRn handling")
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Andrew's comment [1] was left unaddressed. Comments on earlier versions
should be dealt with, one way or another, before re-submitting.

Jan

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1657,6 +1657,8 @@ static void svm_dr_access(struct vcpu *v, struct cpu_user_regs *regs)
>  
>      TRACE(TRC_HVM_DR_WRITE);
>      __restore_debug_registers(vmcb, v);
> +    if ( nestedhvm_enabled(v->domain) && nestedhvm_vcpu_in_guestmode(v) )
> +        vmcb_set_dr_intercepts(v->arch.hvm.svm.vmcb, 0);
>  }
>  
>  static int cf_check svm_msr_read_intercept(