On Mon, Mar 27, 2017 at 06:42:49PM +0200, Alexander Graf wrote: > > > On 27/03/2017 18:33, Eduardo Habkost wrote: > > On Mon, Mar 27, 2017 at 06:10:43PM +0200, Alexander Graf wrote: > > > > > > > > > On 27/03/2017 17:46, Eduardo Habkost wrote: > > > > On Mon, Mar 27, 2017 at 04:26:50PM +0200, Alexander Graf wrote: > > > > > KVM allows trap and emulate (read: NOP) of the MONITOR and MWAIT > > > > > instructions. There is work undergoing to enable actual execution > > > > > of these inside of KVM, but nobody really wants to expose the feature > > > > > to the guest by default, as it would eat up all of the host CPU. > > > > > > > > Isn't this something that should be reported using > > > > KVM_GET_EMULATED_CPUID? (QEMU still doesn't know how to use > > > > KVM_GET_EMULATED_CPUID, however.) > > > > > > Depends how you look at it. In KVM land there are basically 3 ways to deal > > > with MONITOR/MWAIT: > > > > > > 1) #VMEXIT on every execution, treat them as NOP > > > 2) let the guest natively execute them (looks like a busy loop for the > > > host, but saves power) > > > 3) be smart in KVM about it, add actual emulation and adaptively allow for > > > native mwait execution or emulated mwait which means we can run inside host > > > context > > > > Which case is going to be enabled when using your patch? Don't > > you want to make that configurable? If the answer to this question is "no", then we can ignore all the rest of my previous message. :) > > > > This looks like configuration that can't be easily represented > > using the GET_SUPPORTED_CPUID/KVM_SET_CPUID abstraction, as the > > ability to enable the feature can't be represented by a single > > bit. > > > > We already have a few cases where we make > > kvm_arch_get_supported_cpuid() look at something other than just > > GET_SUPPORTED_CPUID return values (most cases are related to > > in-kernel irqchip). We can change kvm_arch_get_supported_cpuid() > > to look at other flags (like one that would configure mwait > > behavior), and decide to report CPUID_EXT_MONITOR on those cases. > > > > Would something like the following make sense? > > > > * Having a "mwait=(none|nop|native|smart)" option, to choose > > mwait behavior > > * Making kvm_arch_get_supported_cpuid() return CPUID_EXT_MONITOR > > as supported only if the "mwait" option is not "none". > > > > Then: > > * "-cpu ...,monitor=on" would fail, because mwait=none would be > > the default > > * "-cpu ...,mwait=none,monitor=on" would fail > > * "-cpu ...,mwait=(nop|native|smart)" would fail if > > host KVM doesn't report the corresponding mwait behavior as > > supported > > * "-cpu ...,mwait=(nop|native|smart),monitor=on" would work > > * "-cpu ...,mwait=(nop|native|smart)" would also work, and > > probably should enable CPUID_EXT_MONITOR flag by default > > > > We could still implement "monitor=force" for debugging, but I > > think we need something safer than "just ignore what KVM is > > telling us" for production. > > I personally think we want the =force for any cpuid bit that we want to > randomly poke into the expose CPUID bitmap and if a customer is daring > enough, leave that to him as an option. > > For anything else, I wouldn't overengineer this interface. I think > eventually we want to just natively support emulated mwait with proper > adaptive logic on when to trap vs execute mwait inside the guest (as > mentioned in the kvm thread on this). Once that's implemented, KVM can just > expose the MONITOR bit as supported and everyone gets it automatically. > > If at that point, the adaptive interface performs much worse than the naive > one, we can still think about pushing this into an enable_cap and add a > specific mwait= option. Sounds good to me. I thought the user still had to choose between the options above somehow, and that KVM was not going to return MONITOR on GET_SUPPORTED_CPUID because of that. If getting adaptative logic that's good enough to add MONITOR to GET_SUPPORTED_CPUID is still a possibility, then that's even better. -- Eduardo
