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

[Zhao Liu <zhao1.liu@intel.com>]

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>