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

[Zhao Liu]

Hi Philippe,

On Thu, Sep 14, 2023 at 09:41:30AM +0200, Philippe Mathieu-Daudé wrote:
> Date: Thu, 14 Sep 2023 09:41:30 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: Re: [PATCH v4 21/21] i386: Add new property to control L2 cache
>  topo in CPUID.04H
> 
> On 14/9/23 09:21, Zhao Liu wrote:
> > 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 overrode by the new topology setting.
> > 
> > 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.
> > 
> > (Tested the cache topology in CPUID[0x04] leaf with "x-l2-cache-topo=[
> > core|cluster]", and tested the live migration between the QEMUs w/ &
> > w/o this patch series.)
> > 
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > Tested-by: Yongwei Ma <yongwei.ma@intel.com>
> > ---
> > Changes since v3:
> >   * Add description about test for live migration compatibility. (Babu)
> > 
> > Changes since v1:
> >   * Rename MODULE branch to CPU_TOPO_LEVEL_MODULE to match the previous
> >     renaming changes.
> > ---
> >   target/i386/cpu.c | 34 +++++++++++++++++++++++++++++++++-
> >   target/i386/cpu.h |  2 ++
> >   2 files changed, 35 insertions(+), 1 deletion(-)
> 
> 
> > @@ -8079,6 +8110,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),
> 
> We use the 'x-' prefix for unstable features, is it the case here?

I thought that if we can have a more general CLI way to define cache
topology in the future, then this option can be removed.

I'm not sure if this option could be treated as unstable, what do you
think?


Thanks,
Zhao

> 
> >       DEFINE_PROP_END_OF_LIST()
> >   };
>