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-7d4c10977...@zhaoxin.com/ > Signed-off-by: Zhao Liu <zhao1....@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)
