On 10/25/22 21:58, Janis Schoetterl-Glausch wrote:
On Wed, 2022-10-12 at 18:20 +0200, Pierre Morel wrote:
In the S390x CPU topology the core_id specifies the CPU address
and the position of the core withing the topology.
Let's build the topology based on the core_id.
s390x/cpu topology: core_id sets s390x CPU topology
In the S390x CPU topology the core_id specifies the CPU address
and the position of the cpu withing the topology.
Let's build the topology based on the core_id.
Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
include/hw/s390x/cpu-topology.h | 45 +++++++++++
hw/s390x/cpu-topology.c | 132 ++++++++++++++++++++++++++++++++
hw/s390x/s390-virtio-ccw.c | 21 +++++
hw/s390x/meson.build | 1 +
4 files changed, 199 insertions(+)
create mode 100644 include/hw/s390x/cpu-topology.h
create mode 100644 hw/s390x/cpu-topology.c
[...]
+/**
+ * s390_topology_realize:
+ * @dev: the device state
+ * @errp: the error pointer (not used)
+ *
+ * During realize the machine CPU topology is initialized with the
+ * QEMU -smp parameters.
+ * The maximum count of CPU TLE in the all Topology can not be greater
+ * than the maximum CPUs.
+ */
+static void s390_topology_realize(DeviceState *dev, Error **errp)
+{
+ MachineState *ms = MACHINE(qdev_get_machine());
+ S390Topology *topo = S390_CPU_TOPOLOGY(dev);
+
+ topo->cpus = ms->smp.cores * ms->smp.threads;
Currently threads are not supported, effectively increasing the number of cpus,
so this is currently correct. Once the machine version limits the threads to 1,
it is also correct. However, once we support multiple threads, this becomes incorrect.
I wonder if it's ok from a backward compatibility point of view to modify the smp values
by doing cores *= threads, threads = 1 for old machines.
Right, this will become incorrect with thread support.
What about having a dedicated function:
topo->cpus = s390_get_cpus(ms);
This function will use the S390CcwMachineClass->max_thread introduced
later to report the correct number of CPUs.
Then you can just use the cores value and it is always correct.
In any case, if you keep it as is, I'd like to see a comment here saying that this
is correct only so long as we don't support threads.
+
+ topo->socket = g_new0(S390TopoContainer, ms->smp.sockets);
+ topo->tle = g_new0(S390TopoTLE, ms->smp.max_cpus);
+
+ topo->ms = ms;
+}
+
[...]
--
Pierre Morel
IBM Lab Boeblingen
