[PATCH V1 0/8] x86/nestedvmx: Fix nested VMX bugs

[PATCH V1 0/8] x86/nestedvmx: Fix nested VMX bugs

[chunjie.zhu]

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

This series fixes several bugs found while running nested VMX guests
(L1 Xen running an L2 guest) on top of Xen:

 - Patch 1: stop leaking L0's widened access bits into the EPT violation
   exit qualification reflected to L1.
 - Patch 2: implement the missing nhvm_hap_walk_L1_p2m2 hook on VMX; the
   NULL hook causes a #UD and host panic when L0 emulates an L2 access.
 - Patch 3: emulate L2 accesses that hit the APIC-access dummy page
   against L1's vLAPIC instead of mapping the page directly.
 - Patch 4: reflect EPT holes hit during L2 emulation to L1 as EPT
   violations rather than injecting a bogus #PF into L2.
 - Patch 5: load L2's EFER on VM entry and restore L1's on VM exit, so
   L2 neither sees L0's LME nor fails VM entry with invalid guest state.
 - Patch 6: invalidate EPT caches after flushing a nested p2m, to avoid
   L2 walking stale paging-structure caches into freed pages.
 - Patch 7: retry a nested page fault if the np2m changes while L1/L0
   p2m are being walked without the np2m lock held.
 - Patch 8: fix several nested VPID problems.

Chunjie Zhu (8):
  x86/nestedvmx: Preserve hardware access bits in reflected EPT
    violations
  x86/nestedvmx: Implement nhvm_hap_walk_L1_p2m2
  x86/nestedvmx: Emulate L2 accesses to L1's LAPIC page
  x86/nestedvmx: Reflect EPT holes hit during L2 emulation to L1
  x86/nestedvmx: Load L2's EFER on VM entry and restore L1's on VM exit
  x86/p2m: invalidate EPT caches after flushing a nested p2m
  x86/nestedhap: Retry nested page fault if the np2m changes during walk
  x86/vmx: Fix nested VPID handling

 xen/arch/x86/hvm/hvm.c                  |   6 ++
 xen/arch/x86/hvm/vmx/vmx.c              |   5 +-
 xen/arch/x86/hvm/vmx/vvmx.c             | 110 ++++++++++++++++++++++--
 xen/arch/x86/include/asm/hvm/vmx/vvmx.h |  11 ++-
 xen/arch/x86/mm/hap/nested_ept.c        |  19 ++--
 xen/arch/x86/mm/hap/nested_hap.c        |  52 ++++++++++-
 xen/arch/x86/mm/p2m.c                   |  17 +++-
 7 files changed, 200 insertions(+), 20 deletions(-)

-- 
2.34.1

[PATCH V1 1/8] x86/nestedvmx: Preserve hardware access bits in reflected EPT violations

[chunjie.zhu]

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

nept_translate_l2ga() rebuilds the exit qualification handed to L1 from
rwx_acc, which is derived from the npfec set up by ept_handle_violation().
That function deliberately treats every write violation as a read as well
(read_access = q.read || q.write), because the SDM leaves bit 0 undefined
for read-modify-write accesses.

As a result, a plain write by L2 that violates L1's EPT is reflected to L1
with bits 0 and 1 both set, whereas bare metal reports only bit 1 (data
write).  The widened access bits are an L0 implementation detail for its
own permission checks and should not leak into L1's view.

Keep bits 0-2 of the qualification as reported by hardware, and only
replace the GPA permission bits (3-5) with those from L1's EPT entries.
rwx_acc is still used for the permission check itself.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/mm/hap/nested_ept.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/hap/nested_ept.c b/xen/arch/x86/mm/hap/nested_ept.c
index d88d677825f1..e116d8254824 100644
--- a/xen/arch/x86/mm/hap/nested_ept.c
+++ b/xen/arch/x86/mm/hap/nested_ept.c
@@ -255,7 +255,12 @@ int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
         rc = EPT_TRANSLATE_VIOLATION;
     /* Fall through to EPT violation if permission check fails. */
     case EPT_TRANSLATE_VIOLATION:
-        *exit_qual = (*exit_qual & 0xffffffc0) | (rwx_bits << 3) | rwx_acc;
+        /*
+         * Keep the access bits (0-2) as the hardware reported them.  rwx_acc
+         * treats every write as a read too, which is not what L1 would see
+         * on bare metal for a plain write.
+         */
+        *exit_qual = (*exit_qual & 0xffffffc7) | (rwx_bits << 3);
         *exit_reason = EXIT_REASON_EPT_VIOLATION;
         break;
 
-- 
2.34.1

[PATCH V1 2/8] x86/nestedvmx: Implement nhvm_hap_walk_L1_p2m2

[chunjie.zhu]

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

