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

["Wu, Dan1" <dan1.wu@intel.com>]

On 12/14/2023 2:20 AM, Sean Christopherson wrote:
> 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).

ok, I will modify it in the next version. Thanks for your review.