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

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

> From: Ross Lagerwall
> Sent: Tuesday, May 26, 2026 2:23 PM
> To: Andrew Cooper; xen-devel@xxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxx>
> >> ---
> >>   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@xxxxxxxxxx/#23578493

Ping, Andrew?

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

Ross