p2m_get_page_from_gfn() on a nested p2m calls
nestedhvm_hap_nested_page_fault2(), which walks L1's paging structures via
hvm_funcs.nhvm_hap_walk_L1_p2m2.  Only SVM provides this hook.  On VMX it
is NULL, so the alternative_call() ends up as a #UD in L0 and the host
panics.

It is reachable from an L2 guest.  An EPT violation on MMIO makes L0
emulate the access, and the emulator walks L2's pagetables through the
nested p2m.

    nhvm_hap_walk_L1_p2m2
    nestedhvm_hap_nested_page_fault2
    p2m_get_page_from_gfn
    hap_p2m_ga_to_gfn_4_levels
    paging_gva_to_gfn
    hvmemul_map_linear_addr
    hvmemul_write
    hvm_hap_nested_page_fault

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/hvm/vmx/vmx.c              |  1 +
 xen/arch/x86/hvm/vmx/vvmx.c             | 39 ++++++++++++++++++++++++-
 xen/arch/x86/include/asm/hvm/vmx/vvmx.h |  7 ++++-
 xen/arch/x86/mm/hap/nested_ept.c        | 10 +++----
 4 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 3c7374a9ff42..ed4ff56e7797 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2892,6 +2892,7 @@ static struct hvm_function_table __initdata_cf_clobber vmx_function_table = {
     .nhvm_domain_relinquish_resources = nvmx_domain_relinquish_resources,
     .update_vlapic_mode = vmx_vlapic_msr_changed,
     .nhvm_hap_walk_L1_p2m = nvmx_hap_walk_L1_p2m,
+    .nhvm_hap_walk_L1_p2m2 = nvmx_hap_walk_L1_p2m2,
 #ifdef CONFIG_VM_EVENT
     .enable_msr_interception = vmx_enable_msr_interception,
 #endif
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 29e48387a110..8d6e0c66f38e 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2355,7 +2355,8 @@ int cf_check nvmx_hap_walk_L1_p2m(
     vmx_vmcs_enter(v);
 
     __vmread(EXIT_QUALIFICATION, &exit_qual);
-    rc = nept_translate_l2ga(v, L2_gpa, page_order, rwx_rights, &gfn, p2m_acc,
+    rc = nept_translate_l2ga(v->domain, nhvm_vcpu_p2m_base(v), L2_gpa,
+                             page_order, rwx_rights, &gfn, p2m_acc,
                              &exit_qual, &exit_reason);
     switch ( rc )
     {
@@ -2383,6 +2384,42 @@ int cf_check nvmx_hap_walk_L1_p2m(
     return rc;
 }
 
+/*
+ * As nvmx_hap_walk_L1_p2m(), for a walk of a specific nested p2m that is not
+ * tied to a VM exit (e.g. L0 emulation reading L2's pagetables).  There is
+ * no EPT violation to reflect to L1, so only the result is reported.
+ */
+int cf_check nvmx_hap_walk_L1_p2m2(
+    struct p2m_domain *nested_p2m, paddr_t L2_gpa, paddr_t *L1_gpa,
+    unsigned int *page_order, uint8_t *p2m_acc, struct npfec npfec)
+{
+    unsigned long gfn;
+    uint64_t exit_qual = 0;
+    uint32_t exit_reason;
+    uint32_t rwx_rights =
+        (npfec.insn_fetch << 2) | (npfec.write_access << 1) | npfec.read_access;
+
+    switch ( nept_translate_l2ga(nested_p2m->domain, nested_p2m->np2m_base,
+                                 L2_gpa, page_order, rwx_rights, &gfn,
+                                 p2m_acc, &exit_qual, &exit_reason) )
+    {
+    case EPT_TRANSLATE_SUCCEED:
+        *L1_gpa = (gfn << PAGE_SHIFT) + (L2_gpa & ~PAGE_MASK);
+        return NESTEDHVM_PAGEFAULT_DONE;
+
+    case EPT_TRANSLATE_VIOLATION:
+    case EPT_TRANSLATE_MISCONFIG:
+        return NESTEDHVM_PAGEFAULT_INJECT;
+
+    case EPT_TRANSLATE_RETRY:
+        return NESTEDHVM_PAGEFAULT_RETRY;
+
+    default:
+        gdprintk(XENLOG_ERR, "GUEST EPT translation error!\n");
+        BUG();
+    }
+}
+
 void nvmx_idtv_handling(void)
 {
     struct vcpu *v = current;
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vvmx.h b/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
index da10d3fa9617..2e0ac5798d5d 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
@@ -89,9 +89,14 @@ bool cf_check nvmx_ept_enabled(struct vcpu *v);
 #define EPT_TRANSLATE_MISCONFIG     2
 #define EPT_TRANSLATE_RETRY         3
 
+struct p2m_domain;
+
 int cf_check nvmx_hap_walk_L1_p2m(
     struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa, unsigned int *page_order,
     uint8_t *p2m_acc, struct npfec npfec);
+int cf_check nvmx_hap_walk_L1_p2m2(
+    struct p2m_domain *nested_p2m, paddr_t L2_gpa, paddr_t *L1_gpa,
+    unsigned int *page_order, uint8_t *p2m_acc, struct npfec npfec);
 
 /*
  * Virtual VMCS layout
@@ -193,7 +198,7 @@ void nvmx_set_cr_read_shadow(struct vcpu *v, unsigned int cr);
 
 uint64_t nept_get_ept_vpid_cap(void);
 
-int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
+int nept_translate_l2ga(struct domain *d, uint64_t eptp_base, paddr_t l2ga,
                         unsigned int *page_order, uint32_t rwx_acc,
                         unsigned long *l1gfn, uint8_t *p2m_acc,
                         uint64_t *exit_qual, uint32_t *exit_reason);
diff --git a/xen/arch/x86/mm/hap/nested_ept.c b/xen/arch/x86/mm/hap/nested_ept.c
index e116d8254824..f8563156b6d0 100644
--- a/xen/arch/x86/mm/hap/nested_ept.c
+++ b/xen/arch/x86/mm/hap/nested_ept.c
@@ -136,13 +136,13 @@ static int ept_lvl_table_offset(unsigned long gpa, int lvl)
 }
 
 static uint32_t
-nept_walk_tables(struct vcpu *v, unsigned long l2ga, ept_walk_t *gw)
+nept_walk_tables(struct domain *d, uint64_t eptp_base, unsigned long l2ga,
+                 ept_walk_t *gw)
 {
     int lvl;
     uint32_t rc = 0, ret = 0, gflags;
-    struct domain *d = v->domain;
     struct p2m_domain *p2m = d->arch.p2m;
-    gfn_t base_gfn = _gfn(nhvm_vcpu_p2m_base(v) >> PAGE_SHIFT);
+    gfn_t base_gfn = _gfn(eptp_base >> PAGE_SHIFT);
     mfn_t lxmfn;
     ept_entry_t *lxp = NULL;
 
@@ -209,7 +209,7 @@ out:
 
 /* Translate a L2 guest address to L1 gpa via L1 EPT paging structure */
 
-int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
+int nept_translate_l2ga(struct domain *d, uint64_t eptp_base, paddr_t l2ga,
                         unsigned int *page_order, uint32_t rwx_acc,
                         unsigned long *l1gfn, uint8_t *p2m_acc,
                         uint64_t *exit_qual, uint32_t *exit_reason)
@@ -220,7 +220,7 @@ int nept_translate_l2ga(struct vcpu *v, paddr_t l2ga,
 
     *l1gfn = gfn_x(INVALID_GFN);
 
-    rc = nept_walk_tables(v, l2ga, &gw);
+    rc = nept_walk_tables(d, eptp_base, l2ga, &gw);
     switch ( rc )
     {
     case EPT_TRANSLATE_SUCCEED:
-- 
2.34.1

[PATCH V1 3/8] x86/nestedvmx: Emulate L2 accesses to L1's LAPIC page

[chunjie.zhu]

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

Return NESTEDHVM_PAGEFAULT_MMIO when the L0 p2m entry is the APIC-access
page, so that the access faults and is emulated against L1's vLAPIC.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/mm/hap/nested_hap.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index 42d588f08821..72706cebaedf 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -17,6 +17,7 @@
 #include <asm/hap.h>
 
 #include <asm/hvm/nestedhvm.h>
+#include <asm/hvm/vmx/vmcs.h>
 
 #include "private.h"
 
@@ -115,6 +116,17 @@ static int nestedhap_walk_L0_p2m(
     mfn = get_gfn_type_access(p2m, L1_gpa >> PAGE_SHIFT, p2mt, p2ma,
                               0, page_order);
 
+    /*
+     * With APIC-access virtualisation, Xen backs the LAPIC page of L1 with a
+     * dummy page.  L2 must not touch it directly: the access is to L1's
+     * emulated LAPIC, so let the caller emulate it.
+     */
+    rc = NESTEDHVM_PAGEFAULT_MMIO;
+    if ( *p2mt == p2m_mmio_direct && cpu_has_vmx &&
+         !mfn_eq(p2m->domain->arch.hvm.vmx.apic_access_mfn, _mfn(0)) &&
+         mfn_eq(mfn, p2m->domain->arch.hvm.vmx.apic_access_mfn) )
+        goto out;
+
     rc = NESTEDHVM_PAGEFAULT_DIRECT_MMIO;
     if ( *p2mt == p2m_mmio_direct )
         goto direct_mmio_out;
-- 
2.34.1

[PATCH V1 4/8] x86/nestedvmx: Reflect EPT holes hit during L2 emulation to L1

[chunjie.zhu]

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

When L0 emulates an L2 instruction, e.g. a `movs` from MMIO, it
translates the other operand with paging_gva_to_gfn().  For an n2 vCPU
this walks L2's pagetables and then L1's EPT.  If L1's EPT has no
mapping for the resulting L2 GPA, the walk returns
NESTEDHVM_PAGEFAULT_INJECT and paging_gva_to_gfn() returns INVALID_GFN.
The emulator then injects #PF into L2, although L2's own pagetables map
the address.  L2 is blamed for a fault that is L1's to handle.

Reflect the hole to L1 as an EPT violation instead:

 * paging_gva_to_gfn() calls nvmx_reflect_emul_ept_violation() on
   INJECT.  If the current L2 exit is an EPT violation, it records the
   missing GPA and sets nv_vmexit_pending.
 * sync_vvmcs_ro() reports that GPA to L1 as the guest-physical address
   of the violation, together with the exit qualification computed by
   the L1 EPT walk.
 * hvm_inject_event() drops events for L2 while a virtual vmexit to L1
   is pending, so the emulator's #PF does not stay in vmcs02.

L1 can then fix its EPT and resume L2, which re-executes the
instruction.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
Assisted-by: Claude:claude-sonnet-5
---
 xen/arch/x86/hvm/hvm.c                  |  6 +++++
 xen/arch/x86/hvm/vmx/vvmx.c             | 30 +++++++++++++++++++++++++
 xen/arch/x86/include/asm/hvm/vmx/vvmx.h |  4 ++++
 xen/arch/x86/mm/p2m.c                   |  6 ++++-
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ef9c7cd05777..b38f8dd68733 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1801,6 +1801,12 @@ void hvm_inject_event(const struct x86_event *event)
         }
     }
 
+    /* L2 is about to exit to L1; an event for L2 would only be lost. */
+    if ( nestedhvm_enabled(curr->domain) &&
+         nestedhvm_vcpu_in_guestmode(curr) &&
+         vcpu_nestedhvm(curr).nv_vmexit_pending )
+        return;
+
     alternative_vcall(hvm_funcs.inject_event, event);
 }
 
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 8d6e0c66f38e..dc8bcbeea033 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1290,7 +1290,12 @@ static void sync_vvmcs_ro(struct vcpu *v)
     {
         set_vvmcs(v, EXIT_QUALIFICATION, nvmx->ept.exit_qual);
         set_vvmcs(v, VM_EXIT_REASON, nvmx->ept.exit_reason);
+
+        if ( nvmx->ept.gpa_valid )
+            set_vvmcs(v, GUEST_PHYSICAL_ADDRESS, nvmx->ept.gpa);
     }
+
+    nvmx->ept.gpa_valid = false;
 }
 
 static void load_vvmcs_host_state(struct vcpu *v)
@@ -2384,6 +2389,31 @@ int cf_check nvmx_hap_walk_L1_p2m(
     return rc;
 }
 
+/*
+ * L0 is emulating an L2 instruction triggered by an EPT violation, and L1's
+ * EPT has a hole for another address the instruction accesses.  Report that
+ * hole to L1 as the EPT violation, instead of blaming L2 with a #PF.  Called
+ * after nvmx_hap_walk_L1_p2m() returned NESTEDHVM_PAGEFAULT_INJECT.
+ */
+bool nvmx_reflect_emul_ept_violation(struct vcpu *v, paddr_t L2_gpa)
+{
+    struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
+    unsigned long reason;
+
+    if ( v != current || !nestedhvm_vcpu_in_guestmode(v) )
+        return false;
+
+    __vmread(VM_EXIT_REASON, &reason);
+    if ( (uint16_t)reason != EXIT_REASON_EPT_VIOLATION )
+        return false;
+
+    nvmx->ept.gpa = L2_gpa;
+    nvmx->ept.gpa_valid = true;
+    vcpu_nestedhvm(v).nv_vmexit_pending = 1;
+
+    return true;
+}
+
 /*
  * As nvmx_hap_walk_L1_p2m(), for a walk of a specific nested p2m that is not
  * tied to a VM exit (e.g. L0 emulation reading L2's pagetables).  There is
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vvmx.h b/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
index 2e0ac5798d5d..92c8a8144527 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vvmx.h
@@ -38,6 +38,9 @@ struct nestedvmx {
         bool     enabled;
         uint32_t exit_reason;
         uint32_t exit_qual;
+        /* Violation raised by L0's emulation, not by the L2 access itself */
+        bool     gpa_valid;
+        paddr_t  gpa;
     } ept;
     uint32_t guest_vpid;
     struct list_head launched_list;
@@ -94,6 +97,7 @@ struct p2m_domain;
 int cf_check nvmx_hap_walk_L1_p2m(
     struct vcpu *v, paddr_t L2_gpa, paddr_t *L1_gpa, unsigned int *page_order,
     uint8_t *p2m_acc, struct npfec npfec);
+bool nvmx_reflect_emul_ept_violation(struct vcpu *v, paddr_t L2_gpa);
 int cf_check nvmx_hap_walk_L1_p2m2(
     struct p2m_domain *nested_p2m, paddr_t L2_gpa, paddr_t *L1_gpa,
     unsigned int *page_order, uint8_t *p2m_acc, struct npfec npfec);
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 61b2d3fbd4df..ae13d36fdab7 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -25,6 +25,8 @@
 #include <asm/p2m.h>
 #include <asm/mem_sharing.h>
 #include <asm/hvm/nestedhvm.h>
+#include <asm/hvm/vmx/vmcs.h>
+#include <asm/hvm/vmx/vvmx.h>
 #include <asm/altp2m.h>
 #include <asm/vm_event.h>
 #include <xsm/xsm.h>
@@ -1675,7 +1677,7 @@ unsigned long paging_gva_to_gfn(struct vcpu *v,
             .write_access = *pfec & PFEC_write_access,
             .insn_fetch   = *pfec & PFEC_insn_fetch,
         };
-        int rv;
+        int rv = NESTEDHVM_PAGEFAULT_DONE;
         uint64_t base, gen;
 
         /* translate l2 guest va into l2 guest gfn */
@@ -1700,6 +1702,8 @@ unsigned long paging_gva_to_gfn(struct vcpu *v,
         {
             if ( base != p2m->np2m_base || gen != p2m->np2m_generation )
                 goto again;
+            if ( rv == NESTEDHVM_PAGEFAULT_INJECT && cpu_has_vmx )
+                nvmx_reflect_emul_ept_violation(v, pfn_to_paddr(l2_gfn));
             return gfn_x(INVALID_GFN);
         }
 
-- 
2.34.1

[PATCH V1 5/8] x86/nestedvmx: Load L2's EFER on VM entry and restore L1's on VM exit

[chunjie.zhu]

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

nvmx_update_entry_control() strips VM_ENTRY_LOAD_GUEST_EFER from the vmcs02
entry controls, and virtual_vmentry() only derives EFER.LMA/LME from
VM_ENTRY_IA32E_MODE.  As a result:

 * While L2 runs, hardware EFER is L0's own value (LME set), and since
   vmx_update_guest_efer() clears the RDMSR intercept when guest_efer matches
   the value it believes is loaded, L2's RDMSR of EFER is passed through and
   returns LME=1 even though L2 never set it.

 * L0's v->arch.hvm.guest_efer has LME cleared for a non-64bit L2, so L2 never
   sets LME in L0's view

L2 Linux AP trampolines read EFER and skip the WRMSR if LME is already set.
L0 therefore never sees LME, and when L2 enables CR0.PG it is treated as
legacy PAE paging.  PDPTEs are loaded from what is really a PML4, and VM
entry fails:

  vmentry failure (reason 0x80000021): Invalid guest state (2)
  PDPTE0 = 0x0000000023604063  EFER(VMCS) = 0

which crashes the L1 domain ("Unhandled nested vmexit: reason 2147483681").

Fix this by:

 * Forcing VM_ENTRY_LOAD_GUEST_EFER in vmcs02 (when the CPU supports it), so
   hardware EFER, and hence passthrough RDMSR, matches guest_efer.

 * In virtual_vmentry(), taking guest_efer from vmcs12 GUEST_EFER when L1 set
   VM_ENTRY_LOAD_GUEST_EFER, preserving LME for an unpaged L2, with LMA still
   following IA32E_MODE.  Then push it to hardware with
   hvm_update_guest_efer().

 * In virtual_vmexit(), restoring guest_efer from vmcs01 GUEST_EFER so that
   L2's SCE/NXE/LME bits don't leak into L1.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
Assisted-by: Claude:claude-sonnet-5
---
 xen/arch/x86/hvm/vmx/vvmx.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index dc8bcbeea033..d1bfc1f23612 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -695,6 +695,12 @@ static void nvmx_update_entry_control(struct vcpu *v)
     shadow_cntrl &= ~(VM_ENTRY_LOAD_GUEST_PAT
                       | VM_ENTRY_LOAD_GUEST_EFER
                       | VM_ENTRY_LOAD_PERF_GLOBAL_CTRL);
+    /*
+     * Always load L2's EFER from v->arch.hvm.guest_efer, so hardware (and
+     * passthrough RDMSR) agrees with it rather than showing L0's own EFER.
+     */
+    if ( cpu_has_vmx_efer )
+        shadow_cntrl |= VM_ENTRY_LOAD_GUEST_EFER;
     __vmwrite(VM_ENTRY_CONTROLS, shadow_cntrl);
 }
 
@@ -1220,13 +1226,24 @@ static void virtual_vmentry(struct cpu_user_regs *regs)
     lm_l1 = hvm_long_mode_active(v);
     lm_l2 = !!(get_vvmcs(v, VM_ENTRY_CONTROLS) & VM_ENTRY_IA32E_MODE);
 
-    if ( lm_l2 )
-        v->arch.hvm.guest_efer |= EFER_LMA | EFER_LME;
+    if ( get_vvmcs(v, VM_ENTRY_CONTROLS) & VM_ENTRY_LOAD_GUEST_EFER )
+    {
+        /*
+         * L2 may have EFER.LME set while still unpaged (LMA clear), e.g. an
+         * AP trampoline which skips the WRMSR because it reads LME already
+         * set.  Keep it, or enabling paging later is treated as legacy PAE.
+         */
+        v->arch.hvm.guest_efer = get_vvmcs(v, GUEST_EFER) & ~EFER_LMA;
+    }
     else
         v->arch.hvm.guest_efer &= ~(EFER_LMA | EFER_LME);
 
+    if ( lm_l2 )
+        v->arch.hvm.guest_efer |= EFER_LMA | EFER_LME;
+
     load_shadow_control(v);
     load_shadow_guest_state(v);
+    hvm_update_guest_efer(v);
 
     if ( lm_l1 != lm_l2 )
         paging_update_paging_modes(v);
@@ -1452,6 +1469,15 @@ static void virtual_vmexit(struct cpu_user_regs *regs)
     lm_l2 = hvm_long_mode_active(v);
     lm_l1 = !!(get_vvmcs(v, VM_EXIT_CONTROLS) & VM_EXIT_IA32E_MODE);
 
+    /* Don't leak L2's EFER (SCE, NXE, LME) into L1. */
+    if ( cpu_has_vmx_efer )
+    {
+        unsigned long efer;
+
+        __vmread(GUEST_EFER, &efer);
+        v->arch.hvm.guest_efer = efer;
+    }
+
     if ( lm_l1 )
         v->arch.hvm.guest_efer |= EFER_LMA | EFER_LME;
     else
-- 
2.34.1

[PATCH V1 6/8] x86/p2m: invalidate EPT caches after flushing a nested p2m

[chunjie.zhu]

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

p2m_flush_table_locked() keeps the np2m root page, so the EP4TA is
unchanged, clears it, and returns the remaining page-table pages to the
paging pool. The only invalidation it triggers is
nestedhvm_vmcx_flushtlb(), whose IPI handler does hvm_asid_flush_core()
and drops the cached nv_p2m.

That only flushes VPID/ASID-tagged translations. EPT paging-structure
caches and guest-physical mappings are tagged by EP4TA and survive.
After the flush they can still point at the freed np2m pages.

vmx_vmenter_helper() issues INVEPT only if the CPU is set in
hostp2m->ept.invalidate. ept_sync_domain_prepare() sets that mask only
when an already-present host p2m entry changes. Nested flushes (L1
INVEPT, np2m LRU recycle, p2m_flush()) never set it. Filling non-present
np2m entries afterwards doesn't set it either.

Once those freed pages are reused, or scrubbed (0xc2 pattern on debug
builds), L2 walks through stale cached pointers and can fetch or read
garbage. In practice this showed up as L2 guests executing 0xc2c2c2c2.

Mark every CPU in the host p2m's ept.invalidate mask at the end of
p2m_flush_table_locked(), so each CPU does an INVEPT on its next VM
entry. The cost is paid only when a flush actually happens.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/mm/p2m.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ae13d36fdab7..fbaa69d4f43f 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1446,6 +1446,17 @@ p2m_flush_table_locked(struct p2m_domain *p2m)
             d->arch.paging.free_page(d, pg);
     }
     page_list_add(top, &p2m->pages);
+
+    /*
+     * The root page is kept (so the EP4TA is unchanged) and the other tables
+     * are freed for reuse, but INVVPID/ASID flushes don't cover the EPT
+     * paging-structure caches or guest-physical mappings, which are tagged by
+     * EP4TA.  Without an INVEPT, a walk can follow a cached pointer into a
+     * freed (possibly reused or scrubbed) table.  vmx_vmenter_helper() does
+     * the INVEPT on every pCPU's next VM entry.
+     */
+    if ( cpu_has_vmx && nestedhvm_enabled(d) )
+        cpumask_setall(p2m_get_hostp2m(d)->ept.invalidate);
 }
 
 /* Reset this p2m table to be empty */
-- 
2.34.1

[PATCH V1 7/8] x86/nestedhap: Retry nested page fault if the np2m changes during walk

[chunjie.zhu]

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

nestedhvm_hap_nested_page_fault() walks the L1 and L0 p2m tables without
holding the nested p2m lock, and only afterwards looks up the np2m with
p2m_get_nestedp2m_locked() to install the result.  If L1 changes its
EPT/NPT base or the np2m is flushed or reassigned during the walk
(np2m_base / np2m_generation change), the translation computed from
the old tables may be installed into a np2m that now represents a
different L2 address space, leaving a stale or wrong mapping.

Snapshot np2m_base and np2m_generation before the walk, and re-check
them after each walk step (L1 and L0) via the new nestedhap_np2m_changed()
helper.  Re-check them again, together with the np2m identity, once the
np2m lock is retaken for nestedhap_fix_p2m(). On any mismatch, drop
the lock and retry the whole fault from the start.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/mm/hap/nested_hap.c | 40 +++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/hap/nested_hap.c b/xen/arch/x86/mm/hap/nested_hap.c
index 72706cebaedf..1f09c706dda5 100644
--- a/xen/arch/x86/mm/hap/nested_hap.c
+++ b/xen/arch/x86/mm/hap/nested_hap.c
@@ -101,6 +101,19 @@ nestedhap_fix_p2m(struct vcpu *v, struct p2m_domain *p2m,
     }
 }
 
