[PATCH v5 0/3] Add SCLP event type CPI

[PATCH v5 0/3] Add SCLP event type CPI

[Shalini Chellathurai Saroja]

Implement the Service-Call Logical Processor (SCLP) event
type Control-Program Identification (CPI) in QEMU.

Changed since v4:
- Remove the object control-program-id and add the CPI attributes
  directly to the sclpcpi object
- Instantiate sclpcpi device from v10.1 instead of v10.0
- Other minor changes

Changed since v3:
- Add QOM object sclpcpi from ccw_init()
- Add SCLPEventCPI state to store the CPI data in the sclpcpi device
- Other minor changes

Changed since v2:
- Add SPDX license tag in the new file hw/s390x/sclpcpi.c
- Store the control-program Identification data in the sclpcpi device
- Update the description of CPI attributes
- Use ldq_be_p() intead of be64_to_cpu()
- Return the CPI attribute system-level as an integer in QMP
- Add compat handling for backward migration
- Other minor changes

Shalini Chellathurai Saroja (3):
  hw/s390x: add SCLP event type CPI
  hw/s390x: add Control-Program Identification to QOM
  hw/s390x: support migration of CPI data

 hw/s390x/event-facility.c          |   2 +
 hw/s390x/meson.build               |   1 +
 hw/s390x/s390-virtio-ccw.c         |  22 +++++
 hw/s390x/sclpcpi.c                 | 154 +++++++++++++++++++++++++++++
 include/hw/s390x/event-facility.h  |  17 ++++
 include/hw/s390x/s390-virtio-ccw.h |   1 +
 qapi/machine.json                  |  58 +++++++++++
 7 files changed, 255 insertions(+)
 create mode 100644 hw/s390x/sclpcpi.c

-- 
2.49.0

[PATCH v5 1/3] hw/s390x: add SCLP event type CPI

[Shalini Chellathurai Saroja]

Implement the Service-Call Logical Processor (SCLP) event
type Control-Program Identification (CPI) in QEMU. This
event is used to send CPI identifiers from the guest to the
host. The CPI identifiers are: system type, system name,
system level and sysplex name.

System type: operating system of the guest (e.g. "LINUX").
System name: user configurable name of the guest (e.g. "TESTVM").
System level: distribution and kernel version, if the system type is Linux
(e.g. 0x50e00).
Sysplex name: name of the cluster which the guest belongs to (if any)
(e.g. "PLEX").

The SCLP event CPI is supported only from "s390-ccw-virtio-10.1" machine
and higher.

Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/s390x/event-facility.c          |  2 +
 hw/s390x/meson.build               |  1 +
 hw/s390x/s390-virtio-ccw.c         | 22 ++++++++
 hw/s390x/sclpcpi.c                 | 89 ++++++++++++++++++++++++++++++
 include/hw/s390x/event-facility.h  | 12 ++++
 include/hw/s390x/s390-virtio-ccw.h |  1 +
 6 files changed, 127 insertions(+)
 create mode 100644 hw/s390x/sclpcpi.c

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 7b7bf237fe..fee286ea63 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -4,6 +4,7 @@
  *       handles SCLP event types
  *          - Signal Quiesce - system power down
  *          - ASCII Console Data - VT220 read and write
+ *          - Control-Program Identification - Send OS data from guest to host
  *
  * Copyright IBM, Corp. 2012
  *
