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

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

[Zide Chen]

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(-)

-- 
2.34.1

[PATCH V3 1/2] vl: Allow multiple -overcommit commands

[Zide Chen]

Both cpu-pm and mem-lock are related to system resource overcommit, but
they are separate from each other, in terms of how they are realized,
and of course, they are applied to different system resources.

It's tempting to use separate command lines to specify their behavior.
e.g., in the following example, the cpu-pm command is quietly
overwritten, and it's not easy to notice it without careful inspection.

  --overcommit mem-lock=on
  --overcommit cpu-pm=on

Fixes: c8c9dc42b7ca ("Remove the deprecated -realtime option")
Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Zide Chen <zide.chen@intel.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
---

V3: added Reviewed-by

v2:
Thanks to Thomas' suggestion, changed to this better approach, which
is more generic and can handle situations like: "enabled the option in
the config file, and now you'd like to disable it on the command line
again".

 system/vl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index a3eede5fa5b8..dfa6cdd9283b 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3545,8 +3545,8 @@ void qemu_init(int argc, char **argv)
                 if (!opts) {
                     exit(1);
                 }
-                enable_mlock = qemu_opt_get_bool(opts, "mem-lock", false);
-                enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", false);
+                enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
+                enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
                 break;
             case QEMU_OPTION_compat:
                 {
-- 
2.34.1

Re: [PATCH V3 1/2] vl: Allow multiple -overcommit commands

[Igor Mammedov]

On Mon,  3 Jun 2024 17:02:21 -0700
Zide Chen <zide.chen@intel.com> wrote:

> Both cpu-pm and mem-lock are related to system resource overcommit, but
> they are separate from each other, in terms of how they are realized,
> and of course, they are applied to different system resources.
> 
> It's tempting to use separate command lines to specify their behavior.
> e.g., in the following example, the cpu-pm command is quietly
> overwritten, and it's not easy to notice it without careful inspection.
> 
>   --overcommit mem-lock=on
>   --overcommit cpu-pm=on
> 
> Fixes: c8c9dc42b7ca ("Remove the deprecated -realtime option")
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Zide Chen <zide.chen@intel.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> 
> V3: added Reviewed-by
> 
> v2:
> Thanks to Thomas' suggestion, changed to this better approach, which
> is more generic and can handle situations like: "enabled the option in
> the config file, and now you'd like to disable it on the command line
> again".
> 
>  system/vl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/system/vl.c b/system/vl.c
> index a3eede5fa5b8..dfa6cdd9283b 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -3545,8 +3545,8 @@ void qemu_init(int argc, char **argv)
>                  if (!opts) {
>                      exit(1);
>                  }
> -                enable_mlock = qemu_opt_get_bool(opts, "mem-lock", false);
> -                enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", false);
> +                enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
> +                enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
>                  break;
>              case QEMU_OPTION_compat:
>                  {

[PATCH V3 2/2] target/i386: Advertise MWAIT iff host supports

[Zide Chen]

host_cpu_realizefn() sets CPUID_EXT_MONITOR without consulting host/KVM
capabilities. This may cause problems:

- If MWAIT/MONITOR is not available on the host, advertising this
  feature to the guest and executing MWAIT/MONITOR from the guest
  triggers #UD and the guest doesn't boot.  This is because typically
  #UD takes priority over VM-Exit interception checks and KVM doesn't
  emulate MONITOR/MWAIT on #UD.

- If KVM doesn't support KVM_X86_DISABLE_EXITS_MWAIT, MWAIT/MONITOR
  from the guest are intercepted by KVM, which is not what cpu-pm=on
  intends to do.

In these cases, MWAIT/MONITOR should not be exposed to the guest.

The logic in kvm_arch_get_supported_cpuid() to handle CPUID_EXT_MONITOR
is correct and sufficient, and we can't set CPUID_EXT_MONITOR after
x86_cpu_filter_features().

This was not an issue before commit 662175b91ff ("i386: reorder call to
cpu_exec_realizefn") because the feature added in the accel-specific
realizefn could be checked against host availability and filtered out.

Additionally, it seems not a good idea to handle guest CPUID leaves in
host_cpu_realizefn(), and this patch merges host_cpu_enable_cpu_pm()
into kvm_cpu_realizefn().

Fixes: f5cc5a5c1686 ("i386: split cpu accelerators from cpu.c, using AccelCPUClass")
Fixes: 662175b91ff2 ("i386: reorder call to cpu_exec_realizefn")
Signed-off-by: Zide Chen <zide.chen@intel.com>
---

V3:
- don't set CPUID_EXT_MONITOR in kvm_cpu_realizefn().
- Change title to reflect the main purpose of this patch.

 target/i386/host-cpu.c    | 12 ------------
 target/i386/kvm/kvm-cpu.c | 11 +++++++++--
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
index 280e427c017c..8b8bf5afeccf 100644
--- a/target/i386/host-cpu.c
+++ b/target/i386/host-cpu.c
@@ -42,15 +42,6 @@ static uint32_t host_cpu_phys_bits(void)
     return host_phys_bits;
 }
 
-static void host_cpu_enable_cpu_pm(X86CPU *cpu)
-{
-    CPUX86State *env = &cpu->env;
-
-    host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
-               &cpu->mwait.ecx, &cpu->mwait.edx);
-    env->features[FEAT_1_ECX] |= CPUID_EXT_MONITOR;
-}
-
 static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
 {
     uint32_t host_phys_bits = host_cpu_phys_bits();
@@ -83,9 +74,6 @@ bool host_cpu_realizefn(CPUState *cs, Error **errp)
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
-    if (cpu->max_features && enable_cpu_pm) {
-        host_cpu_enable_cpu_pm(cpu);
-    }
     if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
         uint32_t phys_bits = host_cpu_adjust_phys_bits(cpu);
 
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index f76972e47e61..148d10ce3711 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -65,8 +65,15 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
      *   cpu_common_realizefn() (via xcc->parent_realize)
      */
     if (cpu->max_features) {
-        if (enable_cpu_pm && kvm_has_waitpkg()) {
-            env->features[FEAT_7_0_ECX] |= CPUID_7_0_ECX_WAITPKG;
+        if (enable_cpu_pm) {
+            if (kvm_has_waitpkg()) {
+                env->features[FEAT_7_0_ECX] |= CPUID_7_0_ECX_WAITPKG;
+            }
+
+            if (env->features[FEAT_1_ECX] & CPUID_EXT_MONITOR) {
+                host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
+                           &cpu->mwait.ecx, &cpu->mwait.edx);
+	    }
         }
         if (cpu->ucode_rev == 0) {
             cpu->ucode_rev =
-- 
2.34.1

Re: [PATCH V3 2/2] target/i386: Advertise MWAIT iff host supports

[Zhao Liu]

On Mon, Jun 03, 2024 at 05:02:22PM -0700, Zide Chen wrote:
> Date: Mon, 3 Jun 2024 17:02:22 -0700
> From: Zide Chen <zide.chen@intel.com>
> Subject: [PATCH V3 2/2] target/i386: Advertise MWAIT iff host supports
> X-Mailer: git-send-email 2.34.1
> 
> host_cpu_realizefn() sets CPUID_EXT_MONITOR without consulting host/KVM
> capabilities. This may cause problems:
> 
> - If MWAIT/MONITOR is not available on the host, advertising this
>   feature to the guest and executing MWAIT/MONITOR from the guest
>   triggers #UD and the guest doesn't boot.  This is because typically
>   #UD takes priority over VM-Exit interception checks and KVM doesn't
>   emulate MONITOR/MWAIT on #UD.
> 
> - If KVM doesn't support KVM_X86_DISABLE_EXITS_MWAIT, MWAIT/MONITOR
>   from the guest are intercepted by KVM, which is not what cpu-pm=on
>   intends to do.
> 
> In these cases, MWAIT/MONITOR should not be exposed to the guest.
> 
> The logic in kvm_arch_get_supported_cpuid() to handle CPUID_EXT_MONITOR
> is correct and sufficient, and we can't set CPUID_EXT_MONITOR after
> x86_cpu_filter_features().
> 
> This was not an issue before commit 662175b91ff ("i386: reorder call to
> cpu_exec_realizefn") because the feature added in the accel-specific
> realizefn could be checked against host availability and filtered out.
> 
> Additionally, it seems not a good idea to handle guest CPUID leaves in
> host_cpu_realizefn(), and this patch merges host_cpu_enable_cpu_pm()
> into kvm_cpu_realizefn().
> 
> Fixes: f5cc5a5c1686 ("i386: split cpu accelerators from cpu.c, using AccelCPUClass")
> Fixes: 662175b91ff2 ("i386: reorder call to cpu_exec_realizefn")
> Signed-off-by: Zide Chen <zide.chen@intel.com>
> ---

LGTM,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH V3 2/2] target/i386: Advertise MWAIT iff host supports

[Xiaoyao Li]

On 6/4/2024 8:02 AM, Zide Chen wrote:
> host_cpu_realizefn() sets CPUID_EXT_MONITOR without consulting host/KVM
> capabilities. This may cause problems:
> 
> - If MWAIT/MONITOR is not available on the host, advertising this
>    feature to the guest and executing MWAIT/MONITOR from the guest
>    triggers #UD and the guest doesn't boot.  This is because typically
>    #UD takes priority over VM-Exit interception checks and KVM doesn't
>    emulate MONITOR/MWAIT on #UD.
> 
> - If KVM doesn't support KVM_X86_DISABLE_EXITS_MWAIT, MWAIT/MONITOR
>    from the guest are intercepted by KVM, which is not what cpu-pm=on
>    intends to do.
> 
> In these cases, MWAIT/MONITOR should not be exposed to the guest.
> 
> The logic in kvm_arch_get_supported_cpuid() to handle CPUID_EXT_MONITOR
> is correct and sufficient, and we can't set CPUID_EXT_MONITOR after
> x86_cpu_filter_features().
> 
> This was not an issue before commit 662175b91ff ("i386: reorder call to
> cpu_exec_realizefn") because the feature added in the accel-specific
> realizefn could be checked against host availability and filtered out.
> 
> Additionally, it seems not a good idea to handle guest CPUID leaves in
> host_cpu_realizefn(), and this patch merges host_cpu_enable_cpu_pm()
> into kvm_cpu_realizefn().
> 
> Fixes: f5cc5a5c1686 ("i386: split cpu accelerators from cpu.c, using AccelCPUClass")
> Fixes: 662175b91ff2 ("i386: reorder call to cpu_exec_realizefn")
> Signed-off-by: Zide Chen <zide.chen@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
> 
> V3:
> - don't set CPUID_EXT_MONITOR in kvm_cpu_realizefn().
> - Change title to reflect the main purpose of this patch.
> 
>   target/i386/host-cpu.c    | 12 ------------
>   target/i386/kvm/kvm-cpu.c | 11 +++++++++--
>   2 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
> index 280e427c017c..8b8bf5afeccf 100644
> --- a/target/i386/host-cpu.c
> +++ b/target/i386/host-cpu.c
> @@ -42,15 +42,6 @@ static uint32_t host_cpu_phys_bits(void)
>       return host_phys_bits;
>   }
>   
> -static void host_cpu_enable_cpu_pm(X86CPU *cpu)
> -{
> -    CPUX86State *env = &cpu->env;
> -
> -    host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> -               &cpu->mwait.ecx, &cpu->mwait.edx);
> -    env->features[FEAT_1_ECX] |= CPUID_EXT_MONITOR;
> -}
> -
>   static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
>   {
>       uint32_t host_phys_bits = host_cpu_phys_bits();
> @@ -83,9 +74,6 @@ bool host_cpu_realizefn(CPUState *cs, Error **errp)
>       X86CPU *cpu = X86_CPU(cs);
>       CPUX86State *env = &cpu->env;
>   
> -    if (cpu->max_features && enable_cpu_pm) {
> -        host_cpu_enable_cpu_pm(cpu);
> -    }
>       if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
>           uint32_t phys_bits = host_cpu_adjust_phys_bits(cpu);
>   
> diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> index f76972e47e61..148d10ce3711 100644
> --- a/target/i386/kvm/kvm-cpu.c
> +++ b/target/i386/kvm/kvm-cpu.c
> @@ -65,8 +65,15 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
>        *   cpu_common_realizefn() (via xcc->parent_realize)
>        */
>       if (cpu->max_features) {
> -        if (enable_cpu_pm && kvm_has_waitpkg()) {
> -            env->features[FEAT_7_0_ECX] |= CPUID_7_0_ECX_WAITPKG;
> +        if (enable_cpu_pm) {
> +            if (kvm_has_waitpkg()) {
> +                env->features[FEAT_7_0_ECX] |= CPUID_7_0_ECX_WAITPKG;
> +            }
> +
> +            if (env->features[FEAT_1_ECX] & CPUID_EXT_MONITOR) {
> +                host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> +                           &cpu->mwait.ecx, &cpu->mwait.edx);
> +	    }
>           }
>           if (cpu->ucode_rev == 0) {
>               cpu->ucode_rev =

Re: [PATCH V3 2/2] target/i386: Advertise MWAIT iff host supports

[Igor Mammedov]

On Mon,  3 Jun 2024 17:02:22 -0700
Zide Chen <zide.chen@intel.com> wrote:

> host_cpu_realizefn() sets CPUID_EXT_MONITOR without consulting host/KVM
> capabilities. This may cause problems:
> 
> - If MWAIT/MONITOR is not available on the host, advertising this
>   feature to the guest and executing MWAIT/MONITOR from the guest
>   triggers #UD and the guest doesn't boot.  This is because typically
>   #UD takes priority over VM-Exit interception checks and KVM doesn't
>   emulate MONITOR/MWAIT on #UD.
> 
> - If KVM doesn't support KVM_X86_DISABLE_EXITS_MWAIT, MWAIT/MONITOR
>   from the guest are intercepted by KVM, which is not what cpu-pm=on
>   intends to do.
> 
> In these cases, MWAIT/MONITOR should not be exposed to the guest.
> 
> The logic in kvm_arch_get_supported_cpuid() to handle CPUID_EXT_MONITOR
> is correct and sufficient, and we can't set CPUID_EXT_MONITOR after
> x86_cpu_filter_features().
> 
> This was not an issue before commit 662175b91ff ("i386: reorder call to
> cpu_exec_realizefn") because the feature added in the accel-specific
> realizefn could be checked against host availability and filtered out.
> 
> Additionally, it seems not a good idea to handle guest CPUID leaves in
> host_cpu_realizefn(), and this patch merges host_cpu_enable_cpu_pm()
> into kvm_cpu_realizefn().
> 
> Fixes: f5cc5a5c1686 ("i386: split cpu accelerators from cpu.c, using AccelCPUClass")
> Fixes: 662175b91ff2 ("i386: reorder call to cpu_exec_realizefn")
> Signed-off-by: Zide Chen <zide.chen@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> 
> V3:
> - don't set CPUID_EXT_MONITOR in kvm_cpu_realizefn().
> - Change title to reflect the main purpose of this patch.
> 
>  target/i386/host-cpu.c    | 12 ------------
>  target/i386/kvm/kvm-cpu.c | 11 +++++++++--
>  2 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/target/i386/host-cpu.c b/target/i386/host-cpu.c
> index 280e427c017c..8b8bf5afeccf 100644
> --- a/target/i386/host-cpu.c
> +++ b/target/i386/host-cpu.c
> @@ -42,15 +42,6 @@ static uint32_t host_cpu_phys_bits(void)
>      return host_phys_bits;
>  }
>  
> -static void host_cpu_enable_cpu_pm(X86CPU *cpu)
> -{
> -    CPUX86State *env = &cpu->env;
> -
> -    host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> -               &cpu->mwait.ecx, &cpu->mwait.edx);
> -    env->features[FEAT_1_ECX] |= CPUID_EXT_MONITOR;
> -}
> -
>  static uint32_t host_cpu_adjust_phys_bits(X86CPU *cpu)
>  {
>      uint32_t host_phys_bits = host_cpu_phys_bits();
> @@ -83,9 +74,6 @@ bool host_cpu_realizefn(CPUState *cs, Error **errp)
>      X86CPU *cpu = X86_CPU(cs);
>      CPUX86State *env = &cpu->env;
>  
> -    if (cpu->max_features && enable_cpu_pm) {
> -        host_cpu_enable_cpu_pm(cpu);
> -    }
>      if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
>          uint32_t phys_bits = host_cpu_adjust_phys_bits(cpu);
>  
> diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> index f76972e47e61..148d10ce3711 100644
> --- a/target/i386/kvm/kvm-cpu.c
> +++ b/target/i386/kvm/kvm-cpu.c
> @@ -65,8 +65,15 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
>       *   cpu_common_realizefn() (via xcc->parent_realize)
>       */
>      if (cpu->max_features) {
> -        if (enable_cpu_pm && kvm_has_waitpkg()) {
> -            env->features[FEAT_7_0_ECX] |= CPUID_7_0_ECX_WAITPKG;
> +        if (enable_cpu_pm) {
> +            if (kvm_has_waitpkg()) {
> +                env->features[FEAT_7_0_ECX] |= CPUID_7_0_ECX_WAITPKG;
> +            }
> +
> +            if (env->features[FEAT_1_ECX] & CPUID_EXT_MONITOR) {
> +                host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
> +                           &cpu->mwait.ecx, &cpu->mwait.edx);
> +	    }
>          }
>          if (cpu->ucode_rev == 0) {
>              cpu->ucode_rev =

Re: [PATCH V3 0/2] improve -overcommit cpu-pm=on|off

[Igor Mammedov]

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(-)
> 

Re: [PATCH V3 0/2] improve -overcommit cpu-pm=on|off

[Chen, Zide]

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(-)
>>
> 

Re: [PATCH V3 0/2] improve -overcommit cpu-pm=on|off

[Michael Tokarev]

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

-- 
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt