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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z6uhFYGdmcq_EWCU@google.com>
Date: Tue, 11 Feb 2025 11:12:21 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Nikita Kalyazin <kalyazin@...zon.com>
Cc: pbonzini@...hat.com, corbet@....net, tglx@...utronix.de, mingo@...hat.com, 
	bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	vkuznets@...hat.com, xiaoyao.li@...el.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, roypat@...zon.co.uk, 
	xmarcalx@...zon.com
Subject: Re: [PATCH 2/2] KVM: x86: async_pf: determine x86 user as cpl == 3

On Wed, Nov 27, 2024, Nikita Kalyazin wrote:
> Suggested-by: Sean Christopherson <seanjc@...gle.com>
> Signed-off-by: Nikita Kalyazin <kalyazin@...zon.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)) {