Re: [PATCH v6 09/10] i386: Simplify CPUID_8000_001E for AMD

[Igor Mammedov]

On Mon, 31 Aug 2020 13:43:01 -0500
Babu Moger <babu.moger@amd.com> wrote:

> Remove all the hardcoded values and replace with generalized
> fields.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  target/i386/cpu.c |   31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ba4667b33c..d434c8545a 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -395,9 +395,10 @@ static int cores_in_core_complex(int nr_cores)
>  }
>  
>  /* Encode cache info for CPUID[8000001D] */
> -static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
> -                                uint32_t *eax, uint32_t *ebx,
> -                                uint32_t *ecx, uint32_t *edx)
> +static void encode_cache_cpuid8000001d(CPUCacheInfo *cache,
> +                                       X86CPUTopoInfo *topo_info,
> +                                       uint32_t *eax, uint32_t *ebx,
> +                                       uint32_t *ecx, uint32_t *edx)
>  {
>      uint32_t l3_cores;
>      assert(cache->size == cache->line_size * cache->associativity *
> @@ -408,10 +409,12 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo 
> *cache, CPUState *cs,
>  
>      /* L3 is shared among multiple cores */
>      if (cache->level == 3) {
> -        l3_cores = cores_in_core_complex(cs->nr_cores);
> -        *eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
> +        l3_cores = DIV_ROUND_UP((topo_info->cores_per_die *
> +                                 topo_info->threads_per_core),
> +                                 topo_info->dies_per_pkg);

from spec:
"
NumSharingCache: number of '''logical processors''' sharing cache.
"

s/l3_cores/l3_vcpus|l3_threads/

Also why not use just:

  val = topo_info->cores_per_die * topo_info->threads_per_core



> +        *eax |= (l3_cores - 1) << 14;
>      } else {
> -        *eax |= ((cs->nr_threads - 1) << 14);
> +        *eax |= ((topo_info->threads_per_core - 1) << 14);
>      }
>  
>      assert(cache->line_size > 0);
> @@ -5994,20 +5997,20 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>          }
>          switch (count) {
>          case 0: /* L1 dcache info */
> -            encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, cs,
> -                                       eax, ebx, ecx, edx);
> +            encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache,
> +                                       &topo_info, eax, ebx, ecx, edx);
>              break;
>          case 1: /* L1 icache info */
> -            encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, cs,
> -                                       eax, ebx, ecx, edx);
> +            encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache,
> +                                       &topo_info, eax, ebx, ecx, edx);
>              break;
>          case 2: /* L2 cache info */
> -            encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, cs,
> -                                       eax, ebx, ecx, edx);
> +            encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache,
> +                                       &topo_info, eax, ebx, ecx, edx);
>              break;
>          case 3: /* L3 cache info */
> -            encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, cs,
> -                                       eax, ebx, ecx, edx);
> +            encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache,
> +                                       &topo_info, eax, ebx, ecx, edx);
>              break;
>          default: /* end of info */
>              *eax = *ebx = *ecx = *edx = 0;
>