Re: [PATCH v5 untested] kvm: better MWAIT emulation for guests

[Michael S. Tsirkin]

On Thu, Mar 16, 2017 at 09:24:27AM -0400, Gabriel L. Somlo wrote:
> After studying your patch a bit more carefully (sorry, it's crazy
> around here right now :) ) I realized you're simply trying to
> (selectively) decide when to exit L1 and emulate as NOP vs. when to
> just allow L1 to execute MONITOR & MWAIT natively.
> 
> Is that right ? Because if so, the issues I saw on my MacPro1,1 are
> weird and inexplicable, given that allowing L>=1 to run MONITOR/MWAIT
> natively was one of the options Alex Graf and Rene Rebe used back in
> the very early days of OS X on QEMU, at the time I got involved with
> that project. Here's part of an out of tree patch against 3.4 which did
> just that, and worked as far as I remember on *any* MWAIT capable
> intel chip I had access to back in 2010:
> 
> ##############################################################################
> # 99-mwait.patch.kvm-kmod (Rene Rebe <rene@xxxxxxxxxxxx>) 2010-04-27
> ##############################################################################
> diff -pNarU5 linux-3.4/arch/x86/kvm/cpuid.c linux-3.4-mac/arch/x86/kvm/cpuid.c
> --- linux-3.4/arch/x86/kvm/cpuid.c	2012-05-20 18:29:13.000000000 -0400
> +++ linux-3.4-mac/arch/x86/kvm/cpuid.c	2012-10-09 11:42:59.921215750 -0400
> @@ -222,11 +222,11 @@ static int do_cpuid_ent(struct kvm_cpuid
>  		f_nx | 0 /* Reserved */ | F(MMXEXT) | F(MMX) |
>  		F(FXSR) | F(FXSR_OPT) | f_gbpages | f_rdtscp |
>  		0 /* Reserved */ | f_lm | F(3DNOWEXT) | F(3DNOW);
>  	/* cpuid 1.ecx */
>  	const u32 kvm_supported_word4_x86_features =
> -		F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
> +		F(XMM3) | F(PCLMULQDQ) | F(MWAIT) /* DTES64, MONITOR */ |
>  		0 /* DS-CPL, VMX, SMX, EST */ |
>  		0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /* Reserved */ |
>  		F(FMA) | F(CX16) | 0 /* xTPR Update, PDCM */ |
>  		0 /* Reserved, DCA */ | F(XMM4_1) |
>  		F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
> diff -pNarU5 linux-3.4/arch/x86/kvm/svm.c linux-3.4-mac/arch/x86/kvm/svm.c
> --- linux-3.4/arch/x86/kvm/svm.c	2012-05-20 18:29:13.000000000 -0400
> +++ linux-3.4-mac/arch/x86/kvm/svm.c	2012-10-09 11:44:41.598997481 -0400
> @@ -1102,12 +1102,10 @@ static void init_vmcb(struct vcpu_svm *s
>  	set_intercept(svm, INTERCEPT_VMSAVE);
>  	set_intercept(svm, INTERCEPT_STGI);
>  	set_intercept(svm, INTERCEPT_CLGI);
>  	set_intercept(svm, INTERCEPT_SKINIT);
>  	set_intercept(svm, INTERCEPT_WBINVD);
> -	set_intercept(svm, INTERCEPT_MONITOR);
> -	set_intercept(svm, INTERCEPT_MWAIT);
>  	set_intercept(svm, INTERCEPT_XSETBV);
>  
>  	control->iopm_base_pa = iopm_base;
>  	control->msrpm_base_pa = __pa(svm->msrpm);
>  	control->int_ctl = V_INTR_MASKING_MASK;
> diff -pNarU5 linux-3.4/arch/x86/kvm/vmx.c linux-3.4-mac/arch/x86/kvm/vmx.c
> --- linux-3.4/arch/x86/kvm/vmx.c	2012-05-20 18:29:13.000000000 -0400
> +++ linux-3.4-mac/arch/x86/kvm/vmx.c	2012-10-09 11:42:59.925215977 -0400
> @@ -1938,11 +1938,11 @@ static __init void nested_vmx_setup_ctls
>  		nested_vmx_procbased_ctls_low, nested_vmx_procbased_ctls_high);
>  	nested_vmx_procbased_ctls_low = 0;
>  	nested_vmx_procbased_ctls_high &=
>  		CPU_BASED_VIRTUAL_INTR_PENDING | CPU_BASED_USE_TSC_OFFSETING |
>  		CPU_BASED_HLT_EXITING | CPU_BASED_INVLPG_EXITING |
> -		CPU_BASED_MWAIT_EXITING | CPU_BASED_CR3_LOAD_EXITING |
> +		CPU_BASED_CR3_LOAD_EXITING |
>  		CPU_BASED_CR3_STORE_EXITING |
>  #ifdef CONFIG_X86_64
>  		CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING |
>  #endif
>  		CPU_BASED_MOV_DR_EXITING | CPU_BASED_UNCOND_IO_EXITING |
> @@ -2404,12 +2404,10 @@ static __init int setup_vmcs_config(stru
>  	      CPU_BASED_CR3_LOAD_EXITING |
>  	      CPU_BASED_CR3_STORE_EXITING |
>  	      CPU_BASED_USE_IO_BITMAPS |
>  	      CPU_BASED_MOV_DR_EXITING |
>  	      CPU_BASED_USE_TSC_OFFSETING |
> -	      CPU_BASED_MWAIT_EXITING |
> -	      CPU_BASED_MONITOR_EXITING |
>  	      CPU_BASED_INVLPG_EXITING |
>  	      CPU_BASED_RDPMC_EXITING;
>  
>  	opt = CPU_BASED_TPR_SHADOW |
>  	      CPU_BASED_USE_MSR_BITMAPS |
> 
> If all you're trying to do is (selectively) revert to this behavior,
> that "shouldn't" mess it up for the MacPro either, so I'm thoroughly
> confused at this point :)

Yes.  Me too. Want to try that other patch and see what happens?

> Back in 2010, running MWAIT in L>=1  behaved 100% exactly like a NOP,
> didn't power down the physical CPU, just immediately moved on to the
> next instruction. As such, there was no power saving and no
> opportunity to yield to another L0 thread either, unlike with NOP
> emulation at L0.
> 
> Did that change on newer Intel chips (i.e., is guest-mode MWAIT now
> doing something smarter than just acting as a guest-mode NOP) ?
> 
> Thanks,
> --Gabriel

Interesting.  What it seems to say is this:

MWAIT. Behavior of the MWAIT instruction (which always causes an invalid-
opcode exceptionâ#UDâif CPL > 0) is determined by the setting of the âMWAIT
exitingâ VM-execution control:
â If the âMWAIT exitingâ VM-execution control is 1, MWAIT causes a VM exit
(see Section 22.1.3).
â If the âMWAIT exitingâ VM-execution control is 0, MWAIT operates normally if
any of the following is true: (1) the âinterrupt-window exitingâ VM-execution
control is 0; (2) ECX[0] is 0; or (3) RFLAGS.IF = 1.
â If the âMWAIT exitingâ VM-execution control is 0, the âinterrupt-window
exitingâ VM-execution control is 1, ECX[0] = 1, and RFLAGS.IF = 0, MWAIT
does not cause the processor to enter an implementation-dependent
optimized state; instead, control passes to the instruction following the
MWAIT instruction.


And since interrupt-window exiting is 0 most of the time for KVM,
I would expect MWAIT to behave normally.


-- 
MST