@@ -40,6 +41,7 @@ struct SCLPEventFacility {
     SysBusDevice parent_obj;
     SCLPEventsBus sbus;
     SCLPEvent quiesce, cpu_hotplug;
+    SCLPEventCPI cpi;
     /* guest's receive mask */
     union {
         uint32_t receive_mask_pieces[2];
diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 3bbebfd817..eb7950489c 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -13,6 +13,7 @@ s390x_ss.add(files(
   's390-skeys.c',
   's390-stattrib.c',
   'sclp.c',
+  'sclpcpi.c',
   'sclpcpu.c',
   'sclpquiesce.c',
   'tod.c',
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index f69a4d8ed3..a0f766c026 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -260,9 +260,21 @@ static void s390_create_sclpconsole(SCLPDevice *sclp,
     qdev_realize_and_unref(dev, ev_fac_bus, &error_fatal);
 }
 
+static void s390_create_sclpcpi(SCLPDevice *sclp)
+{
+    SCLPEventFacility *ef = sclp->event_facility;
+    BusState *ev_fac_bus = sclp_get_event_facility_bus(ef);
+    DeviceState *dev;
+
+    dev = qdev_new(TYPE_SCLP_EVENT_CPI);
+    object_property_add_child(OBJECT(ef), "sclpcpi", OBJECT(dev));
+    qdev_realize_and_unref(dev, ev_fac_bus, &error_fatal);
+}
+
 static void ccw_init(MachineState *machine)
 {
     MachineClass *mc = MACHINE_GET_CLASS(machine);
+    S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
     S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
     int ret;
     VirtualCssBus *css_bus;
@@ -323,6 +335,12 @@ static void ccw_init(MachineState *machine)
 
     /* init the TOD clock */
     s390_init_tod();
+
+    /* init SCLP event Control-Program Identification */
+    if (s390mc->use_cpi) {
+        s390_create_sclpcpi(ms->sclp);
+    }
+
 }
 
 static void s390_cpu_plug(HotplugHandler *hotplug_dev,
@@ -783,6 +801,7 @@ static void ccw_machine_class_init(ObjectClass *oc, const void *data)
     DumpSKeysInterface *dsi = DUMP_SKEYS_INTERFACE_CLASS(oc);
 
     s390mc->max_threads = 1;
+    s390mc->use_cpi = true;
     mc->reset = s390_machine_reset;
     mc->block_default_type = IF_VIRTIO;
     mc->no_cdrom = 1;
@@ -908,6 +927,9 @@ static void ccw_machine_10_0_instance_options(MachineState *machine)
 
 static void ccw_machine_10_0_class_options(MachineClass *mc)
 {
+    S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
+    s390mc->use_cpi = false;
+
     ccw_machine_10_1_class_options(mc);
     compat_props_add(mc->compat_props, hw_compat_10_0, hw_compat_10_0_len);
 }
diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
new file mode 100644
index 0000000000..935fa87acd
--- /dev/null
+++ b/hw/s390x/sclpcpi.c
@@ -0,0 +1,89 @@
+ /*
+  * SPDX-License-Identifier: GPL-2.0-or-later
+  *
+  * SCLP event type 11 - Control-Program Identification (CPI):
+  *    CPI is used to send program identifiers from the guest to the
+  *    Service-Call Logical Processor (SCLP). It is not sent by the SCLP.
+  *    Please refer S390ControlProgramId QOM-type description for details
+  *    on the contents of the CPI.
+  *
+  * Copyright IBM, Corp. 2024
+  *
+  * Authors:
+  *  Shalini Chellathurai Saroja <shalini@linux.ibm.com>
+  *
+  */
+
+#include "qemu/osdep.h"
+#include "hw/s390x/event-facility.h"
+
+typedef struct Data {
+    uint8_t id_format;
+    uint8_t reserved0;
+    uint8_t system_type[8];
+    uint64_t reserved1;
+    uint8_t system_name[8];
+    uint64_t reserved2;
+    uint64_t system_level;
+    uint64_t reserved3;
+    uint8_t sysplex_name[8];
+    uint8_t reserved4[16];
+} QEMU_PACKED Data;
+
+typedef struct ControlProgramIdMsg {
+    EventBufferHeader ebh;
+    Data data;
+} QEMU_PACKED ControlProgramIdMsg;
+
+static bool can_handle_event(uint8_t type)
+{
+    return type == SCLP_EVENT_CTRL_PGM_ID;
+}
+
+static sccb_mask_t send_mask(void)
+{
+    return 0;
+}
+
+/* Enable SCLP to accept buffers of event type CPI from the control-program. */
+static sccb_mask_t receive_mask(void)
+{
+    return SCLP_EVENT_MASK_CTRL_PGM_ID;
+}
+
+static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
+{
+    ControlProgramIdMsg *cpim = container_of(evt_buf_hdr, ControlProgramIdMsg,
+                                             ebh);
+
+    cpim->ebh.flags = SCLP_EVENT_BUFFER_ACCEPTED;
+    return SCLP_RC_NORMAL_COMPLETION;
+}
+
+static void cpi_class_init(ObjectClass *klass, const void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SCLPEventClass *k = SCLP_EVENT_CLASS(klass);
+
+    dc->user_creatable = false;
+
+    k->can_handle_event = can_handle_event;
+    k->get_send_mask = send_mask;
+    k->get_receive_mask = receive_mask;
+    k->write_event_data = write_event_data;
+}
+
+static const TypeInfo sclp_cpi_info = {
+    .name          = TYPE_SCLP_EVENT_CPI,
+    .parent        = TYPE_SCLP_EVENT,
+    .instance_size = sizeof(SCLPEventCPI),
+    .class_init    = cpi_class_init,
+};
+
+static void sclp_cpi_register_types(void)
+{
+    type_register_static(&sclp_cpi_info);
+}
+
+type_init(sclp_cpi_register_types)
+
diff --git a/include/hw/s390x/event-facility.h b/include/hw/s390x/event-facility.h
index ff874e792d..e81bc80498 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -25,6 +25,7 @@
 #define SCLP_EVENT_MESSAGE                      0x02
 #define SCLP_EVENT_CONFIG_MGT_DATA              0x04
 #define SCLP_EVENT_PMSGCMD                      0x09
+#define SCLP_EVENT_CTRL_PGM_ID                  0x0b
 #define SCLP_EVENT_ASCII_CONSOLE_DATA           0x1a
 #define SCLP_EVENT_SIGNAL_QUIESCE               0x1d
 
@@ -35,6 +36,7 @@
 #define SCLP_EVENT_MASK_MSG             SCLP_EVMASK(SCLP_EVENT_MESSAGE)
 #define SCLP_EVENT_MASK_CONFIG_MGT_DATA SCLP_EVMASK(SCLP_EVENT_CONFIG_MGT_DATA)
 #define SCLP_EVENT_MASK_PMSGCMD         SCLP_EVMASK(SCLP_EVENT_PMSGCMD)
+#define SCLP_EVENT_MASK_CTRL_PGM_ID     SCLP_EVMASK(SCLP_EVENT_CTRL_PGM_ID)
 #define SCLP_EVENT_MASK_MSG_ASCII       SCLP_EVMASK(SCLP_EVENT_ASCII_CONSOLE_DATA)
 #define SCLP_EVENT_MASK_SIGNAL_QUIESCE  SCLP_EVMASK(SCLP_EVENT_SIGNAL_QUIESCE)
 
@@ -191,6 +193,16 @@ struct SCLPEventClass {
     bool (*can_handle_event)(uint8_t type);
 };
 
+#define TYPE_SCLP_EVENT_CPI "sclpcpi"
+typedef struct SCLPEventCPIClass SCLPEventCPIClass;
+typedef struct SCLPEventCPI SCLPEventCPI;
+OBJECT_DECLARE_TYPE(SCLPEventCPI, SCLPEventCPIClass,
+                    SCLP_EVENT_CPI)
+
+struct SCLPEventCPI {
+    SCLPEvent event;
+};
+
 #define TYPE_SCLP_EVENT_FACILITY "s390-sclp-event-facility"
 typedef struct SCLPEventFacility SCLPEventFacility;
 typedef struct SCLPEventFacilityClass SCLPEventFacilityClass;
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 321b26df30..526078a4e2 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -54,6 +54,7 @@ struct S390CcwMachineClass {
 
     /*< public >*/
     int max_threads;
+    bool use_cpi;
 };
 
 #endif
-- 
2.49.0

[PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Shalini Chellathurai Saroja]

Add Control-Program Identification (CPI) data to the QEMU Object
Model (QOM), along with the timestamp in which the data was received
as shown below.

virsh # qemu-monitor-command vm --pretty '{"execute":"qom-list",
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi"}}'
{
  "return": [
    [...]
    {
      "name": "system_level",
      "type": "uint64"
    },
    {
      "name": "system_name",
      "type": "string"
    },
    {
      "name": "system_type",
      "type": "string"
    },
    {
      "name": "timestamp",
      "type": "uint64"
    },
    {
      "name": "sysplex_name",
      "type": "string"
    }
  ],
  "id": "libvirt-14"
}

Example CPI data:
virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get",
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi",
"property":"system_type"}}'
{
  "return": "LINUX   ",
  "id": "libvirt-18"
}
virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get",
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi",
"property":"system_name"}}'
{
  "return": "TESTVM  ",
  "id": "libvirt-19"
}
virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get",
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi",
"property":"sysplex_name"}}'
{
  "return": "PLEX    ",
  "id": "libvirt-20"
}
virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get",
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi",
"property":"system_level"}}'
{
  "return": 74872343805430528,
  "id": "libvirt-21"
}
virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get",
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi",
"property":"timestamp"}}'
{
  "return": 1748866753433923000,
  "id": "libvirt-22"
}

Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
---
 hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
 include/hw/s390x/event-facility.h |  5 +++
 qapi/machine.json                 | 58 +++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)

diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
index 935fa87acd..ec711e2291 100644
--- a/hw/s390x/sclpcpi.c
+++ b/hw/s390x/sclpcpi.c
@@ -15,7 +15,9 @@
   */
 
 #include "qemu/osdep.h"
+#include "qemu/timer.h"
 #include "hw/s390x/event-facility.h"
+#include "hw/s390x/ebcdic.h"
 
 typedef struct Data {
     uint8_t id_format;
@@ -55,11 +57,42 @@ static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
 {
     ControlProgramIdMsg *cpim = container_of(evt_buf_hdr, ControlProgramIdMsg,
                                              ebh);
+    SCLPEventCPI *e = SCLP_EVENT_CPI(event);
+
+    ascii_put(e->system_type, (char *)cpim->data.system_type,
+              sizeof(cpim->data.system_type));
+    ascii_put(e->system_name, (char *)cpim->data.system_name,
+              sizeof(cpim->data.system_name));
+    ascii_put(e->sysplex_name, (char *)cpim->data.sysplex_name,
+              sizeof(cpim->data.sysplex_name));
+    e->system_level = ldq_be_p(&cpim->data.system_level);
+    e->timestamp = qemu_clock_get_ns(QEMU_CLOCK_HOST);
 
     cpim->ebh.flags = SCLP_EVENT_BUFFER_ACCEPTED;
     return SCLP_RC_NORMAL_COMPLETION;
 }
 
+static char *get_system_type(Object *obj, Error **errp)
+{
+    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
+
+    return g_strndup((char *) e->system_type, sizeof(e->system_type));
+}
+
+static char *get_system_name(Object *obj, Error **errp)
+{
+    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
+
+    return g_strndup((char *) e->system_name, sizeof(e->system_name));
+}
+
+static char *get_sysplex_name(Object *obj, Error **errp)
+{
+    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
+
+    return g_strndup((char *) e->sysplex_name, sizeof(e->sysplex_name));
+}
+
 static void cpi_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -73,10 +106,24 @@ static void cpi_class_init(ObjectClass *klass, const void *data)
     k->write_event_data = write_event_data;
 }
 
+static void cpi_init(Object *obj)
+{
+    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
+
+    object_property_add_str(obj, "system_type", get_system_type, NULL);
+    object_property_add_str(obj, "system_name", get_system_name, NULL);
+    object_property_add_str(obj, "sysplex_name", get_sysplex_name, NULL);
+    object_property_add_uint64_ptr(obj, "system_level", &(e->system_level),
+                                   OBJ_PROP_FLAG_READ);
+    object_property_add_uint64_ptr(obj, "timestamp", &(e->timestamp),
+                                   OBJ_PROP_FLAG_READ);
+}
+
 static const TypeInfo sclp_cpi_info = {
     .name          = TYPE_SCLP_EVENT_CPI,
     .parent        = TYPE_SCLP_EVENT,
     .instance_size = sizeof(SCLPEventCPI),
+    .instance_init = cpi_init,
     .class_init    = cpi_class_init,
 };
 
