lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aR-pZcbIm7-l0jyh@google.com>
Date: Thu, 20 Nov 2025 15:51:01 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Yosry Ahmed <yosry.ahmed@...ux.dev>
Cc: Jim Mattson <jmattson@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/23] Extend test coverage for nested SVM

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@...ux.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.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