diff mbox series

[v1,2/3] hw/s390x: add CPI values to QOM

Message ID 20250115133106.3034445-2-shalini@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series [v1,1/3] hw/s390x: add SCLP event type CPI | expand

Commit Message

Shalini Chellathurai Saroja Jan. 15, 2025, 1:31 p.m. UTC 
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 <shalini@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(+)

Comments

Nina Schoetterl-Glausch Jan. 24, 2025, 5:49 p.m. UTC | #1 
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 <shalini@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' } }
shalini Feb. 24, 2025, 12:35 p.m. UTC | #2 
On 2025-01-24 18:49, Nina Schoetterl-Glausch wrote:
> 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 <shalini@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.
> 

Hello Nina,

As discussed offline, when interpreted as a number the output was

'system-level': 74872343805430528

however, the desired output is

'system-level': '0x10a000000060b00'

So, this code is not changed in v2. Thank you.

>> +        .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);
>> +}
>> +
diff mbox series

Patch

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),
+        .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;
+    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;
 
 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:
+#
+# 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' } }