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 0x2
> leaf. For AMD CPU, select legacy AMD cache model (in cache_info_amd) as
> the default cache model, otherwise, select legacy Intel cache model (in
> cache_info_cpuid4) as before.
>
> 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 0x2 leaf,
> enable_legacy_vendor_cache flag indicates to pick legacy Intel 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 0x2 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 Intel 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.
>
>   - 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 0x2 leaf regardless of the
>     vendor. Only AMD CPUs have all-0 leaf due to vendor_cpuid_only=true,
>     and this is exactly the behavior of these old machines.
>
>   - For max/host/named CPU (without its own cache model), then the flag
>     enable_legacy_vendor_cache is true, they will use legacy Intel cache
>     model. Similarly, only AMD CPUs have all-0 leaf, and this is exactly
>     the behavior of these old machines.
>
> * 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 0x2 leaf regardless of the
>     vendor. And AMD CPUs have all-0 leaf. Nothing will change.
>
>   - 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 but still get all-0 leaf
>     due to vendor_cpuid_only=true.
>
>     For non-AMD (Intel/Zhaoxin) CPU, it will use legacy Intel cache as
>     expected.
>
>     Here, selecting the legacy cache model based on the vendor does not
>     change the previous (before the change)  behavior.
>
> 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 0x2 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.
>
> Signed-off-by: Zhao Liu <zhao1....@intel.com>
> ---
>  target/i386/cpu.c | 47 +++++++++++++++++++++++++++++++++++++----------
>  target/i386/cpu.h |  1 +
>  2 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index bf8d7a19c88d..524d39de9ace 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -248,23 +248,17 @@ static const CPUCaches legacy_intel_cpuid2_cache_info;
>  
>  /* Encode cache info for CPUID[4] */
>  static void encode_cache_cpuid2(X86CPU *cpu,
> +                                const CPUCaches *caches,
>                                  uint32_t *eax, uint32_t *ebx,
>                                  uint32_t *ecx, uint32_t *edx)
>  {
>      CPUX86State *env = &cpu->env;
> -    const CPUCaches *caches;
>      int l1d, l1i, l2, l3;
>      bool unmatched = false;
>  
>      *eax = 1; /* Number of CPUID[EAX=2] calls required */
>      *ebx = *ecx = *edx = 0;
>  
> -    if (env->enable_legacy_cpuid2_cache) {
> -        caches = &legacy_intel_cpuid2_cache_info;
> -    } else {
> -        caches = &env->cache_info_cpuid4;
> -    }
> -
>      l1d = cpuid2_cache_descriptor(caches->l1d_cache, &unmatched);
>      l1i = cpuid2_cache_descriptor(caches->l1i_cache, &unmatched);
>      l2 = cpuid2_cache_descriptor(caches->l2_cache, &unmatched);
> @@ -7482,8 +7476,37 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>              *ecx &= ~CPUID_EXT_PDCM;
>          }
>          break;
> -    case 2:
> -        /* cache info: needed for Pentium Pro compatibility */
> +    case 2: { /* cache info: needed for Pentium Pro compatibility */
> +        const CPUCaches *caches;
> +
> +        if (env->enable_legacy_cpuid2_cache) {
> +            caches = &legacy_intel_cpuid2_cache_info;
> +        } else if (env->enable_legacy_vendor_cache) {
> +            caches = &legacy_intel_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 use 
> 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. Non-AMD CPUs use cache_info_cpuid4 like before and 
> AMD
> +             *    CPU will use cache_info_amd. But this doesn't matter for 
> AMD
> +             *    CPU, because this leaf encodes all-0 for AMD whatever its 
> cache
> +             *    model is.
> +             */
> +            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;
> @@ -7491,8 +7514,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>              *eax = *ebx = *ecx = *edx = 0;
>              break;
>          }
> -        encode_cache_cpuid2(cpu, eax, ebx, ecx, edx);
> +        encode_cache_cpuid2(cpu, caches, eax, ebx, ecx, edx);
>          break;
> +    }
>      case 4:
>          /* cache info: needed for Core compatibility */
>          if (cpu->cache_info_passthrough) {
> @@ -8979,6 +9003,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>              env->enable_legacy_cpuid2_cache = true;
>          }
>  
> +        if (!cpu->vendor_cpuid_only_v2) {
> +            env->enable_legacy_vendor_cache = true;
> +        }
>          env->cache_info_cpuid4 = legacy_intel_cache_info;
>          env->cache_info_amd = legacy_amd_cache_info;
>      }
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 02cda176798f..243383efd602 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2078,6 +2078,7 @@ typedef struct CPUArchState {
>       */
>      CPUCaches cache_info_cpuid4, cache_info_amd;
>      bool enable_legacy_cpuid2_cache;
> +    bool enable_legacy_vendor_cache;
>  
>      /* MTRRs */
>      uint64_t mtrr_fixed[11];

LGTM. Thanks.

Reviewed-by: Dapeng Mi <dapeng1...@linux.intel.com>



Reply via email to