Re: [PATCH 1/2] KVM: x86: async_pf: remove support for KVM_ASYNC_PF_SEND_ALWAYS

[Nikita Kalyazin <kalyazin@amazon.com>]

On 18/02/2025 15:17, Sean Christopherson wrote:
> On Mon, Feb 17, 2025, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>>
>>> On Wed, Nov 27, 2024, Nikita Kalyazin wrote:
>>>> 3a7c8fafd1b42adea229fd204132f6a2fb3cd2d9 ("x86/kvm: Restrict
>>>> ASYNC_PF to user space") stopped setting KVM_ASYNC_PF_SEND_ALWAYS in
>>>> Linux guests.  While the flag can still be used by legacy guests, the
>>>> mechanism is best effort so KVM is not obliged to use it.
>>>
>>> What's the actual motivation to remove it from KVM?  I agreed KVM isn't required
>>> to honor KVM_ASYNC_PF_SEND_ALWAYS from a guest/host ABI perspective, but that
>>> doesn't mean that dropping a feature has no impact.  E.g. it's entirely possible
>>> removing this support could negatively affect a workload running on an old kernel.
>>>
>>> Looking back at the discussion[*] where Vitaly made this suggestion, I don't see
>>> anything that justifies dropping this code.  It costs KVM practically nothing to
>>> maintain this code.
>>>
>>> [*] https://lore.kernel.org/all/20241118130403.23184-1-kalyazin@amazon.com
>>>
>>
>> How old is old? :-)
>>
>> Linux stopped using KVM_ASYNC_PF_SEND_ALWAYS in v5.8:
> 
> 5.8 is practically a baby.  Maybe a toddler :-)
> 
>> commit 3a7c8fafd1b42adea229fd204132f6a2fb3cd2d9
>> Author: Thomas Gleixner <tglx@linutronix.de>
>> Date:   Fri Apr 24 09:57:56 2020 +0200
>>
>>      x86/kvm: Restrict ASYNC_PF to user space
>>
>> and I was under the impression other OSes never used KVM asynchronous
>> page-fault in the first place (not sure about *BSDs though but certainly
>> not Windows). As Nikita's motivation for the patch was "to avoid the
>> overhead ... in case of kernel-originated faults" I suggested we start
>> by simplifyign the code to not care about 'send_user_only' at all.
> 
> In practice, I don't think it's a meaningful simplification.  There are other
> scenarios where KVM shouldn't inject an async #PF, so kvm_can_deliver_async_pf()
> itself isn't going anywhere.
> 
> AFAICT, what Nikita actually wants is a way to disable host-side async #PF, e.g.

That's correct.  Just wanted to say that the main intention was to do 
that for async PF user [1] where the difference in performance is 
noticeable (at least in my setup).  I'm totally ok with the status quo 
in the async PF kernel.  If however the mechanism to achieve that turns 
out to be generic, it's better to support for both, I guess.

[1]: 
https://lore.kernel.org/kvm/20241118123948.4796-1-kalyazin@amazon.com/T/

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f97d4d435e7f..d461e1b5489c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -13411,7 +13411,8 @@ bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu)
>                       kvm_is_exception_pending(vcpu)))
>                  return false;
> 
> -       if (kvm_hlt_in_guest(vcpu->kvm) && !kvm_can_deliver_async_pf(vcpu))
> +       if ((kvm_hlt_in_guest(vcpu->kvm) || kvm_only_pv_async_pf(vcpu->kvm)) &&
> +           !kvm_can_deliver_async_pf(vcpu))
>                  return false;
> 
>          /*
> 
>> We can keep the code around, I guess, but with no plans to re-introduce
>> KVM_ASYNC_PF_SEND_ALWAYS usage to Linux I still believe it would be good
>> to set a deprecation date.