Re: [PATCH] qapi/machine: Fix missing @modules in topology ordering

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

On Wed, Oct 15, 2025 at 08:21:38AM +0200, Markus Armbruster wrote:
> Date: Wed, 15 Oct 2025 08:21:38 +0200
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [PATCH] qapi/machine: Fix missing @modules in topology ordering
> 
> Zhao Liu <zhao1.liu@intel.com> writes:
> 
> > The module level is between core and cluster levels. Fix the QAPI
> > documentation to add the module level in topology ordering.
> >
> > Reported-by: Markus Armbruster <armbru@redhat.com>
> > Fixes: 8ec0a4634798 ("hw/core/machine: Support modules in -smp")
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >  qapi/machine.json | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qapi/machine.json b/qapi/machine.json
> > index 038eab281c78..5e268479e546 100644
> > --- a/qapi/machine.json
> > +++ b/qapi/machine.json
> > @@ -1624,7 +1624,7 @@
> >  # containers.
> >  #
> >  # The ordering from highest/coarsest to lowest/finest is: @drawers,
> > -# @books, @sockets, @dies, @clusters, @cores, @threads.
> > +# @books, @sockets, @dies, @clusters, @modules, @cores, @threads.
> >  #
> >  # Different architectures support different subsets of topology
> >  # containers.
> 
> Acked-by: Markus Armbruster <armbru@redhat.com>

Thanks!

> However, there are more mentions of @drawers etc. in comments and
> documentation elsewhere.  Quick grep for "drawers" there appended.
> Please double-check for missing mentions of modules.
> 
> 
> docs/about/deprecated.rst:configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is

This case should be cleaned up since I rememberred the realted change is
reverted. I will check in details and submit a patch if necessary.

> docs/devel/s390-cpu-topology.rst:    -smp 1,drawers=3,books=3,sockets=2,cores=2,maxcpus=36 \
> docs/system/s390x/cpu-topology.rst:topology containers: drawers, books and sockets. They define a
> docs/system/s390x/cpu-topology.rst:If none of the containers attributes (drawers, books, sockets) are
> hw/s390x/cpu-topology.c: * (0, 0, 0) up to the last (smp->drawers, smp->books, smp->sockets).

s390x doesn't support module level, so its doc is fine.

> include/hw/boards.h: * @drawers_supported - whether drawers are supported by the machine
> include/hw/boards.h: * @drawers: the number of drawers on the machine

general machine codes has considerred module level.

> tests/functional/s390x/test_topology.py:    the cores, sockets, books and drawers and 2 modifiers attributes,

s390x doesn't support module level.

> tests/unit/test-smp-parse.c: *  -drawers/books/sockets/cores/threads
> tests/unit/test-smp-parse.c: *  -drawers/books/sockets/dies/clusters/modules/cores/threads
> tests/unit/test-smp-parse.c:         *   -smp 8,drawers=1,books=1,sockets=2,dies=1,clusters=1,modules=1,\
> tests/unit/test-smp-parse.c:        /* config: -smp 2,drawers=2 */
> tests/unit/test-smp-parse.c:        /* config: -smp 16,drawers=2,sockets=2,cores=4,threads=2,maxcpus=16 */
> tests/unit/test-smp-parse.c:        /* config: -smp 34,drawers=2,sockets=2,cores=4,threads=2,maxcpus=32 */
> tests/unit/test-smp-parse.c:         * config: -smp 200,drawers=3,books=5,sockets=2,cores=4,\
> tests/unit/test-smp-parse.c:         * config: -smp 242,drawers=3,books=5,sockets=2,cores=4,\
> tests/unit/test-smp-parse.c:         * config: -smp 200,drawers=3,books=5,sockets=2,dies=4,\
> tests/unit/test-smp-parse.c:         * config: -smp 2881,drawers=3,books=5,sockets=2,dies=4,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=3,books=5,sockets=2,dies=4,\
> tests/unit/test-smp-parse.c:         * config: -smp 0,drawers=1,books=1,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:         * Test "drawers=0".
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=0,books=1,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=0,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=1,sockets=0,dies=1,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=1,sockets=1,dies=0,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:         * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
> tests/unit/test-smp-parse.c:        /* when drawers parameter is omitted, it will be set as 1 */
> tests/unit/test-smp-parse.c:        /* when drawers parameter is specified */
> tests/unit/test-smp-parse.c:         * when drawers and books parameters are omitted, they will
> tests/unit/test-smp-parse.c:        /* when drawers and books parameters are both specified */
> tests/unit/test-smp-parse.c:         * when drawers, books, dies, clusters and modules parameters are
> tests/unit/test-smp-parse.c:         * when drawers, books, dies, clusters and modules parameters

test-smp-parse has coverred module case, so it's fine too. But test-smp-parse
doesn't cover smp-cache. I'll consider to add the new test case.

Thanks very much!

Regards,
Zhao