Re: [PATCH 07/16] i386/cpu: Mark CPUID[0x80000005] as reserved for Intel

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

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)