On Wed, 2025-01-15 at 14:31 +0100, Shalini Chellathurai Saroja wrote:
> This commit adds the firmware control-program
> identifiers received from a KVM guest via the
> SCLP event type Control-Program Identification to QOM.
> A timestamp in which the data is received is also
> added to QOM.
>
> Example:
> virsh # qemu-monitor-command vm --pretty '{
> "execute":"qom-get","arguments": {
> "path": "/machine", "property": "s390-cpi"}}'
> {
> "return": {
> "timestamp": 1711620874948254000,
> "system-level": "0x50e00",
> "sysplex-name": "SYSPLEX ",
> "system-name": "TESTVM ",
> "system-type": "LINUX "
> },
> "id": "libvirt-15"
> }
>
> Signed-off-by: Shalini Chellathurai Saroja <shal...@linux.ibm.com>
> ---
> hw/s390x/s390-virtio-ccw.c | 26 ++++++++++++++++++++++++++
> hw/s390x/sclpcpi.c | 10 ++++++++++
> include/hw/s390x/s390-virtio-ccw.h | 8 ++++++++
> qapi/machine.json | 24 ++++++++++++++++++++++++
> 4 files changed, 68 insertions(+)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 38aeba14ee..35fb523af9 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -49,6 +49,7 @@
> #include "hw/virtio/virtio-md-pci.h"
> #include "hw/s390x/virtio-ccw-md.h"
> #include CONFIG_DEVICES
> +#include "qapi/qapi-visit-machine.h"
>
> static Error *pv_mig_blocker;
>
> @@ -773,6 +774,25 @@ static void machine_set_loadparm(Object *obj, Visitor *v,
> s390_ipl_fmt_loadparm(ms->loadparm, val, errp);
> }
>
> +static void machine_get_cpi(Object *obj, Visitor *v,
> + const char *name, void *opaque, Error **errp)
> +{
> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> + S390Cpi *cpi;
> + cpi = &(S390Cpi){
> + .system_type = g_strndup((char *) ms->cpi.system_type,
> + sizeof(ms->cpi.system_type)),
> + .system_name = g_strndup((char *) ms->cpi.system_name,
> + sizeof(ms->cpi.system_name)),
> + .system_level = g_strdup_printf("0x%lx", ms->cpi.system_level),
Is there any way in which it would make sense for the qmp caller to
interpret this as a number? If so exposing it as a number would be preferable.
> + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name,
> + sizeof(ms->cpi.sysplex_name)),
> + .timestamp = ms->cpi.timestamp
> + };
> +
> + visit_type_S390Cpi(v, name, &cpi, &error_abort);
> +}
> +
> static void ccw_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -826,6 +846,12 @@ static void ccw_machine_class_init(ObjectClass *oc, void
> *data)
> "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars
> converted"
> " to upper case) to pass to machine loader, boot manager,"
> " and guest kernel");
> + object_class_property_add(oc, "s390-cpi", "S390Cpi",
> + machine_get_cpi, NULL, NULL, NULL);
> + object_class_property_set_description(oc, "s390-cpi",
> + "Control-progam identifiers provide data about the guest "
> + "operating system");
> +
> }
>
> static inline void s390_machine_initfn(Object *obj)
> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
> index 64bc730f47..75101b8f61 100644
> --- a/hw/s390x/sclpcpi.c
> +++ b/hw/s390x/sclpcpi.c
> @@ -20,8 +20,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 "hw/s390x/s390-virtio-ccw.h"
>
> typedef struct Data {
> uint8_t id_format;
> @@ -60,6 +63,13 @@ static sccb_mask_t receive_mask(void)
> static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
> {
> CPI *cpi = container_of(evt_buf_hdr, CPI, 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 = cpi->data.system_level;
cpi is overlayed with the sccb copied from the guest, correct?
So you need a endianess conversion here.
> + ms->cpi.timestamp = qemu_clock_get_ns(QEMU_CLOCK_HOST);
>
> cpi->ebh.flags = SCLP_EVENT_BUFFER_ACCEPTED;
> return SCLP_RC_NORMAL_COMPLETION;
> diff --git a/include/hw/s390x/s390-virtio-ccw.h
> b/include/hw/s390x/s390-virtio-ccw.h
> index 686d9497d2..c4212ff857 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -19,6 +19,13 @@
>
> OBJECT_DECLARE_TYPE(S390CcwMachineState, S390CcwMachineClass,
> S390_CCW_MACHINE)
>
> +typedef struct Cpi {
> + uint8_t system_type[8];
> + uint8_t system_name[8];
> + uint64_t system_level;
> + uint8_t sysplex_name[8];
> + uint64_t timestamp;
> +} QEMU_PACKED Cpi;
I don't like there being a CPI and a Cpi struct. I'd just go ahead and inline
the
definition into S390CcwMachineState and not name this type.
>
> struct S390CcwMachineState {
> /*< private >*/
> @@ -33,6 +40,7 @@ struct S390CcwMachineState {
> uint64_t max_pagesize;
>
> SCLPDevice *sclp;
> + Cpi cpi;
> };
>
> static inline uint64_t s390_get_memory_limit(S390CcwMachineState *s390ms)
> diff --git a/qapi/machine.json b/qapi/machine.json
> index a6b8795b09..9dcd2790eb 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1898,3 +1898,27 @@
> { 'command': 'x-query-interrupt-controllers',
> 'returns': 'HumanReadableText',
> 'features': [ 'unstable' ]}
> +
> +##
> +# @S390Cpi:
Maybe spelling this out would be preferable?
S390ControlProgramId?
> +#
> +# Control-program identifiers provide data about Linux instance.
> +#
> +# @system-type: operating system of Linux instance
> +#
> +# @system-name: system name of Linux instance
> +#
> +# @system-level: distribution and kernel version of Linux instance
> +#
> +# @sysplex-name: sysplex name of Linux instance
> +#
> +# @timestamp: latest update of CPI data
> +#
> +# Since: 9.2
> +##
> +{ 'struct': 'S390Cpi', 'data': {
> + 'system-type': 'str',
> + 'system-name': 'str',
> + 'system-level': 'str',
> + 'sysplex-name': 'str',
> + 'timestamp': 'uint64' } }
