[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 1/9] s390x: smp: s390x dedicated smp parsing

From: Pierre Morel
Subject: Re: [PATCH v1 1/9] s390x: smp: s390x dedicated smp parsing
Date: Fri, 16 Jul 2021 12:47:14 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

On 7/16/21 10:54 AM, Cornelia Huck wrote:
On Wed, Jul 14 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:

We need a s390x dedicated SMP parsing to handle s390x specificities.

In this patch we only handle threads, cores and sockets for
- do not support threads, we always have 1 single thread per core
- the sockets are filled one after the other with the cores

Both these handlings are different from the standard smp_parse
functionement and reflect the CPU topology in the simple case
where all CPU belong to the same book.

Topology levels above sockets, i.e. books, drawers, are not
considered at this stage and will be introduced in a later patch.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
  hw/s390x/s390-virtio-ccw.c | 42 ++++++++++++++++++++++++++++++++++++++
  1 file changed, 42 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index e4b18aef49..899d3a4137 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -582,6 +582,47 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
      return newsz;
+ * In S390CCW machine we do not support threads for now,
+ * only sockets and cores.
+ */
+static void s390_smp_parse(MachineState *ms, QemuOpts *opts)

It seems you based this on an older version of the code? The current
signature of this function since 1e63fe685804 ("machine: pass QAPI
struct to mc->smp_parse") is

void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp);

That affects your parsing, and also lets you get rid of the ugly exit(1)

hum, yes, thanks

+    unsigned cpus    = qemu_opt_get_number(opts, "cpus", 1);
+    unsigned sockets = qemu_opt_get_number(opts, "sockets", 1);
+    unsigned cores   = qemu_opt_get_number(opts, "cores", 1);
+    if (opts) {
+        if (cpus == 0 || sockets == 0 || cores == 0) {

This behaviour looks different from what we do for other targets: if you
specify the value as 0, a value is calculated from the other values;
here, you error out. It's probably not a good idea to differ.

right, thanks

+            error_report("cpu topology: "
+                         "sockets (%u), cores (%u) or number of CPU(%u) "
+                         "can not be zero", sockets, cores, cpus);
+            exit(1);
+        }
+    }

Pierre Morel
IBM Lab Boeblingen

reply via email to

[Prev in Thread] Current Thread [Next in Thread]