Re: [PATCH v10 2/9] s390x/cpu topology: reporting the CPU topology to the guest

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

 





On 11/7/22 14:20, Janis Schoetterl-Glausch wrote:
On Fri, 2022-10-28 at 12:00 +0200, Pierre Morel wrote:

On 10/27/22 22:42, Janis Schoetterl-Glausch wrote:
On Wed, 2022-10-12 at 18:21 +0200, Pierre Morel wrote:
The guest can use the STSI instruction to get a buffer filled
with the CPU topology description.

Let us implement the STSI instruction for the basis CPU topology
level, level 2.

Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
   include/hw/s390x/cpu-topology.h |   3 +
   target/s390x/cpu.h              |  48 ++++++++++++++
   hw/s390x/cpu-topology.c         |   8 ++-
   target/s390x/cpu_topology.c     | 109 ++++++++++++++++++++++++++++++++
   target/s390x/kvm/kvm.c          |   6 +-
   target/s390x/meson.build        |   1 +
   6 files changed, 172 insertions(+), 3 deletions(-)
   create mode 100644 target/s390x/cpu_topology.c

diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h
index 66c171d0bc..61c11db017 100644
--- a/include/hw/s390x/cpu-topology.h
+++ b/include/hw/s390x/cpu-topology.h
@@ -13,6 +13,8 @@
   #include "hw/qdev-core.h"
   #include "qom/object.h"
+#define S390_TOPOLOGY_POLARITY_H 0x00
+
   typedef struct S390TopoContainer {
       int active_count;
   } S390TopoContainer;
@@ -29,6 +31,7 @@ struct S390Topology {
       S390TopoContainer *socket;
       S390TopoTLE *tle;
       MachineState *ms;
+    QemuMutex topo_mutex;
   };
#define TYPE_S390_CPU_TOPOLOGY "s390-topology"
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7d6d01325b..d604aa9c78 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h

[...]
+
+/* Maxi size of a SYSIB structure is when all CPU are alone in a container */

Max or Maximum.

+#define S390_TOPOLOGY_SYSIB_SIZE (sizeof(SysIB_151x) +                         \
+                                  S390_MAX_CPUS * (sizeof(SysIBTl_container) + \
+                                                   sizeof(SysIBTl_cpu)))

Currently this is 16+248*3*8 == 5968 and will grow with books, drawer support to
16+248*5*8 == 9936 ...

[...]

+
+void insert_stsi_15_1_x(S390CPU *cpu, int sel2, __u64 addr, uint8_t ar)
+{
+    uint64_t page[S390_TOPOLOGY_SYSIB_SIZE / sizeof(uint64_t)] = {};

... so calling this page is a bit misleading. Also why not make it a char[]?
And maybe use a union for type punning.

OK, what about:

      union {
          char place_holder[S390_TOPOLOGY_SYSIB_SIZE];
          SysIB_151x sysib;
      } buffer QEMU_ALIGNED(8);

I don't think you need the QEMU_ALIGNED since SysIB_151x already has it. Not that it hurts to be
explicit. If you declared the tle member as uint64_t[], you should get the correct alignment
automatically and can then drop the explicit one.

I find the explicit statement better. Why make it non explicit?

Btw, [] seems to be preferred over [0], at least there is a commit doing a conversion:
f7795e4096 ("misc: Replace zero-length arrays with flexible array member (automatic)")

OK



+    SysIB_151x *sysib = (SysIB_151x *) page;
+    int len;
+
+    if (s390_is_pv() || !s390_has_topology() ||
+        sel2 < 2 || sel2 > S390_TOPOLOGY_MAX_MNEST) {
+        setcc(cpu, 3);
+        return;
+    }
+
+    len = setup_stsi(sysib, sel2);

This should now be memory safe, but might be larger than 4k,
the maximum size of the SYSIB. I guess you want to set cc code 3
in this case and return.

I do not find why the SYSIB can not be larger than 4k.
Can you point me to this restriction?

Says so at the top of the description of STSI:

The SYSIB is 4K bytes and must begin at a 4 K-byte
boundary; otherwise, a specification exception may
be recognized.

Right, I guess I can not read.

So I will return CC=3 in case the length is greater than 4K


thanks,
Regards,

Pierre


--
Pierre Morel
IBM Lab Boeblingen



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux