Re: [PATCH v2 6/7] i386/cpu: Update cache topology with machine's configuration

[Alireza Sanaee <alireza.sanaee@huawei.com>]

On Sun,  8 Sep 2024 20:59:19 +0800
Zhao Liu <zhao1.liu@intel.com> wrote:

> User will configure smp cache topology via -machine smp-cache.
> 
> For this case, update the x86 CPUs' cache topology with user's
> configuration in MachineState.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> Tested-by: Yongwei Ma <yongwei.ma@intel.com>
> ---
> Changes since RFC v2:
>  * Used smp_cache array to override cache topology.
>  * Wrapped the updating into a function.
> ---
>  target/i386/cpu.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index e9f755000356..6d9f7dc0872a 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7597,6 +7597,38 @@ static void x86_cpu_hyperv_realize(X86CPU *cpu)
>      cpu->hyperv_limits[2] = 0;
>  }
>  
> +#ifndef CONFIG_USER_ONLY
> +static void x86_cpu_update_smp_cache_topo(MachineState *ms, X86CPU
> *cpu) +{
> +    CPUX86State *env = &cpu->env;
> +    CpuTopologyLevel level;
> +
> +    level = machine_get_cache_topo_level(ms,
> CACHE_LEVEL_AND_TYPE_L1D);
> +    if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> +        env->cache_info_cpuid4.l1d_cache->share_level = level;
> +        env->cache_info_amd.l1d_cache->share_level = level;
> +    }
> +
> +    level = machine_get_cache_topo_level(ms,
> CACHE_LEVEL_AND_TYPE_L1I);
> +    if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> +        env->cache_info_cpuid4.l1i_cache->share_level = level;
> +        env->cache_info_amd.l1i_cache->share_level = level;
> +    }
> +
> +    level = machine_get_cache_topo_level(ms,
> CACHE_LEVEL_AND_TYPE_L2);
> +    if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> +        env->cache_info_cpuid4.l2_cache->share_level = level;
> +        env->cache_info_amd.l2_cache->share_level = level;
> +    }
> +
> +    level = machine_get_cache_topo_level(ms,
> CACHE_LEVEL_AND_TYPE_L3);
> +    if (level != CPU_TOPOLOGY_LEVEL_DEFAULT) {
> +        env->cache_info_cpuid4.l3_cache->share_level = level;
> +        env->cache_info_amd.l3_cache->share_level = level;
> +    }
> +}
> +#endif
> +
>  static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>  {
>      CPUState *cs = CPU(dev);
> @@ -7821,6 +7853,13 @@ static void x86_cpu_realizefn(DeviceState
> *dev, Error **errp) 
>  #ifndef CONFIG_USER_ONLY
>      MachineState *ms = MACHINE(qdev_get_machine());
> +
> +    /*
> +     * TODO: Add a SMPCompatProps.has_caches flag to avoid useless
> Updates
> +     * if user didn't set smp_cache.
> +     */
Hi Zhao,

Thanks for sending this patchset so quickly. I really appreciate the
TODO already :) It also helps me avoid going through every single
layer, especially when I want to avoid matching system registers in
ARM, particularly when there's no description in the command line.
> +    x86_cpu_update_smp_cache_topo(ms, cpu);
> +
>      qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
>  
>      if (cpu->env.features[FEAT_1_EDX] & CPUID_APIC || ms->smp.cpus >
> 1) {