[PATCH 0/2] KVM_ASYNC_PF_SEND_ALWAYS

[PATCH 0/2] KVM_ASYNC_PF_SEND_ALWAYS

[Nikita Kalyazin]

As was suggested in
https://lore.kernel.org/kvm/20241118130403.23184-1-kalyazin@amazon.com/T/#ma719a9cb3e036e24ea8512abf9a625ddeaccfc96
, remove support for KVM_ASYNC_PF_SEND_ALWAYS in KVM.

Nikita Kalyazin (2):
  KVM: x86: async_pf: remove support for KVM_ASYNC_PF_SEND_ALWAYS
  KVM: x86: async_pf: determine x86 user as cpl == 3

 Documentation/virt/kvm/x86/msr.rst   | 11 +++++------
 arch/x86/include/asm/kvm_host.h      |  1 -
 arch/x86/include/uapi/asm/kvm_para.h |  2 +-
 arch/x86/kvm/x86.c                   |  4 +---
 4 files changed, 7 insertions(+), 11 deletions(-)


base-commit: 1508bae37044ebffd7c7e09915f041936f338123
-- 
2.40.1

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

[Nikita Kalyazin]

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.

Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
---
 Documentation/virt/kvm/x86/msr.rst   | 11 +++++------
 arch/x86/include/asm/kvm_host.h      |  1 -
 arch/x86/include/uapi/asm/kvm_para.h |  2 +-
 arch/x86/kvm/x86.c                   |  4 +---
 4 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/Documentation/virt/kvm/x86/msr.rst b/Documentation/virt/kvm/x86/msr.rst
index 3aecf2a70e7b..b3bbc12b5d03 100644
--- a/Documentation/virt/kvm/x86/msr.rst
+++ b/Documentation/virt/kvm/x86/msr.rst
@@ -208,12 +208,11 @@ data:
 
 	Bits 5-4 of the MSR are reserved and should be zero. Bit 0 is set to 1
 	when asynchronous page faults are enabled on the vcpu, 0 when disabled.
-	Bit 1 is 1 if asynchronous page faults can be injected when vcpu is in
-	cpl == 0. Bit 2 is 1 if asynchronous page faults are delivered to L1 as
-	#PF vmexits.  Bit 2 can be set only if KVM_FEATURE_ASYNC_PF_VMEXIT is
-	present in CPUID. Bit 3 enables interrupt based delivery of 'page ready'
-	events. Bit 3 can only be set if KVM_FEATURE_ASYNC_PF_INT is present in
-	CPUID.
+	Bit 1 is reserved and should be zero. Bit 2 is 1 if asynchronous page
+	faults are delivered to L1 as #PF vmexits.  Bit 2 can be set only if
+	KVM_FEATURE_ASYNC_PF_VMEXIT is present in CPUID. Bit 3 enables
+	interrupt based delivery of 'page ready' events. Bit 3 can only be set
+	if KVM_FEATURE_ASYNC_PF_INT is present in CPUID.
 
 	'Page not present' events are currently always delivered as synthetic
 	#PF exception. During delivery of these events APF CR2 register contains
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e159e44a6a1b..8ce00b17316f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -975,7 +975,6 @@ struct kvm_vcpu_arch {
 		u64 msr_int_val; /* MSR_KVM_ASYNC_PF_INT */
 		u16 vec;
 		u32 id;
-		bool send_user_only;
 		u32 host_apf_flags;
 		bool delivery_as_pf_vmexit;
 		bool pageready_pending;
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index a1efa7907a0b..5558a1ec3dc9 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -87,7 +87,7 @@ struct kvm_clock_pairing {
 #define KVM_MAX_MMU_OP_BATCH           32
 
 #define KVM_ASYNC_PF_ENABLED			(1 << 0)
-#define KVM_ASYNC_PF_SEND_ALWAYS		(1 << 1)
+#define KVM_ASYNC_PF_SEND_ALWAYS		(1 << 1) /* deprecated */
 #define KVM_ASYNC_PF_DELIVERY_AS_PF_VMEXIT	(1 << 2)
 #define KVM_ASYNC_PF_DELIVERY_AS_INT		(1 << 3)
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2e713480933a..8f784f07d423 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3558,7 +3558,6 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data)
 					sizeof(u64)))
 		return 1;
 
