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

Markus Armbruster
2024-06-04T08:47:53Z

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
##