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

[Paolo Bonzini]

On Mon, Jul 1, 2024 at 6:08 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> > > 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.

That was done by commit 7210a02c585 ("i386: Disable TOPOEXT by default
on "-cpu host"", 2018-08-16) which however does not explain what the
bug was. It talks about missing or inconsistent cache topology
information, but that's not precise enough to decide what the problem
was.

> 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:
>
> 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.

I don't see why it's needed. The bare metal vendor is not visible to
the guest, therefore it should have no bearing on whether a bit is
included in CPUID.

Paolo