On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Per SDM, 0x80000005 leaf is reserved for Intel CPU, and its current
> "assert" check blocks adding new cache model for non-AMD CPUs.
>
> And please note, although Zhaoxin mostly follows Intel behavior,
> this leaf is an exception [1].
>
> So, add a compat property "x-vendor-cpuid-only-v2" (for PC machine v10.0
> and older) to keep the original behavior. For the machine since v10.1,
> check the vendor and encode this leaf as all-0 only for Intel CPU.
>
> This fix also resolves 2 FIXMEs of legacy_l1d_cache_amd and
> legacy_l1i_cache_amd:
>
> /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
>
> In addition, per AMD's APM, update the comment of CPUID[0x80000005].
>
> [1]: https://lore.kernel.org/qemu-devel/fa16f7a8-4917-4731-9d9f-7d4c10977168@zhaoxin.com/
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes since RFC:
> * Only set all-0 for Intel CPU.
> * Add x-vendor-cpuid-only-v2.
> ---
> hw/i386/pc.c | 1 +
> target/i386/cpu.c | 11 ++++++++---
> target/i386/cpu.h | 11 ++++++++++-
> 3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index ad2d6495ebde..9ec3f4db31f3 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -83,6 +83,7 @@
>
> GlobalProperty pc_compat_10_0[] = {
> { TYPE_X86_CPU, "x-consistent-cache", "false" },
> + { TYPE_X86_CPU, "x-vendor-cpuid-only-v2", "false" },
> };
> const size_t pc_compat_10_0_len = G_N_ELEMENTS(pc_compat_10_0);
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 8f174fb971b6..df40d1362566 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -655,7 +655,6 @@ static CPUCacheInfo legacy_l1d_cache = {
> .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> };
>
> -/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
> static CPUCacheInfo legacy_l1d_cache_amd = {
> .type = DATA_CACHE,
> .level = 1,
> @@ -684,7 +683,6 @@ static CPUCacheInfo legacy_l1i_cache = {
> .share_level = CPU_TOPOLOGY_LEVEL_CORE,
> };
>
> -/*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
> static CPUCacheInfo legacy_l1i_cache_amd = {
> .type = INSTRUCTION_CACHE,
> .level = 1,
> @@ -7889,11 +7887,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3];
> break;
> case 0x80000005:
> - /* cache info (L1 cache) */
> + /* cache info (L1 cache/TLB Associativity Field) */
> if (cpu->cache_info_passthrough) {
> x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
> break;
> }
> +
> + if (cpu->vendor_cpuid_only_v2 && IS_INTEL_CPU(env)) {
> + *eax = *ebx = *ecx = *edx = 0;
> + break;
> + }
> +
> *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) |
> (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES);
> *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
> @@ -9464,6 +9468,7 @@ static const Property x86_cpu_properties[] = {
> DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
> DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
> DEFINE_PROP_BOOL("x-vendor-cpuid-only", X86CPU, vendor_cpuid_only, true),
> + DEFINE_PROP_BOOL("x-vendor-cpuid-only-v2", X86CPU, vendor_cpuid_only_v2, true),
> DEFINE_PROP_BOOL("x-amd-topoext-features-only", X86CPU, amd_topoext_features_only, true),
> DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
> DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 8d3ce8a2b678..02cda176798f 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2282,9 +2282,18 @@ struct ArchCPU {
> /* Enable auto level-increase for all CPUID leaves */
> bool full_cpuid_auto_level;
>
> - /* Only advertise CPUID leaves defined by the vendor */
> + /*
> + * Compatibility bits for old machine types (PC machine v6.0 and older).
> + * Only advertise CPUID leaves defined by the vendor.
> + */
> bool vendor_cpuid_only;
>
> + /*
> + * Compatibility bits for old machine types (PC machine v10.0 and older).
> + * Only advertise CPUID leaves defined by the vendor.
> + */
> + bool vendor_cpuid_only_v2;
> +
> /* Only advertise TOPOEXT features that AMD defines */
> bool amd_topoext_features_only;
>
The Intel related part looks good to me. (Not quite familiar with AMD's
Spec, so no reviewed-by tag)