On Thu, Feb 17, 2022 at 02:41:22PM +0100, Pierre Morel wrote:
> S390 CPU topology may have up to 5 topology containers.
> The first container above the cores is level 2, the sockets,
> and the level 3, containing sockets are the books.
>
> We introduce here the drawers, drawers is the level containing books.
>
> Let's add drawers, level4, containers to the CPU topology.
>
> Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
> ---
> hw/core/machine-smp.c | 33 ++++++++++++++++++++++++++-------
> hw/core/machine.c | 2 ++
> hw/s390x/s390-virtio-ccw.c | 1 +
> include/hw/boards.h | 4 ++++
> qapi/machine.json | 7 ++++++-
> softmmu/vl.c | 3 +++
> 6 files changed, 42 insertions(+), 8 deletions(-)
Needs to update -smp args in qemu-options.hx too.
>
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> index d7aa39d540..26150c748f 100644
> --- a/hw/core/machine-smp.c
> +++ b/hw/core/machine-smp.c
> @@ -31,6 +31,10 @@ static char *cpu_hierarchy_to_string(MachineState *ms)
> MachineClass *mc = MACHINE_GET_CLASS(ms);
> GString *s = g_string_new(NULL);
>
> + if (mc->smp_props.drawers_supported) {
> + g_string_append_printf(s, " * drawers (%u)", ms->smp.drawers);
> + }
> +
> if (mc->smp_props.books_supported) {
> g_string_append_printf(s, " * books (%u)", ms->smp.books);
> }
> @@ -77,6 +81,7 @@ void machine_parse_smp_config(MachineState *ms,
> {
> MachineClass *mc = MACHINE_GET_CLASS(ms);
> unsigned cpus = config->has_cpus ? config->cpus : 0;
> + unsigned drawers = config->has_drawers ? config->drawers : 0;
> unsigned books = config->has_books ? config->books : 0;
> unsigned sockets = config->has_sockets ? config->sockets : 0;
> unsigned dies = config->has_dies ? config->dies : 0;
> @@ -90,6 +95,7 @@ void machine_parse_smp_config(MachineState *ms,
> * explicit configuration like "cpus=0" is not allowed.
> */
> if ((config->has_cpus && config->cpus == 0) ||
> + (config->has_drawers && config->drawers == 0) ||
> (config->has_books && config->books == 0) ||
> (config->has_sockets && config->sockets == 0) ||
> (config->has_dies && config->dies == 0) ||
> @@ -124,6 +130,13 @@ void machine_parse_smp_config(MachineState *ms,
>
> books = books > 0 ? books : 1;
>
> + if (!mc->smp_props.drawers_supported && drawers > 1) {
> + error_setg(errp, "drawers not supported by this machine's CPU topology");
> + return;
> + }
> +
> + drawers = drawers > 0 ? drawers : 1;
> +
> /* compute missing values based on the provided ones */
> if (cpus == 0 && maxcpus == 0) {
> sockets = sockets > 0 ? sockets : 1;
> @@ -137,34 +150,40 @@ void machine_parse_smp_config(MachineState *ms,
> if (sockets == 0) {
> cores = cores > 0 ? cores : 1;
> threads = threads > 0 ? threads : 1;
> - sockets = maxcpus / (books * dies * clusters * cores * threads);
> + sockets = maxcpus /
> + (drawers * books * dies * clusters * cores * threads);
> } else if (cores == 0) {
> threads = threads > 0 ? threads : 1;
> - cores = maxcpus / (books * sockets * dies * clusters * threads);
> + cores = maxcpus /
> + (drawers * books * sockets * dies * clusters * threads);
> }
> } else {
> /* prefer cores over sockets since 6.2 */
> if (cores == 0) {
> sockets = sockets > 0 ? sockets : 1;
> threads = threads > 0 ? threads : 1;
> - cores = maxcpus / (books * sockets * dies * clusters * threads);
> + cores = maxcpus /
> + (drawers * books * sockets * dies * clusters * threads);
> } else if (sockets == 0) {
> threads = threads > 0 ? threads : 1;
> - sockets = maxcpus / (books * dies * clusters * cores * threads);
> + sockets = maxcpus /
> + (drawers * books * dies * clusters * cores * threads);
> }
> }
>
> /* try to calculate omitted threads at last */
> if (threads == 0) {
> - threads = maxcpus / (books * sockets * dies * clusters * cores);
> + threads = maxcpus /
> + (drawers * books * sockets * dies * clusters * cores);
> }
> }
>
> - maxcpus = maxcpus > 0 ? maxcpus : books * sockets * dies *
> + maxcpus = maxcpus > 0 ? maxcpus : drawers * books * sockets * dies *
> clusters * cores * threads;
> cpus = cpus > 0 ? cpus : maxcpus;
>
> ms->smp.cpus = cpus;
> + ms->smp.drawers = drawers;
> ms->smp.books = books;
> ms->smp.sockets = sockets;
> ms->smp.dies = dies;
> @@ -174,7 +193,7 @@ void machine_parse_smp_config(MachineState *ms,
> ms->smp.max_cpus = maxcpus;
>
> /* sanity-check of the computed topology */
> - if (books * sockets * dies * clusters * cores * threads != maxcpus) {
> + if (drawers * books * sockets * dies * clusters * cores * threads != maxcpus) {
> g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
> error_setg(errp, "Invalid CPU topology: "
> "product of the hierarchy must match maxcpus: "
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index b8c624d2bf..1db55e36c8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -743,6 +743,7 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name,
> MachineState *ms = MACHINE(obj);
> SMPConfiguration *config = &(SMPConfiguration){
> .has_cpus = true, .cpus = ms->smp.cpus,
> + .has_drawers = true, .drawers = ms->smp.drawers,
> .has_books = true, .books = ms->smp.books,
> .has_sockets = true, .sockets = ms->smp.sockets,
> .has_dies = true, .dies = ms->smp.dies,
> @@ -936,6 +937,7 @@ static void machine_initfn(Object *obj)
> /* default to mc->default_cpus */
> ms->smp.cpus = mc->default_cpus;
> ms->smp.max_cpus = mc->default_cpus;
> + ms->smp.drawers = 1;
> ms->smp.books = 1;
> ms->smp.sockets = 1;
> ms->smp.dies = 1;
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 193883fba3..03829e90b3 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -667,6 +667,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
> nc->nmi_monitor_handler = s390_nmi;
> mc->default_ram_id = "s390.ram";
> mc->smp_props.books_supported = true;
> + mc->smp_props.drawers_supported = true;
> }
>
> static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index bc0f7f22dc..abc5556c50 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -131,12 +131,14 @@ typedef struct {
> * @dies_supported - whether dies are supported by the machine
> * @clusters_supported - whether clusters are supported by the machine
> * @books_supported - whether books are supported by the machine
> + * @drawers_supported - whether drawers are supported by the machine
> */
> typedef struct {
> bool prefer_sockets;
> bool dies_supported;
> bool clusters_supported;
> bool books_supported;
> + bool drawers_supported;
> } SMPCompatProps;
>
> /**
> @@ -301,6 +303,7 @@ typedef struct DeviceMemoryState {
> /**
> * CpuTopology:
> * @cpus: the number of present logical processors on the machine
> + * @drawers: the number of drawers on the machine
> * @books: the number of books on the machine
> * @sockets: the number of sockets on the machine
> * @dies: the number of dies in one socket
> @@ -311,6 +314,7 @@ typedef struct DeviceMemoryState {
> */
> typedef struct CpuTopology {
> unsigned int cpus;
> + unsigned int drawers;
> unsigned int books;
> unsigned int sockets;
> unsigned int dies;
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 73206f811a..fa6bde5617 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -866,13 +866,14 @@
> # a CPU is being hotplugged.
> #
> # @node-id: NUMA node ID the CPU belongs to
> +# @drawer-id: drawer number within node/board the CPU belongs to
> # @book-id: book number within node/board the CPU belongs to
> # @socket-id: socket number within node/board the CPU belongs to
So the lack of change here implies that 'socket-id' is unique
across multiple books/drawers. Is that correct, as its differnt
from semantics for die-id/core-id/thread-id which are scoped
to within the next level of the topology ?
> # @die-id: die number within socket the CPU belongs to (since 4.1)
> # @core-id: core number within die the CPU belongs to
> # @thread-id: thread number within core the CPU belongs to
> #
> -# Note: currently there are 6 properties that could be present
> +# Note: currently there are 7 properties that could be present
> # but management should be prepared to pass through other
> # properties with device_add command to allow for future
> # interface extension. This also requires the filed names to be kept in
> @@ -882,6 +883,7 @@
> ##
> { 'struct': 'CpuInstanceProperties',
> 'data': { '*node-id': 'int',
> + '*drawer-id': 'int',
> '*book-id': 'int',
> '*socket-id': 'int',
> '*die-id': 'int',
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
