Re: [PATCH] nestedsvm: Don't set VMCB(1-2)'s NP_ENABLE and N_CR3 during VMEXIT to L1

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 9/20/26 8:10 PM, Teddy Astie wrote:
> Le 18/09/2026 à 14:54, Ross Lagerwall a écrit :
> > As per the VMRUN pseudocode in APM Vol 3 3.38, the VMCB's NP_ENABLE and
> > N_CR3 fields are not set during a VMEXIT so don't do this when updating
> > VMCB(1-2). At the same time, cleanup the somewhat bogus and irrelevant
> > comments. Not clearing N_CR3 does not introduce a security hole as
> > stated since L1 can set it regardless and it is never used directly when
> > running L2.
> >
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> > ---
> >   xen/arch/x86/hvm/svm/nestedsvm.c | 32 +-------------------------------
> >   1 file changed, 1 insertion(+), 31 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
> > index a8b15d6eae05..c62fca571d75 100644
> > --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> > +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> > @@ -1023,37 +1023,6 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct 
> > cpu_user_regs *regs)
> >       ns_vmcb->event_inj.raw = 0;
> > -    /* Nested paging mode */
> > -    if ( nestedhvm_paging_mode_hap(v) )
> > -    {
> > -        /* host nested paging + guest nested paging. */
> > -        vmcb_set_np(ns_vmcb, vmcb_get_np(n2vmcb));
> > -        ns_vmcb->_cr3 = n2vmcb->_cr3;
> > -        /* The vmcb->h_cr3 is the shadowed h_cr3. The original
> > -         * unshadowed guest h_cr3 is kept in ns_vmcb->h_cr3,
> > -         * hence we keep the ns_vmcb->h_cr3 value. */
> > -    }
> > -    else if ( paging_mode_hap(v->domain) )
> > -    {
> > -        /* host nested paging + guest shadow paging. */
> > -        vmcb_set_np(ns_vmcb, false);
> > -        /* Throw h_cr3 away. Guest is not allowed to set it or
> > -         * it can break out, otherwise (security hole!) */
> > -        ns_vmcb->_h_cr3 = 0x0;
> > -        /* Stop intercepting #PF (already done above
> > -         * by restoring cached intercepts). */
> > -        ns_vmcb->_cr3 = n2vmcb->_cr3;
> > -    }
> > -    else
> > -    {
> > -        /* host shadow paging + guest shadow paging. */
> > -        vmcb_set_np(ns_vmcb, false);
> > -        ns_vmcb->_h_cr3 = 0x0;
> > -        /* The vmcb->_cr3 is the shadowed cr3. The original
> > -         * unshadowed guest cr3 is kept in ns_vmcb->_cr3,
> > -         * hence we keep the ns_vmcb->_cr3 value. */
> > -    }
> > ->       /* LBR virtualization - keep lbr control as is */
> >       /* NextRIP */
> > @@ -1083,6 +1052,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct 
> > cpu_user_regs *regs)
> >       /* CRn */
> >       ns_vmcb->_cr4 = n2vmcb->_cr4;
> > +    ns_vmcb->_cr3 = n2vmcb->_cr3;
> >       ns_vmcb->_cr0 = n2vmcb->_cr0;
> I would suggest to group all the CRn (the CR2 part is currently separated). 
> That can be done separately.
Indeed. The APM says...

"Upon #VMEXIT, the processor performs the following actions in order to return
to the host execution context:"

... so ideally we would rearrange this function to match the order specified
(though it shouldn't make a functional difference).

Ross