diff --git a/include/hw/s390x/event-facility.h b/include/hw/s390x/event-facility.h
index e81bc80498..eac7a51100 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -201,6 +201,11 @@ OBJECT_DECLARE_TYPE(SCLPEventCPI, SCLPEventCPIClass,
 
 struct SCLPEventCPI {
     SCLPEvent event;
+    uint8_t system_type[8];
+    uint8_t system_name[8];
+    uint64_t system_level;
+    uint8_t sysplex_name[8];
+    uint64_t timestamp;
 };
 
 #define TYPE_SCLP_EVENT_FACILITY "s390-sclp-event-facility"
diff --git a/qapi/machine.json b/qapi/machine.json
index 5373e1368c..1f2db68032 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -2279,3 +2279,61 @@
 # Since: 1.2
 ##
 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+
+##
+# @S390ControlProgramId:
+#
+# Control-program identifiers provide data about the guest operating system.
+# The control-program identifiers are: system type, system name, system level
+# and sysplex name.
+#
+# In Linux, all the control-program identifiers are user configurable. The
+# system type, system name, and sysplex name use EBCDIC characters from
+# this set: capital A-Z, 0-9, $, @, #, and blank.  In Linux, the system type,
+# system name and sysplex name are arbitrary free-form texts.
+#
+# In Linux, the 8-byte hexadecimal system-level has the format
+# 0x<a><b><cc><dd><eeee><ff><gg><hh>, where:
+# <a>: is one hexadecimal byte, its most significant bit indicates hypervisor
+# use
+# <b>: is one digit that represents Linux distributions as follows
+# 0: generic Linux
+# 1: Red Hat Enterprise Linux
+# 2: SUSE Linux Enterprise Server
+# 3: Canonical Ubuntu
+# 4: Fedora
+# 5: openSUSE Leap
+# 6: Debian GNU/Linux
+# 7: Red Hat Enterprise Linux CoreOS
+# <cc>: are two digits for a distribution-specific encoding of the major version
+# of the distribution
+# <dd>: are two digits for a distribution-specific encoding of the minor version
+# of the distribution
+# <eeee>: are four digits for the patch level of the distribution
+# <ff>: are two digits for the major version of the kernel
+# <gg>: are two digits for the minor version of the kernel
+# <hh>: are two digits for the stable version of the kernel
+# (e.g. 74872343805430528, when converted to hex is 0x010a000000060b00). On
+# machines prior to z16, some of the values are not available to display.
+#
+# Sysplex refers to a cluster of logical partitions that communicates and
+# co-operates with each other.
+#
+# @system-type: operating system (e.g. "LINUX   ")
+#
+# @system-name: user configurable name of the VM (e.g. "TESTVM  ")
+#
+# @system-level: distribution and kernel version in Linux
+#
+# @sysplex-name: sysplex which the VM belongs to, if any (e.g. "PLEX ")
+#
+# @timestamp: latest update of CPI data in nanoseconds since the UNIX EPOCH
+#
+# Since: 10.1
+##
+{ 'struct': 'S390ControlProgramId', 'data': {
+     'system-type': 'str',
+     'system-name': 'str',
+     'system-level': 'uint64',
+     'sysplex-name': 'str',
+     'timestamp': 'uint64' } }
-- 
2.49.0

[PATCH v5 3/3] hw/s390x: support migration of CPI data

[Shalini Chellathurai Saroja]

Register Control-Program Identification data with the live
migration infrastructure.

Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
 hw/s390x/sclpcpi.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
index ec711e2291..12664b0a57 100644
--- a/hw/s390x/sclpcpi.c
+++ b/hw/s390x/sclpcpi.c
@@ -18,6 +18,7 @@
 #include "qemu/timer.h"
 #include "hw/s390x/event-facility.h"
 #include "hw/s390x/ebcdic.h"
+#include "migration/vmstate.h"
 
 typedef struct Data {
     uint8_t id_format;
@@ -93,12 +94,26 @@ static char *get_sysplex_name(Object *obj, Error **errp)
     return g_strndup((char *) e->sysplex_name, sizeof(e->sysplex_name));
 }
 
+static const VMStateDescription vmstate_sclpcpi = {
+    .name = "s390_control_program_id",
+    .version_id = 0,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT8_ARRAY(system_type, SCLPEventCPI, 8),
+        VMSTATE_UINT8_ARRAY(system_name, SCLPEventCPI, 8),
+        VMSTATE_UINT64(system_level, SCLPEventCPI),
+        VMSTATE_UINT8_ARRAY(sysplex_name, SCLPEventCPI, 8),
+        VMSTATE_UINT64(timestamp, SCLPEventCPI),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void cpi_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     SCLPEventClass *k = SCLP_EVENT_CLASS(klass);
 
     dc->user_creatable = false;
+    dc->vmsd =  &vmstate_sclpcpi;
 
     k->can_handle_event = can_handle_event;
     k->get_send_mask = send_mask;
-- 
2.49.0

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Nina Schoetterl-Glausch]

On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
> Add Control-Program Identification (CPI) data to the QEMU Object
> Model (QOM), along with the timestamp in which the data was received
> as shown below.

[...]
> 
> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> ---
>  hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
>  include/hw/s390x/event-facility.h |  5 +++
>  qapi/machine.json                 | 58 +++++++++++++++++++++++++++++++
>  3 files changed, 110 insertions(+)
> 
> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
> index 935fa87acd..ec711e2291 100644
> --- a/hw/s390x/sclpcpi.c
> +++ b/hw/s390x/sclpcpi.c
> @@ -15,7 +15,9 @@
>    */

[...]
>  
> +static void cpi_init(Object *obj)
> +{
> +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
> +
> +    object_property_add_str(obj, "system_type", get_system_type, NULL);
> +    object_property_add_str(obj, "system_name", get_system_name, NULL);
> +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, NULL);
> +    object_property_add_uint64_ptr(obj, "system_level", &(e->system_level),
> +                                   OBJ_PROP_FLAG_READ);
> +    object_property_add_uint64_ptr(obj, "timestamp", &(e->timestamp),
> +                                   OBJ_PROP_FLAG_READ);
> +}

I think it would be cleaner if those were class properties.
You could use object_class_property_add_str in cpi_class_init,
but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
device_class_set_props.
I'm not sure if DEFINE_PROP_STR can be used with a char array,
in any case you'd need to allocate the extra null byte.
If it doesn't work with the array just go ahead an heap allocate
the strings.
(You could also define a DEFINE_PROP for fixed size strings/char arrays,
which would be useful to have in general, this has come up before)
> +
>  static const TypeInfo sclp_cpi_info = {
>      .name          = TYPE_SCLP_EVENT_CPI,
>      .parent        = TYPE_SCLP_EVENT,
>      .instance_size = sizeof(SCLPEventCPI),
> +    .instance_init = cpi_init,
>      .class_init    = cpi_class_init,
>  };

[...]

> diff --git a/qapi/machine.json b/qapi/machine.json
> index 5373e1368c..1f2db68032 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -2279,3 +2279,61 @@
>  # Since: 1.2
>  ##
>  { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> +
> +##
> +# @S390ControlProgramId:
> +#
> +# Control-program identifiers provide data about the guest operating system.
> +# The control-program identifiers are: system type, system name, system level
> +# and sysplex name.
> +#
> +# In Linux, all the control-program identifiers are user configurable. The
> +# system type, system name, and sysplex name use EBCDIC characters from
> +# this set: capital A-Z, 0-9, $, @, #, and blank.  In Linux, the system type,
> +# system name and sysplex name are arbitrary free-form texts.
> +#
> +# In Linux, the 8-byte hexadecimal system-level has the format
> +# 0x<a><b><cc><dd><eeee><ff><gg><hh>, where:
> +# <a>: is one hexadecimal byte, its most significant bit indicates hypervisor
> +# use
> +# <b>: is one digit that represents Linux distributions as follows
> +# 0: generic Linux
> +# 1: Red Hat Enterprise Linux
> +# 2: SUSE Linux Enterprise Server
> +# 3: Canonical Ubuntu
> +# 4: Fedora
> +# 5: openSUSE Leap
> +# 6: Debian GNU/Linux
> +# 7: Red Hat Enterprise Linux CoreOS
> +# <cc>: are two digits for a distribution-specific encoding of the major version
> +# of the distribution
> +# <dd>: are two digits for a distribution-specific encoding of the minor version
> +# of the distribution
> +# <eeee>: are four digits for the patch level of the distribution
> +# <ff>: are two digits for the major version of the kernel
> +# <gg>: are two digits for the minor version of the kernel
> +# <hh>: are two digits for the stable version of the kernel
> +# (e.g. 74872343805430528, when converted to hex is 0x010a000000060b00). On
> +# machines prior to z16, some of the values are not available to display.
> +#
> +# Sysplex refers to a cluster of logical partitions that communicates and
> +# co-operates with each other.
> +#
> +# @system-type: operating system (e.g. "LINUX   ")
> +#
> +# @system-name: user configurable name of the VM (e.g. "TESTVM  ")
> +#
> +# @system-level: distribution and kernel version in Linux
> +#
> +# @sysplex-name: sysplex which the VM belongs to, if any (e.g. "PLEX ")
> +#
> +# @timestamp: latest update of CPI data in nanoseconds since the UNIX EPOCH
> +#
> +# Since: 10.1
> +##
> +{ 'struct': 'S390ControlProgramId', 'data': {
> +     'system-type': 'str',
> +     'system-name': 'str',
> +     'system-level': 'uint64',
> +     'sysplex-name': 'str',
> +     'timestamp': 'uint64' } }

This is unused now, so you can get rid of it and put the
documentation sclpcpi.c.

-- 
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v5 1/3] hw/s390x: add SCLP event type CPI

