mbox series

[V3,0/2] improve -overcommit cpu-pm=on|off

Message ID 20240604000222.75065-1-zide.chen@intel.com (mailing list archive)
Headers show
Series improve -overcommit cpu-pm=on|off | expand

Message

Chen, Zide June 4, 2024, 12:02 a.m. UTC
Currently, if running "-overcommit cpu-pm=on" on hosts that don't
have MWAIT support, the MWAIT/MONITOR feature is advertised to the
guest and executing MWAIT/MONITOR on the guest triggers #UD.

Typically #UD takes priority over VM-Exit interception checks and
KVM doesn't emulate MONITOR/MWAIT. This causes the guest fail to
boot.

V2:
- [PATCH 1]: took Thomas' suggestion for more generic fix
- [PATCH 2/3]: no changes

V3:
- dropped [PATCH 1/3]. Took the simpler approach not to re-order
  cpu_exec_realizefn() call.
- changed patch title in [PATCH V3 1/2]
- don't set CPUID_EXT_MONITOR in kvm_cpu_realizefn() 

Zide Chen (2):
  vl: Allow multiple -overcommit commands
  target/i386: Advertise MWAIT iff host supports

 system/vl.c               |  4 ++--
 target/i386/host-cpu.c    | 12 ------------
 target/i386/kvm/kvm-cpu.c | 11 +++++++++--
 3 files changed, 11 insertions(+), 16 deletions(-)

Comments

Igor Mammedov June 5, 2024, 1:49 p.m. UTC | #1
On Mon,  3 Jun 2024 17:02:20 -0700
Zide Chen <zide.chen@intel.com> wrote:

> Currently, if running "-overcommit cpu-pm=on" on hosts that don't
> have MWAIT support, the MWAIT/MONITOR feature is advertised to the
> guest and executing MWAIT/MONITOR on the guest triggers #UD.
> 
> Typically #UD takes priority over VM-Exit interception checks and
> KVM doesn't emulate MONITOR/MWAIT. This causes the guest fail to
> boot.
> 
> V2:
> - [PATCH 1]: took Thomas' suggestion for more generic fix
> - [PATCH 2/3]: no changes
> 
> V3:
> - dropped [PATCH 1/3]. Took the simpler approach not to re-order
>   cpu_exec_realizefn() call.
> - changed patch title in [PATCH V3 1/2]
> - don't set CPUID_EXT_MONITOR in kvm_cpu_realizefn() 

on top of above we should make make
  -overcommit cpu-pm=on
to error out if KVM_X86_DISABLE_EXITS_MWAIT is not supported/failed

if we don't do this user gets false assumption that cpu-pm=on
works as expected, and instead of effective CPU usage/IPI delivery
all they get is a storm of mwait exits.

> 
> Zide Chen (2):
>   vl: Allow multiple -overcommit commands
>   target/i386: Advertise MWAIT iff host supports
> 
>  system/vl.c               |  4 ++--
>  target/i386/host-cpu.c    | 12 ------------
>  target/i386/kvm/kvm-cpu.c | 11 +++++++++--
>  3 files changed, 11 insertions(+), 16 deletions(-)
>
Chen, Zide June 5, 2024, 6:33 p.m. UTC | #2
On 6/5/2024 6:49 AM, Igor Mammedov wrote:
> On Mon,  3 Jun 2024 17:02:20 -0700
> Zide Chen <zide.chen@intel.com> wrote:
> 
>> Currently, if running "-overcommit cpu-pm=on" on hosts that don't
>> have MWAIT support, the MWAIT/MONITOR feature is advertised to the
>> guest and executing MWAIT/MONITOR on the guest triggers #UD.
>>
>> Typically #UD takes priority over VM-Exit interception checks and
>> KVM doesn't emulate MONITOR/MWAIT. This causes the guest fail to
>> boot.
>>
>> V2:
>> - [PATCH 1]: took Thomas' suggestion for more generic fix
>> - [PATCH 2/3]: no changes
>>
>> V3:
>> - dropped [PATCH 1/3]. Took the simpler approach not to re-order
>>   cpu_exec_realizefn() call.
>> - changed patch title in [PATCH V3 1/2]
>> - don't set CPUID_EXT_MONITOR in kvm_cpu_realizefn() 
> 
> on top of above we should make make
>   -overcommit cpu-pm=on
> to error out if KVM_X86_DISABLE_EXITS_MWAIT is not supported/failed
> 
> if we don't do this user gets false assumption that cpu-pm=on
> works as expected, and instead of effective CPU usage/IPI delivery
> all they get is a storm of mwait exits.
> 

Agree. But seems it's quite complicated. Please refer to the comments on
[PATCH V2 2/3].

We may remove the "-overcommit cpu-pm", and similar to notify-vmexit,
add individual -accel options for flexibility and better cpu-pm control.
But will be over complicated?

KVM_X86_DISABLE_EXITS_MWAIT
KVM_X86_DISABLE_EXITS_HLT
KVM_X86_DISABLE_EXITS_PAUSE
KVM_X86_DISABLE_EXITS_CSTATE


>>
>> Zide Chen (2):
>>   vl: Allow multiple -overcommit commands
>>   target/i386: Advertise MWAIT iff host supports
>>
>>  system/vl.c               |  4 ++--
>>  target/i386/host-cpu.c    | 12 ------------
>>  target/i386/kvm/kvm-cpu.c | 11 +++++++++--
>>  3 files changed, 11 insertions(+), 16 deletions(-)
>>
>
Michael Tokarev June 17, 2024, 12:47 p.m. UTC | #3
04.06.2024 03:02, Zide Chen wrote:
> Currently, if running "-overcommit cpu-pm=on" on hosts that don't
> have MWAIT support, the MWAIT/MONITOR feature is advertised to the
> guest and executing MWAIT/MONITOR on the guest triggers #UD.
> 
> Typically #UD takes priority over VM-Exit interception checks and
> KVM doesn't emulate MONITOR/MWAIT. This causes the guest fail to
> boot.

Applied to trivial-patches, thanks!

/mjt