linux-kernel - Re: [PATCH v2 6/6] KVM: x86: Register "emergency disable" callbacks when virt is enabled

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, 23 May 2024 13:59:49 +0800
From: Chao Gao <chao.gao@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Paolo Bonzini <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Kai Huang <kai.huang@...el.com>
Subject: Re: [PATCH v2 6/6] KVM: x86: Register "emergency disable" callbacks
 when virt is enabled

On Tue, May 21, 2024 at 07:28:27PM -0700, Sean Christopherson wrote:
>Register the "disable virtualization in an emergency" callback just
>before KVM enables virtualization in hardware, as there is no functional
>need to keep the callbacks registered while KVM happens to be loaded, but
>is inactive, i.e. if KVM hasn't enabled virtualization.
>
>Note, unregistering the callback every time the last VM is destroyed could
>have measurable latency due to the synchronize_rcu() needed to ensure all
>references to the callback are dropped before KVM is unloaded.  But the
>latency should be a small fraction of the total latency of disabling
>virtualization across all CPUs, and userspace can set enable_virt_at_load
>to completely eliminate the runtime overhead.
>
>Add a pointer in kvm_x86_ops to allow vendor code to provide its callback.
>There is no reason to force vendor code to do the registration, and either
>way KVM would need a new kvm_x86_ops hook.
>
>Suggested-by: Kai Huang <kai.huang@...el.com>
>Signed-off-by: Sean Christopherson <seanjc@...gle.com>

Reviewed-by: Chao Gao <chao.gao@...el.com>

..

>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -753,7 +753,7 @@ static int kvm_cpu_vmxoff(void)
> 	return -EIO;
> }
> 
>-static void vmx_emergency_disable(void)
>+void vmx_emergency_disable(void)
> {
> 	int cpu = raw_smp_processor_id();
> 	struct loaded_vmcs *v;
>@@ -8613,8 +8613,6 @@ static void __vmx_exit(void)
> {
> 	allow_smaller_maxphyaddr = false;
> 
>-	cpu_emergency_unregister_virt_callback(vmx_emergency_disable);
>-
> 	vmx_cleanup_l1d_flush();
> }
> 
>@@ -8661,8 +8659,6 @@ static int __init vmx_init(void)
> 		pi_init_cpu(cpu);
> 	}
> 
>-	cpu_emergency_register_virt_callback(vmx_emergency_disable);
>-

Nit: with the removal of calls to cpu_emergency_(un)register_virt_callback,
there is no need to include asm/reboot.h in vmx.c any more. right?