[Nina Schoetterl-Glausch]

On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
> Implement the Service-Call Logical Processor (SCLP) event
> type Control-Program Identification (CPI) in QEMU. This
> event is used to send CPI identifiers from the guest to the
> host. The CPI identifiers are: system type, system name,
> system level and sysplex name.
> 
> System type: operating system of the guest (e.g. "LINUX").
> System name: user configurable name of the guest (e.g. "TESTVM").
> System level: distribution and kernel version, if the system type is Linux
> (e.g. 0x50e00).
> Sysplex name: name of the cluster which the guest belongs to (if any)
> (e.g. "PLEX").
> 
> The SCLP event CPI is supported only from "s390-ccw-virtio-10.1" machine
> and higher.
> 
> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

-- 
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Thomas Huth]

On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
> On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
>> Add Control-Program Identification (CPI) data to the QEMU Object
>> Model (QOM), along with the timestamp in which the data was received
>> as shown below.
> 
> [...]
>>
>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> ---
>>   hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
>>   include/hw/s390x/event-facility.h |  5 +++
>>   qapi/machine.json                 | 58 +++++++++++++++++++++++++++++++
>>   3 files changed, 110 insertions(+)
>>
>> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>> index 935fa87acd..ec711e2291 100644
>> --- a/hw/s390x/sclpcpi.c
>> +++ b/hw/s390x/sclpcpi.c
>> @@ -15,7 +15,9 @@
>>     */
> 
> [...]
>>   
>> +static void cpi_init(Object *obj)
>> +{
>> +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
>> +
>> +    object_property_add_str(obj, "system_type", get_system_type, NULL);
>> +    object_property_add_str(obj, "system_name", get_system_name, NULL);
>> +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, NULL);
>> +    object_property_add_uint64_ptr(obj, "system_level", &(e->system_level),
>> +                                   OBJ_PROP_FLAG_READ);
>> +    object_property_add_uint64_ptr(obj, "timestamp", &(e->timestamp),
>> +                                   OBJ_PROP_FLAG_READ);
>> +}
> 
> I think it would be cleaner if those were class properties.
> You could use object_class_property_add_str in cpi_class_init,
> but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
> device_class_set_props.

For "normal" properties I'd say "yes" ... but in this case, this would also 
allow the user to set the properties from the host side - which would be a 
little bit weird? So I think it might be cleaner to keep it this way here 
without the "setter" functions? WDYT?

>> +{ 'struct': 'S390ControlProgramId', 'data': {
>> +     'system-type': 'str',
>> +     'system-name': 'str',
>> +     'system-level': 'uint64',
>> +     'sysplex-name': 'str',
>> +     'timestamp': 'uint64' } }
> 
> This is unused now, so you can get rid of it and put the
> documentation sclpcpi.c.

Agreed, that looks like it could be cleaned up now, indeed.

  Thomas

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Thomas Huth]

On 05/06/2025 13.56, Thomas Huth wrote:
> On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
>> On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
>>> Add Control-Program Identification (CPI) data to the QEMU Object
>>> Model (QOM), along with the timestamp in which the data was received
>>> as shown below.
>>
>> [...]
>>>
>>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>>> ---
>>>   hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
>>>   include/hw/s390x/event-facility.h |  5 +++
>>>   qapi/machine.json                 | 58 +++++++++++++++++++++++++++++++
>>>   3 files changed, 110 insertions(+)
>>>
>>> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>>> index 935fa87acd..ec711e2291 100644
>>> --- a/hw/s390x/sclpcpi.c
>>> +++ b/hw/s390x/sclpcpi.c
>>> @@ -15,7 +15,9 @@
>>>     */
>>
>> [...]
>>> +static void cpi_init(Object *obj)
>>> +{
>>> +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
>>> +
>>> +    object_property_add_str(obj, "system_type", get_system_type, NULL);
>>> +    object_property_add_str(obj, "system_name", get_system_name, NULL);
>>> +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, NULL);
>>> +    object_property_add_uint64_ptr(obj, "system_level", &(e->system_level),
>>> +                                   OBJ_PROP_FLAG_READ);
>>> +    object_property_add_uint64_ptr(obj, "timestamp", &(e->timestamp),
>>> +                                   OBJ_PROP_FLAG_READ);
>>> +}
>>
>> I think it would be cleaner if those were class properties.
>> You could use object_class_property_add_str in cpi_class_init,
>> but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
>> device_class_set_props.
> 
> For "normal" properties I'd say "yes" ... but in this case, this would also 
> allow the user to set the properties from the host side - which would be a 
> little bit weird? So I think it might be cleaner to keep it this way here 
> without the "setter" functions? WDYT?

By the way, Shalini, could you please add a description of the properties 
via object_property_set_description() here, please?

  Thanks,
   Thomas

PS: FWIW, Markus recently also provided a nice summery about the different 
property types here:
  https://lore.kernel.org/qemu-devel/87o6vfzdig.fsf@pond.sub.org/

Re: [PATCH v5 3/3] hw/s390x: support migration of CPI data

[Thomas Huth]

On 03/06/2025 15.56, Shalini Chellathurai Saroja wrote:
> Register Control-Program Identification data with the live
> migration infrastructure.
> 
> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>   hw/s390x/sclpcpi.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Nina Schotterl-Glausch]

