Re: [PATCH 08/16] i386/cpu: Fix CPUID[0x80000006] for Intel CPU

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

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> Per SDM, Intel supports CPUID[0x80000006]. But only L2 information is
> encoded in ECX (note that L2 associativity field encodings rules
> consistent with AMD are used), all other fields are reserved.
>
> Therefore, make the following changes to CPUID[0x80000006]:
>  * Rename AMD_ENC_ASSOC to X86_ENC_ASSOC since Intel also uses the same
>    rules. (While there are some slight differences between the rules in
>    AMD APM v4.07 no.40332 and those in the current QEMU, generally they
>    are consistent.)
>  * Check the vendor in CPUID[0x80000006] and just encode L2 to ECX for
>    Intel.
>  * Assert L2's lines_per_tag is not 0 for AMD, and assert it is 0 for
>    Intel.
>  * Apply the encoding change of Intel for Zhaoxin as well [1].
>
> This fix also resolves the FIXME of legacy_l2_cache_amd:
>
> /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
>
> In addition, per AMD's APM, update the comment of CPUID[0x80000006].
>
> [1]: https://lore.kernel.org/qemu-devel/c522ebb5-04d5-49c6-9ad8-d755b8998988@zhaoxin.com/
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Changes since RFC:
>  * Check vendor_cpuid_only_v2 instead of vendor_cpuid_only.
>  * Move lines_per_tag assert check into encode_cache_cpuid80000006().
> ---
>  target/i386/cpu.c | 42 +++++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index df40d1362566..0b292aa2e07b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -506,8 +506,8 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
>  
>  #define ASSOC_FULL 0xFF
>  
> -/* AMD associativity encoding used on CPUID Leaf 0x80000006: */
> -#define AMD_ENC_ASSOC(a) (a <=   1 ? a   : \
> +/* x86 associativity encoding used on CPUID Leaf 0x80000006: */
> +#define X86_ENC_ASSOC(a) (a <=   1 ? a   : \
>                            a ==   2 ? 0x2 : \
>                            a ==   4 ? 0x4 : \
>                            a ==   8 ? 0x6 : \
> @@ -526,23 +526,26 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache)
>   */
>  static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
>                                         CPUCacheInfo *l3,
> -                                       uint32_t *ecx, uint32_t *edx)
> +                                       uint32_t *ecx, uint32_t *edx,
> +                                       bool lines_per_tag_supported)
>  {
>      assert(l2->size % 1024 == 0);
>      assert(l2->associativity > 0);
> -    assert(l2->lines_per_tag > 0);
> -    assert(l2->line_size > 0);

why remove the assert for l2->line_size?


> +    assert(lines_per_tag_supported ?
> +           l2->lines_per_tag > 0 : l2->lines_per_tag == 0);
>      *ecx = ((l2->size / 1024) << 16) |
> -           (AMD_ENC_ASSOC(l2->associativity) << 12) |
> +           (X86_ENC_ASSOC(l2->associativity) << 12) |
>             (l2->lines_per_tag << 8) | (l2->line_size);
>  
> +    /* For Intel, EDX is reserved. */
>      if (l3) {
>          assert(l3->size % (512 * 1024) == 0);
>          assert(l3->associativity > 0);
> -        assert(l3->lines_per_tag > 0);
> +        assert(lines_per_tag_supported ?
> +               l3->lines_per_tag > 0 : l3->lines_per_tag == 0);
>          assert(l3->line_size > 0);
>          *edx = ((l3->size / (512 * 1024)) << 18) |
> -               (AMD_ENC_ASSOC(l3->associativity) << 12) |
> +               (X86_ENC_ASSOC(l3->associativity) << 12) |
>                 (l3->lines_per_tag << 8) | (l3->line_size);
>      } else {
>          *edx = 0;
> @@ -711,7 +714,6 @@ static CPUCacheInfo legacy_l2_cache = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
>  static CPUCacheInfo legacy_l2_cache_amd = {
>      .type = UNIFIED_CACHE,
>      .level = 2,
> @@ -7906,23 +7908,33 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
>          break;
>      case 0x80000006:
> -        /* cache info (L2 cache) */
> +        /* cache info (L2 cache/TLB/L3 cache) */
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>              break;
>          }
> -        *eax = (AMD_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) |
> +
> +        if (cpu->vendor_cpuid_only_v2 &&
> +            (IS_INTEL_CPU(env) || IS_ZHAOXIN_CPU(env))) {
> +            *eax = *ebx = 0;
> +            encode_cache_cpuid80000006(env->cache_info_cpuid4.l2_cache,
> +                                       NULL, ecx, edx, false);
> +            break;
> +        }
> +
> +        *eax = (X86_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) |
>                 (L2_DTLB_2M_ENTRIES << 16) |
> -               (AMD_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) |
> +               (X86_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) |
>                 (L2_ITLB_2M_ENTRIES);
> -        *ebx = (AMD_ENC_ASSOC(L2_DTLB_4K_ASSOC) << 28) |
> +        *ebx = (X86_ENC_ASSOC(L2_DTLB_4K_ASSOC) << 28) |
>                 (L2_DTLB_4K_ENTRIES << 16) |
> -               (AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) |
> +               (X86_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) |
>                 (L2_ITLB_4K_ENTRIES);
> +
>          encode_cache_cpuid80000006(env->cache_info_amd.l2_cache,
>                                     cpu->enable_l3_cache ?
>                                     env->cache_info_amd.l3_cache : NULL,
> -                                   ecx, edx);
> +                                   ecx, edx, true);
>          break;
>      case 0x80000007:
>          *eax = 0;