linux-kernel - Re: [PATCH 1/4] KVM: Add dedicated arch hook for querying if vCPU was preempted in-kernel

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] [thread-next>] [day] [month] [year] [list]

Date: Thu, 11 Jan 2024 20:48:35 +0800
From: Yuan Yao <yuan.yao@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, Like Xu <like.xu.linux@...il.com>
Subject: Re: [PATCH 1/4] KVM: Add dedicated arch hook for querying if vCPU
 was preempted in-kernel

On Tue, Jan 09, 2024 at 04:39:35PM -0800, Sean Christopherson wrote:
> Plumb in a dedicated hook for querying whether or not a vCPU was preempted
> in-kernel.  Unlike literally every other architecture, x86's VMX can check
> if a vCPU is in kernel context if and only if the vCPU is loaded on the
> current pCPU.
>
> x86's kvm_arch_vcpu_in_kernel() works around the limitation by querying
> kvm_get_running_vcpu() and redirecting to vcpu->arch.preempted_in_kernel
> as needed.  But that's unnecessary, confusing, and fragile, e.g. x86 has
> had at least one bug where KVM incorrectly used a stale
> preempted_in_kernel.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>

Reviewed-by: Yuan Yao <yuan.yao@...el.com>

> ---
>  arch/x86/kvm/x86.c       |  5 +++++
>  include/linux/kvm_host.h |  1 +
>  virt/kvm/kvm_main.c      | 15 +++++++++++++--
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 27e23714e960..415509918c7f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -13091,6 +13091,11 @@ bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu)
>  	return false;
>  }
>
> +bool kvm_arch_vcpu_preempted_in_kernel(struct kvm_vcpu *vcpu)
> +{
> +	return kvm_arch_vcpu_in_kernel(vcpu);
> +}
> +
>  bool kvm_arch_dy_runnable(struct kvm_vcpu *vcpu)
>  {
>  	if (READ_ONCE(vcpu->arch.pv.pv_unhalted))
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 7e7fd25b09b3..28b020404a41 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1505,6 +1505,7 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
>  int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
>  bool kvm_arch_dy_runnable(struct kvm_vcpu *vcpu);
>  bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
> +bool kvm_arch_vcpu_preempted_in_kernel(struct kvm_vcpu *vcpu);
>  int kvm_arch_post_init_vm(struct kvm *kvm);
>  void kvm_arch_pre_destroy_vm(struct kvm *kvm);
>  int kvm_arch_create_vm_debugfs(struct kvm *kvm);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 10bfc88a69f7..6326852bfb3d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4042,11 +4042,22 @@ static bool vcpu_dy_runnable(struct kvm_vcpu *vcpu)
>  	return false;
>  }
>
> +/*
> + * By default, simply query the target vCPU's current mode when checking if a
> + * vCPU was preempted in kernel mode.  All architectures except x86 (or more
> + * specifical, except VMX) allow querying whether or not a vCPU is in kernel
> + * mode even if the vCPU is NOT loaded, i.e. using kvm_arch_vcpu_in_kernel()
> + * directly for cross-vCPU checks is functionally correct and accurate.
> + */
> +bool __weak kvm_arch_vcpu_preempted_in_kernel(struct kvm_vcpu *vcpu)
> +{
> +	return kvm_arch_vcpu_in_kernel(vcpu);
> +}
> +
>  bool __weak kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu)
>  {
>  	return false;
>  }
> -
>  void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool yield_to_kernel_mode)
>  {
>  	struct kvm *kvm = me->kvm;
> @@ -4080,7 +4091,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me, bool yield_to_kernel_mode)
>  				continue;
>  			if (READ_ONCE(vcpu->preempted) && yield_to_kernel_mode &&
>  			    !kvm_arch_dy_has_pending_interrupt(vcpu) &&
> -			    !kvm_arch_vcpu_in_kernel(vcpu))
> +			    !kvm_arch_vcpu_preempted_in_kernel(vcpu))
>  				continue;
>  			if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>  				continue;
> --
> 2.43.0.472.g3155946c3a-goog
>
>