| Message ID | 20240220092504.726064-7-zhao1.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce SMP Cache Topology | expand |
> -----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
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
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) {