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

Ross Lagerwall <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <CH8PR03MB8274EFC6FA9AA86D68E4277FF0D22@CH8PR03MB8274.namprd03.prod.outlook.com>
> From: Ross Lagerwall
> Sent: Tuesday, May 26, 2026 2:23 PM
> To: Andrew Cooper; [email protected]
> 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 <[email protected]>
> >> ---
> >>   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/[email protected]/#23578493

Ping, Andrew?

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

Ross
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.