On Thu, 2025-06-05 at 13:56 +0200, Thomas Huth wrote:
> On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
> > On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
> > > Add Control-Program Identification (CPI) data to the QEMU Object
> > > Model (QOM), along with the timestamp in which the data was received
> > > as shown below.
> > 
> > [...]
> > > 
> > > Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> > > ---
> > >   hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
> > >   include/hw/s390x/event-facility.h |  5 +++
> > >   qapi/machine.json                 | 58 +++++++++++++++++++++++++++++++
> > >   3 files changed, 110 insertions(+)
> > > 
> > > diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
> > > index 935fa87acd..ec711e2291 100644
> > > --- a/hw/s390x/sclpcpi.c
> > > +++ b/hw/s390x/sclpcpi.c
> > > @@ -15,7 +15,9 @@
> > >     */
> > 
> > [...]
> > >   
> > > +static void cpi_init(Object *obj)
> > > +{
> > > +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
> > > +
> > > +    object_property_add_str(obj, "system_type", get_system_type, NULL);
> > > +    object_property_add_str(obj, "system_name", get_system_name, NULL);
> > > +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, NULL);
> > > +    object_property_add_uint64_ptr(obj, "system_level", &(e->system_level),
> > > +                                   OBJ_PROP_FLAG_READ);
> > > +    object_property_add_uint64_ptr(obj, "timestamp", &(e->timestamp),
> > > +                                   OBJ_PROP_FLAG_READ);
> > > +}
> > 
> > I think it would be cleaner if those were class properties.
> > You could use object_class_property_add_str in cpi_class_init,
> > but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
> > device_class_set_props.
> 
> For "normal" properties I'd say "yes" ... but in this case, this would also 
> allow the user to set the properties from the host side - which would be a 
> little bit weird? So I think it might be cleaner to keep it this way here 
> without the "setter" functions? WDYT?

Oh, yeah I didn't consider this.
Still, object_class_property_add_str would be better I think, all instances
of the class should have these properties.


[...]

-- 
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Nina Schoetterl-Glausch]

On Thu, 2025-06-05 at 14:03 +0200, Thomas Huth wrote:
> On 05/06/2025 13.56, Thomas Huth wrote:
> > On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
> > > On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
> > > > Add Control-Program Identification (CPI) data to the QEMU Object
> > > > Model (QOM), along with the timestamp in which the data was received
> > > > as shown below.
> > > 
> > > [...]
> > > > 
> > > > Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> > > > ---

[...]
> 
> PS: FWIW, Markus recently also provided a nice summery about the different 
> property types here:
>   https://lore.kernel.org/qemu-devel/87o6vfzdig.fsf@pond.sub.org/

Thanks!

> This Property part of qdev isn't actually device-specific.  We could
> lift it into Object.  But would that be an improvement?  I'm not sure;
> QOM is confusing enough as it is.

:D indeed it is.

-- 
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Shalini Chellathurai Saroja]

On 2025-06-05 13:56, Thomas Huth wrote:
> On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
>> On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
>>> Add Control-Program Identification (CPI) data to the QEMU Object
>>> Model (QOM), along with the timestamp in which the data was received
>>> as shown below.
>> 
>> [...]
>>> 
>>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>>> ---
>>>   hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
>>>   include/hw/s390x/event-facility.h |  5 +++
>>>   qapi/machine.json                 | 58 
>>> +++++++++++++++++++++++++++++++
>>>   3 files changed, 110 insertions(+)
>>> 
>>> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>>> index 935fa87acd..ec711e2291 100644
>>> --- a/hw/s390x/sclpcpi.c
>>> +++ b/hw/s390x/sclpcpi.c
>>> @@ -15,7 +15,9 @@
>>>     */
>> 
>> [...]
>>>   +static void cpi_init(Object *obj)
>>> +{
>>> +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
>>> +
>>> +    object_property_add_str(obj, "system_type", get_system_type, 
>>> NULL);
>>> +    object_property_add_str(obj, "system_name", get_system_name, 
>>> NULL);
>>> +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, 
>>> NULL);
>>> +    object_property_add_uint64_ptr(obj, "system_level", 
>>> &(e->system_level),
>>> +                                   OBJ_PROP_FLAG_READ);
>>> +    object_property_add_uint64_ptr(obj, "timestamp", 
>>> &(e->timestamp),
>>> +                                   OBJ_PROP_FLAG_READ);
>>> +}
>> 
>> I think it would be cleaner if those were class properties.
>> You could use object_class_property_add_str in cpi_class_init,
>> but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
>> device_class_set_props.
> 
> For "normal" properties I'd say "yes" ... but in this case, this would
> also allow the user to set the properties from the host side - which
> would be a little bit weird? So I think it might be cleaner to keep it
> this way here without the "setter" functions? WDYT?
> 
>>> +{ 'struct': 'S390ControlProgramId', 'data': {
>>> +     'system-type': 'str',
>>> +     'system-name': 'str',
>>> +     'system-level': 'uint64',
>>> +     'sysplex-name': 'str',
>>> +     'timestamp': 'uint64' } }
>> 
>> This is unused now, so you can get rid of it and put the
>> documentation sclpcpi.c.
> 
> Agreed, that looks like it could be cleaned up now, indeed.
> 

Hello Thomas, Nina,

Yes, I will do this. I am sorry that I missed to do this earlier.
Thank you very much for the quick review.



>  Thomas


-- 
Mit freundlichen Grüßen / Kind regards
Shalini Chellathurai Saroja
Software Developer
Linux on IBM Z & KVM Development
IBM Deutschland Research & Development GmbH
Dept 1419, Schoenaicher Str. 220, 71032 Boeblingen
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht 
Stuttgart, HRB 243294

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Thomas Huth]

