| Message ID | 20240704031603.1744546-9-zhao1.liu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce SMP Cache Topology | expand |
Zhao Liu <zhao1.liu@intel.com> writes: > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> This patch is just documentation. The code got added in some previous patch. Would it make sense to squash this patch into that previous patch? > --- > Changes since RFC v2: > * Rewrote the document of smp-cache object. > > Changes since RFC v1: > * Use "*_cache=topo_level" as -smp example as the original "level" > term for a cache has a totally different meaning. (Jonathan) > --- > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > > diff --git a/qemu-options.hx b/qemu-options.hx > index 8ca7f34ef0c8..4b84f4508a6e 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -159,6 +159,15 @@ SRST > :: > > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 > + > + ``smp-cache='id'`` > + Allows to configure cache property (now only the cache topology level). > + > + For example: > + :: > + > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' > + -machine smp-cache=cache > ERST > > DEF("M", HAS_ARG, QEMU_OPTION_M, > @@ -5871,6 +5880,55 @@ SRST > :: > > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 > + > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` > + Create an smp-cache object that configures machine's cache > + property. Currently, cache property only include cache topology > + level. > + > + This option must be written in JSON format to support JSON list. Why? > + > + The ``caches`` parameter accepts a list of cache property in JSON > + format. > + > + A list element requires the cache name to be specified in the > + ``name`` parameter (currently ``l1d``, ``l1i``, ``l2`` and ``l3`` > + are supported). ``topo`` parameter accepts CPU topology levels > + including ``thread``, ``core``, ``module``, ``cluster``, ``die``, > + ``socket``, ``book``, ``drawer`` and ``default``. The ``topo`` > + parameter indicates CPUs winthin the same CPU topology container > + are sharing the same cache. > + > + Some machines may have their own cache topology model, and this > + object may override the machine-specific cache topology setting > + by specifying smp-cache object in the -machine. When specifying > + the cache topology level of ``default``, it will honor the default > + machine-specific cache topology setting. For other topology levels, > + they will override the default setting. > + > + An example list of caches to configure the cache model (l1d cache > + per core, l1i cache per core, l2 cache per module and l3 cache per > + socket) supported by PC machine might look like: > + > + :: > + > + { > + "caches": [ > + { "name": "l1d", "topo": "core" }, > + { "name": "l1i", "topo": "core" }, > + { "name": "l2", "topo": "module" }, > + { "name": "l3", "topo": "socket" }, > + ] > + } > + > + An example smp-cache object would look like:() > + > + .. parsed-literal:: > + > + # |qemu_system| \\ > + ... \\ > + -object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}' \\ > + ... > ERST
Hi Markus, On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: > Date: Mon, 22 Jul 2024 15:37:43 +0200 > From: Markus Armbruster <armbru@redhat.com> > Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > object > > Zhao Liu <zhao1.liu@intel.com> writes: > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > > This patch is just documentation. The code got added in some previous > patch. Would it make sense to squash this patch into that previous > patch? OK, I'll merge them. > > --- > > Changes since RFC v2: > > * Rewrote the document of smp-cache object. > > > > Changes since RFC v1: > > * Use "*_cache=topo_level" as -smp example as the original "level" > > term for a cache has a totally different meaning. (Jonathan) > > --- > > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 58 insertions(+) > > > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 8ca7f34ef0c8..4b84f4508a6e 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -159,6 +159,15 @@ SRST > > :: > > > > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 > > + > > + ``smp-cache='id'`` > > + Allows to configure cache property (now only the cache topology level). > > + > > + For example: > > + :: > > + > > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' > > + -machine smp-cache=cache > > ERST > > > > DEF("M", HAS_ARG, QEMU_OPTION_M, > > @@ -5871,6 +5880,55 @@ SRST > > :: > > > > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 > > + > > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` > > + Create an smp-cache object that configures machine's cache > > + property. Currently, cache property only include cache topology > > + level. > > + > > + This option must be written in JSON format to support JSON list. > > Why? I'm not familiar with this, so I hope you could educate me if I'm wrong. All I know so far is for -object that defining a list can only be done in JSON format and not with a numeric index like a keyval based option, like: -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing the above doesn't work. Is there any other way to specify a list in command line? > > + > > + The ``caches`` parameter accepts a list of cache property in JSON > > + format. > > + > > + A list element requires the cache name to be specified in the > > + ``name`` parameter (currently ``l1d``, ``l1i``, ``l2`` and ``l3`` > > + are supported). ``topo`` parameter accepts CPU topology levels > > + including ``thread``, ``core``, ``module``, ``cluster``, ``die``, > > + ``socket``, ``book``, ``drawer`` and ``default``. The ``topo`` > > + parameter indicates CPUs winthin the same CPU topology container > > + are sharing the same cache. > > + > > + Some machines may have their own cache topology model, and this > > + object may override the machine-specific cache topology setting > > + by specifying smp-cache object in the -machine. When specifying > > + the cache topology level of ``default``, it will honor the default > > + machine-specific cache topology setting. For other topology levels, > > + they will override the default setting. > > + > > + An example list of caches to configure the cache model (l1d cache > > + per core, l1i cache per core, l2 cache per module and l3 cache per > > + socket) supported by PC machine might look like: > > + > > + :: > > + > > + { > > + "caches": [ > > + { "name": "l1d", "topo": "core" }, > > + { "name": "l1i", "topo": "core" }, > > + { "name": "l2", "topo": "module" }, > > + { "name": "l3", "topo": "socket" }, > > + ] > > + } > > + > > + An example smp-cache object would look like:() > > + > > + .. parsed-literal:: > > + > > + # |qemu_system| \\ > > + ... \\ > > + -object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}' \\ > > + ... > > ERST >
Zhao Liu <zhao1.liu@intel.com> writes: > Hi Markus, > > On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: >> Date: Mon, 22 Jul 2024 15:37:43 +0200 >> From: Markus Armbruster <armbru@redhat.com> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache >> object >> >> Zhao Liu <zhao1.liu@intel.com> writes: >> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> >> >> This patch is just documentation. The code got added in some previous >> patch. Would it make sense to squash this patch into that previous >> patch? > > OK, I'll merge them. > >> > --- >> > Changes since RFC v2: >> > * Rewrote the document of smp-cache object. >> > >> > Changes since RFC v1: >> > * Use "*_cache=topo_level" as -smp example as the original "level" >> > term for a cache has a totally different meaning. (Jonathan) >> > --- >> > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 58 insertions(+) >> > >> > diff --git a/qemu-options.hx b/qemu-options.hx >> > index 8ca7f34ef0c8..4b84f4508a6e 100644 >> > --- a/qemu-options.hx >> > +++ b/qemu-options.hx >> > @@ -159,6 +159,15 @@ SRST >> > :: >> > >> > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 >> > + >> > + ``smp-cache='id'`` >> > + Allows to configure cache property (now only the cache topology level). >> > + >> > + For example: >> > + :: >> > + >> > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' >> > + -machine smp-cache=cache >> > ERST >> > >> > DEF("M", HAS_ARG, QEMU_OPTION_M, >> > @@ -5871,6 +5880,55 @@ SRST >> > :: >> > >> > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 >> > + >> > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` >> > + Create an smp-cache object that configures machine's cache >> > + property. Currently, cache property only include cache topology >> > + level. >> > + >> > + This option must be written in JSON format to support JSON list. >> >> Why? > > I'm not familiar with this, so I hope you could educate me if I'm wrong. > > All I know so far is for -object that defining a list can only be done in > JSON format and not with a numeric index like a keyval based option, like: > > -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing > > the above doesn't work. > > Is there any other way to specify a list in command line? The command line is a big, sprawling mess :) -object supports either a JSON or a QemuOpts argument. *Not* keyval! Both QemuOpts and keyval parse something like KEY=VALUE,... Keyval supports arrays and objects via dotted keys. QemuOpts doesn't natively support arrays and objects, but its users can hack around that limitation in various ways. -object doesn't. So you're right, it's JSON or bust here. However, if we used one object per cache instead, we could get something like -object smp-cache,name=l1d,... -object smp-cache,name=l1u,... -object smp-cache,name=l2,... ... >> > + >> > + The ``caches`` parameter accepts a list of cache property in JSON >> > + format. >> > + >> > + A list element requires the cache name to be specified in the >> > + ``name`` parameter (currently ``l1d``, ``l1i``, ``l2`` and ``l3`` >> > + are supported). ``topo`` parameter accepts CPU topology levels >> > + including ``thread``, ``core``, ``module``, ``cluster``, ``die``, >> > + ``socket``, ``book``, ``drawer`` and ``default``. The ``topo`` >> > + parameter indicates CPUs winthin the same CPU topology container >> > + are sharing the same cache. >> > + >> > + Some machines may have their own cache topology model, and this >> > + object may override the machine-specific cache topology setting >> > + by specifying smp-cache object in the -machine. When specifying >> > + the cache topology level of ``default``, it will honor the default >> > + machine-specific cache topology setting. For other topology levels, >> > + they will override the default setting. >> > + >> > + An example list of caches to configure the cache model (l1d cache >> > + per core, l1i cache per core, l2 cache per module and l3 cache per >> > + socket) supported by PC machine might look like: >> > + >> > + :: >> > + >> > + { >> > + "caches": [ >> > + { "name": "l1d", "topo": "core" }, >> > + { "name": "l1i", "topo": "core" }, >> > + { "name": "l2", "topo": "module" }, >> > + { "name": "l3", "topo": "socket" }, >> > + ] >> > + } >> > + >> > + An example smp-cache object would look like:() >> > + >> > + .. parsed-literal:: >> > + >> > + # |qemu_system| \\ >> > + ... \\ >> > + -object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}' \\ >> > + ... >> > ERST >>
Hi Markus and Daniel, I have the questions about the -object per cache implementation: On Wed, Jul 24, 2024 at 02:39:29PM +0200, Markus Armbruster wrote: > Date: Wed, 24 Jul 2024 14:39:29 +0200 > From: Markus Armbruster <armbru@redhat.com> > Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > object > > Zhao Liu <zhao1.liu@intel.com> writes: > > > Hi Markus, > > > > On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: > >> Date: Mon, 22 Jul 2024 15:37:43 +0200 > >> From: Markus Armbruster <armbru@redhat.com> > >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > >> object > >> > >> Zhao Liu <zhao1.liu@intel.com> writes: > >> > >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > >> > >> This patch is just documentation. The code got added in some previous > >> patch. Would it make sense to squash this patch into that previous > >> patch? > > > > OK, I'll merge them. > > > >> > --- > >> > Changes since RFC v2: > >> > * Rewrote the document of smp-cache object. > >> > > >> > Changes since RFC v1: > >> > * Use "*_cache=topo_level" as -smp example as the original "level" > >> > term for a cache has a totally different meaning. (Jonathan) > >> > --- > >> > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > >> > 1 file changed, 58 insertions(+) > >> > > >> > diff --git a/qemu-options.hx b/qemu-options.hx > >> > index 8ca7f34ef0c8..4b84f4508a6e 100644 > >> > --- a/qemu-options.hx > >> > +++ b/qemu-options.hx > >> > @@ -159,6 +159,15 @@ SRST > >> > :: > >> > > >> > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 > >> > + > >> > + ``smp-cache='id'`` > >> > + Allows to configure cache property (now only the cache topology level). > >> > + > >> > + For example: > >> > + :: > >> > + > >> > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' > >> > + -machine smp-cache=cache > >> > ERST > >> > > >> > DEF("M", HAS_ARG, QEMU_OPTION_M, > >> > @@ -5871,6 +5880,55 @@ SRST > >> > :: > >> > > >> > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 > >> > + > >> > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` > >> > + Create an smp-cache object that configures machine's cache > >> > + property. Currently, cache property only include cache topology > >> > + level. > >> > + > >> > + This option must be written in JSON format to support JSON list. > >> > >> Why? > > > > I'm not familiar with this, so I hope you could educate me if I'm wrong. > > > > All I know so far is for -object that defining a list can only be done in > > JSON format and not with a numeric index like a keyval based option, like: > > > > -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing > > > > the above doesn't work. > > > > Is there any other way to specify a list in command line? > > The command line is a big, sprawling mess :) > > -object supports either a JSON or a QemuOpts argument. *Not* keyval! > > Both QemuOpts and keyval parse something like KEY=VALUE,... Keyval > supports arrays and objects via dotted keys. QemuOpts doesn't natively > support arrays and objects, but its users can hack around that > limitation in various ways. -object doesn't. So you're right, it's > JSON or bust here. > > However, if we used one object per cache instead, we could get something > like > > -object smp-cache,name=l1d,... > -object smp-cache,name=l1u,... > -object smp-cache,name=l2,... > ... Current, I use -object to create a smp_cache object, and link it to MachineState by -machine,smp-cache=obj_id. Then for the objects per cache, how could I link them to machine? Is it possible that I create something static in smp_cache.c and expose all the cache information to machine through some interface? Additionally, I would like to consider for the long term heterogeneous cache, as asked before in [1], does the object per cache conflict with the cache device I'm considering? Considering cache device is further because I want to create CPU/cache topology via -device and build a topology tree. [1]: https://lore.kernel.org/qemu-devel/Zl88DYwLE3ScDF5F@intel.com/ I think this is becoming a nightmare I can't get around. Naming is difficult, and sorting out interface design I think is also a difficult task. If you feel that there is indeed a conflict, then I'm also willing to fall back to -smp again and do it based on keyval's list, as originally suggested by Daniel. Sorry for the repetition on thoughts/design, I hope that discussion with you I can make sense of the current and subsequent paths without getting out of hand! Best Regards, Zhao
Zhao Liu <zhao1.liu@intel.com> writes: > Hi Markus and Daniel, > > I have the questions about the -object per cache implementation: > > On Wed, Jul 24, 2024 at 02:39:29PM +0200, Markus Armbruster wrote: >> Date: Wed, 24 Jul 2024 14:39:29 +0200 >> From: Markus Armbruster <armbru@redhat.com> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache >> object >> >> Zhao Liu <zhao1.liu@intel.com> writes: >> >> > Hi Markus, >> > >> > On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: >> >> Date: Mon, 22 Jul 2024 15:37:43 +0200 >> >> From: Markus Armbruster <armbru@redhat.com> >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache >> >> object >> >> >> >> Zhao Liu <zhao1.liu@intel.com> writes: >> >> >> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> >> >> >> >> This patch is just documentation. The code got added in some previous >> >> patch. Would it make sense to squash this patch into that previous >> >> patch? >> > >> > OK, I'll merge them. >> > >> >> > --- >> >> > Changes since RFC v2: >> >> > * Rewrote the document of smp-cache object. >> >> > >> >> > Changes since RFC v1: >> >> > * Use "*_cache=topo_level" as -smp example as the original "level" >> >> > term for a cache has a totally different meaning. (Jonathan) >> >> > --- >> >> > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ >> >> > 1 file changed, 58 insertions(+) >> >> > >> >> > diff --git a/qemu-options.hx b/qemu-options.hx >> >> > index 8ca7f34ef0c8..4b84f4508a6e 100644 >> >> > --- a/qemu-options.hx >> >> > +++ b/qemu-options.hx >> >> > @@ -159,6 +159,15 @@ SRST >> >> > :: >> >> > >> >> > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 >> >> > + >> >> > + ``smp-cache='id'`` >> >> > + Allows to configure cache property (now only the cache topology level). >> >> > + >> >> > + For example: >> >> > + :: >> >> > + >> >> > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' >> >> > + -machine smp-cache=cache >> >> > ERST >> >> > >> >> > DEF("M", HAS_ARG, QEMU_OPTION_M, >> >> > @@ -5871,6 +5880,55 @@ SRST >> >> > :: >> >> > >> >> > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 >> >> > + >> >> > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` >> >> > + Create an smp-cache object that configures machine's cache >> >> > + property. Currently, cache property only include cache topology >> >> > + level. >> >> > + >> >> > + This option must be written in JSON format to support JSON list. >> >> >> >> Why? >> > >> > I'm not familiar with this, so I hope you could educate me if I'm wrong. >> > >> > All I know so far is for -object that defining a list can only be done in >> > JSON format and not with a numeric index like a keyval based option, like: >> > >> > -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing >> > >> > the above doesn't work. >> > >> > Is there any other way to specify a list in command line? >> >> The command line is a big, sprawling mess :) >> >> -object supports either a JSON or a QemuOpts argument. *Not* keyval! >> >> Both QemuOpts and keyval parse something like KEY=VALUE,... Keyval >> supports arrays and objects via dotted keys. QemuOpts doesn't natively >> support arrays and objects, but its users can hack around that >> limitation in various ways. -object doesn't. So you're right, it's >> JSON or bust here. >> >> However, if we used one object per cache instead, we could get something >> like >> >> -object smp-cache,name=l1d,... >> -object smp-cache,name=l1u,... >> -object smp-cache,name=l2,... >> ... > > Current, I use -object to create a smp_cache object, and link it to > MachineState by -machine,smp-cache=obj_id. > > Then for the objects per cache, how could I link them to machine? > > Is it possible that I create something static in smp_cache.c and expose > all the cache information to machine through some interface? Good questions. However, before we head deeper into the weeds here, I feel we should discuss the things below. And before we do that, I need a clear understanding of the use case. Elsewhere in this thread, I just described the use case as I understand it. Please reply there. I'll then come back to this message. [...]
On Thu, Jul 25, 2024 at 11:07:12AM +0200, Markus Armbruster wrote: > Date: Thu, 25 Jul 2024 11:07:12 +0200 > From: Markus Armbruster <armbru@redhat.com> > Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > object > > Zhao Liu <zhao1.liu@intel.com> writes: > > > Hi Markus and Daniel, > > > > I have the questions about the -object per cache implementation: > > > > On Wed, Jul 24, 2024 at 02:39:29PM +0200, Markus Armbruster wrote: > >> Date: Wed, 24 Jul 2024 14:39:29 +0200 > >> From: Markus Armbruster <armbru@redhat.com> > >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > >> object > >> > >> Zhao Liu <zhao1.liu@intel.com> writes: > >> > >> > Hi Markus, > >> > > >> > On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: > >> >> Date: Mon, 22 Jul 2024 15:37:43 +0200 > >> >> From: Markus Armbruster <armbru@redhat.com> > >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > >> >> object > >> >> > >> >> Zhao Liu <zhao1.liu@intel.com> writes: > >> >> > >> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > >> >> > >> >> This patch is just documentation. The code got added in some previous > >> >> patch. Would it make sense to squash this patch into that previous > >> >> patch? > >> > > >> > OK, I'll merge them. > >> > > >> >> > --- > >> >> > Changes since RFC v2: > >> >> > * Rewrote the document of smp-cache object. > >> >> > > >> >> > Changes since RFC v1: > >> >> > * Use "*_cache=topo_level" as -smp example as the original "level" > >> >> > term for a cache has a totally different meaning. (Jonathan) > >> >> > --- > >> >> > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > >> >> > 1 file changed, 58 insertions(+) > >> >> > > >> >> > diff --git a/qemu-options.hx b/qemu-options.hx > >> >> > index 8ca7f34ef0c8..4b84f4508a6e 100644 > >> >> > --- a/qemu-options.hx > >> >> > +++ b/qemu-options.hx > >> >> > @@ -159,6 +159,15 @@ SRST > >> >> > :: > >> >> > > >> >> > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 > >> >> > + > >> >> > + ``smp-cache='id'`` > >> >> > + Allows to configure cache property (now only the cache topology level). > >> >> > + > >> >> > + For example: > >> >> > + :: > >> >> > + > >> >> > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' > >> >> > + -machine smp-cache=cache > >> >> > ERST > >> >> > > >> >> > DEF("M", HAS_ARG, QEMU_OPTION_M, > >> >> > @@ -5871,6 +5880,55 @@ SRST > >> >> > :: > >> >> > > >> >> > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 > >> >> > + > >> >> > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` > >> >> > + Create an smp-cache object that configures machine's cache > >> >> > + property. Currently, cache property only include cache topology > >> >> > + level. > >> >> > + > >> >> > + This option must be written in JSON format to support JSON list. > >> >> > >> >> Why? > >> > > >> > I'm not familiar with this, so I hope you could educate me if I'm wrong. > >> > > >> > All I know so far is for -object that defining a list can only be done in > >> > JSON format and not with a numeric index like a keyval based option, like: > >> > > >> > -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing > >> > > >> > the above doesn't work. > >> > > >> > Is there any other way to specify a list in command line? > >> > >> The command line is a big, sprawling mess :) > >> > >> -object supports either a JSON or a QemuOpts argument. *Not* keyval! > >> > >> Both QemuOpts and keyval parse something like KEY=VALUE,... Keyval > >> supports arrays and objects via dotted keys. QemuOpts doesn't natively > >> support arrays and objects, but its users can hack around that > >> limitation in various ways. -object doesn't. So you're right, it's > >> JSON or bust here. > >> > >> However, if we used one object per cache instead, we could get something > >> like > >> > >> -object smp-cache,name=l1d,... > >> -object smp-cache,name=l1u,... > >> -object smp-cache,name=l2,... > >> ... > > > > Current, I use -object to create a smp_cache object, and link it to > > MachineState by -machine,smp-cache=obj_id. > > > > Then for the objects per cache, how could I link them to machine? > > > > Is it possible that I create something static in smp_cache.c and expose > > all the cache information to machine through some interface? > > Good questions. However, before we head deeper into the weeds here, I > feel we should discuss the things below. And before we do that, I need > a clear understanding of the use case. Elsewhere in this thread, I just > described the use case as I understand it. Please reply there. I'll > then come back to this message. > > [...] Jonathan and I provided different use cases for x86 and Arm. Could we come back here to continue the discussion? :) Thanks, Zhao
Zhao Liu <zhao1.liu@intel.com> writes: > On Thu, Jul 25, 2024 at 11:07:12AM +0200, Markus Armbruster wrote: >> Date: Thu, 25 Jul 2024 11:07:12 +0200 >> From: Markus Armbruster <armbru@redhat.com> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache >> object >> >> Zhao Liu <zhao1.liu@intel.com> writes: >> >> > Hi Markus and Daniel, >> > >> > I have the questions about the -object per cache implementation: >> > >> > On Wed, Jul 24, 2024 at 02:39:29PM +0200, Markus Armbruster wrote: >> >> Date: Wed, 24 Jul 2024 14:39:29 +0200 >> >> From: Markus Armbruster <armbru@redhat.com> >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache >> >> object >> >> >> >> Zhao Liu <zhao1.liu@intel.com> writes: >> >> >> >> > Hi Markus, >> >> > >> >> > On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: >> >> >> Date: Mon, 22 Jul 2024 15:37:43 +0200 >> >> >> From: Markus Armbruster <armbru@redhat.com> >> >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache >> >> >> object >> >> >> >> >> >> Zhao Liu <zhao1.liu@intel.com> writes: >> >> >> >> >> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> >> >> >> >> >> >> This patch is just documentation. The code got added in some previous >> >> >> patch. Would it make sense to squash this patch into that previous >> >> >> patch? >> >> > >> >> > OK, I'll merge them. >> >> > >> >> >> > --- >> >> >> > Changes since RFC v2: >> >> >> > * Rewrote the document of smp-cache object. >> >> >> > >> >> >> > Changes since RFC v1: >> >> >> > * Use "*_cache=topo_level" as -smp example as the original "level" >> >> >> > term for a cache has a totally different meaning. (Jonathan) >> >> >> > --- >> >> >> > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ >> >> >> > 1 file changed, 58 insertions(+) >> >> >> > >> >> >> > diff --git a/qemu-options.hx b/qemu-options.hx >> >> >> > index 8ca7f34ef0c8..4b84f4508a6e 100644 >> >> >> > --- a/qemu-options.hx >> >> >> > +++ b/qemu-options.hx >> >> >> > @@ -159,6 +159,15 @@ SRST >> >> >> > :: >> >> >> > >> >> >> > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 >> >> >> > + >> >> >> > + ``smp-cache='id'`` >> >> >> > + Allows to configure cache property (now only the cache topology level). >> >> >> > + >> >> >> > + For example: >> >> >> > + :: >> >> >> > + >> >> >> > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' >> >> >> > + -machine smp-cache=cache >> >> >> > ERST >> >> >> > >> >> >> > DEF("M", HAS_ARG, QEMU_OPTION_M, >> >> >> > @@ -5871,6 +5880,55 @@ SRST >> >> >> > :: >> >> >> > >> >> >> > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 >> >> >> > + >> >> >> > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` >> >> >> > + Create an smp-cache object that configures machine's cache >> >> >> > + property. Currently, cache property only include cache topology >> >> >> > + level. >> >> >> > + >> >> >> > + This option must be written in JSON format to support JSON list. >> >> >> >> >> >> Why? >> >> > >> >> > I'm not familiar with this, so I hope you could educate me if I'm wrong. >> >> > >> >> > All I know so far is for -object that defining a list can only be done in >> >> > JSON format and not with a numeric index like a keyval based option, like: >> >> > >> >> > -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing >> >> > >> >> > the above doesn't work. >> >> > >> >> > Is there any other way to specify a list in command line? >> >> >> >> The command line is a big, sprawling mess :) >> >> >> >> -object supports either a JSON or a QemuOpts argument. *Not* keyval! >> >> >> >> Both QemuOpts and keyval parse something like KEY=VALUE,... Keyval >> >> supports arrays and objects via dotted keys. QemuOpts doesn't natively >> >> support arrays and objects, but its users can hack around that >> >> limitation in various ways. -object doesn't. So you're right, it's >> >> JSON or bust here. >> >> >> >> However, if we used one object per cache instead, we could get something >> >> like >> >> >> >> -object smp-cache,name=l1d,... >> >> -object smp-cache,name=l1u,... >> >> -object smp-cache,name=l2,... >> >> ... >> > >> > Current, I use -object to create a smp_cache object, and link it to >> > MachineState by -machine,smp-cache=obj_id. >> > >> > Then for the objects per cache, how could I link them to machine? >> > >> > Is it possible that I create something static in smp_cache.c and expose >> > all the cache information to machine through some interface? >> >> Good questions. However, before we head deeper into the weeds here, I >> feel we should discuss the things below. And before we do that, I need >> a clear understanding of the use case. Elsewhere in this thread, I just >> described the use case as I understand it. Please reply there. I'll >> then come back to this message. >> >> [...] > > Jonathan and I provided different use cases for x86 and Arm. Could we > come back here to continue the discussion? :) Can you provide a brief summary of the design alternatives that have been proposed so far? Because I've lost track.
On Thu, Aug 01, 2024 at 01:28:27PM +0200, Markus Armbruster wrote: > Date: Thu, 01 Aug 2024 13:28:27 +0200 > From: Markus Armbruster <armbru@redhat.com> > Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > object > > Zhao Liu <zhao1.liu@intel.com> writes: > > > On Thu, Jul 25, 2024 at 11:07:12AM +0200, Markus Armbruster wrote: > >> Date: Thu, 25 Jul 2024 11:07:12 +0200 > >> From: Markus Armbruster <armbru@redhat.com> > >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > >> object > >> > >> Zhao Liu <zhao1.liu@intel.com> writes: > >> > >> > Hi Markus and Daniel, > >> > > >> > I have the questions about the -object per cache implementation: > >> > > >> > On Wed, Jul 24, 2024 at 02:39:29PM +0200, Markus Armbruster wrote: > >> >> Date: Wed, 24 Jul 2024 14:39:29 +0200 > >> >> From: Markus Armbruster <armbru@redhat.com> > >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > >> >> object > >> >> > >> >> Zhao Liu <zhao1.liu@intel.com> writes: > >> >> > >> >> > Hi Markus, > >> >> > > >> >> > On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: > >> >> >> Date: Mon, 22 Jul 2024 15:37:43 +0200 > >> >> >> From: Markus Armbruster <armbru@redhat.com> > >> >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > >> >> >> object > >> >> >> > >> >> >> Zhao Liu <zhao1.liu@intel.com> writes: > >> >> >> > >> >> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > >> >> >> > >> >> >> This patch is just documentation. The code got added in some previous > >> >> >> patch. Would it make sense to squash this patch into that previous > >> >> >> patch? > >> >> > > >> >> > OK, I'll merge them. > >> >> > > >> >> >> > --- > >> >> >> > Changes since RFC v2: > >> >> >> > * Rewrote the document of smp-cache object. > >> >> >> > > >> >> >> > Changes since RFC v1: > >> >> >> > * Use "*_cache=topo_level" as -smp example as the original "level" > >> >> >> > term for a cache has a totally different meaning. (Jonathan) > >> >> >> > --- > >> >> >> > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > >> >> >> > 1 file changed, 58 insertions(+) > >> >> >> > > >> >> >> > diff --git a/qemu-options.hx b/qemu-options.hx > >> >> >> > index 8ca7f34ef0c8..4b84f4508a6e 100644 > >> >> >> > --- a/qemu-options.hx > >> >> >> > +++ b/qemu-options.hx > >> >> >> > @@ -159,6 +159,15 @@ SRST > >> >> >> > :: > >> >> >> > > >> >> >> > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 > >> >> >> > + > >> >> >> > + ``smp-cache='id'`` > >> >> >> > + Allows to configure cache property (now only the cache topology level). > >> >> >> > + > >> >> >> > + For example: > >> >> >> > + :: > >> >> >> > + > >> >> >> > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' > >> >> >> > + -machine smp-cache=cache > >> >> >> > ERST > >> >> >> > > >> >> >> > DEF("M", HAS_ARG, QEMU_OPTION_M, > >> >> >> > @@ -5871,6 +5880,55 @@ SRST > >> >> >> > :: > >> >> >> > > >> >> >> > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 > >> >> >> > + > >> >> >> > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` > >> >> >> > + Create an smp-cache object that configures machine's cache > >> >> >> > + property. Currently, cache property only include cache topology > >> >> >> > + level. > >> >> >> > + > >> >> >> > + This option must be written in JSON format to support JSON list. > >> >> >> > >> >> >> Why? > >> >> > > >> >> > I'm not familiar with this, so I hope you could educate me if I'm wrong. > >> >> > > >> >> > All I know so far is for -object that defining a list can only be done in > >> >> > JSON format and not with a numeric index like a keyval based option, like: > >> >> > > >> >> > -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing > >> >> > > >> >> > the above doesn't work. > >> >> > > >> >> > Is there any other way to specify a list in command line? > >> >> > >> >> The command line is a big, sprawling mess :) > >> >> > >> >> -object supports either a JSON or a QemuOpts argument. *Not* keyval! > >> >> > >> >> Both QemuOpts and keyval parse something like KEY=VALUE,... Keyval > >> >> supports arrays and objects via dotted keys. QemuOpts doesn't natively > >> >> support arrays and objects, but its users can hack around that > >> >> limitation in various ways. -object doesn't. So you're right, it's > >> >> JSON or bust here. > >> >> > >> >> However, if we used one object per cache instead, we could get something > >> >> like > >> >> > >> >> -object smp-cache,name=l1d,... > >> >> -object smp-cache,name=l1u,... > >> >> -object smp-cache,name=l2,... > >> >> ... > >> > > >> > Current, I use -object to create a smp_cache object, and link it to > >> > MachineState by -machine,smp-cache=obj_id. > >> > > >> > Then for the objects per cache, how could I link them to machine? > >> > > >> > Is it possible that I create something static in smp_cache.c and expose > >> > all the cache information to machine through some interface? > >> > >> Good questions. However, before we head deeper into the weeds here, I > >> feel we should discuss the things below. And before we do that, I need > >> a clear understanding of the use case. Elsewhere in this thread, I just > >> described the use case as I understand it. Please reply there. I'll > >> then come back to this message. > >> > >> [...] > > > > Jonathan and I provided different use cases for x86 and Arm. Could we > > come back here to continue the discussion? :) > > 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. * 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. 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 * 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 * 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? - 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. Regards, Zhao
Hi Markus, Just a kindly ping. Hopefully we can continue this discussion when you're free. Regards, Zhao On Fri, Aug 02, 2024 at 03:58:02PM +0800, Zhao Liu wrote: > Date: Fri, 2 Aug 2024 15:58:02 +0800 > From: Zhao Liu <zhao1.liu@intel.com> > Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > object > > On Thu, Aug 01, 2024 at 01:28:27PM +0200, Markus Armbruster wrote: > > Date: Thu, 01 Aug 2024 13:28:27 +0200 > > From: Markus Armbruster <armbru@redhat.com> > > Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > > object > > > > Zhao Liu <zhao1.liu@intel.com> writes: > > > > > On Thu, Jul 25, 2024 at 11:07:12AM +0200, Markus Armbruster wrote: > > >> Date: Thu, 25 Jul 2024 11:07:12 +0200 > > >> From: Markus Armbruster <armbru@redhat.com> > > >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > > >> object > > >> > > >> Zhao Liu <zhao1.liu@intel.com> writes: > > >> > > >> > Hi Markus and Daniel, > > >> > > > >> > I have the questions about the -object per cache implementation: > > >> > > > >> > On Wed, Jul 24, 2024 at 02:39:29PM +0200, Markus Armbruster wrote: > > >> >> Date: Wed, 24 Jul 2024 14:39:29 +0200 > > >> >> From: Markus Armbruster <armbru@redhat.com> > > >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > > >> >> object > > >> >> > > >> >> Zhao Liu <zhao1.liu@intel.com> writes: > > >> >> > > >> >> > Hi Markus, > > >> >> > > > >> >> > On Mon, Jul 22, 2024 at 03:37:43PM +0200, Markus Armbruster wrote: > > >> >> >> Date: Mon, 22 Jul 2024 15:37:43 +0200 > > >> >> >> From: Markus Armbruster <armbru@redhat.com> > > >> >> >> Subject: Re: [PATCH 8/8] qemu-options: Add the description of smp-cache > > >> >> >> object > > >> >> >> > > >> >> >> Zhao Liu <zhao1.liu@intel.com> writes: > > >> >> >> > > >> >> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > > >> >> >> > > >> >> >> This patch is just documentation. The code got added in some previous > > >> >> >> patch. Would it make sense to squash this patch into that previous > > >> >> >> patch? > > >> >> > > > >> >> > OK, I'll merge them. > > >> >> > > > >> >> >> > --- > > >> >> >> > Changes since RFC v2: > > >> >> >> > * Rewrote the document of smp-cache object. > > >> >> >> > > > >> >> >> > Changes since RFC v1: > > >> >> >> > * Use "*_cache=topo_level" as -smp example as the original "level" > > >> >> >> > term for a cache has a totally different meaning. (Jonathan) > > >> >> >> > --- > > >> >> >> > qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ > > >> >> >> > 1 file changed, 58 insertions(+) > > >> >> >> > > > >> >> >> > diff --git a/qemu-options.hx b/qemu-options.hx > > >> >> >> > index 8ca7f34ef0c8..4b84f4508a6e 100644 > > >> >> >> > --- a/qemu-options.hx > > >> >> >> > +++ b/qemu-options.hx > > >> >> >> > @@ -159,6 +159,15 @@ SRST > > >> >> >> > :: > > >> >> >> > > > >> >> >> > -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 > > >> >> >> > + > > >> >> >> > + ``smp-cache='id'`` > > >> >> >> > + Allows to configure cache property (now only the cache topology level). > > >> >> >> > + > > >> >> >> > + For example: > > >> >> >> > + :: > > >> >> >> > + > > >> >> >> > + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' > > >> >> >> > + -machine smp-cache=cache > > >> >> >> > ERST > > >> >> >> > > > >> >> >> > DEF("M", HAS_ARG, QEMU_OPTION_M, > > >> >> >> > @@ -5871,6 +5880,55 @@ SRST > > >> >> >> > :: > > >> >> >> > > > >> >> >> > (qemu) qom-set /objects/iothread1 poll-max-ns 100000 > > >> >> >> > + > > >> >> >> > + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` > > >> >> >> > + Create an smp-cache object that configures machine's cache > > >> >> >> > + property. Currently, cache property only include cache topology > > >> >> >> > + level. > > >> >> >> > + > > >> >> >> > + This option must be written in JSON format to support JSON list. > > >> >> >> > > >> >> >> Why? > > >> >> > > > >> >> > I'm not familiar with this, so I hope you could educate me if I'm wrong. > > >> >> > > > >> >> > All I know so far is for -object that defining a list can only be done in > > >> >> > JSON format and not with a numeric index like a keyval based option, like: > > >> >> > > > >> >> > -object smp-cache,id=cache0,caches.0.name=l1i,caches.0.topo=core: Parameter 'caches' is missing > > >> >> > > > >> >> > the above doesn't work. > > >> >> > > > >> >> > Is there any other way to specify a list in command line? > > >> >> > > >> >> The command line is a big, sprawling mess :) > > >> >> > > >> >> -object supports either a JSON or a QemuOpts argument. *Not* keyval! > > >> >> > > >> >> Both QemuOpts and keyval parse something like KEY=VALUE,... Keyval > > >> >> supports arrays and objects via dotted keys. QemuOpts doesn't natively > > >> >> support arrays and objects, but its users can hack around that > > >> >> limitation in various ways. -object doesn't. So you're right, it's > > >> >> JSON or bust here. > > >> >> > > >> >> However, if we used one object per cache instead, we could get something > > >> >> like > > >> >> > > >> >> -object smp-cache,name=l1d,... > > >> >> -object smp-cache,name=l1u,... > > >> >> -object smp-cache,name=l2,... > > >> >> ... > > >> > > > >> > Current, I use -object to create a smp_cache object, and link it to > > >> > MachineState by -machine,smp-cache=obj_id. > > >> > > > >> > Then for the objects per cache, how could I link them to machine? > > >> > > > >> > Is it possible that I create something static in smp_cache.c and expose > > >> > all the cache information to machine through some interface? > > >> > > >> Good questions. However, before we head deeper into the weeds here, I > > >> feel we should discuss the things below. And before we do that, I need > > >> a clear understanding of the use case. Elsewhere in this thread, I just > > >> described the use case as I understand it. Please reply there. I'll > > >> then come back to this message. > > >> > > >> [...] > > > > > > Jonathan and I provided different use cases for x86 and Arm. Could we > > > come back here to continue the discussion? :) > > > > 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. > > > * 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. > > 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 > > > * 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 > > > * 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? > > - 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. > > Regards, > Zhao > > >
I apologize for the delay. 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. > * 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. 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. > * 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 > * 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. > - 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. 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? What else will you need to configure in the future? By the way, extending -machine to support JSON looks feasible to me at a glance.
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
diff --git a/qemu-options.hx b/qemu-options.hx index 8ca7f34ef0c8..4b84f4508a6e 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -159,6 +159,15 @@ SRST :: -machine cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=128G,cxl-fmw.0.interleave-granularity=512 + + ``smp-cache='id'`` + Allows to configure cache property (now only the cache topology level). + + For example: + :: + + -object '{"qom-type":"smp-cache","id":"cache","caches":[{"name":"l1d","topo":"core"},{"name":"l1i","topo":"core"},{"name":"l2","topo":"module"},{"name":"l3","topo":"die"}]}' + -machine smp-cache=cache ERST DEF("M", HAS_ARG, QEMU_OPTION_M, @@ -5871,6 +5880,55 @@ SRST :: (qemu) qom-set /objects/iothread1 poll-max-ns 100000 + + ``-object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}'`` + Create an smp-cache object that configures machine's cache + property. Currently, cache property only include cache topology + level. + + This option must be written in JSON format to support JSON list. + + The ``caches`` parameter accepts a list of cache property in JSON + format. + + A list element requires the cache name to be specified in the + ``name`` parameter (currently ``l1d``, ``l1i``, ``l2`` and ``l3`` + are supported). ``topo`` parameter accepts CPU topology levels + including ``thread``, ``core``, ``module``, ``cluster``, ``die``, + ``socket``, ``book``, ``drawer`` and ``default``. The ``topo`` + parameter indicates CPUs winthin the same CPU topology container + are sharing the same cache. + + Some machines may have their own cache topology model, and this + object may override the machine-specific cache topology setting + by specifying smp-cache object in the -machine. When specifying + the cache topology level of ``default``, it will honor the default + machine-specific cache topology setting. For other topology levels, + they will override the default setting. + + An example list of caches to configure the cache model (l1d cache + per core, l1i cache per core, l2 cache per module and l3 cache per + socket) supported by PC machine might look like: + + :: + + { + "caches": [ + { "name": "l1d", "topo": "core" }, + { "name": "l1i", "topo": "core" }, + { "name": "l2", "topo": "module" }, + { "name": "l3", "topo": "socket" }, + ] + } + + An example smp-cache object would look like:() + + .. parsed-literal:: + + # |qemu_system| \\ + ... \\ + -object '{"qom-type":"smp-cache","id":id,"caches":[{"name":cache_name,"topo":cache_topo}]}' \\ + ... ERST
Signed-off-by: Zhao Liu <zhao1.liu@intel.com> --- Changes since RFC v2: * Rewrote the document of smp-cache object. Changes since RFC v1: * Use "*_cache=topo_level" as -smp example as the original "level" term for a cache has a totally different meaning. (Jonathan) --- qemu-options.hx | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)