Re: [RFC v2 3/7] hw/core: Add cache topology options in -smp

[Zhao Liu]

Hi Markus and Daniel,

(I replied to both of you because I discussed the idea of cache object
at the end)

On Tue, Jun 04, 2024 at 10:32:14AM +0100, Daniel P. Berrangé wrote:
> Date: Tue, 4 Jun 2024 10:32:14 +0100
> From: "Daniel P. Berrangé" <berrange@redhat.com>
> Subject: Re: [RFC v2 3/7] hw/core: Add cache topology options in -smp
> 
> On Tue, Jun 04, 2024 at 10:54:51AM +0200, Markus Armbruster wrote:
> > Zhao Liu <zhao1.liu@intel.com> writes:
> > 
> > > Add "l1d-cache", "l1i-cache". "l2-cache", and "l3-cache" options in
> > > -smp to define the cache topology for SMP system.
> > >
> > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > 
> > [...]
> > 
> > > diff --git a/qapi/machine.json b/qapi/machine.json
> > > index 7ac5a05bb9c9..8fa5af69b1bf 100644
> > > --- a/qapi/machine.json
> > > +++ b/qapi/machine.json
> > > @@ -1746,6 +1746,23 @@
> > >  #
> > >  # @threads: number of threads per core
> > >  #
> > > +# @l1d-cache: topology hierarchy of L1 data cache. It accepts the CPU
> > > +#     topology enumeration as the parameter, i.e., CPUs in the same
> > > +#     topology container share the same L1 data cache. (since 9.1)
> > > +#
> > > +# @l1i-cache: topology hierarchy of L1 instruction cache. It accepts
> > > +#     the CPU topology enumeration as the parameter, i.e., CPUs in the
> > > +#     same topology container share the same L1 instruction cache.
> > > +#     (since 9.1)
> > > +#
> > > +# @l2-cache: topology hierarchy of L2 unified cache. It accepts the CPU
> > > +#     topology enumeration as the parameter, i.e., CPUs in the same
> > > +#     topology container share the same L2 unified cache. (since 9.1)
> > > +#
> > > +# @l3-cache: topology hierarchy of L3 unified cache. It accepts the CPU
> > > +#     topology enumeration as the parameter, i.e., CPUs in the same
> > > +#     topology container share the same L3 unified cache. (since 9.1)
> > > +#
> > >  # Since: 6.1
> > >  ##
> > 
> > The new members are all optional.  What does "absent" mean?  No such
> > cache?  Some default topology?

I suppose "absent" means using the the default arch-specific cache topo.

For example, x86 has its own default cache topo. my purpose in providing
this interface is to allow users to override the default (arch-specific)
cache topology.

Daniel suggested in cover letter's reply that I could add the value
“default”, I think omitting it would have the same effect as setting it
to “default”, and omitting it shouldn't be regarded as disabling a
particular cache, since the interface is only about the topology!

> > Is this sufficiently general?  Do all machines of interest have a split
> > level 1 cache, a level 2 cache, and a level 3 cache?

The different cache support can be extended by such control fields in
MachineClass:

typedef struct {
    ...
    bool l1_separated_cache_supported;
    bool l2_unified_cache_supported;
    bool l3_unified_cache_supported;
} SMPCompatProps;

For example, if a machine with l1 unified cache appears, it can add the
"l1_unified_cache_supported", and add "l1" option in QAPI.

Then separate L1 cache has “l1i” and “l1d” options, and unified L1 cache
should have “l1” option.

> Level 4 cache is apparently a thing
> 
> https://www.guru3d.com/story/intel-confirms-l4-cache-in-upcoming-meteor-lake-cpus/
> 
> but given that any new cache levels will require new code in QEMU to
> wire up, its not a big deal to add new properties at the same time.
> 
> That said see my reply just now to the cover letter, where I suggest
> we should have a "caches" property that takes an array of cache
> info objects.

Yes, I agree.

> > 
> > Is the CPU topology level the only cache property we'll want to
> > configure here?  If the answer isn't "yes", then we should perhaps wrap
> > it in an object, so we can easily add more members later.
> 
> Cache size is a piece of info I could see us wanting to express

For the simplicity and clarity, I think it's better to only cover
topology in -smp, including the current CPU topology and the cache
topology I'm working on.

I've thought about the idea of converting the cache into the device,
which in turn could define more properties for the cache.

This one is mostly in connection with the previous qom-topo idea (aka
abstracting CPU topology device [1]):

    -device cpu-socket,id=sock0 \
    -device cpu-die,id=die0,parent=sock0 \
    -device cpu-core,id=core0,parent=die0,nr-threads=2 \
    -device cpu-cache,id=cache0,parent=core0,level=l1,type=inst \
    -device cpu-cache,id=cache1,parent=core0,level=l1,type=data \
    -device cpu-cache,id=cache2,parent=core0,level=l2,type=unif \
    -device cpu-cache,id=cache3,parent=die0,level=l3,type=unif

Cache device idea was mainly to support hybrid cache topology, because
Intel client hybrid architecture has hybrid cache topology (on MTL,
compute die has a L3 and SOC die has no L3).

Based on such a cache device, we can define more properties for the
cache and also meet flexible topology requirements at the same time.

This cache device I had planned to implement after the cpu topo device.
It's a long and hard way to go, I wonder if this future I'm portraying
will alleviate your concerns about more cache properties support. ;-)

Soon I'm also planning to refresh the qom-topo series again, reducing
hack changes, deleting dead codes, etc. Maybe it shouldn't be called
qom-topo, because I want to display topology tree in qom path by
qdev_set_id().

[1]: 
https://lore.kernel.org/qemu-devel/20231130144203.2307629-1-zhao1.liu@linux.intel.com/

> > Two spaces between sentences for consistency, please.
> >

Sure, thanks!

Regards,
Zhao