Re: [PATCH v4 1/4] hw/s390x: add SCLP event type CPI

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

On 2025-04-28 11:22, Janis Schoetterl-Glausch wrote:
> On Thu, 2025-04-10 at 17:09 +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").
>> 
>> 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        | 14 +++++
>>  hw/s390x/sclpcpi.c                | 92 
>> +++++++++++++++++++++++++++++++
>>  include/hw/s390x/event-facility.h | 13 +++++
>>  5 files changed, 122 insertions(+)
>>  create mode 100644 hw/s390x/sclpcpi.c
>> 
>> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
>> index 2b0332c20e..60237b8581 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 75b32182eb..7f28cbd1de 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -260,6 +260,17 @@ 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);
> 
> I wonder if we should drop this and add control-program-id directly
> on the event facility. I don't see what purpose having the intermediate
> level sclpcpi serves.
> 

Hello Nina,

sclpcpi is the qemu device which handles the SCLP event type 
Control-Program Identification by performing the following.
1. The sclpcpi device indicates to the guest OS that QEMU is able to 
process the event buffers of the SCLP event type Control-Program 
Identification.
2. The sclpcpi device receives the data from the event buffers of the 
SCLP event type Control-Program Identification sent by the guest OS.
The received data is stored in control-program-id, which is the property 
of the device sclpcpi. So, the sclpcpi device cannot be dropped.

In addition, Thomas Huth told that the migration data of a device must 
be stored in the device itself during his review of the previous 
versions of this patch series. So control-program-id cannot be stored in 
the event facility.

It is possible to add the identifiers directly as the properties of 
sclpcpi (eg. system type as shown below) and remove the 
control-program-id.

virsh # qemu-monitor-command vm --pretty 
'{"execute":"qom-list","arguments": {"path": 
"/machine/sclp/s390-sclp-event-facility/sclpcpi"}}'
{
   "return": [
     {
       "name": "type",
       "type": "string"
     },
     {
       "name": "parent_bus",
       "type": "link<bus>"
     },
     {
       "name": "realized",
       "type": "bool"
     },
     {
       "name": "hotplugged",
       "type": "bool"
     },
     {
       "name": "hotpluggable",
       "type": "bool"
     },
     {
       "name": "system_type",
       "type": "string"
     },
     {
       "name": "control-program-id",
       "type": "S390ControlProgramId"
     }
   ],
   "id": "libvirt-16"
}

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-17"
}

However, Hendrik Brückner suggested to group the identifiers together 
during the initial discussion of this line item. So, I would prefer to 
leave this as it is. Please let me know if you still think otherwise.

>> +}
> 
> [...]
> 
>> diff --git a/include/hw/s390x/event-facility.h 
>> b/include/hw/s390x/event-facility.h
>> index ff874e792d..ef469e62ae 100644
>> --- a/include/hw/s390x/event-facility.h
>> +++ b/include/hw/s390x/event-facility.h
> 
> [...]
>    I.
>> +struct SCLPEventCPI {
>> +    DeviceState qdev;
>> +    SCLPEvent event;
> 
> I think you should just inherit from SCLPEvent, which in turn inherits
> from DeviceState.
> So without the qdev.
> 

Ok, I will do so. Thank you very much for the review.

>> +};
>> +
>>  #define TYPE_SCLP_EVENT_FACILITY "s390-sclp-event-facility"
>>  typedef struct SCLPEventFacility SCLPEventFacility;
>>  typedef struct SCLPEventFacilityClass SCLPEventFacilityClass;

-- 
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