diff mbox series

[v9,04/10] hw/core: introducing drawer and books for s390x

Message ID 20220902075531.188916-5-pmorel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: CPU Topology | expand

Commit Message

Pierre Morel Sept. 2, 2022, 7:55 a.m. UTC
S390x defines two topology levels above sockets: nbooks and drawers.

Let's add these two levels inside the CPU topology implementation.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 hw/core/machine-smp.c | 48 ++++++++++++++++++++++++++++++++++++-------
 hw/core/machine.c     |  4 ++++
 include/hw/boards.h   |  8 ++++++++
 qapi/machine.json     | 14 +++++++++++--
 qemu-options.hx       |  6 ++++--
 softmmu/vl.c          |  6 ++++++
 6 files changed, 75 insertions(+), 11 deletions(-)

Comments

Markus Armbruster Sept. 6, 2022, 8:59 a.m. UTC | #1
Pierre Morel <pmorel@linux.ibm.com> writes:

> S390x defines two topology levels above sockets: nbooks and drawers.

nbooks or books?

> Let's add these two levels inside the CPU topology implementation.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---

[...]

> diff --git a/qapi/machine.json b/qapi/machine.json
> index 6afd1936b0..bdd92e3cb1 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -900,13 +900,15 @@
>  # a CPU is being hotplugged.
>  #
>  # @node-id: NUMA node ID the CPU belongs to
> -# @socket-id: socket number within node/board the CPU belongs to
> +# @drawer-id: drawer number within node/board the CPU belongs to
> +# @book-id: book number within drawer/node/board the CPU belongs to
> +# @socket-id: socket number within book/node/board the CPU belongs to
>  # @die-id: die number within socket the CPU belongs to (since 4.1)
>  # @cluster-id: cluster number within die the CPU belongs to (since 7.1)
>  # @core-id: core number within cluster 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

Should this be 8?

>  #       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
   #       sync with the properties passed to -device/device_add.

Not your patch's fault, but the second sentence is less than clear.
What are "the filed names"?  A typo perhaps?