On 03/06/2025 15.56, Shalini Chellathurai Saroja wrote:
> Add Control-Program Identification (CPI) data to the QEMU Object
> Model (QOM), along with the timestamp in which the data was received
> as shown below.
...
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 5373e1368c..1f2db68032 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -2279,3 +2279,61 @@
>   # Since: 1.2
>   ##
>   { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> +
> +##
> +# @S390ControlProgramId:
> +#
> +# Control-program identifiers provide data about the guest operating system.
> +# The control-program identifiers are: system type, system name, system level
> +# and sysplex name.
> +#
> +# In Linux, all the control-program identifiers are user configurable. The
> +# system type, system name, and sysplex name use EBCDIC characters from
> +# this set: capital A-Z, 0-9, $, @, #, and blank.  In Linux, the system type,
> +# system name and sysplex name are arbitrary free-form texts.
> +#
> +# In Linux, the 8-byte hexadecimal system-level has the format
> +# 0x<a><b><cc><dd><eeee><ff><gg><hh>, where:
> +# <a>: is one hexadecimal byte, its most significant bit indicates hypervisor
> +# use

  Hi Shalini!

While testing the patches, the above description caused some headache for 
me, but I think it's simply typo here: This is not a hexadecimal byte, it's 
just a nibble / 4-bit digit for this <a>, right? Could you please fix the 
description in the next version of the patch series, please?

  Thanks,
   Thomas


> +# <b>: is one digit that represents Linux distributions as follows
> +# 0: generic Linux
> +# 1: Red Hat Enterprise Linux
> +# 2: SUSE Linux Enterprise Server
> +# 3: Canonical Ubuntu
> +# 4: Fedora
> +# 5: openSUSE Leap
> +# 6: Debian GNU/Linux
> +# 7: Red Hat Enterprise Linux CoreOS
> +# <cc>: are two digits for a distribution-specific encoding of the major version
> +# of the distribution
> +# <dd>: are two digits for a distribution-specific encoding of the minor version
> +# of the distribution
> +# <eeee>: are four digits for the patch level of the distribution
> +# <ff>: are two digits for the major version of the kernel
> +# <gg>: are two digits for the minor version of the kernel
> +# <hh>: are two digits for the stable version of the kernel
> +# (e.g. 74872343805430528, when converted to hex is 0x010a000000060b00). On
> +# machines prior to z16, some of the values are not available to display.
> +#
> +# Sysplex refers to a cluster of logical partitions that communicates and
> +# co-operates with each other.
> +#
> +# @system-type: operating system (e.g. "LINUX   ")
> +#
> +# @system-name: user configurable name of the VM (e.g. "TESTVM  ")
> +#
> +# @system-level: distribution and kernel version in Linux
> +#
> +# @sysplex-name: sysplex which the VM belongs to, if any (e.g. "PLEX ")
> +#
> +# @timestamp: latest update of CPI data in nanoseconds since the UNIX EPOCH
> +#
> +# Since: 10.1
> +##
> +{ 'struct': 'S390ControlProgramId', 'data': {
> +     'system-type': 'str',
> +     'system-name': 'str',
> +     'system-level': 'uint64',
> +     'sysplex-name': 'str',
> +     'timestamp': 'uint64' } }

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Shalini Chellathurai Saroja]

On 2025-06-13 11:59, Thomas Huth wrote:
> On 03/06/2025 15.56, Shalini Chellathurai Saroja wrote:
>> Add Control-Program Identification (CPI) data to the QEMU Object
>> Model (QOM), along with the timestamp in which the data was received
>> as shown below.
> ...
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 5373e1368c..1f2db68032 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -2279,3 +2279,61 @@
>>   # Since: 1.2
>>   ##
>>   { 'command': 'query-cpu-definitions', 'returns': 
>> ['CpuDefinitionInfo'] }
>> +
>> +##
>> +# @S390ControlProgramId:
>> +#
>> +# Control-program identifiers provide data about the guest operating 
>> system.
>> +# The control-program identifiers are: system type, system name, 
>> system level
>> +# and sysplex name.
>> +#
>> +# In Linux, all the control-program identifiers are user 
>> configurable. The
>> +# system type, system name, and sysplex name use EBCDIC characters 
>> from
>> +# this set: capital A-Z, 0-9, $, @, #, and blank.  In Linux, the 
>> system type,
>> +# system name and sysplex name are arbitrary free-form texts.
>> +#
>> +# In Linux, the 8-byte hexadecimal system-level has the format
>> +# 0x<a><b><cc><dd><eeee><ff><gg><hh>, where:
>> +# <a>: is one hexadecimal byte, its most significant bit indicates 
>> hypervisor
>> +# use
> 
>  Hi Shalini!
> 
> While testing the patches, the above description caused some headache
> for me, but I think it's simply typo here: This is not a hexadecimal
> byte, it's just a nibble / 4-bit digit for this <a>, right? Could you
> please fix the description in the next version of the patch series,
> please?


Hello Thomas,
Yes, that is correct. <a> is a 4-bit digit and not a byte. I will 
correct this
in the next version. Thank you very much.

> 
>  Thanks,
>   Thomas
> 
> 
>> +# <b>: is one digit that represents Linux distributions as follows
>> +# 0: generic Linux
>> +# 1: Red Hat Enterprise Linux
>> +# 2: SUSE Linux Enterprise Server
>> +# 3: Canonical Ubuntu
>> +# 4: Fedora
>> +# 5: openSUSE Leap
>> +# 6: Debian GNU/Linux
>> +# 7: Red Hat Enterprise Linux CoreOS
>> +# <cc>: are two digits for a distribution-specific encoding of the 
>> major version
>> +# of the distribution
>> +# <dd>: are two digits for a distribution-specific encoding of the 
>> minor version
>> +# of the distribution
>> +# <eeee>: are four digits for the patch level of the distribution
>> +# <ff>: are two digits for the major version of the kernel
>> +# <gg>: are two digits for the minor version of the kernel
>> +# <hh>: are two digits for the stable version of the kernel
>> +# (e.g. 74872343805430528, when converted to hex is 
>> 0x010a000000060b00). On
>> +# machines prior to z16, some of the values are not available to 
>> display.
>> +#
>> +# Sysplex refers to a cluster of logical partitions that communicates 
>> and
>> +# co-operates with each other.
>> +#
>> +# @system-type: operating system (e.g. "LINUX   ")
>> +#
>> +# @system-name: user configurable name of the VM (e.g. "TESTVM  ")
>> +#
>> +# @system-level: distribution and kernel version in Linux
>> +#
>> +# @sysplex-name: sysplex which the VM belongs to, if any (e.g. "PLEX 
>> ")
>> +#
>> +# @timestamp: latest update of CPI data in nanoseconds since the UNIX 
>> EPOCH
>> +#
>> +# Since: 10.1
>> +##
>> +{ 'struct': 'S390ControlProgramId', 'data': {
>> +     'system-type': 'str',
>> +     'system-name': 'str',
>> +     'system-level': 'uint64',
>> +     'sysplex-name': 'str',
>> +     'timestamp': 'uint64' } }

