Re: [PATCH qemu v2 2/3] hw/s390x: add SCLP event type CPI

[Thomas Huth <thuth@redhat.com>]

On 24/02/2025 13.04, Shalini Chellathurai Saroja wrote:
> Implement the SCLP event type Control-Program Identification
> (CPI) in QEMU. This event is used to send CPI identifiers,
> which provide information about the guest OS to the host.
> 
> Save the information obtained from the SCLP CPI event to the
> QOM along with the timestamp in which the data was received.
> 
> Example:
> virsh # qemu-monitor-command vm --pretty '{
> "execute":"qom-get","arguments": {
> "path": "/machine", "property": "s390-control-program_id"}}'

I guess it should be a "-" instead of a "_" between "program" and "id"?

> {
>    "return": {
>      "timestamp": 1711620874948254000,
>      "system-level": "0x50e00",
>      "sysplex-name": "SYSPLEX ",
>      "system-name": "TESTVM  ",
>      "system-type": "LINUX   "
>    },
>    "id": "libvirt-15"
> }
> 
> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>   hw/s390x/event-facility.c         |  12 +++-
>   hw/s390x/meson.build              |   1 +
>   hw/s390x/sclpcpi.c                | 105 ++++++++++++++++++++++++++++++
>   include/hw/s390x/event-facility.h |   3 +
>   4 files changed, 120 insertions(+), 1 deletion(-)
>   create mode 100644 hw/s390x/sclpcpi.c
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 2b0332c20e..c0fb6e098c 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -39,7 +39,7 @@ typedef struct SCLPEventsBus {
>   struct SCLPEventFacility {
>       SysBusDevice parent_obj;
>       SCLPEventsBus sbus;
> -    SCLPEvent quiesce, cpu_hotplug;
> +    SCLPEvent quiesce, cpu_hotplug, cpi;
>       /* guest's receive mask */
>       union {
>           uint32_t receive_mask_pieces[2];
> @@ -436,6 +436,10 @@ static void init_event_facility(Object *obj)
>       object_initialize_child(obj, TYPE_SCLP_CPU_HOTPLUG,
>                               &event_facility->cpu_hotplug,
>                               TYPE_SCLP_CPU_HOTPLUG);
> +
> +    object_initialize_child(obj, TYPE_SCLP_CPI,
> +                            &event_facility->cpi,
> +                            TYPE_SCLP_CPI);
>   }
>   
>   static void realize_event_facility(DeviceState *dev, Error **errp)
> @@ -451,6 +455,12 @@ static void realize_event_facility(DeviceState *dev, Error **errp)
>           qdev_unrealize(DEVICE(&event_facility->quiesce));
>           return;
>       }
> +    if (!qdev_realize(DEVICE(&event_facility->cpi),
> +                      BUS(&event_facility->sbus), errp)) {
> +        qdev_unrealize(DEVICE(&event_facility->quiesce));
> +        qdev_unrealize(DEVICE(&event_facility->cpu_hotplug));
> +        return;
> +    }
>   }
>   
>   static void reset_event_facility(DeviceState *dev)
> 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/sclpcpi.c b/hw/s390x/sclpcpi.c
> new file mode 100644
> index 0000000000..f2830d2d57
> --- /dev/null
> +++ b/hw/s390x/sclpcpi.c
> @@ -0,0 +1,105 @@
> +/*

scripts/checkpatch.pl recently learnt a new check to mandate SPDX license 
tags now for new files ... so please add one here now!

> + * SCLP event type 11 - Control-Program Identification(CPI):
> + *    CPI is used to send program identifiers from the control-program to the
> + *    SCLP. The program identifiers provide data about the guest instance. It
> + *    is not sent by the SCLP.
> + *
> + *    The program identifiers are system type, system name, sysplex name and
> + *    system level. The system type, system name, and sysplex name use EBCDIC
> + *    ucharacters from this set: capital A-Z, 0-9, $, @, #, and blank. The
> + *    system level is a hex value.
> + *
> + * Copyright IBM, Corp. 2024
> + *
> + * Authors:
> + *  Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at your
> + * option) any later version.  See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#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 "hw/s390x/s390-virtio-ccw.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_CPI;
> +}
> +
> +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_CPI;
> +}
> +
> +static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
> +{
> +    ControlProgramIdMsg *cpi = container_of(evt_buf_hdr, ControlProgramIdMsg,
> +                                            ebh);
> +    S390CcwMachineState *ms = S390_CCW_MACHINE(qdev_get_machine());
> +
> +    ascii_put(ms->cpi.system_type, (char *)cpi->data.system_type, 8);
> +    ascii_put(ms->cpi.system_name, (char *)cpi->data.system_name, 8);
> +    ascii_put(ms->cpi.sysplex_name, (char *)cpi->data.sysplex_name, 8);
> +    ms->cpi.system_level = be64_to_cpu(cpi->data.system_level);

Can we be confident that system_level is always properly aligned?
Otherwise it's maybe better to use ldq_be_p() instead?

  Thomas