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

[Shalini Chellathurai Saroja <shalini@linux.ibm.com>]

On 2025-04-01 15:55, Nina Schoetterl-Glausch wrote:
> On Mon, 2025-03-31 at 16:00 +0200, Shalini Chellathurai Saroja wrote:
>> Add Control-Program Identification data to the QEMU Object
>> Model (QOM), along with the timestamp in which the data was received.
>> 
>> Example:
>> virsh # qemu-monitor-command vm --pretty '{
>> "execute": "qom-get",
>> "arguments": {
>> "path": "/machine/sclp/s390-sclp-event-facility/sclpcpi",
>> "property": "control-program-id" }}'
>> {
>>   "return": {
>>     "timestamp": 1742390410685762000,
>>     "system-level": 74872343805430528,
>>     "sysplex-name": "PLEX ",
>>     "system-name": "TESTVM  ",
>>     "system-type": "LINUX   "
>>   },
>>   "id": "libvirt-15"
>> }
>> 
>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> ---
>>  hw/s390x/sclpcpi.c                | 38 ++++++++++++++++++++
>>  include/hw/s390x/event-facility.h |  9 +++++
>>  qapi/machine.json                 | 58 
>> +++++++++++++++++++++++++++++++
>>  3 files changed, 105 insertions(+)
>> 
>> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>> index 7ace5dd64e..969c15e43d 100644
>> --- a/hw/s390x/sclpcpi.c
>> +++ b/hw/s390x/sclpcpi.c
>> @@ -57,8 +57,11 @@
>>    */
>> 
>>  #include "qemu/osdep.h"
>> +#include "qemu/timer.h"
>>  #include "hw/s390x/sclp.h"
>>  #include "hw/s390x/event-facility.h"
>> +#include "hw/s390x/ebcdic.h"
>> +#include "qapi/qapi-visit-machine.h"
>> 
>>  typedef struct Data {
>>      uint8_t id_format;
>> @@ -99,10 +102,37 @@ static int write_event_data(SCLPEvent *event, 
>> EventBufferHeader *evt_buf_hdr)
>>      ControlProgramIdMsg *cpim = container_of(evt_buf_hdr, 
>> ControlProgramIdMsg,
>>                                               ebh);
>> 
>> +    ascii_put(event->cpi.system_type, (char *)cpim->data.system_type, 
>> 8);
>> +    ascii_put(event->cpi.system_name, (char *)cpim->data.system_name, 
>> 8);
>> +    ascii_put(event->cpi.sysplex_name, (char 
>> *)cpim->data.sysplex_name, 8);
>> +    event->cpi.system_level = ldq_be_p(&cpim->data.system_level);
>> +    event->cpi.timestamp = qemu_clock_get_ns(QEMU_CLOCK_HOST);
>> +
>>      cpim->ebh.flags = SCLP_EVENT_BUFFER_ACCEPTED;
>>      return SCLP_RC_NORMAL_COMPLETION;
>>  }
>> 
>> +static void get_control_program_id(Object *obj, Visitor *v,
>> +                                   const char *name, void *opaque,
>> +                                   Error **errp)
>> +{
>> +    SCLPEvent *event = (SCLPEvent *)(obj);
> 
> Do a checked cast with SCLP_EVENT(obj).

Hello Nina,

ok, thank you.
> 
>> +    S390ControlProgramId *cpi;
>> +
>> +    cpi = &(S390ControlProgramId){
>> +        .system_type = g_strndup((char *) event->cpi.system_type,
>> +                                 sizeof(event->cpi.system_type)),
>> +        .system_name = g_strndup((char *) event->cpi.system_name,
>> +                                 sizeof(event->cpi.system_name)),
>> +        .system_level = event->cpi.system_level,
>> +        .sysplex_name = g_strndup((char *) event->cpi.sysplex_name,
>> +                                  sizeof(event->cpi.sysplex_name)),
>> +        .timestamp = event->cpi.timestamp
>> +    };
>> +
>> +    visit_type_S390ControlProgramId(v, name, &cpi, errp);
>> +}
>> +
>>  static void cpi_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -114,6 +144,14 @@ static void cpi_class_init(ObjectClass *klass, 
>> void *data)
>>      k->get_send_mask = send_mask;
>>      k->get_receive_mask = receive_mask;
>>      k->write_event_data = write_event_data;
>> +
>> +    object_class_property_add(klass, "control-program-id",
>> +                              "S390ControlProgramId",
>> +                              get_control_program_id,
>> +                              NULL, NULL, NULL);
>> +    object_class_property_set_description(klass, 
>> "control-program-id",
>> +        "Control-program identifiers provide data about the guest "
>> +        "operating system");
>>  }
>> 
>>  static const TypeInfo sclp_cpi_info = {
>> diff --git a/include/hw/s390x/event-facility.h 
>> b/include/hw/s390x/event-facility.h
>> index f445d2f9f5..39e589ed44 100644
>> --- a/include/hw/s390x/event-facility.h
>> +++ b/include/hw/s390x/event-facility.h
>> @@ -169,10 +169,19 @@ typedef struct ReadEventData {
>>      };
>>  } QEMU_PACKED ReadEventData;
>> 
>> +typedef struct ControlProgramId {
>> +    uint8_t system_type[8];
>> +    uint8_t system_name[8];
>> +    uint64_t system_level;
>> +    uint8_t sysplex_name[8];
>> +    uint64_t timestamp;
>> +} QEMU_PACKED ControlProgramId;
>> +
>>  struct SCLPEvent {
>>      DeviceState qdev;
>>      bool event_pending;
>>      char *name;
>> +    ControlProgramId cpi;
> 
> I don't think this should go into SCLPEvent.
> Rather SCLPEventFacility or SCLPDevice. Otherwise all events,
> so also quiesce and cpu_hotplug have a cpi field.
> 
ok, that is correct.

I gave it a try by moving ControlProgramId to SCLPDevice. With this, the 
migration data is stored in dc->vmsd of TYPE_SCLP as shown below.

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 5945c9b1d8..4d6d5bb857 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -424,6 +424,29 @@ static void sclp_init(Object *obj)
      sclp_memory_init(sclp);
  }

+static const VMStateDescription vmstate_control_program_id = {
+    .name = "s390_control_program_id",
+    .version_id = 0,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT8_ARRAY(system_type, ControlProgramId, 8),
+        VMSTATE_UINT8_ARRAY(system_name, ControlProgramId, 8),
+        VMSTATE_UINT64(system_level, ControlProgramId),
+        VMSTATE_UINT8_ARRAY(sysplex_name, ControlProgramId, 8),
+        VMSTATE_UINT64(timestamp, ControlProgramId),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_sclpcpi = {
+    .name = "s390_sclpcpi",
+    .version_id = 0,
+    .fields = (const VMStateField[]) {
+        VMSTATE_STRUCT(cpi, SCLPDevice, 0, vmstate_control_program_id,
+                       ControlProgramId),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
  static void sclp_class_init(ObjectClass *oc, void *data)
  {
      SCLPDeviceClass *sc = SCLP_CLASS(oc);
@@ -438,6 +461,7 @@ static void sclp_class_init(ObjectClass *oc, void 
*data)
       * which is a non-pluggable sysbus device
       */
      dc->user_creatable = false;
+    dc->vmsd =  &vmstate_sclpcpi;

      sc->read_SCP_info = read_SCP_info;
      sc->read_cpu_info = sclp_read_cpu_info;

@Thomas Huth: Is this ok?, 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