[v1] Introduce SMP Cache Topology

[RFC 6/8] i386/cpu: Update cache topology with machine's configuration

Posted by Zhao Liu 8 months, 3 weeks ago

From: Zhao Liu <zhao1.liu@intel.com>

User will configure SMP cache topology via -smp.

For this case, update the x86 CPUs' cache topology with user's
configuration in MachineState.

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d7cb0f1e49b4..4b5c551fe7f0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7582,6 +7582,27 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
 
 #ifndef CONFIG_USER_ONLY
     MachineState *ms = MACHINE(qdev_get_machine());
+
+    if (ms->smp_cache.l1d != CPU_TOPO_LEVEL_INVALID) {
+        env->cache_info_cpuid4.l1d_cache->share_level = ms->smp_cache.l1d;
+        env->cache_info_amd.l1d_cache->share_level = ms->smp_cache.l1d;
+    }
+
+    if (ms->smp_cache.l1i != CPU_TOPO_LEVEL_INVALID) {
+        env->cache_info_cpuid4.l1i_cache->share_level = ms->smp_cache.l1i;
+        env->cache_info_amd.l1i_cache->share_level = ms->smp_cache.l1i;
+    }
+
+    if (ms->smp_cache.l2 != CPU_TOPO_LEVEL_INVALID) {
+        env->cache_info_cpuid4.l2_cache->share_level = ms->smp_cache.l2;
+        env->cache_info_amd.l2_cache->share_level = ms->smp_cache.l2;
+    }
+
+    if (ms->smp_cache.l3 != CPU_TOPO_LEVEL_INVALID) {
+        env->cache_info_cpuid4.l3_cache->share_level = ms->smp_cache.l3;
+        env->cache_info_amd.l3_cache->share_level = ms->smp_cache.l3;
+    }
+
     qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
 
     if (cpu->env.features[FEAT_1_EDX] & CPUID_APIC || ms->smp.cpus > 1) {
-- 
2.34.1

RE: [RFC 6/8] i386/cpu: Update cache topology with machine's configuration

Posted by JeeHeng Sia 8 months, 2 weeks ago


> -----Original Message-----
> From: Zhao Liu <zhao1.liu@linux.intel.com>
> Sent: Tuesday, February 20, 2024 5:25 PM
> To: Daniel P . Berrangé <berrange@redhat.com>; Eduardo Habkost <eduardo@habkost.net>; Marcel Apfelbaum
> <marcel.apfelbaum@gmail.com>; Philippe Mathieu-Daudé <philmd@linaro.org>; Yanan Wang <wangyanan55@huawei.com>;
> Michael S . Tsirkin <mst@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard Henderson <richard.henderson@linaro.org>;
> Eric Blake <eblake@redhat.com>; Markus Armbruster <armbru@redhat.com>; Marcelo Tosatti <mtosatti@redhat.com>; Alex Bennée
> <alex.bennee@linaro.org>; Peter Maydell <peter.maydell@linaro.org>; Jonathan Cameron <Jonathan.Cameron@huawei.com>;
> JeeHeng Sia <jeeheng.sia@starfivetech.com>
> Cc: qemu-devel@nongnu.org; kvm@vger.kernel.org; qemu-riscv@nongnu.org; qemu-arm@nongnu.org; Zhenyu Wang
> <zhenyu.z.wang@intel.com>; Dapeng Mi <dapeng1.mi@linux.intel.com>; Yongwei Ma <yongwei.ma@intel.com>; Zhao Liu
> <zhao1.liu@intel.com>
> Subject: [RFC 6/8] i386/cpu: Update cache topology with machine's configuration
> 
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> User will configure SMP cache topology via -smp.
> 
> For this case, update the x86 CPUs' cache topology with user's
> configuration in MachineState.
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>  target/i386/cpu.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index d7cb0f1e49b4..4b5c551fe7f0 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7582,6 +7582,27 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> 
>  #ifndef CONFIG_USER_ONLY
>      MachineState *ms = MACHINE(qdev_get_machine());
> +
> +    if (ms->smp_cache.l1d != CPU_TOPO_LEVEL_INVALID) {
> +        env->cache_info_cpuid4.l1d_cache->share_level = ms->smp_cache.l1d;
> +        env->cache_info_amd.l1d_cache->share_level = ms->smp_cache.l1d;
> +    }
> +
> +    if (ms->smp_cache.l1i != CPU_TOPO_LEVEL_INVALID) {
> +        env->cache_info_cpuid4.l1i_cache->share_level = ms->smp_cache.l1i;
> +        env->cache_info_amd.l1i_cache->share_level = ms->smp_cache.l1i;
> +    }
> +
> +    if (ms->smp_cache.l2 != CPU_TOPO_LEVEL_INVALID) {
> +        env->cache_info_cpuid4.l2_cache->share_level = ms->smp_cache.l2;
> +        env->cache_info_amd.l2_cache->share_level = ms->smp_cache.l2;
> +    }
> +
> +    if (ms->smp_cache.l3 != CPU_TOPO_LEVEL_INVALID) {
> +        env->cache_info_cpuid4.l3_cache->share_level = ms->smp_cache.l3;
> +        env->cache_info_amd.l3_cache->share_level = ms->smp_cache.l3;
> +    }
> +
I think this block of code can be further optimized. Maybe we can create
a function called updateCacheShareLevel() that takes a cache pointer and
a share level as arguments. This function encapsulates the common
pattern of updating cache share levels for different caches. You can define
it like this:
void updateCacheShareLevel(XxxCacheInfo *cache, int shareLevel) {
    if (shareLevel != CPU_TOPO_LEVEL_INVALID) {
        cache->share_level = shareLevel;
    }
}
>      qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
> 
>      if (cpu->env.features[FEAT_1_EDX] & CPUID_APIC || ms->smp.cpus > 1) {
> --
> 2.34.1

Re: [RFC 6/8] i386/cpu: Update cache topology with machine's configuration

Posted by Zhao Liu 8 months, 2 weeks ago

Hi JeeHeng,

> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index d7cb0f1e49b4..4b5c551fe7f0 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -7582,6 +7582,27 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> > 
> >  #ifndef CONFIG_USER_ONLY
> >      MachineState *ms = MACHINE(qdev_get_machine());
> > +
> > +    if (ms->smp_cache.l1d != CPU_TOPO_LEVEL_INVALID) {
> > +        env->cache_info_cpuid4.l1d_cache->share_level = ms->smp_cache.l1d;
> > +        env->cache_info_amd.l1d_cache->share_level = ms->smp_cache.l1d;
> > +    }
> > +
> > +    if (ms->smp_cache.l1i != CPU_TOPO_LEVEL_INVALID) {
> > +        env->cache_info_cpuid4.l1i_cache->share_level = ms->smp_cache.l1i;
> > +        env->cache_info_amd.l1i_cache->share_level = ms->smp_cache.l1i;
> > +    }
> > +
> > +    if (ms->smp_cache.l2 != CPU_TOPO_LEVEL_INVALID) {
> > +        env->cache_info_cpuid4.l2_cache->share_level = ms->smp_cache.l2;
> > +        env->cache_info_amd.l2_cache->share_level = ms->smp_cache.l2;
> > +    }
> > +
> > +    if (ms->smp_cache.l3 != CPU_TOPO_LEVEL_INVALID) {
> > +        env->cache_info_cpuid4.l3_cache->share_level = ms->smp_cache.l3;
> > +        env->cache_info_amd.l3_cache->share_level = ms->smp_cache.l3;
> > +    }
> > +
>
> I think this block of code can be further optimized. Maybe we can create
> a function called updateCacheShareLevel() that takes a cache pointer and
> a share level as arguments. This function encapsulates the common
> pattern of updating cache share levels for different caches. You can define
> it like this:
> void updateCacheShareLevel(XxxCacheInfo *cache, int shareLevel) {
>     if (shareLevel != CPU_TOPO_LEVEL_INVALID) {
>         cache->share_level = shareLevel;
>     }
> }
>

Good idea! Will try this way.

Thanks,
Zhao

[RFC 1/8] hw/core: Rename CpuTopology to CPUTopology
[RFC 2/8] hw/core: Move CPU topology enumeration into arch-agnostic file
[RFC 3/8] hw/core: Define cache topology for machine
[RFC 4/8] hw/core: Add cache topology options in -smp
[RFC 5/8] i386/cpu: Support thread and module level cache topology
[RFC 6/8] i386/cpu: Update cache topology with machine's configuration
[RFC 7/8] i386/pc: Support cache topology in -smp for PC machine
[RFC 8/8] qemu-options: Add the cache topology description of -smp