Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration arch-agnostic

[Zhao Liu]

On Tue, Jun 04, 2024 at 10:47:44AM +0200, Markus Armbruster wrote:
> Date: Tue, 04 Jun 2024 10:47:44 +0200
> From: Markus Armbruster <armbru@redhat.com>
> Subject: Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration
>  arch-agnostic
> 
> Zhao Liu <zhao1.liu@intel.com> writes:
> 
> > Hi Markus,
> >
> > On Mon, Jun 03, 2024 at 02:25:36PM +0200, Markus Armbruster wrote:
> >> Date: Mon, 03 Jun 2024 14:25:36 +0200
> >> From: Markus Armbruster <armbru@redhat.com>
> >> Subject: Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration
> >>  arch-agnostic
> >> 
> >> Zhao Liu <zhao1.liu@intel.com> writes:
> >> 
> >> > Cache topology needs to be defined based on CPU topology levels. Thus,
> >> > define CPU topology enumeration in qapi/machine.json to make it generic
> >> > for all architectures.
> >> >
> >> > To match the general topology naming style, rename CPU_TOPO_LEVEL_SMT
> >> > and CPU_TOPO_LEVEL_PACKAGE to CPU_TOPO_LEVEL_THREAD and
> >> > CPU_TOPO_LEVEL_SOCKET.
> >> >
> >> > Also, enumerate additional topology levels for non-i386 arches, and add
> >> > helpers for topology enumeration and string conversion.
> >> >
> >> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> >> 
> >> [...]
> >> 
> >> > diff --git a/qapi/machine.json b/qapi/machine.json
> >> > index bce6e1bbc412..7ac5a05bb9c9 100644
> >> > --- a/qapi/machine.json
> >> > +++ b/qapi/machine.json
> >> > @@ -1667,6 +1667,46 @@
> >> >       '*reboot-timeout': 'int',
> >> >       '*strict': 'bool' } }
> >> >  
> >> > +##
> >> > +# @CPUTopoLevel:
> >> 
> >> I understand you're moving existing enum CPUTopoLevel into the QAPI
> >> schema.  I think the idiomatic QAPI name would be CpuTopologyLevel.
> >> Would you be willing to rename it, or would that be too much churn?
> >
> > Sure, I'll rename it as you suggested.
> >
> >> > +#
> >> > +# An enumeration of CPU topology levels.
> >> > +#
> >> > +# @invalid: Invalid topology level, used as a placeholder.
> >> > +#
> >> > +# @thread: thread level, which would also be called SMT level or logical
> >> > +#     processor level. The @threads option in -smp is used to configure
> >> > +#     the topology of this level.
> >> > +#
> >> > +# @core: core level. The @cores option in -smp is used to configure the
> >> > +#     topology of this level.
> >> > +#
> >> > +# @module: module level. The @modules option in -smp is used to
> >> > +#     configure the topology of this level.
> >> > +#
> >> > +# @cluster: cluster level. The @clusters option in -smp is used to
> >> > +#     configure the topology of this level.
> >> > +#
> >> > +# @die: die level. The @dies option in -smp is used to configure the
> >> > +#     topology of this level.
> >> > +#
> >> > +# @socket: socket level, which would also be called package level. The
> >> > +#     @sockets option in -smp is used to configure the topology of this
> >> > +#     level.
> >> > +#
> >> > +# @book: book level. The @books option in -smp is used to configure the
> >> > +#     topology of this level.
> >> > +#
> >> > +# @drawer: drawer level. The @drawers option in -smp is used to
> >> > +#     configure the topology of this level.
> >> 
> >> docs/devel/qapi-code-gen.rst section Documentation markup:
> >> 
> >>     For legibility, wrap text paragraphs so every line is at most 70
> >>     characters long.
> >> 
> >>     Separate sentences with two spaces.
> >
> > Thank you for pointing this.
> >
> > About separating sentences, is this what I should be doing?
> >
> > # @drawer: drawer level. The @drawers option in -smp is used to
> > #  configure the topology of this level.
> 
> Like this:
> 
> ##
> # @CPUTopoLevel:
> #
> # An enumeration of CPU topology levels.
> #
> # @invalid: Invalid topology level.
> #
> # @thread: thread level, which would also be called SMT level or
> #     logical processor level.  The @threads option in -smp is used to
> #     configure the topology of this level.
> #
> # @core: core level.  The @cores option in -smp is used to configure
> #     the topology of this level.
> #
> # @module: module level.  The @modules option in -smp is used to
> #     configure the topology of this level.
> #
> # @cluster: cluster level.  The @clusters option in -smp is used to
> #     configure the topology of this level.
> #
> # @die: die level.  The @dies option in -smp is used to configure the
> #     topology of this level.
> #
> # @socket: socket level, which would also be called package level.
> #     The @sockets option in -smp is used to configure the topology of
> #     this level.
> #
> # @book: book level.  The @books option in -smp is used to configure
> #     the topology of this level.
> #
> # @drawer: drawer level.  The @drawers option in -smp is used to
> #     configure the topology of this level.
> #
> # Since: 9.1
> ##
>

I see, thanks very much for your patience.