KVM: vmx: remove save/restore of host BNDCGFS MSR
diff mbox

Message ID 20180711165430.8422-1-sean.j.christopherson@intel.com
State New
Headers show

Commit Message

Sean Christopherson July 11, 2018, 4:54 p.m. UTC
Linux does not support Memory Protection Extensions (MPX) in the
kernel itself, thus the BNDCFGS (Bound Config Supervisor) MSR will
always be zero in the KVM host, i.e. RDMSR in vmx_save_host_state()
is superfluous.  KVM unconditionally sets VM_EXIT_CLEAR_BNDCFGS,
i.e. BNDCFGS will always be zero after VMEXIT, thus manually loading
BNDCFGS is also superfluous.

And in the event the MPX kernel support is added (unlikely given
that MPX for userspace is in its death throes[1]), BNDCFGS will
likely be common across all CPUs[2], and at the least shouldn't
change on a regular basis, i.e. saving the MSR on every VMENTRY is
completely unnecessary.

WARN_ONCE in hardware_setup() if the host's BNDCFGS is non-zero to
document that KVM does not preserve BNDCFGS and to serve as a hint
as to how BNDCFGS likely should be handled if MPX is used in the
kernel, e.g. BNDCFGS should be saved once during KVM setup.

[1] https://lkml.org/lkml/2018/4/27/1046
[2] http://www.openwall.com/lists/kernel-hardening/2017/07/24/28

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Paolo Bonzini July 18, 2018, 5:17 p.m. UTC | #1
On 11/07/2018 18:54, Sean Christopherson wrote:
> Linux does not support Memory Protection Extensions (MPX) in the
> kernel itself, thus the BNDCFGS (Bound Config Supervisor) MSR will
> always be zero in the KVM host, i.e. RDMSR in vmx_save_host_state()
> is superfluous.  KVM unconditionally sets VM_EXIT_CLEAR_BNDCFGS,
> i.e. BNDCFGS will always be zero after VMEXIT, thus manually loading
> BNDCFGS is also superfluous.
> 
> And in the event the MPX kernel support is added (unlikely given
> that MPX for userspace is in its death throes[1]), BNDCFGS will
> likely be common across all CPUs[2], and at the least shouldn't
> change on a regular basis, i.e. saving the MSR on every VMENTRY is
> completely unnecessary.
> 
> WARN_ONCE in hardware_setup() if the host's BNDCFGS is non-zero to
> document that KVM does not preserve BNDCFGS and to serve as a hint
> as to how BNDCFGS likely should be handled if MPX is used in the
> kernel, e.g. BNDCFGS should be saved once during KVM setup.
> 
> [1] https://lkml.org/lkml/2018/4/27/1046
> [2] http://www.openwall.com/lists/kernel-hardening/2017/07/24/28
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 1689f433f3a0..2630ab38d72c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -802,7 +802,6 @@ struct vcpu_vmx {
>  #endif
>  		int           gs_ldt_reload_needed;
>  		int           fs_reload_needed;
> -		u64           msr_host_bndcfgs;
>  	} host_state;
>  	struct {
>  		int vm86_active;
> @@ -2621,8 +2620,6 @@ static void vmx_save_host_state(struct kvm_vcpu *vcpu)
>  	vmcs_writel(HOST_FS_BASE, segment_base(vmx->host_state.fs_sel));
>  	vmcs_writel(HOST_GS_BASE, segment_base(vmx->host_state.gs_sel));
>  #endif
> -	if (boot_cpu_has(X86_FEATURE_MPX))
> -		rdmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_bndcfgs);
>  	for (i = 0; i < vmx->save_nmsrs; ++i)
>  		kvm_set_shared_msr(vmx->guest_msrs[i].index,
>  				   vmx->guest_msrs[i].data,
> @@ -2660,8 +2657,6 @@ static void __vmx_load_host_state(struct vcpu_vmx *vmx)
>  #ifdef CONFIG_X86_64
>  	wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
>  #endif
> -	if (vmx->host_state.msr_host_bndcfgs)
> -		wrmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_bndcfgs);
>  	load_fixmap_gdt(raw_smp_processor_id());
>  }
>  
> @@ -7478,6 +7473,7 @@ static void vmx_enable_tdp(void)
>  
>  static __init int hardware_setup(void)
>  {
> +	unsigned long host_bndcfgs;
>  	int r = -ENOMEM, i;
>  
>  	rdmsrl_safe(MSR_EFER, &host_efer);
> @@ -7502,6 +7498,11 @@ static __init int hardware_setup(void)
>  	if (boot_cpu_has(X86_FEATURE_NX))
>  		kvm_enable_efer_bits(EFER_NX);
>  
> +	if (boot_cpu_has(X86_FEATURE_MPX)) {
> +		rdmsrl(MSR_IA32_BNDCFGS, host_bndcfgs);
> +		WARN_ONCE(host_bndcfgs, "KVM: BNDCFGS in host will be lost");
> +	}
> +
>  	if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
>  		!(cpu_has_vmx_invvpid_single() || cpu_has_vmx_invvpid_global()))
>  		enable_vpid = 0;
> 

Queued, thanks.

Paolo

Patch
diff mbox

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1689f433f3a0..2630ab38d72c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -802,7 +802,6 @@  struct vcpu_vmx {
 #endif
 		int           gs_ldt_reload_needed;
 		int           fs_reload_needed;
-		u64           msr_host_bndcfgs;
 	} host_state;
 	struct {
 		int vm86_active;
@@ -2621,8 +2620,6 @@  static void vmx_save_host_state(struct kvm_vcpu *vcpu)
 	vmcs_writel(HOST_FS_BASE, segment_base(vmx->host_state.fs_sel));
 	vmcs_writel(HOST_GS_BASE, segment_base(vmx->host_state.gs_sel));
 #endif
-	if (boot_cpu_has(X86_FEATURE_MPX))
-		rdmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_bndcfgs);
 	for (i = 0; i < vmx->save_nmsrs; ++i)
 		kvm_set_shared_msr(vmx->guest_msrs[i].index,
 				   vmx->guest_msrs[i].data,
@@ -2660,8 +2657,6 @@  static void __vmx_load_host_state(struct vcpu_vmx *vmx)
 #ifdef CONFIG_X86_64
 	wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_host_kernel_gs_base);
 #endif
-	if (vmx->host_state.msr_host_bndcfgs)
-		wrmsrl(MSR_IA32_BNDCFGS, vmx->host_state.msr_host_bndcfgs);
 	load_fixmap_gdt(raw_smp_processor_id());
 }
 
@@ -7478,6 +7473,7 @@  static void vmx_enable_tdp(void)
 
 static __init int hardware_setup(void)
 {
+	unsigned long host_bndcfgs;
 	int r = -ENOMEM, i;
 
 	rdmsrl_safe(MSR_EFER, &host_efer);
@@ -7502,6 +7498,11 @@  static __init int hardware_setup(void)
 	if (boot_cpu_has(X86_FEATURE_NX))
 		kvm_enable_efer_bits(EFER_NX);
 
+	if (boot_cpu_has(X86_FEATURE_MPX)) {
+		rdmsrl(MSR_IA32_BNDCFGS, host_bndcfgs);
+		WARN_ONCE(host_bndcfgs, "KVM: BNDCFGS in host will be lost");
+	}
+
 	if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
 		!(cpu_has_vmx_invvpid_single() || cpu_has_vmx_invvpid_global()))
 		enable_vpid = 0;