Re: [PATCH 2/2] KVM: selftests: Test KVM_PRE_FAULT_MEMORY against a full dirty ring

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Tue, Nov 10, 2026 at 07:53:35AM -0800, Sean Christopherson wrote:
> On Tue, Nov 10, 2026, Fuad Tabba wrote:
> > Hi Lorenzo,
> >
> > On Tue, 10 Nov 2026 14:38:27 +0000, "Lorenzo Stoakes (ARM)"
> > <ljs@kernel.org> wrote:
> > > On Tue, Nov 10, 2026 at 11:07:24AM +0000, Fuad Tabba wrote:
> > > > Pre-fault more populated, dirty-logged pages than the vCPU's dirty ring
> > > > holds, and check that each call stops before the ring overflows, that a
> > > > retry returns ENOSPC without progress, and that the harvested entries
> > > > are the pages in order. A read pre-fault can map a page read-only, or
> > >
> > > Are we asserting an invariant that prefaulting happens in address order
> > > here?  Is that explicitly presumed in the prefault API, or just how it
> > > happens to pan out?
> > >
> > > I wonder if we might decide for whatever reason in future to do things
> > > differently there. Bit nitty I know, but wonder if it's asserting an
> > > implementation detail?
> >
> > Yes, the API doesn't guarantee an order. I'll assert that every page
> > is reported exactly once instead.
> >
> > [...]
> > > > diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c
> > [...]
> > > > +static void test_pre_fault_dirty_ring(void)
> > [...]
> > > > +    /* Populate the pages, so that pre-faulting can map them writable. */
> > >
> > > Reviewed below, but I wonder how x86 could then read fault these?
> >
> > KVM_PRE_FAULT_MEMORY is defined as a read fault, but x86 maps the page
> > writable when the host page is, and marks it dirty, unless it falls
> > back to access tracking (below). Populating the pages makes sure they
> > aren't backed by the zero page.
>
> Before we effectively solidify the behavior, I think we should have an explicit
> discussion about the desired behavior of KVM_PRE_FAULT_MEMORY when dirty logging
> is enabled.  E.g. the only reason I kept the existing behavior in my auto-prefault
> series was because I couldn't think of a good reason to change it.  But we never
> made a concious decision in the first place, i.e. we never considered what would
> happen if prefaulting collided with dirty logging (as evidenced by this bug).

Yeah, I do worry about implicit assumptions (also commented re: ordering).

I guess what makes sense, given the general approach for prefaulting = injecting
a synthetic read fault, is for the expectation to be that it matches the
behaviour of an ordinary read fault.

Or do you feel that it makes sense for the pre-faulting to do something special
in these cases and establish a sensible invariant?

If the pre-faulting does indeed dirty the pages, you'd need the dirty logging to
do its usual thing (not familiar with the code, so I'm guessing logging the page
:) though right?

--
Cheers, Lorenzo