All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] kvm: better MWAIT emulation for guests
@ 2017-03-13 16:19 Michael S. Tsirkin
  2017-03-13 20:18 ` kvm-kmod (Re: [PATCH v2] kvm: better MWAIT emulation for guests) Gabriel L. Somlo
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2017-03-13 16:19 UTC (permalink / raw)
  To: linux-kernel
  Cc: Gabriel L. Somlo, Paolo Bonzini, Radim Krčmář,
	Jonathan Corbet, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	x86, kvm, linux-doc

Guests running Mac OS 5, 6, and 7 (Leopard through Lion) have a problem:
unless explicitly provided with kernel command line argument
"idlehalt=0" they'd implicitly assume MONITOR and MWAIT availability,
without checking CPUID.

We currently emulate that as a NOP but on VMX we can do better: let
guest stop the CPU until timer, IPI or memory change.  CPU will be busy
but that isn't any worse than a NOP emulation.

Note that mwait within guests is not the same as on real hardware
because halt causes an exit while mwait doesn't.  For this reason it
might not be a good idea to use the regular MWAIT flag in CPUID to
signal this capability: halt is sometimes better as you are giving up
the rest of your CPU slice.  Add a flag in the hypervisor leaf instead.

Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

changes from v1:
- typo fix resulting in rest of leaf flags being overwritten
  Reported by: Wanpeng Li <kernellwp@gmail.com>
- updated commit log with data about guests helped by this feature
- better document differences between mwait and halt for guests
  


 Documentation/virtual/kvm/cpuid.txt  | 3 +++
 arch/x86/include/uapi/asm/kvm_para.h | 1 +
 arch/x86/kvm/cpuid.c                 | 3 +++
 arch/x86/kvm/vmx.c                   | 4 ----
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index 3c65feb..5caa234 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -54,6 +54,9 @@ KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
                                    ||       || before enabling paravirtualized
                                    ||       || spinlock support.
 ------------------------------------------------------------------------------
+KVM_FEATURE_MWAIT                  ||     8 || guest can use monitor/mwait
+                                   ||       || to halt the VCPU.
+------------------------------------------------------------------------------
 KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||    24 || host will warn if no guest-side
                                    ||       || per-cpu warps are expected in
                                    ||       || kvmclock.
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index cff0bb6..9cc77a7 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -24,6 +24,7 @@
 #define KVM_FEATURE_STEAL_TIME		5
 #define KVM_FEATURE_PV_EOI		6
 #define KVM_FEATURE_PV_UNHALT		7
+#define KVM_FEATURE_MWAIT		8
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index efde6cc..3c7fca83 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -594,6 +594,9 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		if (sched_info_on())
 			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
 
+		if (this_cpu_has(X86_FEATURE_MWAIT))
+			entry->eax |= (1 << KVM_FEATURE_MWAIT);
+
 		entry->ebx = 0;
 		entry->ecx = 0;
 		entry->edx = 0;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4bfe349..b167aba 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3547,13 +3547,9 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	      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;
 
-	printk(KERN_ERR "cleared CPU_BASED_MWAIT_EXITING + CPU_BASED_MONITOR_EXITING\n");
-
 	opt = CPU_BASED_TPR_SHADOW |
 	      CPU_BASED_USE_MSR_BITMAPS |
 	      CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
-- 
MST

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* kvm-kmod (Re: [PATCH v2] kvm: better MWAIT emulation for guests)
  2017-03-13 16:19 [PATCH v2] kvm: better MWAIT emulation for guests Michael S. Tsirkin
@ 2017-03-13 20:18 ` Gabriel L. Somlo
  2017-03-14 10:53   ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: Gabriel L. Somlo @ 2017-03-13 20:18 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Michael S. Tsirkin, linux-kernel

Paolo,

I'd like to test Michael's MWAIT patch on my copies of the affected
OS X versions, and wanted to use kvm-kmod to build the latest KVM on
my F24 box. I did:

