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

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi all,

On Tue, Nov 10, 2026 at 04:02:21PM +0000, Lorenzo Stoakes (ARM) wrote:
> 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.

Unfortunately, what happens in this case is not as obvious as it sounds,
because a read fault on a memslot backed by guest_memfd maps the GPA with
read *and* write permission on arm64 (if the memslot is not readonly,
haven't checked what x86 does).

Thanks,
Alex

> 
> 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
>