Re: [kvm-unit-tests PATCH v1 0/3] x86: fix async page fault issues

[Sean Christopherson <seanjc@google.com>]

On Wed, Dec 13, 2023, Dan1 Wu wrote:
> 
> On 12/12/2023 11:17 PM, Sean Christopherson wrote:
> > On Tue, Dec 12, 2023, Dan Wu wrote:
> > > When running asyncpf test, it gets skipped without a clear reason:
> > > 
> > >      ./asyncpf
> > > 
> > >      enabling apic
> > >      smp: waiting for 0 APs
> > >      paging enabled
> > >      cr0 = 80010011
> > >      cr3 = 107f000
> > >      cr4 = 20
> > >      install handler
> > >      enable async pf
> > >      alloc memory
> > >      start loop
> > >      end loop
> > >      start loop
> > >      end loop
> > >      SUMMARY: 0 tests
> > >      SKIP asyncpf (0 tests)
> > > 
> > > The reason is that KVM changed to use interrupt-based 'page-ready' notification
> > > and abandoned #PF-based 'page-ready' notification mechanism. Interrupt-based
> > > 'page-ready' notification requires KVM_ASYNC_PF_DELIVERY_AS_INT to be set as well
> > > in MSR_KVM_ASYNC_PF_EN to enable asyncpf.
> > > 
> > > This series tries to fix the problem by separating two testcases for different mechanisms.
> > > 
> > > - For old #PF-based notification, changes current asyncpf.c to add CPUID check
> > >    at the beginning. It checks (KVM_FEATURE_ASYNC_PF && !KVM_FEATURE_ASYNC_PF_INT),
> > >    otherwise it gets skipped.
> > > 
> > > - For new interrupt-based notification, add a new test, asyncpf-int.c, to check
> > >    (KVM_FEATURE_ASYNC_PF && KVM_FEATURE_ASYNC_PF_INT) and implement interrupt-based
> > >    'page-ready' handler.
> > Using #PF to deliver page-ready is completely dead, no?  Unless I'm mistaken, let's
> > just drop the existing support and replace it with the interrupted-based mechanism.
> > I see no reason to continue maintaining the old crud.  If someone wants to verify
> > an old, broken KVM, then they can use the old version of  KUT.
> 
> Yes, since Linux v5.10 the feature asyncpf is deprecated.
> 
> So, just drop asyncpf.c and add asyncpf_int.c is enough, right?

I would rather not add asyncpf_int.c, and instead keep asyncpf.c and modify it to
use ASYNC_PF_INT.  It _might_ be a bit more churn, but modifying the existing code
instead of dropping in a new file will better preserve the history, and may also
allow for finer grained patches (not sure on that one).