git clone git://git.kernel.org/pub/scm/virt/kvm/kvm.git
git clone https://github.com/bonzini/kvm-kmod.git
cd kvm-kmod
./configure
make LINUX=../kvm clean sync all

Then, I get a bunch of errors:

make -C /lib/modules/4.9.10-100.fc24.x86_64/build M=`pwd` clean
make[1]: Entering directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
make[1]: Leaving directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
./sync -v  for-linus -l ../kvm
make -C /lib/modules/4.9.10-100.fc24.x86_64/build M=`pwd` \
        LINUXINCLUDE="-I`pwd`/include -I`pwd`/include/uapi -Iinclude \
                 -Iinclude2 -I/lib/modules/4.9.10-100.fc24.x86_64/source/include -I/lib/modules/4.9.10-100.fc24.x86_64/source/include/uapi -I/lib/modules/4.9.10-100.fc24.x86_64/source/arch/x86/include -I/lib/modules/4.9.10-100.fc24.x86_64/source/arch/x86/include/uapi \
                -Iinclude/generated/uapi -Iarch/x86/include/generated \
                -Iarch/x86/include/generated/uapi \
                -I`pwd`/include-compat -I`pwd`/x86 \
                -include  include/generated/autoconf.h \
                -include `pwd`/x86/external-module-compat.h" \
        "$@"
make[1]: Entering directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
  LD      /home/somlo/FOO/kvm-kmod/x86/built-in.o
  CC [M]  /home/somlo/FOO/kvm-kmod/x86/kvm_main.o
In file included from /home/somlo/FOO/kvm-kmod/x86/external-module-compat.h:46:,
                 from <command-line>:0:
/home/somlo/FOO/kvm-kmod/x86/../external-module-compat-comm.h:1724:53: warning:struct static_key_deferred’ declared inside parameter list will not be visible outside of this definition or declaration
 static inline void static_key_deferred_flush(struct static_key_deferred *key)
                                                     ^~~~~~~~~~~~~~~~~~~
/home/somlo/FOO/kvm-kmod/x86/../external-module-compat-comm.h: In function ‘static_key_deferred_flush’:
/home/somlo/FOO/kvm-kmod/x86/../external-module-compat-comm.h:1726:25: error: dereferencing pointer to incomplete type ‘struct static_key_deferred’
  flush_delayed_work(&key->work);
                         ^~
In file included from <command-line>:0:0:
/home/somlo/FOO/kvm-kmod/x86/external-module-compat.h: At top level:
/home/somlo/FOO/kvm-kmod/x86/external-module-compat.h:1090:22: fatal error: asm/i387.h: No such file or directory
 #include <asm/i387.h>
                      ^
compilation terminated.
scripts/Makefile.build:293: recipe for target '/home/somlo/FOO/kvm-kmod/x86/kvm_main.o' failed
make[3]: *** [/home/somlo/FOO/kvm-kmod/x86/kvm_main.o] Error 1
scripts/Makefile.build:544: recipe for target '/home/somlo/FOO/kvm-kmod/x86' failed
make[2]: *** [/home/somlo/FOO/kvm-kmod/x86] Error 2
Makefile:1494: recipe for target '_module_/home/somlo/FOO/kvm-kmod' failed
make[1]: *** [_module_/home/somlo/FOO/kvm-kmod] Error 2
make[1]: Leaving directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
Makefile:21: recipe for target 'all' failed
make: *** [all] Error 2

Any idea where things might be going wrong?
Is F24 (4.9.10-100.fc24.x86_64) too old for this?

Thanks,
--Gabriel

