Re: [PATCH 06/16] i386/cpu: Drop CPUID 0x2 specific cache info in X86CPUState

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

On 6/20/2025 5:27 PM, Zhao Liu wrote:
> With the pre-defined cache model legacy_intel_cpuid2_cache_info,
> for X86CPUState there's no need to cache special cache information
> for CPUID 0x2 leaf.
>
> Drop the cache_info_cpuid2 field of X86CPUState and use the
> legacy_intel_cpuid2_cache_info directly.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 31 +++++++++++--------------------
>  target/i386/cpu.h |  3 ++-
>  2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a06aa1d629dc..8f174fb971b6 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -244,19 +244,27 @@ static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache, bool *unmacthed)
>      return CACHE_DESCRIPTOR_UNAVAILABLE;
>  }
>  
> +static const CPUCaches legacy_intel_cpuid2_cache_info;
> +
>  /* Encode cache info for CPUID[4] */
>  static void encode_cache_cpuid2(X86CPU *cpu,
>                                  uint32_t *eax, uint32_t *ebx,
>                                  uint32_t *ecx, uint32_t *edx)
>  {
>      CPUX86State *env = &cpu->env;
> -    CPUCaches *caches = &env->cache_info_cpuid2;
> +    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);
> @@ -705,17 +713,6 @@ static CPUCacheInfo legacy_l2_cache = {
>      .share_level = CPU_TOPOLOGY_LEVEL_CORE,
>  };
>  
> -/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
> -static CPUCacheInfo legacy_l2_cache_cpuid2 = {
> -    .type = UNIFIED_CACHE,
> -    .level = 2,
> -    .size = 2 * MiB,
> -    .line_size = 64,
> -    .associativity = 8,
> -    .share_level = CPU_TOPOLOGY_LEVEL_INVALID,
> -};
> -
> -
>  /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
>  static CPUCacheInfo legacy_l2_cache_amd = {
>      .type = UNIFIED_CACHE,
> @@ -8951,18 +8948,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>                         "CPU model '%s' doesn't support legacy-cache=off", name);
>              return;
>          }
> -        env->cache_info_cpuid2 = env->cache_info_cpuid4 = env->cache_info_amd =
> -            *cache_info;
> +        env->cache_info_cpuid4 = env->cache_info_amd = *cache_info;
>      } else {
>          /* Build legacy cache information */
> -        env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache;
> -        env->cache_info_cpuid2.l1i_cache = &legacy_l1i_cache;
>          if (!cpu->consistent_cache) {
> -            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2;
> -        } else {
> -            env->cache_info_cpuid2.l2_cache = &legacy_l2_cache;
> +            env->enable_legacy_cpuid2_cache = true;
>          }
> -        env->cache_info_cpuid2.l3_cache = &legacy_l3_cache;
>  
>          env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache;
>          env->cache_info_cpuid4.l1i_cache = &legacy_l1i_cache;
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 3c7e59ffb12a..8d3ce8a2b678 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -2076,7 +2076,8 @@ typedef struct CPUArchState {
>       * on each CPUID leaf will be different, because we keep compatibility
>       * with old QEMU versions.
>       */
> -    CPUCaches cache_info_cpuid2, cache_info_cpuid4, cache_info_amd;
> +    CPUCaches cache_info_cpuid4, cache_info_amd;
> +    bool enable_legacy_cpuid2_cache;
>  
>      /* MTRRs */
>      uint64_t mtrr_fixed[11];

LGTM.

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