> @@ -916,6 +918,8 @@
>  ##
>  { 'struct': 'CpuInstanceProperties',
>    'data': { '*node-id': 'int',
> +            '*drawer-id': 'int',
> +            '*book-id': 'int',
>              '*socket-id': 'int',
>              '*die-id': 'int',
>              '*cluster-id': 'int',
> @@ -1465,6 +1469,10 @@
>  #
>  # @cpus: number of virtual CPUs in the virtual machine
>  #
> +# @drawers: number of drawers in the CPU topology
> +#
> +# @books: number of books in the CPU topology
> +#
>  # @sockets: number of sockets in the CPU topology
>  #
>  # @dies: number of dies per socket in the CPU topology
> @@ -1481,6 +1489,8 @@
>  ##
>  { 'struct': 'SMPConfiguration', 'data': {
>       '*cpus': 'int',
> +     '*drawers': 'int',
> +     '*books': 'int',
>       '*sockets': 'int',
>       '*dies': 'int',
>       '*clusters': 'int',

[...]
Pierre Morel Sept. 28, 2022, 9:04 a.m. UTC | #2
On 9/6/22 10:59, Markus Armbruster wrote:
> Pierre Morel <pmorel@linux.ibm.com> writes:
> 
>> S390x defines two topology levels above sockets: nbooks and drawers.
> 
> nbooks or books?
> 
>> Let's add these two levels inside the CPU topology implementation.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
> 
> [...]
> 
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 6afd1936b0..bdd92e3cb1 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -900,13 +900,15 @@
>>   # a CPU is being hotplugged.
>>   #
>>   # @node-id: NUMA node ID the CPU belongs to
>> -# @socket-id: socket number within node/board the CPU belongs to
>> +# @drawer-id: drawer number within node/board the CPU belongs to
>> +# @book-id: book number within drawer/node/board the CPU belongs to
>> +# @socket-id: socket number within book/node/board the CPU belongs to
>>   # @die-id: die number within socket the CPU belongs to (since 4.1)
>>   # @cluster-id: cluster number within die the CPU belongs to (since 7.1)
>>   # @core-id: core number within cluster 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
> 
> Should this be 8?

when one can count .. yes :)

Thanks, I will update in next spin

> 
>>   #       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
>     #       sync with the properties passed to -device/device_add.
> 
> Not your patch's fault, but the second sentence is less than clear.
> What are "the filed names"?  A typo perhaps?

I understood that it means the names in the structure here under which 
are filed inside the single quotes.


In this case, may be "filed names" should be replaced by
"names in the CpuInstanceProperties structure"


> 
>> @@ -916,6 +918,8 @@
>>   ##
>>   { 'struct': 'CpuInstanceProperties',
>>     'data': { '*node-id': 'int',
>> +            '*drawer-id': 'int',
>> +            '*book-id': 'int',
>>               '*socket-id': 'int',
>>               '*die-id': 'int',
>>               '*cluster-id': 'int',
>> @@ -1465,6 +1469,10 @@
>>   #

Regards,
Pierre
Pierre Morel Sept. 28, 2022, 9:06 a.m. UTC | #3
On 9/6/22 10:59, Markus Armbruster wrote:
> Pierre Morel <pmorel@linux.ibm.com> writes:
> 
>> S390x defines two topology levels above sockets: nbooks and drawers.
> 
> nbooks or books?

Sorry, forgot this.

Yes typo, I mean "books"

Thanks,
Pierre
diff mbox series

Patch

diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index b39ed21e65..26150c748f 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -31,6 +31,14 @@  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);
+    }
+
     g_string_append_printf(s, "sockets (%u)", ms->smp.sockets);
 
     if (mc->smp_props.dies_supported) {
@@ -73,6 +81,8 @@  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;
     unsigned clusters = config->has_clusters ? config->clusters : 0;
@@ -85,6 +95,8 @@  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) ||
         (config->has_clusters && config->clusters == 0) ||
@@ -111,6 +123,20 @@  void machine_parse_smp_config(MachineState *ms,
     dies = dies > 0 ? dies : 1;
     clusters = clusters > 0 ? clusters : 1;
 
+    if (!mc->smp_props.books_supported && books > 1) {
+        error_setg(errp, "books not supported by this machine's CPU topology");
+        return;
+    }
+
+    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;
@@ -124,33 +150,41 @@  void machine_parse_smp_config(MachineState *ms,
             if (sockets == 0) {
                 cores = cores > 0 ? cores : 1;
                 threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (dies * clusters * cores * threads);
+                sockets = maxcpus /
+                          (drawers * books * dies * clusters * cores * threads);
             } else if (cores == 0) {
                 threads = threads > 0 ? threads : 1;
-                cores = maxcpus / (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 / (sockets * dies * clusters * threads);
+                cores = maxcpus /
+                        (drawers * books * sockets * dies * clusters * threads);
             } else if (sockets == 0) {
                 threads = threads > 0 ? threads : 1;
-                sockets = maxcpus / (dies * clusters * cores * threads);
+                sockets = maxcpus /
+                         (drawers * books * dies * clusters * cores * threads);
             }
         }
 
         /* try to calculate omitted threads at last */
         if (threads == 0) {
-            threads = maxcpus / (sockets * dies * clusters * cores);
+            threads = maxcpus /
+                      (drawers * books * sockets * dies * clusters * cores);
         }
     }
 
-    maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * clusters * cores * threads;
+    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;
     ms->smp.clusters = clusters;
@@ -159,7 +193,7 @@  void machine_parse_smp_config(MachineState *ms,
     ms->smp.max_cpus = maxcpus;
 
     /* sanity-check of the computed topology */
-    if (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 a673302cce..4c5c8d1655 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -821,6 +821,8 @@  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,
         .has_clusters = true, .clusters = ms->smp.clusters,
@@ -1087,6 +1089,8 @@  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;
     ms->smp.clusters = 1;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 7b416c9787..69e20c1252 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -130,11 +130,15 @@  typedef struct {
  * @prefer_sockets - whether sockets are preferred over cores in smp parsing
  * @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;
 
 /**
@@ -299,6 +303,8 @@  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
  * @clusters: the number of clusters in one die
@@ -308,6 +314,8 @@  typedef struct DeviceMemoryState {
  */
 typedef struct CpuTopology {
     unsigned int cpus;
+    unsigned int drawers;
+    unsigned int books;
     unsigned int sockets;
     unsigned int dies;
     unsigned int clusters;
diff --git a/qapi/machine.json b/qapi/machine.json
index 6afd1936b0..bdd92e3cb1 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -900,13 +900,15 @@ 
 # a CPU is being hotplugged.
 #
 # @node-id: NUMA node ID the CPU belongs to
-# @socket-id: socket number within node/board the CPU belongs to
+# @drawer-id: drawer number within node/board the CPU belongs to
+# @book-id: book number within drawer/node/board the CPU belongs to
+# @socket-id: socket number within book/node/board the CPU belongs to
 # @die-id: die number within socket the CPU belongs to (since 4.1)
 # @cluster-id: cluster number within die the CPU belongs to (since 7.1)
 # @core-id: core number within cluster 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
@@ -916,6 +918,8 @@ 
 ##
 { 'struct': 'CpuInstanceProperties',
   'data': { '*node-id': 'int',
+            '*drawer-id': 'int',
+            '*book-id': 'int',
             '*socket-id': 'int',
             '*die-id': 'int',
             '*cluster-id': 'int',
@@ -1465,6 +1469,10 @@ 
 #
 # @cpus: number of virtual CPUs in the virtual machine
 #
+# @drawers: number of drawers in the CPU topology
+#
+# @books: number of books in the CPU topology
+#
 # @sockets: number of sockets in the CPU topology
 #
 # @dies: number of dies per socket in the CPU topology
@@ -1481,6 +1489,8 @@ 
 ##
 { 'struct': 'SMPConfiguration', 'data': {
      '*cpus': 'int',
+     '*drawers': 'int',
+     '*books': 'int',
      '*sockets': 'int',
      '*dies': 'int',
      '*clusters': 'int',
diff --git a/qemu-options.hx b/qemu-options.hx
index 311e75a98e..e78e51ab49 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -245,11 +245,13 @@  SRST
 ERST
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n"
+    "-smp [[cpus=]n][,maxcpus=maxcpus][,drawers=drawers][,books=books][,sockets=sockets][,dies=dies][,clusters=clusters][,cores=cores][,threads=threads]\n"
     "                set the number of initial CPUs to 'n' [default=1]\n"
     "                maxcpus= maximum number of total CPUs, including\n"
     "                offline CPUs for hotplug, etc\n"
-    "                sockets= number of sockets on the machine board\n"
+    "                drawers= number of drawers on the machine board\n"
+    "                books= number of books in one drawer\n"
+    "                sockets= number of sockets in one book\n"
     "                dies= number of dies in one socket\n"
     "                clusters= number of clusters in one die\n"
     "                cores= number of cores in one cluster\n"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 706bd7cff7..80ea35fa26 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -726,6 +726,12 @@  static QemuOptsList qemu_smp_opts = {
         {
             .name = "cpus",
             .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "drawers",
+            .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "books",
+            .type = QEMU_OPT_NUMBER,
         }, {
             .name = "sockets",
             .type = QEMU_OPT_NUMBER,