Skip to content

Commit bf09fb6

Browse files
Sean Christophersonbonzini
authored andcommitted
KVM: VMX: Stop context switching MSR_IA32_UMWAIT_CONTROL
Remove support for context switching between the guest's and host's desired UMWAIT_CONTROL. Propagating the guest's value to hardware isn't required for correct functionality, e.g. KVM intercepts reads and writes to the MSR, and the latency effects of the settings controlled by the MSR are not architecturally visible. As a general rule, KVM should not allow the guest to control power management settings unless explicitly enabled by userspace, e.g. see KVM_CAP_X86_DISABLE_EXITS. E.g. Intel's SDM explicitly states that C0.2 can improve the performance of SMT siblings. A devious guest could disable C0.2 so as to improve the performance of their workloads at the detriment to workloads running in the host or on other VMs. Wholesale removal of UMWAIT_CONTROL context switching also fixes a race condition where updates from the host may cause KVM to enter the guest with the incorrect value. Because updates are are propagated to all CPUs via IPI (SMP function callback), the value in hardware may be stale with respect to the cached value and KVM could enter the guest with the wrong value in hardware. As above, the guest can't observe the bad value, but it's a weird and confusing wart in the implementation. Removal also fixes the unnecessary usage of VMX's atomic load/store MSR lists. Using the lists is only necessary for MSRs that are required for correct functionality immediately upon VM-Enter/VM-Exit, e.g. EFER on old hardware, or for MSRs that need to-the-uop precision, e.g. perf related MSRs. For UMWAIT_CONTROL, the effects are only visible in the kernel via TPAUSE/delay(), and KVM doesn't do any form of delay in vcpu_vmx_run(). Using the atomic lists is undesirable as they are more expensive than direct RDMSR/WRMSR. Furthermore, even if giving the guest control of the MSR is legitimate, e.g. in pass-through scenarios, it's not clear that the benefits would outweigh the overhead. E.g. saving and restoring an MSR across a VMX roundtrip costs ~250 cycles, and if the guest diverged from the host that cost would be paid on every run of the guest. In other words, if there is a legitimate use case then it should be enabled by a new per-VM capability. Note, KVM still needs to emulate MSR_IA32_UMWAIT_CONTROL so that it can correctly expose other WAITPKG features to the guest, e.g. TPAUSE, UMWAIT and UMONITOR. Fixes: 6e3ba4a ("KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL") Cc: [email protected] Cc: Jingqi Liu <[email protected]> Cc: Tao Xu <[email protected]> Signed-off-by: Sean Christopherson <[email protected]> Message-Id: <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 2dbebf7 commit bf09fb6

File tree

3 files changed

+0
-26
lines changed

3 files changed

+0
-26
lines changed

arch/x86/include/asm/mwait.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@
2525
#define TPAUSE_C01_STATE 1
2626
#define TPAUSE_C02_STATE 0
2727

28-
u32 get_umwait_control_msr(void);
29-
3028
static inline void __monitor(const void *eax, unsigned long ecx,
3129
unsigned long edx)
3230
{

arch/x86/kernel/cpu/umwait.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,6 @@
1818
*/
1919
static u32 umwait_control_cached = UMWAIT_CTRL_VAL(100000, UMWAIT_C02_ENABLE);
2020

21-
u32 get_umwait_control_msr(void)
22-
{
23-
return umwait_control_cached;
24-
}
25-
EXPORT_SYMBOL_GPL(get_umwait_control_msr);
26-
2721
/*
2822
* Cache the original IA32_UMWAIT_CONTROL MSR value which is configured by
2923
* hardware or BIOS before kernel boot.

arch/x86/kvm/vmx/vmx.c

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6606,23 +6606,6 @@ static void atomic_switch_perf_msrs(struct vcpu_vmx *vmx)
66066606
msrs[i].host, false);
66076607
}
66086608

6609-
static void atomic_switch_umwait_control_msr(struct vcpu_vmx *vmx)
6610-
{
6611-
u32 host_umwait_control;
6612-
6613-
if (!vmx_has_waitpkg(vmx))
6614-
return;
6615-
6616-
host_umwait_control = get_umwait_control_msr();
6617-
6618-
if (vmx->msr_ia32_umwait_control != host_umwait_control)
6619-
add_atomic_switch_msr(vmx, MSR_IA32_UMWAIT_CONTROL,
6620-
vmx->msr_ia32_umwait_control,
6621-
host_umwait_control, false);
6622-
else
6623-
clear_atomic_switch_msr(vmx, MSR_IA32_UMWAIT_CONTROL);
6624-
}
6625-
66266609
static void vmx_update_hv_timer(struct kvm_vcpu *vcpu)
66276610
{
66286611
struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -6729,7 +6712,6 @@ static fastpath_t vmx_vcpu_run(struct kvm_vcpu *vcpu)
67296712
pt_guest_enter(vmx);
67306713

67316714
atomic_switch_perf_msrs(vmx);
6732-
atomic_switch_umwait_control_msr(vmx);
67336715

67346716
if (enable_preemption_timer)
67356717
vmx_update_hv_timer(vcpu);

0 commit comments

Comments
 (0)