Re: [PATCH 2/2] target/i386: drop AMD machine check bits from Intel CPUID

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

On Fri, Jun 28, 2024 at 03:23:11PM +0200, Paolo Bonzini wrote:
> Date: Fri, 28 Jun 2024 15:23:11 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 2/2] target/i386: drop AMD machine check bits from
>  Intel CPUID
> 
> Il ven 28 giu 2024, 10:32 Xiaoyao Li <xiaoyao.li@intel.com> ha scritto:
> 
> > On 6/27/2024 10:06 PM, Paolo Bonzini wrote:
> > > The recent addition of the SUCCOR bit to kvm_arch_get_supported_cpuid()
> > > causes the bit to be visible when "-cpu host" VMs are started on Intel
> > > processors.
> > >
> > > While this should in principle be harmless, it's not tidy and we don't
> > > even know for sure that it doesn't cause any guest OS to take unexpected
> > > paths.  Since x86_cpu_get_supported_feature_word() can return different
> > > different values depending on the guest, adjust it to hide the SUCCOR
> >
> > superfluous different
> >
> > > bit if the guest has non-AMD vendor.
> >
> > It seems to adjust it based on vendor in kvm_arch_get_supported_cpuid()
> > is better than in x86_cpu_get_supported_feature_word(). Otherwise
> > kvm_arch_get_supported_cpuid() still returns "risky" value for Intel VMs.
> >
> 
> But the cpuid bit is only invalid for Intel *guest* vendor, not host. It is
> not a problem to have it if you run on Intel host but have a guest model
> with AMD vendor.
> 
> I will check if there are other callers of kvm_arch_get_supported_cpuid(),
> or callers of x86_cpu_get_supported_feature_word() with NULL cpu, that
> might care about the difference.

Another example is CPUID_EXT3_TOPOEXT, though it's a no_autoenable_flags,
it can be set by "-cpu host,+topoext" on Intel platforms.

For this case, we have recognized that that the host/max CPU should only
contain vender specific features, and I think it would be hard to expand
such a rule afterwards, especially since there's other x86 vender like
zhaoxin who implement a subset of Intel/AMD:

https://lore.kernel.org/qemu-devel/d4c0dae5-b9d5-4deb-b300-78492ab11ed8@zhaoxin.com/#t

What about a new flag "host_bare_metal_check" in FeatureWordInfo? Then
if a feature is marked as "host_bare_metal_check", in addition to the
current checks in x86_cpu_get_supported_feature_word(), bare-metal CPUID
check is also needed (by host_cpuid()) for "host" CPU.

-Zhao