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

["Yosry Ahmed" <yosry.ahmed@linux.dev>]

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.

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.

> 
> > 
> > #define is_present_pte(mmu, pte) \
> >  (PTE_PRESENT_MASK(mmu) ? \
> >  !!(*(pte) & PTE_PRESENT_MASK(mmu)) : \
> >  !!(*(pte) & (PTE_READABLE_MASK(mmu) | PTE_EXECUTABLE_MASK(mmu))))
> >  
> >  and then Intel will introduce VMX_EPT_WRITE_ONLY_BIT :P
> >
>