qemu-s390x
[Top][All Lists]
Advanced

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

Re: [PATCH v3 3/4] s390x: topology: CPU topology objects and structures


From: Thomas Huth
Subject: Re: [PATCH v3 3/4] s390x: topology: CPU topology objects and structures
Date: Thu, 14 Oct 2021 09:16:43 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0

On 16/09/2021 15.50, 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
book.
The book is built as a SYSBUS bridge during the CPU initialization.

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.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
  hw/s390x/cpu-topology.c         | 353 ++++++++++++++++++++++++++++++++
  hw/s390x/meson.build            |   1 +
  hw/s390x/s390-virtio-ccw.c      |   4 +
  include/hw/s390x/cpu-topology.h |  67 ++++++
  target/s390x/cpu.h              |  47 +++++
  5 files changed, 472 insertions(+)
  create mode 100644 hw/s390x/cpu-topology.c
  create mode 100644 include/hw/s390x/cpu-topology.h

diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
new file mode 100644
index 0000000000..f0ffd86a4f
--- /dev/null
+++ b/hw/s390x/cpu-topology.c
@@ -0,0 +1,353 @@
+/*
+ * CPU Topology
+ *
+ * Copyright 2021 IBM Corp.
+ * Author(s): Pierre Morel <pmorel@linux.ibm.com>
+
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "hw/sysbus.h"
+#include "hw/s390x/cpu-topology.h"
+#include "hw/qdev-properties.h"
+#include "hw/boards.h"
+#include "qemu/typedefs.h"
+#include "target/s390x/cpu.h"
+#include "hw/s390x/s390-virtio-ccw.h"
+
+static S390TopologyCores *s390_create_cores(S390TopologySocket *socket,
+                                            int origin)
+{
+    DeviceState *dev;
+    S390TopologyCores *cores;
+    const MachineState *ms = MACHINE(qdev_get_machine());
+
+    if (socket->bus->num_children >= ms->smp.cores) {
+        return NULL;
+    }
+
+    dev = qdev_new(TYPE_S390_TOPOLOGY_CORES);
+    qdev_realize_and_unref(dev, socket->bus, &error_fatal);
+
+    cores = S390_TOPOLOGY_CORES(dev);
+    cores->origin = origin;
+    socket->cnt += 1;
+
+    return cores;
+}
+
+static S390TopologySocket *s390_create_socket(S390TopologyBook *book, int id)
+{
+    DeviceState *dev;
+    S390TopologySocket *socket;
+    const MachineState *ms = MACHINE(qdev_get_machine());
+
+    if (book->bus->num_children >= ms->smp.sockets) {
+        return NULL;
+    }
+
+    dev = qdev_new(TYPE_S390_TOPOLOGY_SOCKET);
+    qdev_realize_and_unref(dev, book->bus, &error_fatal);
+
+    socket = S390_TOPOLOGY_SOCKET(dev);
+    socket->socket_id = id;
+    book->cnt++;
+
+    return socket;
+}
+
+/*
+ * s390_get_cores:
+ * @socket: the socket to search into
+ * @origin: the origin specified for the S390TopologyCores
+ *
+ * returns a pointer to a S390TopologyCores structure within a socket having
+ * the specified origin.
+ * First search if the socket is already containing the S390TopologyCores
+ * structure and if not create one with this origin.
+ */
+static S390TopologyCores *s390_get_cores(S390TopologySocket *socket, int 
origin)
+{
+    S390TopologyCores *cores;
+    BusChild *kid;
+
+    QTAILQ_FOREACH(kid, &socket->bus->children, sibling) {
+        cores = S390_TOPOLOGY_CORES(kid->child);
+        if (cores->origin == origin) {
+            return cores;
+        }
+    }
+    return s390_create_cores(socket, origin);
+}
+
+/*
+ * s390_get_socket:
+ * @book: The book to search into
+ * @socket_id: the identifier of the socket to search for
+ *
+ * returns a pointer to a S390TopologySocket structure within a book having
+ * the specified socket_id.
+ * First search if the book is already containing the S390TopologySocket
+ * structure and if not create one with this socket_id.
+ */
+static S390TopologySocket *s390_get_socket(S390TopologyBook *book,
+                                           int socket_id)
+{
+    S390TopologySocket *socket;
+    BusChild *kid;
+
+    QTAILQ_FOREACH(kid, &book->bus->children, sibling) {
+        socket = S390_TOPOLOGY_SOCKET(kid->child);
+        if (socket->socket_id == socket_id) {
+            return socket;
+        }
+    }
+    return s390_create_socket(book, socket_id);
+}
+
+/*
+ * s390_topology_new_cpu:
+ * @core_id: the core ID is machine wide
+ *
+ * We have a single book returned by s390_get_topology(),
+ * then we build the hierarchy on demand.
+ * Note that we do not destroy the hierarchy on error creating
+ * an entry in the topology, we just keep it empty.
+ * We do not need to worry about not finding a topology level
+ * entry this would have been caught during smp parsing.
+ */
+void s390_topology_new_cpu(int core_id)
+{
+    const MachineState *ms = MACHINE(qdev_get_machine());
+    S390TopologyBook *book;
+    S390TopologySocket *socket;
+    S390TopologyCores *cores;
+    int cores_per_socket, sock_idx;
+    int origin, bit;
+
+    book = s390_get_topology();
+
+    cores_per_socket = ms->smp.max_cpus / ms->smp.sockets;
+
+    sock_idx = (core_id / cores_per_socket);
+    socket = s390_get_socket(book, sock_idx);
+
+    /*
+     * We assume that all CPU are identical IFL, shared and
+     * horizontal topology, the only reason to have several
+     * S390TopologyCores is to have more than 64 CPUs.
+     */
+    origin = 64 * (core_id / 64);
+
+    cores = s390_get_cores(socket, origin);
+
+    bit = 63 - (core_id - origin);
+    set_bit(bit, &cores->mask);
+    cores->origin = origin;
+}

Are these "64" an artificial limit by the hardware? Or is it just an implementation detail in your code since you chose to use a uint64_t variable for the mask? In the latter case, why not use a bitfield? ... anyway, could you please add some more comments to the code why you are using "64" here? (e.g. the sentences "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." should go into the cpu-topology.c file, too).

 Thomas




reply via email to

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