Re: [PATCH v19 01/21] s390x/cpu topology: add s390 specifics to CPU topology

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

 




On 4/18/23 10:53, Nina Schoetterl-Glausch wrote:
On Mon, 2023-04-03 at 18:28 +0200, Pierre Morel wrote:
S390 adds two new SMP levels, drawers and books to the CPU
topology.
The S390 CPU have specific topology features like dedication
and entitlement to give to the guest indications on the host
vCPUs scheduling and help the guest take the best decisions
on the scheduling of threads on the vCPUs.

Let us provide the SMP properties with books and drawers levels
and S390 CPU with dedication and entitlement,

Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
Reviewed-by: Thomas Huth <thuth@xxxxxxxxxx>
---
  MAINTAINERS                     |  5 ++++
  qapi/machine-common.json        | 22 ++++++++++++++
  qapi/machine-target.json        | 12 ++++++++
  qapi/machine.json               | 17 +++++++++--
  include/hw/boards.h             | 10 ++++++-
  include/hw/s390x/cpu-topology.h | 15 ++++++++++
Is hw/s390x the right path for cpu-topology?
I haven't understood the difference between hw/s390x and target/s390x
but target/s390x feels more correct, I could be mistaken though.

AFAIK target/s390 is for CPU emulation code while hw/s390 is for other emulation.

So it depends how we classify the CPU topology, it is related to CPU but it is no emulation.

Since Thomas approved this layout I would like to keep it like this.



  target/s390x/cpu.h              |  5 ++++
  hw/core/machine-smp.c           | 53 ++++++++++++++++++++++++++++-----
  hw/core/machine.c               |  4 +++
  hw/s390x/s390-virtio-ccw.c      |  2 ++
  softmmu/vl.c                    |  6 ++++
  target/s390x/cpu.c              |  7 +++++
  qapi/meson.build                |  1 +
  qemu-options.hx                 |  7 +++--
  14 files changed, 152 insertions(+), 14 deletions(-)
  create mode 100644 qapi/machine-common.json
  create mode 100644 include/hw/s390x/cpu-topology.h

[...]

diff --git a/qapi/machine-common.json b/qapi/machine-common.json
new file mode 100644
index 0000000000..73ea38d976
--- /dev/null
+++ b/qapi/machine-common.json
@@ -0,0 +1,22 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+
+##
+# = Machines S390 data types
+##
+
+##
+# @CpuS390Entitlement:
+#
+# An enumeration of cpu entitlements that can be assumed by a virtual
+# S390 CPU
+#
+# Since: 8.1
+##
+{ 'enum': 'CpuS390Entitlement',
+  'prefix': 'S390_CPU_ENTITLEMENT',
+  'data': [ 'horizontal', 'low', 'medium', 'high' ] }
You can get rid of the horizontal value now that the entitlement is ignored if the
polarization is vertical.


Right, horizontal is not used, but what would you like?

- replace horizontal with 'none' ?

- add or substract 1 when we do the conversion between enum string and value ?

frankly I prefer to keep horizontal here which is exactly what is given in the documentation for entitlement = 0




[...]

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index b10a8541ff..57165fa3a0 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -37,6 +37,7 @@
  #ifndef CONFIG_USER_ONLY
  #include "sysemu/reset.h"
  #endif
+#include "hw/s390x/cpu-topology.h"
#define CR0_RESET 0xE0UL
  #define CR14_RESET      0xC2000000UL;
@@ -259,6 +260,12 @@ static gchar *s390_gdb_arch_name(CPUState *cs)
  static Property s390x_cpu_properties[] = {
  #if !defined(CONFIG_USER_ONLY)
      DEFINE_PROP_UINT32("core-id", S390CPU, env.core_id, 0),
+    DEFINE_PROP_INT32("socket-id", S390CPU, env.socket_id, -1),
+    DEFINE_PROP_INT32("book-id", S390CPU, env.book_id, -1),
+    DEFINE_PROP_INT32("drawer-id", S390CPU, env.drawer_id, -1),
+    DEFINE_PROP_BOOL("dedicated", S390CPU, env.dedicated, false),
+    DEFINE_PROP_UINT8("entitlement", S390CPU, env.entitlement,
+                      S390_CPU_ENTITLEMENT__MAX),
I would define an entitlement PropertyInfo in qdev-properties-system.[ch],
then one can use e.g.

-device z14-s390x-cpu,core-id=11,entitlement=high


Don't you think it is an enhancement we can do later?



on the command line and cpu hotplug.

I think setting the default entitlement to medium here should be fine.

[...]

right, I had medium before and should not have change it.

Anyway what ever the default is, it must be changed later depending on dedication.






[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