[PATCH v1 0/6] nestedsvm: Misc fixes

[PATCH v1 0/6] nestedsvm: Misc fixes

[Ross Lagerwall]

Before this series, running Linux on Xen on Xen on a modern AMD
processor would lock up L1 shortly after L2 reached the bootloader.
Furthermore, L1's domain could not be destroyed.

After this series, repeating the same results in L2 crashing shortly
after it reaches the Linux kernel but L1 survives and its domain can be
properly destroyed. This is not great but is at least some small amount
of progress.

Thanks,
Ross

Ross Lagerwall (6):
  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
  nestedsvm: Fix deferred event injection
  nestedsvm: Allow destroying the domain fully

 xen/arch/x86/hvm/svm/intr.c      |  4 +--
 xen/arch/x86/hvm/svm/nestedhvm.h |  1 +
 xen/arch/x86/hvm/svm/nestedsvm.c | 60 ++++++++++++++++++++++++--------
 xen/arch/x86/hvm/svm/svm.c       |  3 ++
 xen/arch/x86/hvm/svm/svm.h       |  3 ++
 xen/arch/x86/hvm/svm/vmcb.c      |  6 ++--
 6 files changed, 57 insertions(+), 20 deletions(-)

-- 
2.53.0

[PATCH v1 1/6] nestedsvm: Fix CR3 MBZ check

[Ross Lagerwall]

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 975a1eaef806..9ada491e57db 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -347,10 +347,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.53.0

[PATCH v1 2/6] nestedsvm: Adjust L2's DR intercept when adjusting L1

[Ross Lagerwall]

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 49fcdd906cf8..209edcba321a 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.53.0

[PATCH v1 3/6] nestedsvm: Use the correct VMCB for vGIF

[Ross Lagerwall]

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 9899cb2147b1..dca07d27d923 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1193,10 +1193,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;
 
@@ -1460,11 +1461,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.53.0

[PATCH v1 4/6] nestedsvm: Set GIF during VMRUN if vGIF is enabled

[Ross Lagerwall]

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 dca07d27d923..9b0bd0358ce4 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -603,10 +603,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);
 
@@ -651,7 +653,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.53.0

[PATCH v1 5/6] nestedsvm: Fix deferred event injection

[Ross Lagerwall]

If an event for L1 occurs while L2 is running, Xen should inject
VMEXIT_INTR and the event into L1.

nestedsvm_vcpu_interrupt() and nestedsvm_vmexit_defer() set this up to
be handled later by nsvm_vcpu_vmexit_inject() after the switch back to
L1. However, the code there appears to be bogus and completely ignores
the source/vector set up in the first place. Fix this by using the
values to properly inject the event.

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

diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
index cf0621d2f628..8914375b6c21 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_extint(struct vcpu *v, int vector)
+void svm_inject_extint(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_extint(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/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 9b0bd0358ce4..d4fd838ca0b6 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -733,11 +733,25 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct cpu_user_regs *regs,
         switch ( exitcode )
         {
         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;
+        {
+            struct hvm_intack intack = {
+                .source = svm->ns_vmexit.exitinfo1,
+                .vector = svm->ns_vmexit.exitinfo2
+            };
+
+            /* See the comment in svm_intr_assist() for why this is necessary */
+            if ( unlikely(vmcb->event_inj.v) ||
+                 hvm_interrupt_blocked(v, intack) )
+            {
+                svm_enable_intr_window(v, intack);
+                break;
+            }
+
+            svm_inject_extint(v, intack.vector);
+            pt_intr_post(v, intack);
             break;
+        }
+
         case VMEXIT_EXCEPTION_PF:
             ns_vmcb->_cr2 = ns_vmcb->ei.exc.cr2;
             fallthrough;
diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
index cfa411ad5ae1..186e0905967c 100644
--- a/xen/arch/x86/hvm/svm/svm.h
+++ b/xen/arch/x86/hvm/svm/svm.h
@@ -95,6 +95,9 @@ enum vmcb_sync_state {
 
 void svm_sync_vmcb(struct vcpu *v, enum vmcb_sync_state new_state);
 
+void svm_inject_extint(struct vcpu *v, int vector);
+void svm_enable_intr_window(struct vcpu *v, struct hvm_intack intack);
+
 #endif /* __X86_HVM_SVM_SVM_PRIV_H__ */
 
 /*
-- 
2.53.0

[PATCH v1 6/6] nestedsvm: Allow destroying the domain fully

[Ross Lagerwall]

Unmapping the virtual VMCB is performed near the end of the domain
destroy procedure but the mapped guest frame prevents domain destroy
from getting to that point. This means guests that call VMRUN cannot
be fully destroyed.

Move the unmap of the virtual VMCB earlier to fix the issue.

Fixes: bcf557675d85 ("x86: properly use map_domain_page() in nested HVM code")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedhvm.h |  1 +
 xen/arch/x86/hvm/svm/nestedsvm.c | 15 +++++++++++++--
 xen/arch/x86/hvm/svm/svm.c       |  1 +
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedhvm.h b/xen/arch/x86/hvm/svm/nestedhvm.h
index 9bfed5ffd71b..9bb04a043430 100644
--- a/xen/arch/x86/hvm/svm/nestedhvm.h
+++ b/xen/arch/x86/hvm/svm/nestedhvm.h
@@ -48,6 +48,7 @@ bool cf_check nsvm_vmcb_guest_intercepts_event(
     struct vcpu *v, unsigned int vector, int errcode);
 bool cf_check nsvm_vmcb_hap_enabled(struct vcpu *v);
 enum hvm_intblk cf_check nsvm_intr_blocked(struct vcpu *v);
+void cf_check nsvm_domain_relinquish_resources(struct domain *d);
 
 /* Interrupts, vGIF */
 void svm_vmexit_do_clgi(struct cpu_user_regs *regs, struct vcpu *v);
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index d4fd838ca0b6..6f4684f5c21b 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -110,8 +110,6 @@ void cf_check nsvm_vcpu_destroy(struct vcpu *v)
         svm->ns_merged_msrpm = NULL;
     }
 
-    hvm_unmap_guest_frame(nv->nv_vvmcx, 1);
-    nv->nv_vvmcx = NULL;
     if ( nv->nv_n2vmcx )
     {
         free_vmcb(nv->nv_n2vmcx);
@@ -122,6 +120,19 @@ void cf_check nsvm_vcpu_destroy(struct vcpu *v)
     svm->ns_iomap = NULL;
 }
 
+void cf_check nsvm_domain_relinquish_resources(struct domain *d)
+{
+    struct vcpu *v;
+    struct nestedvcpu *nv;
+
+    for_each_vcpu ( d, v )
+    {
+        nv = &vcpu_nestedhvm(v);
+        hvm_unmap_guest_frame(nv->nv_vvmcx, 1);
+        nv->nv_vvmcx = NULL;
+    }
+}
+
 int cf_check nsvm_vcpu_reset(struct vcpu *v)
 {
     struct nestedsvm *svm = &vcpu_nestedsvm(v);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 209edcba321a..e6b5c9ec3b9c 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2422,6 +2422,7 @@ static struct hvm_function_table __initdata_cf_clobber svm_function_table = {
     .nhvm_vmcx_hap_enabled = nsvm_vmcb_hap_enabled,
     .nhvm_intr_blocked = nsvm_intr_blocked,
     .nhvm_hap_walk_L1_p2m = nsvm_hap_walk_L1_p2m,
+    .nhvm_domain_relinquish_resources = nsvm_domain_relinquish_resources,
 
     .get_reg = svm_get_reg,
     .set_reg = svm_set_reg,
-- 
2.53.0

Re: [PATCH v1 1/6] nestedsvm: Fix CR3 MBZ check

[Andrew Cooper]

On 26/05/2026 1:40 pm, Ross Lagerwall wrote:
> 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 975a1eaef806..9ada491e57db 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -347,10 +347,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);

The APM does say MBZ for VMRUN, but the end result of a VMEntry (virtual
or otherwise) must be a legal CR3 value.

For 5.2.1 CR3 Register (Legacy) and 5.3.2 CR3 (Long), the APM states:

Reserved Bits. Reserved fields should be cleared to 0 by software when
writing CR3.

What's the real behaviour for trying to set a reserved, non-MBZ bit in
CR3?  On Intel it's strictly a #GP, and I really hope it's the same on AMD.

i.e. I really hope this is a documentation error on AMD's behalf, and
not a misfeature we need to support.

~Andrew

Re: [PATCH v1 1/6] nestedsvm: Fix CR3 MBZ check

[Ross Lagerwall]

On 5/26/26 2:01 PM, Andrew Cooper wrote:
> On 26/05/2026 1:40 pm, Ross Lagerwall wrote:
>> 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 975a1eaef806..9ada491e57db 100644
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -347,10 +347,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);
> 
> The APM does say MBZ for VMRUN, but the end result of a VMEntry (virtual
> or otherwise) must be a legal CR3 value.
> 
> For 5.2.1 CR3 Register (Legacy) and 5.3.2 CR3 (Long), the APM states:
> 
> Reserved Bits. Reserved fields should be cleared to 0 by software when
> writing CR3.
> 
> What's the real behaviour for trying to set a reserved, non-MBZ bit in
> CR3?  On Intel it's strictly a #GP, and I really hope it's the same on AMD.
> 
> i.e. I really hope this is a documentation error on AMD's behalf, and
> not a misfeature we need to support.
> 

An hvm32pae XTF test that does this...

     write_cr3(read_cr3() | 1);
     printk("cr3 is %lx\n", read_cr3());

... succeeds and prints:

     cr3 is 105001

This was similarly observed by the KVM folks in this thread:
https://patchwork.kernel.org/project/kvm/patch/20200713043908.39605-1-namit@vmware.com/#23578493

Ross

Re: [PATCH v1 2/6] nestedsvm: Adjust L2's DR intercept when adjusting L1

[Andrew Cooper]

On 26/05/2026 1:40 pm, Ross Lagerwall wrote:
> 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 49fcdd906cf8..209edcba321a 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(

In Xen, debug registers are generally lazily.  When DR7 is not active
(which is expected to be ~100% of the time for a regular guest), there's
no point context switching DR{0..3} or (on AMD) the mask DBG Mask MSRs[1].

The debug registers are brought into sync if DR7 is active at context
switch, or any DR is accessed, or if a #DB is injected[2].

Now, in the logic above, you're saying that L1 didn't intercept DR which
is why we didn't Virtual VMExit earlier, so when we're bringing DRs into
sync we need to drop the L02 intercept too.  I think this is fine, but
it deserves a comment explaining that it's an artefact of Xen's lazy
context DR switching.

But, what about emulated MOV DR, or a #DB injection?  Those paths will
still end up being wrong.

I think this logic to alter the DR intercepts needs to be inside
__restore_debug_registers(), and needs to cross-check the L12 settings
before modifying L02.

~Andrew

[1] Although the Mask MSRs are currently inefficiently switched because
I didn't have time to optimise things after the last XSA fixing them.
[2] This path is wonky.  DRs should be made active irrespective of TF
because the #DB handler always needs to read DR6[3].
[3] This is a fun FRED bug, as the FRED #DB handler does not need to
read DR6, meaning that I think we need to force DR6 always to be in
sync.  And this gets extra complicated on Intel...

Re: [PATCH v1 1/6] nestedsvm: Fix CR3 MBZ check

[Jan Beulich]

On 26.05.2026 15:01, Andrew Cooper wrote:
> On 26/05/2026 1:40 pm, Ross Lagerwall wrote:
>> 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 975a1eaef806..9ada491e57db 100644
>> --- a/xen/arch/x86/hvm/svm/vmcb.c
>> +++ b/xen/arch/x86/hvm/svm/vmcb.c
>> @@ -347,10 +347,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);
> 
> The APM does say MBZ for VMRUN, but the end result of a VMEntry (virtual
> or otherwise) must be a legal CR3 value.
> 
> For 5.2.1 CR3 Register (Legacy) and 5.3.2 CR3 (Long), the APM states:
> 
> Reserved Bits. Reserved fields should be cleared to 0 by software when
> writing CR3.
> 
> What's the real behaviour for trying to set a reserved, non-MBZ bit in
> CR3?  On Intel it's strictly a #GP, and I really hope it's the same on AMD.

As to Intel - are you sure? The MOV to/from control register page has this:
"When PCIDs are not enabled, bits 2:0 and bits 11:5 of CR3 are not used and
attempts to set them are ignored."

Jan

Re: [PATCH v1 6/6] nestedsvm: Allow destroying the domain fully

[Jan Beulich]

On 26.05.2026 14:40, Ross Lagerwall wrote:
> Unmapping the virtual VMCB is performed near the end of the domain
> destroy procedure but the mapped guest frame prevents domain destroy
> from getting to that point. This means guests that call VMRUN cannot
> be fully destroyed.
> 
> Move the unmap of the virtual VMCB earlier to fix the issue.
> 
> Fixes: bcf557675d85 ("x86: properly use map_domain_page() in nested HVM code")
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Afaict this is independent of the earlier patches, and hence could go in once
the tree is properly (or at least partly) open again after branching?

Jan

Re: [PATCH v1 1/6] nestedsvm: Fix CR3 MBZ check

[Ross Lagerwall]

> From: Ross Lagerwall
> Sent: Tuesday, May 26, 2026 2:23 PM
> To: Andrew Cooper; xen-devel@lists.xenproject.org
> Cc: Jan Beulich; Roger Pau Monne; Jason Andryuk; Teddy Astie
> Subject: Re: [PATCH v1 1/6] nestedsvm: Fix CR3 MBZ check
> 
> On 5/26/26 2:01 PM, Andrew Cooper wrote:
> > On 26/05/2026 1:40 pm, Ross Lagerwall wrote:
> >> 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 975a1eaef806..9ada491e57db 100644
> >> --- a/xen/arch/x86/hvm/svm/vmcb.c
> >> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> >> @@ -347,10 +347,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);
> >
> > The APM does say MBZ for VMRUN, but the end result of a VMEntry (virtual
> > or otherwise) must be a legal CR3 value.
> >
> > For 5.2.1 CR3 Register (Legacy) and 5.3.2 CR3 (Long), the APM states:
> >
> > Reserved Bits. Reserved fields should be cleared to 0 by software when
> > writing CR3.
> >
> > What's the real behaviour for trying to set a reserved, non-MBZ bit in
> > CR3?  On Intel it's strictly a #GP, and I really hope it's the same on AMD.
> >
> > i.e. I really hope this is a documentation error on AMD's behalf, and
> > not a misfeature we need to support.
> >
> 
> An hvm32pae XTF test that does this...
> 
>      write_cr3(read_cr3() | 1);
>      printk("cr3 is %lx\n", read_cr3());
> 
> ... succeeds and prints:
> 
>      cr3 is 105001
> 
> This was similarly observed by the KVM folks in this thread:
> https://patchwork.kernel.org/project/kvm/patch/20200713043908.39605-1-namit@vmware.com/#23578493

Ping, Andrew?

The existing check doesn't mirror what hardware does and causes real-world failures.
Can this patch go in?

Ross

Re: [PATCH v1 0/6] nestedsvm: Misc fixes

[Ross Lagerwall]

> From: Ross Lagerwall
> Sent: Tuesday, May 26, 2026 1:40 PM
> To: xen-devel@lists.xenproject.org
> Cc: Ross Lagerwall; Jan Beulich; Andrew Cooper; Roger Pau Monne; Jason Andryuk; Teddy Astie
> Subject: [PATCH v1 0/6] nestedsvm: Misc fixes
> 
> Before this series, running Linux on Xen on Xen on a modern AMD
> processor would lock up L1 shortly after L2 reached the bootloader.
> Furthermore, L1's domain could not be destroyed.
> 
> After this series, repeating the same results in L2 crashing shortly
> after it reaches the Linux kernel but L1 survives and its domain can be
> properly destroyed. This is not great but is at least some small amount
> of progress.
> 
> Thanks,
> Ross
> 
> Ross Lagerwall (6):
>   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
>   nestedsvm: Fix deferred event injection
>   nestedsvm: Allow destroying the domain fully

Ping? Can I please get some reviews on patches 3, 4, and 5 please?

Thanks,
Ross

Re: [PATCH v1 5/6] nestedsvm: Fix deferred event injection

[Andrew Cooper]

On 26/05/2026 1:40 pm, Ross Lagerwall wrote:
> If an event for L1 occurs while L2 is running, Xen should inject
> VMEXIT_INTR and the event into L1.
>
> nestedsvm_vcpu_interrupt() and nestedsvm_vmexit_defer() set this up to
> be handled later by nsvm_vcpu_vmexit_inject() after the switch back to
> L1. However, the code there appears to be bogus and completely ignores
> the source/vector set up in the first place. Fix this by using the
> values to properly inject the event.
>
> Fixes: 9a779e4fc161 ("Implement SVM specific part for Nested Virtualization")
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

I'm reasonably sure this will be unnecessary when we fix the underlying
problem of deferred entry/exit.

In the meantime, while reviewing this I found a systematic naming error
in the code.  I've submitted a fix separately.

~Andrew

Re: [PATCH v1 5/6] nestedsvm: Fix deferred event injection

[Ross Lagerwall]

On 9/11/26 11:56 AM, Andrew Cooper wrote:
> On 26/05/2026 1:40 pm, Ross Lagerwall wrote:
>> If an event for L1 occurs while L2 is running, Xen should inject
>> VMEXIT_INTR and the event into L1.
>>
>> nestedsvm_vcpu_interrupt() and nestedsvm_vmexit_defer() set this up to
>> be handled later by nsvm_vcpu_vmexit_inject() after the switch back to
>> L1. However, the code there appears to be bogus and completely ignores
>> the source/vector set up in the first place. Fix this by using the
>> values to properly inject the event.
>>
>> Fixes: 9a779e4fc161 ("Implement SVM specific part for Nested Virtualization")
>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> 
> I'm reasonably sure this will be unnecessary when we fix the underlying
> problem of deferred entry/exit.
> 

Are you suggesting if the VMEXIT is done inline in nestedsvm_vcpu_interrupt(),
then it can just fall through to the normal L1 interrupt injection code (in
svm_intr_assist())?

Ross