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

2024-06-04 Thread 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 
> Subject: Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration
>  arch-agnostic
> 
> Zhao Liu  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 
> >> Subject: Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration
> >>  arch-agnostic
> >> 
> >> Zhao Liu  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 
> >> 
> >> [...]
> >> 
> >> > 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.




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

2024-06-04 Thread Markus Armbruster
Zhao Liu  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 
>> Subject: Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration
>>  arch-agnostic
>> 
>> Zhao Liu  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 
>> 
>> [...]
>> 
>> > 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
##




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

2024-06-04 Thread Markus Armbruster
Zhao Liu  writes:

[...]

>> > +##
>> > +# @CPUTopoLevel:
>> > +#
>> > +# An enumeration of CPU topology levels.
>> > +#
>> > +# @invalid: Invalid topology level, used as a placeholder.
>> 
>> Placeholder for what?
>
> I was trying to express that when no specific topology level is
> specified, it will be initialized to this value by default.
>
> Or what about just deleting this placeholder related words and just
> saying it's "Invalid topology level"?

Works for me.

[...]




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

2024-06-04 Thread Zhao Liu
[snip]

> > +CPUTopoInfo cpu_topo_descriptors[] = {
> > +[CPU_TOPO_LEVEL_INVALID] = { .name = "invalid", },
> > +[CPU_TOPO_LEVEL_THREAD]  = { .name = "thread",  },
> > +[CPU_TOPO_LEVEL_CORE]= { .name = "core",},
> > +[CPU_TOPO_LEVEL_MODULE]  = { .name = "module",  },
> > +[CPU_TOPO_LEVEL_CLUSTER] = { .name = "cluster", },
> > +[CPU_TOPO_LEVEL_DIE] = { .name = "die", },
> > +[CPU_TOPO_LEVEL_SOCKET]  = { .name = "socket",  },
> > +[CPU_TOPO_LEVEL_BOOK]= { .name = "book",},
> > +[CPU_TOPO_LEVEL_DRAWER]  = { .name = "drawer",  },
> > +[CPU_TOPO_LEVEL__MAX]= { .name = NULL,  },
> > +};
> 
> This looks redundant with generated
> 
> const QEnumLookup CPUTopoLevel_lookup = {
> .array = (const char *const[]) {
> [CPU_TOPO_LEVEL_INVALID] = "invalid",
> [CPU_TOPO_LEVEL_THREAD] = "thread",
> [CPU_TOPO_LEVEL_CORE] = "core",
> [CPU_TOPO_LEVEL_MODULE] = "module",
> [CPU_TOPO_LEVEL_CLUSTER] = "cluster",
> [CPU_TOPO_LEVEL_DIE] = "die",
> [CPU_TOPO_LEVEL_SOCKET] = "socket",
> [CPU_TOPO_LEVEL_BOOK] = "book",
> [CPU_TOPO_LEVEL_DRAWER] = "drawer",
> },
> .size = CPU_TOPO_LEVEL__MAX
> };
> 
> > +
> > +const char *cpu_topo_to_string(CPUTopoLevel topo)
> > +{
> > +return cpu_topo_descriptors[topo].name;
> > +}
> 
> And this with generated CPUTopoLevel_str().

Thanks! I missed these generated helpers.

[snip]

> > +##
> > +# @CPUTopoLevel:
> > +#
> > +# An enumeration of CPU topology levels.
> > +#
> > +# @invalid: Invalid topology level, used as a placeholder.
> 
> Placeholder for what?

I was trying to express that when no specific topology level is
specified, it will be initialized to this value by default.

Or what about just deleting this placeholder related words and just
saying it's "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.
> 
> As far as I can tell, -smp is sugar for machine property "smp" of QAPI
> type SMPConfiguration.  Should we refer to SMPConfiguration instead of
> -smp?

Yes, SMPConfiguration is better.

Thanks,
Zhao




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

