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

[Janis Schoetterl-Glausch <scgl@linux.vnet.ibm.com>]

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.

> +}

[...]

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

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

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