Re: [Qemu-devel] [PATCH v7 5/9] i386: Use the statically loaded cache definitions

[Eduardo Habkost <ehabkost@redhat.com>]

On Thu, Apr 26, 2018 at 11:26:45AM -0500, Babu Moger wrote:
> Use the statically loaded cache definitions if available
> and legacy-cache parameter is not set.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Tested-by: Geoffrey McRae <geoff@hostfission.com>

Looks good, but I suggest squashing this with patch 4/9.

Additional suggestion below:

> ---
>  target/i386/cpu.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a27b658..56d2f0b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3941,8 +3941,13 @@ 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(&l1d_cache_amd);
> -        *edx = encode_cache_cpuid80000005(&l1i_cache_amd);

We could rename l1*_cache_amd and l2_cache_amd to
"legacy_*_cache_amd", to make it clear they are just legacy cache
settings.

> +        if (env->cache_info.valid && !cpu->legacy_cache) {
> +            *ecx = encode_cache_cpuid80000005(&env->cache_info.l1d_cache);
> +            *edx = encode_cache_cpuid80000005(&env->cache_info.l1i_cache);
> +        } else {
> +            *ecx = encode_cache_cpuid80000005(&l1d_cache_amd);
> +            *edx = encode_cache_cpuid80000005(&l1i_cache_amd);
> +        }
>          break;
>      case 0x80000006:
>          /* cache info (L2 cache) */
> @@ -3958,9 +3963,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                 (L2_DTLB_4K_ENTRIES << 16) | \
>                 (AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) | \
>                 (L2_ITLB_4K_ENTRIES);
> -        encode_cache_cpuid80000006(&l2_cache_amd,
> -                                   cpu->enable_l3_cache ? &l3_cache : NULL,
> -                                   ecx, edx);
> +        if (env->cache_info.valid && !cpu->legacy_cache) {
> +            encode_cache_cpuid80000006(&env->cache_info.l2_cache,
> +                                       cpu->enable_l3_cache ?
> +                                       &env->cache_info.l3_cache : NULL,
> +                                       ecx, edx);
> +        } else {
> +            encode_cache_cpuid80000006(&l2_cache_amd,
> +                                       cpu->enable_l3_cache ? &l3_cache : NULL,
> +                                       ecx, edx);
> +        }
>          break;
>      case 0x80000007:
>          *eax = 0;
> -- 
> 2.7.4
> 
> 

-- 
Eduardo