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

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

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.
>       /* DRn */
Reviewed-by: Teddy Astie <teddy.astie@xxxxxxxxxx>

Teddy

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature