Re: [PATCH RESEND 18/18] i386: Add new property to control L2 cache topo in CPUID.04H

[Zhao Liu]

On Thu, Feb 16, 2023 at 09:14:54PM +0800, wangyanan (Y) wrote:
> Date: Thu, 16 Feb 2023 21:14:54 +0800
> From: "wangyanan (Y)" <wangyanan55@huawei.com>
> Subject: Re: [PATCH RESEND 18/18] i386: Add new property to control L2
>  cache topo in CPUID.04H
> 
> 在 2023/2/13 17:36, Zhao Liu 写道:
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > The property x-l2-cache-topo will be used to change the L2 cache
> > topology in CPUID.04H.
> > 
> > Now it allows user to set the L2 cache is shared in core level or
> > cluster level.
> > 
> > If user passes "-cpu x-l2-cache-topo=[core|cluster]" then older L2 cache
> > topology will be overrided by the new topology setting.
> Currently x-l2-cache-topo only defines the share level *globally*.

Yes, will set for all CPUs.

> I'm thinking how we can make the property more powerful so that it
> can specify which CPUs share l2 on core level and which CPUs share
> l2 on cluster level.
> 
> What would Intel's Hybrid CPUs do? Determine the l2 share level
> is core or cluster according to the CPU core type (Atom or Core)?
> While ARM does not have the core type concept but have CPUs
> that l2 is shared on different levels in the same system.

For example, Alderlake's "core" shares 1 L2 per core and every 4 "atom"s
share 1 L2. For this case, we can set the topology as:

cluster0 has 1 "core" and cluster1 has 4 "atom". Then set L2 shared on
cluster level.

Since cluster0 has only 1 "core" type core, then L2 per "core" works.

Not sure if this idea can be applied to arm?

> 
> Thanks,
> Yanan
> > Here we expose to user "cluster" instead of "module", to be consistent
> > with "cluster-id" naming.
> > 
> > Since CPUID.04H is used by intel CPUs, this property is available on
> > intel CPUs as for now.
> > 
> > When necessary, it can be extended to CPUID.8000001DH for amd CPUs.
> > 
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >   target/i386/cpu.c | 33 ++++++++++++++++++++++++++++++++-
> >   target/i386/cpu.h |  2 ++
> >   2 files changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 5816dc99b1d4..cf84c720a431 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -240,12 +240,15 @@ static uint32_t 
> > max_processor_ids_for_cache(CPUCacheInfo *cache,
> >       case CORE:
> >           num_ids = 1 << apicid_core_offset(topo_info);
> >           break;
> > +    case MODULE:
> > +        num_ids = 1 << apicid_module_offset(topo_info);
> > +        break;
> >       case DIE:
> >           num_ids = 1 << apicid_die_offset(topo_info);
> >           break;
> >       default:
> >           /*
> > -         * Currently there is no use case for SMT, MODULE and PACKAGE, so 
> > use
> > +         * Currently there is no use case for SMT and PACKAGE, so use
> >            * assert directly to facilitate debugging.
> >            */
> >           g_assert_not_reached();
> > @@ -6633,6 +6636,33 @@ static void x86_cpu_realizefn(DeviceState *dev, 
> > Error **errp)
> >           env->cache_info_amd.l3_cache = &legacy_l3_cache;
> >       }
> > +    if (cpu->l2_cache_topo_level) {
> > +        /*
> > +         * FIXME: Currently only supports changing CPUID[4] (for intel), 
> > and
> > +         * will support changing CPUID[0x8000001D] when necessary.
> > +         */
> > +        if (!IS_INTEL_CPU(env)) {
> > +            error_setg(errp, "only intel cpus supports x-l2-cache-topo");
> > +            return;
> > +        }
> > +
> > +        if (!strcmp(cpu->l2_cache_topo_level, "core")) {
> > +            env->cache_info_cpuid4.l2_cache->share_level = CORE;
> > +        } else if (!strcmp(cpu->l2_cache_topo_level, "cluster")) {
> > +            /*
> > +             * We expose to users "cluster" instead of "module", to be
> > +             * consistent with "cluster-id" naming.
> > +             */
> > +            env->cache_info_cpuid4.l2_cache->share_level = MODULE;
> > +        } else {
> > +            error_setg(errp,
> > +                       "x-l2-cache-topo doesn't support '%s', "
> > +                       "and it only supports 'core' or 'cluster'",
> > +                       cpu->l2_cache_topo_level);
> > +            return;
> > +        }
> > +    }
> > +
> >   #ifndef CONFIG_USER_ONLY
> >       MachineState *ms = MACHINE(qdev_get_machine());
> >       qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
> > @@ -7135,6 +7165,7 @@ static Property x86_cpu_properties[] = {
> >                        false),
> >       DEFINE_PROP_BOOL("x-intel-pt-auto-level", X86CPU, intel_pt_auto_level,
> >                        true),
> > +    DEFINE_PROP_STRING("x-l2-cache-topo", X86CPU, l2_cache_topo_level),
> >       DEFINE_PROP_END_OF_LIST()
> >   };
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 5a955431f759..aa7e96c586c7 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1987,6 +1987,8 @@ struct ArchCPU {
> >       int32_t thread_id;
> >       int32_t hv_max_vps;
> > +
> > +    char *l2_cache_topo_level;
> >   };
>