Re: [PATCH 13/16] i386/cpu: Select legacy cache model based on vendor in CPUID 0x80000005

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

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> As preparation for merging cache_info_cpuid4 and cache_info_amd in
> X86CPUState, set legacy cache model based on vendor in the CPUID
> 0x80000005 leaf. For AMD CPU, select legacy AMD cache model (in
> cache_info_amd) as the default cache model like before, otherwise,
> select legacy Intel cache model (in cache_info_cpuid4).
>
> To ensure compatibility is not broken, add an enable_legacy_vendor_cache
> flag based on x-vendor-only-v2 to indicate cases where the legacy cache
> model should be used regardless of the vendor. For CPUID 0x80000005
> leaf, enable_legacy_vendor_cache flag indicates to pick legacy AMD cache
> model, which is for compatibility with the behavior of PC machine v10.0
> and older.
>
> The following explains how current vendor-based default legacy cache
> model ensures correctness without breaking compatibility.
>
> * For the PC machine v6.0 and older, vendor_cpuid_only=false, and
>   vendor_cpuid_only_v2=false.
>
>   - If the named CPU model has its own cache model, and doesn't use
>     legacy cache model (legacy_cache=false), then cache_info_cpuid4 and
>     cache_info_amd are same, so 0x80000005 leaf uses its own cache model
>     regardless of the vendor.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is true, they will use legacy AMD cache
>     model just like their previous behavior.
>
> * For the PC machine v10.0 and older (to v6.1), vendor_cpuid_only=true,
>   and vendor_cpuid_only_v2=false.
>
>   - No change, since this leaf doesn't aware vendor_cpuid_only.
>
> * For the PC machine v10.1 and newer, vendor_cpuid_only=true, and
>   vendor_cpuid_only_v2=true.
>
>   - If the named CPU model has its own cache model (legacy_cache=false),
>     then cache_info_cpuid4 & cache_info_amd both equal to its own cache
>     model, so it uses its own cache model in 0x80000005 leaf regardless
>     of the vendor. Only Intel CPUs have all-0 leaf due to
>     vendor_cpuid_only_2=true, and this is exactly the expected behavior.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is false, the legacy cache model is
>     selected based on vendor.
>
>     For AMD CPU, it will use legacy AMD cache as expected.
>
>     For Intel CPU, it will use legacy Intel cache but still get all-0
>     leaf due to vendor_cpuid_only_2=true as expected.
>
>     (Note) And for Zhaoxin CPU, it will use legacy Intel cache model
>     instead of AMD's. This is the difference brought by this change! But
>     it's correct since then Zhaoxin could have the consistent cache info
>     in CPUID 0x2, 0x4 and 0x80000005 leaves.
>
>     Here, except Zhaoxin, selecting the legacy cache model based on the
>     vendor does not change the previous (before the change) behavior.
>     And the change for Zhaoxin is also a good improvement.
>
> Therefore, the above analysis proves that, with the help of the flag
> enable_legacy_vendor_cache, it is acceptable to select the default
> legacy cache model based on the vendor.
>
> For the CPUID 0x80000005 leaf, in X86CPUState, a unified cache_info is
> enough. It only needs to be initialized and configured with the
> corresponding legacy cache model based on the vendor.
>
> Cc: EwanHai <ewanhai-oc@zhaoxin.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> Note, side effect of this patch: fix the inconsistency cache info for
> Zhaoxin. For more details, see the commit message above.
> ---
>  target/i386/cpu.c | 36 ++++++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index afbf11569ab4..16b4ecb76113 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7945,8 +7945,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          *ecx = env->cpuid_model[(index - 0x80000002) * 4 + 2];
>          *edx = env->cpuid_model[(index - 0x80000002) * 4 + 3];
>          break;
> -    case 0x80000005:
> -        /* cache info (L1 cache/TLB Associativity Field) */
> +    case 0x80000005: { /* cache info (L1 cache/TLB Associativity Field) */
Better put the comment into an independent line.

> +        const CPUCaches *caches;
> +
> +        if (env->enable_legacy_vendor_cache) {
> +            caches = &legacy_amd_cache_info;
> +        } else {
> +            /*
> +             * FIXME: Temporarily select cache info model here based on
> +             * vendor, and merge these 2 cache info models later.
> +             *
> +             * This condition covers the following cases (with
> +             * enable_legacy_vendor_cache=false):
> +             *  - When CPU model has its own cache model and doesn't uses legacy
> +             *    cache model (legacy_model=off). Then cache_info_amd and
> +             *    cache_info_cpuid4 are the same.
> +             *
> +             *  - For v10.1 and newer machines, when CPU model uses legacy cache
> +             *    model. AMD CPUs use cache_info_amd like before and non-AMD
> +             *    CPU will use cache_info_cpuid4. But this doesn't matter,
> +             *    because for Intel CPU, it will get all-0 leaf, and Zhaoxin CPU
> +             *    will get correct cache info. Both are expected.
> +             */
> +            if (IS_AMD_CPU(env)) {
> +                caches = &env->cache_info_amd;
> +            } else {
> +                caches = &env->cache_info_cpuid4;
> +            }
> +        }
> +
>          if (cpu->cache_info_passthrough) {
>              x86_cpu_get_cache_cpuid(index, 0, eax, ebx, ecx, edx);
>              break;
> @@ -7961,9 +7988,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                 (L1_ITLB_2M_ASSOC <<  8) | (L1_ITLB_2M_ENTRIES);
>          *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) |
>                 (L1_ITLB_4K_ASSOC <<  8) | (L1_ITLB_4K_ENTRIES);
> -        *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache);
> -        *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache);
> +        *ecx = encode_cache_cpuid80000005(caches->l1d_cache);
> +        *edx = encode_cache_cpuid80000005(caches->l1i_cache);
>          break;
> +    }
>      case 0x80000006:
>          /* cache info (L2 cache/TLB/L3 cache) */
>          if (cpu->cache_info_passthrough) {