kvm-kmod (Re: [PATCH v2] kvm: better MWAIT emulation for guests)

From: Gabriel L. Somlo
Date: Mon Mar 13 2017 - 16:18:52 EST


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@xxxxxxxxx>
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> ---
>
> changes from v1:
> - typo fix resulting in rest of leaf flags being overwritten
> Reported by: Wanpeng Li <kernellwp@xxxxxxxxx>
> - 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