Re: [PATCH v1 5/5] x86/nestedsvm: fix up

[chunjie.zhu@xxxxxxxxxx]

On Wed, Sep 23, 2026 at 10:55:41 AM, Ross Lagerwall <ross.lagerwall@xxxxxxxxxx> 
wrote:

> On 9/23/26 10:42 AM, Ross Lagerwall wrote:
> > On 9/23/26 10:20 AM, chunjie.zhu@xxxxxxxxxx wrote:
> >> From: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>
> >>
> >> Signed-off-by: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>
> >> ---
> >>   xen/arch/x86/hvm/svm/nestedsvm.c | 19 ++++++++++++++++---
> >>   xen/arch/x86/hvm/svm/vmcb.c      |  9 +++++----
> >>   xen/arch/x86/mm/p2m.c            |  5 +++++
> >>   3 files changed, 26 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c 
> >> b/xen/arch/x86/hvm/svm/nestedsvm.c
> >> index 82e01e513e69..91872af8aa7b 100644
> >> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> >> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> >> @@ -355,7 +355,7 @@ static void nestedsvm_vmcb_set_nestedp2m(struct vcpu 
> >> *v,
> >>       vcpu_nestedsvm(v).ns_vmcb_hostcr3 = vvmcb->_h_cr3;
> >>       p2m = p2m_get_nestedp2m(v);
> >> -    n2vmcb->_h_cr3 = pagetable_get_paddr(p2m_get_pagetable(p2m));
> >> +    vmcb_set_h_cr3(n2vmcb, pagetable_get_paddr(p2m_get_pagetable(p2m)));
> >>   }
> >>   void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
> >> @@ -373,6 +373,7 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
> >>           return;
> >>       p2m = p2m_get_nestedp2m(v);
> >> +    nv->stale_np2m = false;
> >>       /*
> >>        * This may happen if we've handled VMEXIT_NPF at the same time as a
> >> @@ -383,8 +384,6 @@ void nsvm_vcpu_update_nestedp2m(struct vcpu *v)
> >>            vmcb_get_h_cr3(nv->nv_n2vmcx) !=
> >>            pagetable_get_paddr(p2m_get_pagetable(p2m)) )
> >>           nestedsvm_vmcb_set_nestedp2m(v, nv->nv_vvmcx, nv->nv_n2vmcx);
> >> -
> >> -    nv->stale_np2m = false;
> >>   }
> >>   static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct cpu_user_regs 
> >> *regs)
> >> @@ -1024,6 +1023,20 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct 
> >> cpu_user_regs *regs)
> >>       struct vmcb_struct *ns_vmcb = nv->nv_vvmcx;
> >>       struct vmcb_struct *n2vmcb = nv->nv_n2vmcx;
> >> +    ASSERT(v == current);
> >> +
> >> +    /*
> >> +     * The physical VMCB fields covered by VMSAVE/VMLOAD may not be in 
> >> sync
> >> +     * with v's vmcb if a context switch happened since the last VMLOAD.
> >> +     * VMSAVE below would otherwise capture stale/foreign register state
> >> +     * into the L1 shadow VMCB.
> >> +     */
> >> +    if ( v->arch.hvm.svm.vmcb_sync_state == vmcb_needs_vmload )
> >> +    {
> >> +        svm_vmload_pa(v->arch.hvm.svm.vmcb_pa);
> >> +        v->arch.hvm.svm.vmcb_sync_state = vmcb_in_sync;
> >> +    }
> >> +
> >>       svm_vmsave_pa(nv->nv_n1vmcx_pa);
> >>       /* Cache guest physical address of virtual vmcb
> >> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> >> index 5354c4f1b85f..2f7053ed7eec 100644
> >> --- a/xen/arch/x86/hvm/svm/vmcb.c
> >> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> >> @@ -357,10 +357,11 @@ 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))) )
> >> +         ((!(cr4 & X86_CR4_PCIDE) &&
> >> +           ((cr3 & 7) ||
> >> +            ((!(cr4 & X86_CR4_PAE) || (efer & EFER_LMA)) &&
> >> +             (cr3 & 0xfe0)))) ||
> >> +          (cr3 >> v->domain->arch.cpuid->extd.maxphysaddr)) )
> >>           PRINTF("CR3: MBZ bits are set (%#"PRIx64")\n", cr3);
> >>       valid = hvm_cr4_guest_valid_bits(v->domain);
> >> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> >> index 027b9ae69be3..d1fafc479c78 100644
> >> --- a/xen/arch/x86/mm/p2m.c
> >> +++ b/xen/arch/x86/mm/p2m.c
> >> @@ -1517,7 +1517,12 @@ p2m_get_nestedp2m_locked(struct vcpu *v)
> >>       np2m_base &= ~(0xfffULL);
> >>       if ( nv->nv_flushp2m && nv->nv_p2m )
> >> +    {
> >> +        p2m = nv->nv_p2m;
> >> +        if ( p2m )
> >> +            p2m_flush_table(p2m);
> >>           nv->nv_p2m = NULL;
> >> +    }
> >>       nestedp2m_lock(d);
> >>       p2m = nv->nv_p2m;
> > 
> > This seems to have combined several bug fixes from our internal patchqueue 
> > (I
> > know because I wrote some of them...), some of which have already been 
> > posted
> > to the list.
> > 
> > It's not clear why they were submitted as part of this series.
> > 
>
> Looking further, patches 1, 3, and 4 also contain seemingly unrelated patches
> from our internal patchqueue. Can you resubmit this series without these
> patches mixed in? Or if they really are required dependencies, (where they
> haven't already been posted to xen-devel previously) submit them as separate
> patches at the start of this series, maintaining authorship information.
>
> Ross

Sure. Will send the v2 series.

>
>