[Top][All Lists]

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

Re: [PATCH v8 02/12] s390x/cpu_topology: CPU topology objects and struct

From: Pierre Morel
Subject: Re: [PATCH v8 02/12] s390x/cpu_topology: CPU topology objects and structures
Date: Tue, 23 Aug 2022 18:30:31 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0

On 8/23/22 15:30, Thomas Huth wrote:
On 20/06/2022 16.03, Pierre Morel wrote:
We use new objects to have a dynamic administration of the CPU topology.
The highest level object in this implementation is the s390 book and
in this first implementation of CPU topology for S390 we have a single
The book is built as a SYSBUS bridge during the CPU initialization.
Other objects, sockets and core will be built after the parsing
of the QEMU -smp argument.

Every object under this single book will be build dynamically
immediately after a CPU has be realized if it is needed.
The CPU will fill the sockets once after the other, according to the
number of core per socket defined during the smp parsing.

Each CPU inside a socket will be represented by a bit in a 64bit
unsigned long. Set on plug and clear on unplug of a CPU.

For the S390 CPU topology, thread and cores are merged into
topology cores and the number of topology cores is the multiplication
of cores by the numbers of threads.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
  hw/s390x/cpu-topology.c         | 391 ++++++++++++++++++++++++++++++++
  hw/s390x/meson.build            |   1 +
  hw/s390x/s390-virtio-ccw.c      |   6 +
  include/hw/s390x/cpu-topology.h |  74 ++++++
  target/s390x/cpu.h              |  47 ++++
  5 files changed, 519 insertions(+)
  create mode 100644 hw/s390x/cpu-topology.c
  create mode 100644 include/hw/s390x/cpu-topology.h
+bool s390_topology_new_cpu(MachineState *ms, int core_id, Error **errp)
+    S390TopologyBook *book;
+    S390TopologySocket *socket;
+    S390TopologyCores *cores;
+    int nb_cores_per_socket;
+    int origin, bit;
+    book = s390_get_topology();
+    nb_cores_per_socket = ms->smp.cores * ms->smp.threads;
+    socket = s390_get_socket(ms, book, core_id / nb_cores_per_socket, errp);
+    if (!socket) {
+        return false;
+    }
+    /*
+     * At the core level, each CPU is represented by a bit in a 64bit
+     * unsigned long. Set on plug and clear on unplug of a CPU.
+     * The firmware assume that all CPU in the core description have the same
+     * type, polarization and are all dedicated or shared.
+     * In the case a socket contains CPU with different type, polarization +     * or dedication then they will be defined in different CPU containers. +     * Currently we assume all CPU are identical and the only reason to have +     * several S390TopologyCores inside a socket is to have more than 64 CPUs +     * in that case the origin field, representing the offset of the first CPU +     * in the CPU container allows to represent up to the maximal number of
+     * CPU inside several CPU containers inside the socket container.
+     */
+    origin = 64 * (core_id / 64);

Maybe faster:

     origin = core_id & ~63;

yes thanks

By the way, where is this limitation to 64 coming from? Just because we're using a "unsigned long" for now? Or is this a limitation from the architecture?

It is a limitation from the architecture who use a 64bit field to represent the CPUs in a CPU TLE mask.

but this patch serie is quite old now and I changed some things according to the comments I received
I plan to send the new version before the end of the week.

+    cores = s390_get_cores(ms, socket, origin, errp);
+    if (!cores) {
+        return false;
+    }
+    bit = 63 - (core_id - origin);
+    set_bit(bit, &cores->mask);
+    cores->origin = origin;
+    return true;
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index cc3097bfee..a586875b24 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -43,6 +43,7 @@
  #include "sysemu/sysemu.h"
  #include "hw/s390x/pv.h"
  #include "migration/blocker.h"
+#include "hw/s390x/cpu-topology.h"
  static Error *pv_mig_blocker;
@@ -89,6 +90,7 @@ static void s390_init_cpus(MachineState *machine)
      /* initialize possible_cpus */
+    s390_topology_setup(machine);

Is this safe with regards to migration? Did you tried a ping-pong migration from an older QEMU to a QEMU with your modifications and back to the older one? If it does not work, we might need to wire this setup to the machine types...

I did not, I will add this test.

      for (i = 0; i < machine->smp.cpus; i++) {
          s390x_new_cpu(machine->cpu_type, i, &error_fatal);
@@ -306,6 +308,10 @@ static void s390_cpu_plug(HotplugHandler *hotplug_dev,
      ms->possible_cpus->cpus[cpu->env.core_id].cpu = OBJECT(dev);
+    if (!s390_topology_new_cpu(ms, cpu->env.core_id, errp)) {
+        return;
+    }
      if (dev->hotplugged) {


Thanks Thomas,


Pierre Morel
IBM Lab Boeblingen

reply via email to

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