Re: [PATCH v20 03/21] target/s390x/cpu topology: handle STSI(15) and build the SYSIB

[Thomas Huth]

On 25/04/2023 18.14, Pierre Morel wrote:
On interception of STSI(15.1.x) the System Information Block
(SYSIB) is built from the list of pre-ordered topology entries.
Signed-off-by: Pierre Morel <pmo...@linux.ibm.com>
---
  MAINTAINERS                     |   1 +
  include/hw/s390x/cpu-topology.h |  24 +++
  include/hw/s390x/sclp.h         |   1 +
  target/s390x/cpu.h              |  72 ++++++++
  hw/s390x/cpu-topology.c         |  13 +-
  target/s390x/kvm/cpu_topology.c | 308 ++++++++++++++++++++++++++++++++
  target/s390x/kvm/kvm.c          |   5 +-
  target/s390x/kvm/meson.build    |   3 +-
  8 files changed, 424 insertions(+), 3 deletions(-)
  create mode 100644 target/s390x/kvm/cpu_topology.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bb7b34d0d8..de9052f753 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1659,6 +1659,7 @@ M: Pierre Morel <pmo...@linux.ibm.com>
  S: Supported
  F: include/hw/s390x/cpu-topology.h
  F: hw/s390x/cpu-topology.c
+F: target/s390x/kvm/cpu_topology.c

It's somewhat weird to have one file "cpu-topology.c" (in hw/s390x, with a 
dash), and one file cpu_topology.c (in target/s390x, with an underscore) ... 
could you come up with a better naming? Maybe call the new file 
stsi-topology.c or so?

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index bb7cfb0cab..9f97989bd7 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -561,6 +561,25 @@ typedef struct SysIB_322 {
  } SysIB_322;
  QEMU_BUILD_BUG_ON(sizeof(SysIB_322) != 4096);


Maybe add a short comment here what MAG stands for (magnitude fields?)?
+#define S390_TOPOLOGY_MAG  6
+#define S390_TOPOLOGY_MAG6 0
+#define S390_TOPOLOGY_MAG5 1
+#define S390_TOPOLOGY_MAG4 2
+#define S390_TOPOLOGY_MAG3 3
+#define S390_TOPOLOGY_MAG2 4
+#define S390_TOPOLOGY_MAG1 5
+/* Configuration topology */
+typedef struct SysIB_151x {
+    uint8_t  reserved0[2];
+    uint16_t length;
+    uint8_t  mag[S390_TOPOLOGY_MAG];
+    uint8_t  reserved1;
+    uint8_t  mnest;
+    uint32_t reserved2;
+    char tle[];
+} SysIB_151x;
+QEMU_BUILD_BUG_ON(sizeof(SysIB_151x) != 16);
...

diff --git a/target/s390x/kvm/cpu_topology.c b/target/s390x/kvm/cpu_topology.c
new file mode 100644
index 0000000000..86a286afe2
--- /dev/null
+++ b/target/s390x/kvm/cpu_topology.c
@@ -0,0 +1,308 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * QEMU S390x CPU Topology
+ *
+ * Copyright IBM Corp. 2022,2023
+ * Author(s): Pierre Morel <pmo...@linux.ibm.com>
+ *
+ */
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/s390x/pv.h"
+#include "hw/sysbus.h"
+#include "hw/s390x/sclp.h"
+#include "hw/s390x/cpu-topology.h"
+
+/**
+ * fill_container:
+ * @p: The address of the container TLE to fill
+ * @level: The level of nesting for this container
+ * @id: The container receives a uniq ID inside its own container

s/uniq/unique/

 Thomas