Hi Michael,
On Tue, Oct 03, 2023 at 08:57:27AM -0400, Michael S. Tsirkin wrote:
> Date: Tue, 3 Oct 2023 08:57:27 -0400
> From: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
> Subject: Re: [PATCH v4 21/21] i386: Add new property to control L2 cache
> topo in CPUID.04H
>
> On Fri, Sep 15, 2023 at 03:53:25PM +0800, Zhao Liu wrote:
> > 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@xxxxxxxxxx>
> > > 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@xxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxx>
> > > > Tested-by: Yongwei Ma <yongwei.ma@xxxxxxxxx>
> > > > ---
> > > > 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
>
> Then, please work on this new generic thing.
> What we don't want is people relying on unstable options.
>
Okay, I'll remove this option in the next refresh.
BTW, about the generic cache topology, what about porting this option to
smp? Just like:
-smp cpus=4,sockets=2,cores=2,threads=1, \
l3-cache=socket,l2-cache=core,l1-i-cache=core,l1-d-cache=core
>From the previous discussion [1] with Jonathan, it seems this format
could also meet the requirement for ARM.
If you like this, I'll move forward in this direction. ;-)
[1]: https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg03997.html
Thanks,
Zhao
