Re: [PATCH 8/8] qemu-options: Add the description of smp-cache object

[Zhao Liu <zhao1.liu@intel.com>]

Hi Markus,

On Fri, Aug 09, 2024 at 02:24:48PM +0200, Markus Armbruster wrote:
> Date: Fri, 09 Aug 2024 14:24:48 +0200
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache
>  object
> 
> I apologize for the delay.

You're welcome! I appreciate your time, guidance and feedback.

> Zhao Liu <zhao1.liu@intel.com> writes:
> 
> > On Thu, Aug 01, 2024 at 01:28:27PM +0200, Markus Armbruster wrote:
> 
> [...]
> 
> >> Can you provide a brief summary of the design alternatives that have
> >> been proposed so far?  Because I've lost track.
> >
> > No problem!
> >
> > Currently, we have the following options:
> >
> > * 1st: The first one is just to configure cache topology with several
> >   options in -smp:
> >
> >   -smp l1i-cache-topo=core,l1d-cache-topo-core
> >
> >   This one lacks scalability to support the cache size that ARM will
> >   need in the future.
> 
> -smp sets machine property "smp" of QAPI type SMPConfiguration.
> 
> So this one adds members l1i-cache-topo, l1d-cache-topo, ... to
> SMPConfiguration.

Yes.

> > * 2nd: The cache list object in -smp.
> >
> >   The idea was to use JSON to configure the cache list. However, the
> >   underlying implementation of -smp at the moment is keyval parsing,
> >   which is not compatible with JSON.
> 
> Keyval is a variation of the QEMU's traditional KEY=VALUE,... syntax
> that can serve as an alternative to JSON, with certain restrictions.
> Ideally, we provide both JSON and keyval syntax on the command line.

I see. It's the ideal state of the CLI, and -machine and -smp haven't
arrived here yet.

> Example: -blockdev supports both JSON and keyval.
>     JSON:   -blockdev '{"driver": "null-co", "node-name": "node0"}'
>     keyval: -blockdev null-co,node-name=node0
> 
> Unfortunately, we have many old interfaces that still lack JSON support.
> 
> >   If we can not insist on JSON format, then cache lists can also be
> >   implemented in the following way:
> >   
> >   -smp caches.0.name=l1i,caches.0.topo=core,\
> >        caches.1.name=l1d,caches.1.topo=core
> 
> This one adds a single member caches to SMPConfiguration.  It is an
> array of objects.

Yes.

> > * 3rd: The cache list object linked in -machine.
> >
> >   Considering that -object is JSON-compatible so that defining lists via
> >   JSON is more friendly, I implemented the caches list via -object and
> >   linked it to MachineState:
> >
> >   -object '{"qom-type":"smp-cache","id":"obj","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"}]}'
> >   -machine smp-caches=obj
> 
> This one wraps the same array of objects in a new user-creatable object,
> then sets machine property "smp-caches" to that object.
> 
> We can set machine properties directly with -machine.  But -machine
> doesn't support JSON, yet.
> 
> Wrapping in an object moves the configuration to -object, which does
> support JSON.
> 
> Half way between 2nd and 3rd:
> 
>   * Cache list object in machine
> 
>     -machine caches.0.name=l1i,caches.0.topo=core,\
>              caches.1.name=l1d,caches.1.topo=core

I got your point, and putting the array in -machine does align with the
design of the other machine options nowadays.

> > * 4th: The per cache object without any list:
> >
> >   -object smp-cache,id=cache0,name=l1i,topo=core \
> >   -object smp-cache,id=cache1,name=l1d,topo=core
> >
> >   This proposal is clearer, but there are a few opens:
> >   - I plan to push qom-topo forward, which would abstract CPU related
> >     topology levels and cache to "device" instead of object. Is there a
> >     conflict here?
> 
> Can't say, since I don't understand where you want to go.
> 
> Looks like your trying to design an interface for what you want to do
> now, and are wondering whether it could evolve to accomodate what you
> want to do later.
> 
> It's often better to design the interface for everything you already
> know you want to do, then take out the parts you want to do later.

Thanks! From this point of view, then per cache of objects does not meet
my needs.

> >   - Multiple cache objects can't be linked to the machine on the command
> >     line, so I maintain a static cache list in smp_cache.c and expose
> >     the cache information to the machine through some interface. is this
> >     way acceptable?
> >
> >
> > In summary, the 4th proposal was the most up in the air, as it looked to
> > be conflict with the hybrid topology I wanted to do (and while hybrid
> > topology may not be accepted by the community either, I thought it would
> > be best for the two work to be in the same direction).
> >
> > The difference between 2nd and 3rd is about the JSON requirement, if JSON
> > is mandatory for now then it's 3rd, if it's not mandatory (or accept to
> > make -machine/-smp support JSON in the future), 2nd looks cleaner, which
> > puts the caches list in -smp.
> 
> I'd rather not let syntactic limitations of our CLI dictate the
> structure of our configuration data.  Design the structure *first*.
> Only then start to think about CLI.  Our CLI is an unholy mess, and
> thinking about it too early risks getting lost in the weeds.  I fear
> this is what happened to you.

Indeed, that's my dilemma, lost in the world of CLIs.

> If I forcibly ignore all the considerations related to concrete syntax
> in your message, a structure seems to emerge: there's a set of caches
> identified by name (l1i, l1d, ...), and for each cache, we have a number
> of configurable properties (topology level, ...).  Makes sense?

Yes, you're right!

> What else will you need to configure in the future?

Maybe cache size, as Jonathan mentioned for Arm side.

> By the way, extending -machine to support JSON looks feasible to me at a
> glance.
 
Thanks again! Then I made it clear that it would be most appropriate to
place the cache array in -machine, i.e., it's extensible and consistent
with the design of the rest of the machine's properties, as well as
consistent with my long-term needs.

Later on, if -machine is able to support JSON, it will also benefit from
it. If I have time, I will also think about how -machine can support
JSON.

Regards,
Zhao