2024-06-04 Thread Zhao Liu
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 
> Subject: Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration
>  arch-agnostic
> 
> Zhao Liu  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 
> 
> [...]
> 
> > 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.


Thanks,
Zhao




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

2024-06-03 Thread Markus Armbruster
Zhao Liu  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 

[...]

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

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

> +#
> +# Since: 9.1
> +##
> +{ 'enum': 'CPUTopoLevel',
> +  'prefix': 'CPU_TOPO_LEVEL',
> +  'data': [ 'invalid', 'thread', 'core', 'module', 'cluster',
> +'die', 'socket', 'book', 'drawer' ] }
> +
>  ##
>  # @SMPConfiguration:
>  #

[...]




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

2024-06-03 Thread Markus Armbruster
Zhao Liu  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 
> ---
> Changes since RFC v1:
>  * Use QAPI to enumerate CPU topology levels.
>  * Drop string_to_cpu_topo() since QAPI will help to parse the topo
>levels.
> ---
>  MAINTAINERS|  2 ++
>  hw/core/cpu-topology.c | 36 ++
>  hw/core/meson.build|  1 +
>  include/hw/core/cpu-topology.h | 20 +
>  include/hw/i386/topology.h | 18 +--
>  qapi/machine.json  | 40 ++
>  target/i386/cpu.c  | 30 -
>  target/i386/cpu.h  |  4 ++--
>  8 files changed, 117 insertions(+), 34 deletions(-)
>  create mode 100644 hw/core/cpu-topology.c
>  create mode 100644 include/hw/core/cpu-topology.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 448dc951c509..09173e8c953d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1875,6 +1875,7 @@ R: Yanan Wang 
>  S: Supported
>  F: hw/core/cpu-common.c
>  F: hw/core/cpu-sysemu.c
> +F: hw/core/cpu-topology.c
>  F: hw/core/machine-qmp-cmds.c
>  F: hw/core/machine.c
>  F: hw/core/machine-smp.c
> @@ -1886,6 +1887,7 @@ F: qapi/machine-common.json
>  F: qapi/machine-target.json
>  F: include/hw/boards.h
>  F: include/hw/core/cpu.h
> +F: include/hw/core/cpu-topology.h
>  F: include/hw/cpu/cluster.h
>  F: include/sysemu/numa.h
>  F: tests/unit/test-smp-parse.c
> diff --git a/hw/core/cpu-topology.c b/hw/core/cpu-topology.c
> new file mode 100644
> index ..20b5d708cb54
> --- /dev/null
> +++ b/hw/core/cpu-topology.c
> @@ -0,0 +1,36 @@
> +/*
> + * QEMU CPU Topology Representation
> + *
> + * Copyright (c) 2024 Intel Corporation
> + *
> + * Authors:
> + *  Zhao Liu 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/core/cpu-topology.h"
> +
> +typedef struct CPUTopoInfo {
> +const char *name;
> +} CPUTopoInfo;
> +
> +CPUTopoInfo cpu_topo_descriptors[] = {
> +[CPU_TOPO_LEVEL_INVALID] = { .name = "invalid", },
> +[CPU_TOPO_LEVEL_THREAD]  = { .name = "thread",  },
> +[CPU_TOPO_LEVEL_CORE]= { .name = "core",},
> +[CPU_TOPO_LEVEL_MODULE]  = { .name = "module",  },
> +[CPU_TOPO_LEVEL_CLUSTER] = { .name = "cluster", },
> +[CPU_TOPO_LEVEL_DIE] = { .name = "die", },
> +[CPU_TOPO_LEVEL_SOCKET]  = { .name = "socket",  },
> +[CPU_TOPO_LEVEL_BOOK]= { .name = "book",},
> +[CPU_TOPO_LEVEL_DRAWER]  = { .name = "drawer",  },
> +[CPU_TOPO_LEVEL__MAX]= { .name = NULL,  },
> +};

This looks redundant with generated

const QEnumLookup CPUTopoLevel_lookup = {
.array = (const char *const[]) {
[CPU_TOPO_LEVEL_INVALID] = "invalid",
[CPU_TOPO_LEVEL_THREAD] = "thread",
[CPU_TOPO_LEVEL_CORE] = "core",
[CPU_TOPO_LEVEL_MODULE] = "module",
[CPU_TOPO_LEVEL_CLUSTER] = "cluster",
[CPU_TOPO_LEVEL_DIE] = "die",
[CPU_TOPO_LEVEL_SOCKET] = "socket",
[CPU_TOPO_LEVEL_BOOK] = "book",
[CPU_TOPO_LEVEL_DRAWER] = "drawer",
},
.size = CPU_TOPO_LEVEL__MAX
};

> +
> +const char *cpu_topo_to_string(CPUTopoLevel topo)
> +{
> +return cpu_topo_descriptors[topo].name;
> +}

And this with generated CPUTopoLevel_str().

> diff --git a/hw/core/meson.build b/hw/core/meson.build
> index a3d9bab9f42a..71dc396e9bfc 100644
> --- a/hw/core/meson.build
> +++ b/hw/core/meson.build
> @@ -13,6 +13,7 @@ hwcore_ss.add(files(
>  ))
>  
>  common_ss.add(files('cpu-common.c'))
> +common_ss.add(files('cpu-topology.c'))
>  common_ss.add(files('machine-smp.c'))
>  system_ss.add(when: 'CONFIG_FITLOADER', if_true: files('loader-fit.c'))
>  system_ss.add(when: 'CONFIG_GENERIC_LOADER', if_true: 
> files('generic-loader.c'))
> diff --git a/include/hw/core/cpu-topology.h b/include/hw/core/cpu-topology.h
> new file mode 100644
> index ..0e21fe8a9bf8
> --- /dev/null
> +++ b/include/hw/core/cpu-topology.h
> @@ -0,0 +1,20 @@
> +/*
> + * QEMU CPU Topology Representation Header
> + *
> + * Copyright (c) 2024 Intel Corporation
> + *
> + * Authors:
> + *  Zhao Liu 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef CPU_TOPOLOGY_H
> +#define CPU_TOPOLOGY_H
> +
> +#include "qapi/qapi-types-machine.h"
> +
> 

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

2024-05-30 Thread Zhao Liu
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 
---
Changes since RFC v1:
 * Use QAPI to enumerate CPU topology levels.
 * Drop string_to_cpu_topo() since QAPI will help to parse the topo
   levels.
---
 MAINTAINERS|  2 ++
 hw/core/cpu-topology.c | 36 ++
 hw/core/meson.build|  1 +
 include/hw/core/cpu-topology.h | 20 +
 include/hw/i386/topology.h | 18 +--
 qapi/machine.json  | 40 ++
 target/i386/cpu.c  | 30 -
 target/i386/cpu.h  |  4 ++--
 8 files changed, 117 insertions(+), 34 deletions(-)
 create mode 100644 hw/core/cpu-topology.c
 create mode 100644 include/hw/core/cpu-topology.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 448dc951c509..09173e8c953d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1875,6 +1875,7 @@ R: Yanan Wang 
 S: Supported
 F: hw/core/cpu-common.c
 F: hw/core/cpu-sysemu.c
+F: hw/core/cpu-topology.c
 F: hw/core/machine-qmp-cmds.c
 F: hw/core/machine.c
 F: hw/core/machine-smp.c
@@ -1886,6 +1887,7 @@ F: qapi/machine-common.json
 F: qapi/machine-target.json
 F: include/hw/boards.h
 F: include/hw/core/cpu.h
+F: include/hw/core/cpu-topology.h
 F: include/hw/cpu/cluster.h
 F: include/sysemu/numa.h
 F: tests/unit/test-smp-parse.c
diff --git a/hw/core/cpu-topology.c b/hw/core/cpu-topology.c
new file mode 100644
index ..20b5d708cb54
--- /dev/null
+++ b/hw/core/cpu-topology.c
@@ -0,0 +1,36 @@
+/*
+ * QEMU CPU Topology Representation
+ *
+ * Copyright (c) 2024 Intel Corporation
+ *
+ * Authors:
+ *  Zhao Liu 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/core/cpu-topology.h"
+
+typedef struct CPUTopoInfo {
+const char *name;
+} CPUTopoInfo;
+
+CPUTopoInfo cpu_topo_descriptors[] = {
+[CPU_TOPO_LEVEL_INVALID] = { .name = "invalid", },
+[CPU_TOPO_LEVEL_THREAD]  = { .name = "thread",  },
+[CPU_TOPO_LEVEL_CORE]= { .name = "core",},
+[CPU_TOPO_LEVEL_MODULE]  = { .name = "module",  },
+[CPU_TOPO_LEVEL_CLUSTER] = { .name = "cluster", },
+[CPU_TOPO_LEVEL_DIE] = { .name = "die", },
+[CPU_TOPO_LEVEL_SOCKET]  = { .name = "socket",  },
+[CPU_TOPO_LEVEL_BOOK]= { .name = "book",},
+[CPU_TOPO_LEVEL_DRAWER]  = { .name = "drawer",  },
+[CPU_TOPO_LEVEL__MAX]= { .name = NULL,  },
+};
+
+const char *cpu_topo_to_string(CPUTopoLevel topo)
+{
+return cpu_topo_descriptors[topo].name;
+}
diff --git a/hw/core/meson.build b/hw/core/meson.build
index a3d9bab9f42a..71dc396e9bfc 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -13,6 +13,7 @@ hwcore_ss.add(files(
 ))
 
 common_ss.add(files('cpu-common.c'))
+common_ss.add(files('cpu-topology.c'))
 common_ss.add(files('machine-smp.c'))
 system_ss.add(when: 'CONFIG_FITLOADER', if_true: files('loader-fit.c'))
 system_ss.add(when: 'CONFIG_GENERIC_LOADER', if_true: 
files('generic-loader.c'))
diff --git a/include/hw/core/cpu-topology.h b/include/hw/core/cpu-topology.h
new file mode 100644
index ..0e21fe8a9bf8
--- /dev/null
+++ b/include/hw/core/cpu-topology.h
@@ -0,0 +1,20 @@
+/*
+ * QEMU CPU Topology Representation Header
+ *
+ * Copyright (c) 2024 Intel Corporation
+ *
+ * Authors:
+ *  Zhao Liu 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#ifndef CPU_TOPOLOGY_H
+#define CPU_TOPOLOGY_H
+
+#include "qapi/qapi-types-machine.h"
+
+const char *cpu_topo_to_string(CPUTopoLevel topo);
+
+#endif /* CPU_TOPOLOGY_H */
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index dff49fce1154..c6ff75f23991 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -39,7 +39,7 @@
  *  CPUID Fn8000_0008_ECX[ApicIdCoreIdSize[3:0]] is set to apicid_core_width().
  */
 
-
+#include "hw/core/cpu-topology.h"
 #include "qemu/bitops.h"
 
 /*
@@ -62,22 +62,6 @@ typedef struct X86CPUTopoInfo {
 unsigned threads_per_core;
 } X86CPUTopoInfo;
 
-/*
- * CPUTopoLevel is the general i386 topology hierarchical representation,
- * ordered by increasing hierarchical relationship.
- * Its enumeration value is not bound to the type value of Intel (CPUID[0x1F])
- * or AMD (CPUID[0x8026]).
- */
-enum CPUTopoLevel {
-CPU_TOPO_LEVEL_INVALID,
-CPU_TOPO_LEVEL_SMT,
-CPU_TOPO_LEVEL_CORE,
-CPU_TOPO_LEVEL_MODULE,
-