Re: [PATCH v1 1/6] nestedsvm: Fix CR3 MBZ check
Ross Lagerwall <[email protected]> Thu, 6 Aug 2026 10:43:56 +0000
| Newsgroups | gmane.comp.emulators.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