Re: [PATCH v2 00/23] Extend test coverage for nested SVM

[Sean Christopherson <seanjc@google.com>]

On Wed, Nov 19, 2025, Yosry Ahmed wrote:
> On Tue, Nov 18, 2025 at 03:49:55PM -0800, Sean Christopherson wrote:
> > On Tue, Nov 18, 2025, Yosry Ahmed wrote:
> > > On Tue, Nov 18, 2025 at 03:00:26PM -0800, Jim Mattson wrote:
> > > > On Tue, Nov 18, 2025 at 2:26 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
> > > > > diff --git a/tools/testing/selftests/kvm/lib/x86_64/vmx.c b/tools/testing/selftests/kvm/lib/x86_64/vmx.c
> > > > > index 358143bf8dd0d..8bacb74c00053 100644
> > > > > --- a/tools/testing/selftests/kvm/lib/x86/vmx.c
> > > > > +++ b/tools/testing/selftests/kvm/lib/x86/vmx.c
> > > > > @@ -203,7 +203,7 @@ static inline void init_vmcs_control_fields(struct vmx_pages *vmx)
> > > > >                 uint64_t ept_paddr;
> > > > >                 struct eptPageTablePointer eptp = {
> > > > >                         .memory_type = X86_MEMTYPE_WB,
> > > > > -                       .page_walk_length = 3, /* + 1 */
> > > > > +                       .page_walk_length = get_cr4() & X86_CR4_LA57 ? 4 : 3, /* + 1 */
> > > > 
> > > > LA57 does not imply support for 5-level EPT. (SRF, IIRC)
> > 
> > Yuuuup.  And similarly, MAXPHYADDR=52 doesn't imply 5-level EPT (thank you TDX!).
> > 
> > > Huh, that's annoying. We can keep the EPTs hardcoded to 4 levels and
> > > pass in the max level to __virt_pg_map() instead of hardcoding
> > > vm->pgtable_levels.
> > 
> > I haven't looked at the series in-depth so I don't know exactly what you're trying
> > to do, but why not check MSR_IA32_VMX_EPT_VPID_CAP for PWL5?
> 
> The second part of the series reuses __virt_pg_map() to be used for
> nested EPTs (and NPTs). __virt_pg_map() uses vm->pgtable_levels to find
> out how many page table levels we have.
> 
> So we need to either:
> 
> (a) Always use the same number of levels for page tables and EPTs.
> 
> (b) Make __virt_pg_map() take the number of page table levels as a
>   parameter, and always pass 4 for EPTs (for now).
> 
> I suggested (a) initially, but it doesn't work because we can
> technically have LA57 but not MSR_IA32_VMX_EPT_VPID_CAP, so we need to
> do (b). We can still check MSR_IA32_VMX_EPT_VPID_CAP and use PWL5 for
> EPTs, but that's an orthogonal change at this point.

I choose option (c)

 (c) Add a "struct kvm_mmu" and use it to hold the PTE masks, root, root level,
     and any other metadata that comes along in the future.

Then we only need to do the core plumbing once, to get "struct kvm_mmu *mmu"
passed in.  After that, adding each piece only needs to touch code that actually
cares about those things.

That was going to be my vote even without this particular discussion (I forgot
why I even started reviewing the series, *sigh*), because the root_gpa and
pte_masks should be passed as a single entity.  The root+level are even more
tightly coupled.