Re: [PATCH v3 10/16] KVM: selftests: Reuse virt mapping functions for nested EPTs

[Sean Christopherson <seanjc@google.com>]

On Tue, Dec 30, 2025, Yosry Ahmed wrote:
> December 29, 2025 at 4:08 PM, "Sean Christopherson" <seanjc@google.com mailto:seanjc@google.com?to=%22Sean%20Christopherson%22%20%3Cseanjc%40google.com%3E > wrote:
> > 
> > On Tue, Dec 23, 2025, Yosry Ahmed wrote:
> > 
> > > On Tue, Dec 23, 2025 at 03:12:09PM -0800, Sean Christopherson wrote:
> > >  On Thu, Nov 27, 2025, Yosry Ahmed wrote:
> > >  > diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> > >  > index fb2b2e53d453..62e10b296719 100644
> > >  > --- a/tools/testing/selftests/kvm/include/x86/processor.h
> > >  > +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> > >  > @@ -1447,6 +1447,7 @@ struct pte_masks {
> > >  > uint64_t dirty;
> > >  > uint64_t huge;
> > >  > uint64_t nx;
> > >  > + uint64_t x;
> > >  
> > >  To be consistent with e.g. writable, call this executable.
> > >  
> > >  Was trying to be consistent with 'nx' :) 
> > >  
> > >  
> > >  > uint64_t c;
> > >  > uint64_t s;
> > >  > };
> > >  > @@ -1464,6 +1465,7 @@ struct kvm_mmu {
> > >  > #define PTE_DIRTY_MASK(mmu) ((mmu)->pte_masks.dirty)
> > >  > #define PTE_HUGE_MASK(mmu) ((mmu)->pte_masks.huge)
> > >  > #define PTE_NX_MASK(mmu) ((mmu)->pte_masks.nx)
> > >  > +#define PTE_X_MASK(mmu) ((mmu)->pte_masks.x)
> > >  > #define PTE_C_MASK(mmu) ((mmu)->pte_masks.c)
> > >  > #define PTE_S_MASK(mmu) ((mmu)->pte_masks.s)
> > >  > 
> > >  > @@ -1474,6 +1476,7 @@ struct kvm_mmu {
> > >  > #define pte_dirty(mmu, pte) (!!(*(pte) & PTE_DIRTY_MASK(mmu)))
> > >  > #define pte_huge(mmu, pte) (!!(*(pte) & PTE_HUGE_MASK(mmu)))
> > >  > #define pte_nx(mmu, pte) (!!(*(pte) & PTE_NX_MASK(mmu)))
> > >  > +#define pte_x(mmu, pte) (!!(*(pte) & PTE_X_MASK(mmu)))
> > >  
> > >  And then here to not assume PRESENT == READABLE, just check if the MMU even has
> > >  a PRESENT bit. We may still need changes, e.g. the page table builders actually
> > >  need to verify a PTE is _writable_, not just present, but that's largely an
> > >  orthogonal issue.
> > >  
> > >  Not sure what you mean? How is the PTE being writable relevant to
> > >  assuming PRESENT == READABLE?
> > > 
> > Only tangentially, I was try to say that if we ever get to a point where selftests
> > support read-only mappings, then the below check won't suffice because walking
> > page tables would get false positives on whether or not an entry is usable, e.g.
> > if a test wants to create a writable mapping and ends up re-using a read-only
> > mapping.
> > 
> > The PRESENT == READABLE thing is much more about execute-only mappings (which
> > selftests also don't support, but as you allude to below, don't require new
> > hardware functionality).
> 
> Oh okay, thanks for clarifying. Yeah that makes sense, if/when read-only
> mappings are ever supported the page table builders will need to be updated
> accordingly.
> 
> Although now that you point this out, I think it would be easy to miss. If
> new helpers are introduced that just modify existing page tables to remove
> the write bit, then we'll probably miss updating the page table builders to
> check for writable mappings. Then again, we'll probably only update the leaf
> PTEs to be read-only, and the page table builders already do not re-use leaf
> entries.

Yep, unless we rewrite the KUT access test :-)

> We could be paranoid and add some TEST_ASSERT() calls to guard against that
> (e.g. in virt_create_upper_pte()), but probably not worth it.

Agreed.