On Mon, Mar 13, 2017 at 06:19:07PM +0200, Michael S. Tsirkin wrote:
> Guests running Mac OS 5, 6, and 7 (Leopard through Lion) have a problem:
> unless explicitly provided with kernel command line argument
> "idlehalt=0" they'd implicitly assume MONITOR and MWAIT availability,
> without checking CPUID.
> 
> We currently emulate that as a NOP but on VMX we can do better: let
> guest stop the CPU until timer, IPI or memory change.  CPU will be busy
> but that isn't any worse than a NOP emulation.
> 
> Note that mwait within guests is not the same as on real hardware
> because halt causes an exit while mwait doesn't.  For this reason it
> might not be a good idea to use the regular MWAIT flag in CPUID to
> signal this capability: halt is sometimes better as you are giving up
> the rest of your CPU slice.  Add a flag in the hypervisor leaf instead.
> 
> Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> changes from v1:
> - typo fix resulting in rest of leaf flags being overwritten
>   Reported by: Wanpeng Li <kernellwp@gmail.com>
> - updated commit log with data about guests helped by this feature
> - better document differences between mwait and halt for guests
>   
> 
> 
>  Documentation/virtual/kvm/cpuid.txt  | 3 +++
>  arch/x86/include/uapi/asm/kvm_para.h | 1 +
>  arch/x86/kvm/cpuid.c                 | 3 +++
>  arch/x86/kvm/vmx.c                   | 4 ----
>  4 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
> index 3c65feb..5caa234 100644
> --- a/Documentation/virtual/kvm/cpuid.txt
> +++ b/Documentation/virtual/kvm/cpuid.txt
> @@ -54,6 +54,9 @@ KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
>                                     ||       || before enabling paravirtualized
>                                     ||       || spinlock support.
>  ------------------------------------------------------------------------------
> +KVM_FEATURE_MWAIT                  ||     8 || guest can use monitor/mwait
> +                                   ||       || to halt the VCPU.
> +------------------------------------------------------------------------------
>  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||    24 || host will warn if no guest-side
>                                     ||       || per-cpu warps are expected in
>                                     ||       || kvmclock.
> diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
> index cff0bb6..9cc77a7 100644
> --- a/arch/x86/include/uapi/asm/kvm_para.h
> +++ b/arch/x86/include/uapi/asm/kvm_para.h
> @@ -24,6 +24,7 @@
>  #define KVM_FEATURE_STEAL_TIME		5
>  #define KVM_FEATURE_PV_EOI		6
>  #define KVM_FEATURE_PV_UNHALT		7
> +#define KVM_FEATURE_MWAIT		8
>  
>  /* The last 8 bits are used to indicate how to interpret the flags field
>   * in pvclock structure. If no bits are set, all flags are ignored.
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index efde6cc..3c7fca83 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -594,6 +594,9 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		if (sched_info_on())
>  			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
>  
> +		if (this_cpu_has(X86_FEATURE_MWAIT))
> +			entry->eax |= (1 << KVM_FEATURE_MWAIT);
> +
>  		entry->ebx = 0;
>  		entry->ecx = 0;
>  		entry->edx = 0;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 4bfe349..b167aba 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3547,13 +3547,9 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  	      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;
>  
> -	printk(KERN_ERR "cleared CPU_BASED_MWAIT_EXITING + CPU_BASED_MONITOR_EXITING\n");
> -
>  	opt = CPU_BASED_TPR_SHADOW |
>  	      CPU_BASED_USE_MSR_BITMAPS |
>  	      CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
> -- 
> MST

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: kvm-kmod (Re: [PATCH v2] kvm: better MWAIT emulation for guests)
  2017-03-13 20:18 ` kvm-kmod (Re: [PATCH v2] kvm: better MWAIT emulation for guests) Gabriel L. Somlo
@ 2017-03-14 10:53   ` Paolo Bonzini
  0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2017-03-14 10:53 UTC (permalink / raw)
  To: Gabriel L. Somlo; +Cc: Michael S. Tsirkin, linux-kernel



On 13/03/2017 21:18, Gabriel L. Somlo wrote:
> I'd like to test Michael's MWAIT patch on my copies of the affected
> OS X versions, and wanted to use kvm-kmod to build the latest KVM on
> my F24 box. I did:
> 
> git clone git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> git clone https://github.com/bonzini/kvm-kmod.git
> cd kvm-kmod
> ./configure
> make LINUX=../kvm clean sync all
> 
> Then, I get a bunch of errors:
> 
> make -C /lib/modules/4.9.10-100.fc24.x86_64/build M=`pwd` clean
> make[1]: Entering directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
> make[1]: Leaving directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
> ./sync -v  for-linus -l ../kvm
> make -C /lib/modules/4.9.10-100.fc24.x86_64/build M=`pwd` \
>         LINUXINCLUDE="-I`pwd`/include -I`pwd`/include/uapi -Iinclude \
>                  -Iinclude2 -I/lib/modules/4.9.10-100.fc24.x86_64/source/include -I/lib/modules/4.9.10-100.fc24.x86_64/source/include/uapi -I/lib/modules/4.9.10-100.fc24.x86_64/source/arch/x86/include -I/lib/modules/4.9.10-100.fc24.x86_64/source/arch/x86/include/uapi \
>                 -Iinclude/generated/uapi -Iarch/x86/include/generated \
>                 -Iarch/x86/include/generated/uapi \
>                 -I`pwd`/include-compat -I`pwd`/x86 \
>                 -include  include/generated/autoconf.h \
>                 -include `pwd`/x86/external-module-compat.h" \
>         "$@"
> make[1]: Entering directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
>   LD      /home/somlo/FOO/kvm-kmod/x86/built-in.o
>   CC [M]  /home/somlo/FOO/kvm-kmod/x86/kvm_main.o
> In file included from /home/somlo/FOO/kvm-kmod/x86/external-module-compat.h:46:,
>                  from <command-line>:0:
> /home/somlo/FOO/kvm-kmod/x86/../external-module-compat-comm.h:1724:53: warning:struct static_key_deferred’ declared inside parameter list will not be visible outside of this definition or declaration
>  static inline void static_key_deferred_flush(struct static_key_deferred *key)
>                                                      ^~~~~~~~~~~~~~~~~~~
> /home/somlo/FOO/kvm-kmod/x86/../external-module-compat-comm.h: In function ‘static_key_deferred_flush’:
> /home/somlo/FOO/kvm-kmod/x86/../external-module-compat-comm.h:1726:25: error: dereferencing pointer to incomplete type ‘struct static_key_deferred’
>   flush_delayed_work(&key->work);
>                          ^~
> In file included from <command-line>:0:0:
> /home/somlo/FOO/kvm-kmod/x86/external-module-compat.h: At top level:
> /home/somlo/FOO/kvm-kmod/x86/external-module-compat.h:1090:22: fatal error: asm/i387.h: No such file or directory
>  #include <asm/i387.h>
>                       ^
> compilation terminated.
> scripts/Makefile.build:293: recipe for target '/home/somlo/FOO/kvm-kmod/x86/kvm_main.o' failed
> make[3]: *** [/home/somlo/FOO/kvm-kmod/x86/kvm_main.o] Error 1
> scripts/Makefile.build:544: recipe for target '/home/somlo/FOO/kvm-kmod/x86' failed
> make[2]: *** [/home/somlo/FOO/kvm-kmod/x86] Error 2
> Makefile:1494: recipe for target '_module_/home/somlo/FOO/kvm-kmod' failed
> make[1]: *** [_module_/home/somlo/FOO/kvm-kmod] Error 2
> make[1]: Leaving directory '/usr/src/kernels/4.9.10-100.fc24.x86_64'
> Makefile:21: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> Any idea where things might be going wrong?
> Is F24 (4.9.10-100.fc24.x86_64) too old for this?

Let me push a newer version of kvm-kmod.  But I only tested it with 4.11.

Paolo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-14 10:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 16:19 [PATCH v2] kvm: better MWAIT emulation for guests Michael S. Tsirkin
2017-03-13 20:18 ` kvm-kmod (Re: [PATCH v2] kvm: better MWAIT emulation for guests) Gabriel L. Somlo
2017-03-14 10:53   ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.