+static bool nestedhap_np2m_changed(struct p2m_domain *p2m,
+                                   uint64_t base, uint64_t generation)
+{
+    bool changed;
+
+    p2m_lock(p2m);
+    changed = p2m->np2m_base != base ||
+              p2m->np2m_generation != generation;
+    p2m_unlock(p2m);
+
+    return changed;
+}
+
 /* This function uses L1_gpa to walk the P2M table in L0 hypervisor. If the
  * walk is successful, the translated value is returned in L0_gpa. The return 
  * value tells the upper level what to do.
@@ -176,16 +189,26 @@ int nestedhvm_hap_nested_page_fault(
     p2m_type_t p2mt_10;
     p2m_access_t p2ma_10 = p2m_access_rwx;
     uint8_t p2ma_21 = p2m_access_rwx;
+    uint64_t np2m_base, np2m_generation;
 
     if ( *L2_gpa >> v->domain->arch.cpuid->extd.maxphysaddr )
         return NESTEDHVM_PAGEFAULT_INJECT;
 
     p2m = p2m_get_hostp2m(d); /* L0 p2m */
 
+ retry:
+    nested_p2m = p2m_get_nestedp2m_locked(v);
+    np2m_base = nested_p2m->np2m_base;
+    np2m_generation = nested_p2m->np2m_generation;
+    p2m_unlock(nested_p2m);
+
     /* walk the L1 P2M table */
     rv = nhvm_hap_walk_L1_p2m(v, *L2_gpa, &L1_gpa, &page_order_21, &p2ma_21,
                               npfec);
 