-	vcpu->arch.apf.send_user_only = !(data & KVM_ASYNC_PF_SEND_ALWAYS);
 	vcpu->arch.apf.delivery_as_pf_vmexit = data & KVM_ASYNC_PF_DELIVERY_AS_PF_VMEXIT;
 
 	kvm_async_pf_wakeup_all(vcpu);
@@ -13361,8 +13360,7 @@ static bool kvm_can_deliver_async_pf(struct kvm_vcpu *vcpu)
 	if (!kvm_pv_async_pf_enabled(vcpu))
 		return false;
 
-	if (vcpu->arch.apf.send_user_only &&
-	    kvm_x86_call(get_cpl)(vcpu) == 0)
+	if (kvm_x86_call(get_cpl)(vcpu) == 0)
 		return false;
 
 	if (is_guest_mode(vcpu)) {
-- 
2.40.1

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

[Sean Christopherson]

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

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

[Vitaly Kuznetsov]

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: 

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. 

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.

-- 
Vitaly

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

[Sean Christopherson]

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.

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.

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

[Nikita Kalyazin]

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.

[PATCH 2/2] KVM: x86: async_pf: determine x86 user as cpl == 3

[Nikita Kalyazin]

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
---
 arch/x86/kvm/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8f784f07d423..168dcf1d4625 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13360,7 +13360,7 @@ static bool kvm_can_deliver_async_pf(struct kvm_vcpu *vcpu)
 	if (!kvm_pv_async_pf_enabled(vcpu))
 		return false;
 
-	if (kvm_x86_call(get_cpl)(vcpu) == 0)
+	if (kvm_x86_call(get_cpl)(vcpu) != 3)
 		return false;
 
 	if (is_guest_mode(vcpu)) {
-- 
2.40.1

Re: [PATCH 2/2] KVM: x86: async_pf: determine x86 user as cpl == 3

[Sean Christopherson]

On Wed, Nov 27, 2024, Nikita Kalyazin wrote:
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Nikita Kalyazin <kalyazin@amazon.com>
> ---
>  arch/x86/kvm/x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 8f784f07d423..168dcf1d4625 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -13360,7 +13360,7 @@ static bool kvm_can_deliver_async_pf(struct kvm_vcpu *vcpu)
>  	if (!kvm_pv_async_pf_enabled(vcpu))
>  		return false;
>  
> -	if (kvm_x86_call(get_cpl)(vcpu) == 0)
> +	if (kvm_x86_call(get_cpl)(vcpu) != 3)

Ugh, looking at the documentation (explicitly says "vcpu is in cpl == 0"), and
what KVM consideres "in kernel" in other flows, e.g. kvm_arch_vcpu_in_kernel(),
I think the existing code is working as intended.  The only thing that's "wrong"
is the name of KVM's internal variable.  Paolo will probably complain about
checking for a negative, but I think the below is actually what we want.  I'll
post a patch.

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b15cde0a9b5c..528057105c26 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -996,8 +996,8 @@ struct kvm_vcpu_arch {
                u64 msr_int_val; /* MSR_KVM_ASYNC_PF_INT */
                u16 vec;
                u32 id;
-               bool send_user_only;
                u32 host_apf_flags;
+               bool send_always;
                bool delivery_as_pf_vmexit;
                bool pageready_pending;
        } apf;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8e77e61d4fbd..c47cdccc7c5c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3544,7 +3544,7 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data)
                                        sizeof(u64)))
                return 1;
 
-       vcpu->arch.apf.send_user_only = !(data & KVM_ASYNC_PF_SEND_ALWAYS);
+       vcpu->arch.apf.send_always = (data & KVM_ASYNC_PF_SEND_ALWAYS);
        vcpu->arch.apf.delivery_as_pf_vmexit = data & KVM_ASYNC_PF_DELIVERY_AS_PF_VMEXIT;
 
        kvm_async_pf_wakeup_all(vcpu);
@@ -13378,8 +13378,7 @@ static bool kvm_can_deliver_async_pf(struct kvm_vcpu *vcpu)
        if (!kvm_pv_async_pf_enabled(vcpu))
                return false;
 
-       if (vcpu->arch.apf.send_user_only &&
-           kvm_x86_call(get_cpl)(vcpu) == 0)
+       if (!vcpu->arch.apf.send_always && kvm_x86_call(get_cpl)(vcpu) == 0)
                return false;
 
        if (is_guest_mode(vcpu)) {