-- 
Mit freundlichen Grüßen / Kind regards
Shalini Chellathurai Saroja
Software Developer
Linux on IBM Z & KVM Development
IBM Deutschland Research & Development GmbH
Dept 1419, Schoenaicher Str. 220, 71032 Boeblingen
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht 
Stuttgart, HRB 243294

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Shalini Chellathurai Saroja]

On 2025-06-05 15:16, Nina Schotterl-Glausch wrote:
> On Thu, 2025-06-05 at 13:56 +0200, Thomas Huth wrote:
>> On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
>> > On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
>> > > Add Control-Program Identification (CPI) data to the QEMU Object
>> > > Model (QOM), along with the timestamp in which the data was received
>> > > as shown below.
>> >
>> > [...]
>> > >
>> > > Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> > > ---
>> > >   hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
>> > >   include/hw/s390x/event-facility.h |  5 +++
>> > >   qapi/machine.json                 | 58 +++++++++++++++++++++++++++++++
>> > >   3 files changed, 110 insertions(+)
>> > >
>> > > diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>> > > index 935fa87acd..ec711e2291 100644
>> > > --- a/hw/s390x/sclpcpi.c
>> > > +++ b/hw/s390x/sclpcpi.c
>> > > @@ -15,7 +15,9 @@
>> > >     */
>> >
>> > [...]
>> > >
>> > > +static void cpi_init(Object *obj)
>> > > +{
>> > > +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
>> > > +
>> > > +    object_property_add_str(obj, "system_type", get_system_type, NULL);
>> > > +    object_property_add_str(obj, "system_name", get_system_name, NULL);
>> > > +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, NULL);
>> > > +    object_property_add_uint64_ptr(obj, "system_level", &(e->system_level),
>> > > +                                   OBJ_PROP_FLAG_READ);
>> > > +    object_property_add_uint64_ptr(obj, "timestamp", &(e->timestamp),
>> > > +                                   OBJ_PROP_FLAG_READ);
>> > > +}
>> >
>> > I think it would be cleaner if those were class properties.
>> > You could use object_class_property_add_str in cpi_class_init,
>> > but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
>> > device_class_set_props.
>> 
>> For "normal" properties I'd say "yes" ... but in this case, this would 
>> also
>> allow the user to set the properties from the host side - which would 
>> be a
>> little bit weird? So I think it might be cleaner to keep it this way 
>> here
>> without the "setter" functions? WDYT?
> 
> Oh, yeah I didn't consider this.
> Still, object_class_property_add_str would be better I think, all 
> instances
> of the class should have these properties.
> 
Hello Nina,

Ok, I will use object_class_property_add_str and 
object_class_property_add methods instead.
Thank you.
> 
> [...]

-- 
Mit freundlichen Grüßen / Kind regards
Shalini Chellathurai Saroja
Software Developer
Linux on IBM Z & KVM Development
IBM Deutschland Research & Development GmbH
Dept 1419, Schoenaicher Str. 220, 71032 Boeblingen
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht 
Stuttgart, HRB 243294

Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

[Shalini Chellathurai Saroja]

On 2025-06-05 14:03, Thomas Huth wrote:
> On 05/06/2025 13.56, Thomas Huth wrote:
>> On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
>>> On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
>>>> Add Control-Program Identification (CPI) data to the QEMU Object
>>>> Model (QOM), along with the timestamp in which the data was received
>>>> as shown below.
>>> 
>>> [...]
>>>> 
>>>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>>>> ---
>>>>   hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
>>>>   include/hw/s390x/event-facility.h |  5 +++
>>>>   qapi/machine.json                 | 58 
>>>> +++++++++++++++++++++++++++++++
>>>>   3 files changed, 110 insertions(+)
>>>> 
>>>> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>>>> index 935fa87acd..ec711e2291 100644
>>>> --- a/hw/s390x/sclpcpi.c
>>>> +++ b/hw/s390x/sclpcpi.c
>>>> @@ -15,7 +15,9 @@
>>>>     */
>>> 
>>> [...]
>>>> +static void cpi_init(Object *obj)
>>>> +{
>>>> +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
>>>> +
>>>> +    object_property_add_str(obj, "system_type", get_system_type, 
>>>> NULL);
>>>> +    object_property_add_str(obj, "system_name", get_system_name, 
>>>> NULL);
>>>> +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, 
>>>> NULL);
>>>> +    object_property_add_uint64_ptr(obj, "system_level", 
>>>> &(e->system_level),
>>>> +                                   OBJ_PROP_FLAG_READ);
>>>> +    object_property_add_uint64_ptr(obj, "timestamp", 
>>>> &(e->timestamp),
>>>> +                                   OBJ_PROP_FLAG_READ);
>>>> +}
>>> 
>>> I think it would be cleaner if those were class properties.
>>> You could use object_class_property_add_str in cpi_class_init,
>>> but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
>>> device_class_set_props.
>> 
>> For "normal" properties I'd say "yes" ... but in this case, this would 
>> also allow the user to set the properties from the host side - which 
>> would be a little bit weird? So I think it might be cleaner to keep it 
>> this way here without the "setter" functions? WDYT?
> 
> By the way, Shalini, could you please add a description of the
> properties via object_property_set_description() here, please?
> 
>  Thanks,
>   Thomas
> 

Hello Thomas,
I will add description for the properties via 
object_class_property_set_description(),
as I will be using object_class_property_add_str and 
object_class_property_add methods
based on Nina's feedback and Markus's summary on properties which you 
pointed out.

Thank you very much.

> PS: FWIW, Markus recently also provided a nice summery about the
> different property types here:
>  https://lore.kernel.org/qemu-devel/87o6vfzdig.fsf@pond.sub.org/

-- 
Mit freundlichen Grüßen / Kind regards
Shalini Chellathurai Saroja
Software Developer
Linux on IBM Z & KVM Development
IBM Deutschland Research & Development GmbH
Dept 1419, Schoenaicher Str. 220, 71032 Boeblingen
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht 
Stuttgart, HRB 243294