+    if ( nestedhap_np2m_changed(nested_p2m, np2m_base, np2m_generation) )
+        goto retry;
+
     /* let caller to handle these two cases */
     switch (rv) {
     case NESTEDHVM_PAGEFAULT_INJECT:
@@ -203,6 +226,9 @@ int nestedhvm_hap_nested_page_fault(
     rv = nestedhap_walk_L0_p2m(p2m, L1_gpa, &L0_gpa, &p2mt_10, &p2ma_10,
                                &page_order_10, npfec);
 
+    if ( nestedhap_np2m_changed(nested_p2m, np2m_base, np2m_generation) )
+        goto retry;
+
     /* let upper level caller to handle these two cases */
     switch (rv) {
     case NESTEDHVM_PAGEFAULT_INJECT:
@@ -249,7 +275,19 @@ int nestedhvm_hap_nested_page_fault(
     p2ma_10 &= (p2m_access_t)p2ma_21;
 
     /* fix p2m_get_pagetable(nested_p2m) */
-    nested_p2m = p2m_get_nestedp2m_locked(v);
+    {
+        struct p2m_domain *current_p2m = p2m_get_nestedp2m_locked(v);
+
+        if ( current_p2m != nested_p2m ||
+             current_p2m->np2m_base != np2m_base ||
+             current_p2m->np2m_generation != np2m_generation )
+        {
+            p2m_unlock(current_p2m);
+            goto retry;
+        }
+
+        nested_p2m = current_p2m;
+    }
     nestedhap_fix_p2m(v, nested_p2m, *L2_gpa, L0_gpa, page_order_20,
         p2mt_10, p2ma_10);
     p2m_unlock(nested_p2m);
-- 
2.34.1

[PATCH V1 8/8] x86/vmx: Fix nested VPID handling

[chunjie.zhu]

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

Some nested VPID problems:

* nept_get_ept_vpid_cap() ORed NVPID_CAP_BITS into the low half of
  IA32_VMX_EPT_VPID_CAP.  The VPID capability bits (INVVPID support
  and types) live in bits 32 and up, so the low-half write advertised
  unrelated EPT capabilities to L1 and never the intended INVVPID ones.
  Shift them into place.

* virtual_vmentry() only tracked the L2 VPID when L1 enabled VPID.
  When L1 does not use VPID, all of L2's linear mappings are tagged
  with VPID 0 and must be invalidated on every virtual VM entry, so
  flush the n2asid in that case as well.

* vmx_vmenter_helper() jumped over the whole ASID/EPT invalidation
  block when the CPU lacks VPID, which also skipped the EPT
  invalidation that has nothing to do with VPID.  Skip only the VPID
  part.

* Require VPID hardware support for nested virt.  Without VPID, every
  virtual VM entry/exit flushes the whole TLB, which makes nested VMX
  too slow to be useful.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
---
 xen/arch/x86/hvm/vmx/vmx.c       |  4 ++--
 xen/arch/x86/hvm/vmx/vvmx.c      | 11 ++++++-----
 xen/arch/x86/mm/hap/nested_ept.c |  2 +-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index ed4ff56e7797..8b06a462602d 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4943,7 +4943,7 @@ bool asmlinkage vmx_vmenter_helper(const struct cpu_user_regs *regs)
         vmx_pi_do_resume(curr);
 
     if ( !cpu_has_vmx_vpid )
-        goto out;
+        goto skip_vpid;
     if ( nestedhvm_vcpu_in_guestmode(curr) )
         p_asid = &vcpu_nestedhvm(curr).nv_n2asid;
     else
@@ -4975,6 +4975,7 @@ bool asmlinkage vmx_vmenter_helper(const struct cpu_user_regs *regs)
     if ( unlikely(need_flush) )
         vpid_sync_all();
 
+ skip_vpid:
     if ( !paging_mode_shadow(curr->domain) )
     {
         struct ept_data *ept = &p2m_get_hostp2m(currd)->ept;
@@ -5017,7 +5018,6 @@ bool asmlinkage vmx_vmenter_helper(const struct cpu_user_regs *regs)
                      inv == 1 ? single->eptp          : 0);
     }
 
- out:
     if ( unlikely(curr->arch.hvm.vmx.lbr_flags & LBR_FIXUP_MASK) )
         lbr_fixup();
 
diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index d1bfc1f23612..6c2629f20742 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1264,12 +1264,14 @@ static void virtual_vmentry(struct cpu_user_regs *regs)
         __vmwrite(EPT_POINTER, get_host_eptp(v));
 
     /* nested VPID support! */
-    if ( cpu_has_vmx_vpid && nvmx_vpid_enabled(v) )
+    if ( cpu_has_vmx_vpid )
     {
         struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
-        uint32_t new_vpid = get_vvmcs(v, VIRTUAL_PROCESSOR_ID);
+        uint32_t new_vpid = nvmx_vpid_enabled(v) ?
+                            get_vvmcs(v, VIRTUAL_PROCESSOR_ID) : 0;
 
-        if ( nvmx->guest_vpid != new_vpid )
+        /* Without VPID, every virtual VM entry invalidates linear mappings. */
+        if ( !new_vpid || nvmx->guest_vpid != new_vpid )
         {
             hvm_asid_flush_vcpu_asid(&vcpu_nestedhvm(v).nv_n2asid);
             nvmx->guest_vpid = new_vpid;
@@ -2909,9 +2911,8 @@ void nvmx_set_cr_read_shadow(struct vcpu *v, unsigned int cr)
 
 void __init start_nested_vmx(struct hvm_function_table *hvm_function_table)
 {
-    /* TODO: Require hardware support before enabling nested virt */
     hvm_function_table->caps.nested_virt =
-        hvm_function_table->caps.hap;
+        hvm_function_table->caps.hap && cpu_has_vmx_vpid;
 }
 
 /*
diff --git a/xen/arch/x86/mm/hap/nested_ept.c b/xen/arch/x86/mm/hap/nested_ept.c
index f8563156b6d0..18d5de870013 100644
--- a/xen/arch/x86/mm/hap/nested_ept.c
+++ b/xen/arch/x86/mm/hap/nested_ept.c
@@ -101,7 +101,7 @@ uint64_t nept_get_ept_vpid_cap(void)
     if ( !cpu_has_vmx_ept_exec_only_supported )
         caps &= ~VMX_EPT_EXEC_ONLY_SUPPORTED;
     if ( cpu_has_vmx_vpid )
-        caps |= NVPID_CAP_BITS;
+        caps |= (uint64_t)NVPID_CAP_BITS << 32;
 
     return caps;
 }
-- 
2.34.1