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

[Nina Schoetterl-Glausch <nsg@linux.ibm.com>]

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