qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v3 2/4] hw/s390x: add Control-Program Identification to QOM
@ 2025-04-03 14:33 Shalini Chellathurai Saroja
  2025-04-09  5:30 ` Thomas Huth
  0 siblings, 1 reply; 7+ messages in thread
From: Shalini Chellathurai Saroja @ 2025-04-03 14:33 UTC (permalink / raw)
  To: Nina Schoetterl-Glausch, Thomas Huth
  Cc: qemu-s390x mailing list, Daniel Berrange, qemu-devel mailing list,
	Hendrik Brueckner

On 2025-04-01 15:55, Nina Schoetterl-Glausch wrote:
> On Mon, 2025-03-31 at 16:00 +0200, Shalini Chellathurai Saroja wrote:
>> Add Control-Program Identification data to the QEMU Object
>> Model (QOM), along with the timestamp in which the data was received.
>> 
>> Example:
>> virsh # qemu-monitor-command vm --pretty '{
>> "execute": "qom-get",
>> "arguments": {
>> "path": "/machine/sclp/s390-sclp-event-facility/sclpcpi",
>> "property": "control-program-id" }}'
>> {
>>   "return": {
>>     "timestamp": 1742390410685762000,
>>     "system-level": 74872343805430528,
>>     "sysplex-name": "PLEX ",
>>     "system-name": "TESTVM  ",
>>     "system-type": "LINUX   "
>>   },
>>   "id": "libvirt-15"
>> }
>> 
>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> ---
>>  hw/s390x/sclpcpi.c                | 38 ++++++++++++++++++++
>>  include/hw/s390x/event-facility.h |  9 +++++
>>  qapi/machine.json                 | 58 
>> +++++++++++++++++++++++++++++++
>>  3 files changed, 105 insertions(+)
>> 
>> diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>> index 7ace5dd64e..969c15e43d 100644
>> --- a/hw/s390x/sclpcpi.c
>> +++ b/hw/s390x/sclpcpi.c
>> @@ -57,8 +57,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 "qapi/qapi-visit-machine.h"
>> 
>>  typedef struct Data {
>>      uint8_t id_format;
>> @@ -99,10 +102,37 @@ static int write_event_data(SCLPEvent *event, 
>> EventBufferHeader *evt_buf_hdr)
>>      ControlProgramIdMsg *cpim = container_of(evt_buf_hdr, 
>> ControlProgramIdMsg,
>>                                               ebh);
>> 
>> +    ascii_put(event->cpi.system_type, (char *)cpim->data.system_type, 
>> 8);
>> +    ascii_put(event->cpi.system_name, (char *)cpim->data.system_name, 
>> 8);
>> +    ascii_put(event->cpi.sysplex_name, (char 
>> *)cpim->data.sysplex_name, 8);
>> +    event->cpi.system_level = ldq_be_p(&cpim->data.system_level);
>> +    event->cpi.timestamp = qemu_clock_get_ns(QEMU_CLOCK_HOST);
>> +
>>      cpim->ebh.flags = SCLP_EVENT_BUFFER_ACCEPTED;
>>      return SCLP_RC_NORMAL_COMPLETION;
>>  }
>> 
>> +static void get_control_program_id(Object *obj, Visitor *v,
>> +                                   const char *name, void *opaque,
>> +                                   Error **errp)
>> +{
>> +    SCLPEvent *event = (SCLPEvent *)(obj);
> 
> Do a checked cast with SCLP_EVENT(obj).

Hello Nina,

ok, thank you.
> 
>> +    S390ControlProgramId *cpi;
>> +
>> +    cpi = &(S390ControlProgramId){
>> +        .system_type = g_strndup((char *) event->cpi.system_type,
>> +                                 sizeof(event->cpi.system_type)),
>> +        .system_name = g_strndup((char *) event->cpi.system_name,
>> +                                 sizeof(event->cpi.system_name)),
>> +        .system_level = event->cpi.system_level,
>> +        .sysplex_name = g_strndup((char *) event->cpi.sysplex_name,
>> +                                  sizeof(event->cpi.sysplex_name)),
>> +        .timestamp = event->cpi.timestamp
>> +    };
>> +
>> +    visit_type_S390ControlProgramId(v, name, &cpi, errp);
>> +}
>> +
>>  static void cpi_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -114,6 +144,14 @@ static void cpi_class_init(ObjectClass *klass, 
>> void *data)
>>      k->get_send_mask = send_mask;
>>      k->get_receive_mask = receive_mask;
>>      k->write_event_data = write_event_data;
>> +
>> +    object_class_property_add(klass, "control-program-id",
>> +                              "S390ControlProgramId",
>> +                              get_control_program_id,
>> +                              NULL, NULL, NULL);
>> +    object_class_property_set_description(klass, 
>> "control-program-id",
>> +        "Control-program identifiers provide data about the guest "
>> +        "operating system");
>>  }
>> 
>>  static const TypeInfo sclp_cpi_info = {
>> diff --git a/include/hw/s390x/event-facility.h 
>> b/include/hw/s390x/event-facility.h
>> index f445d2f9f5..39e589ed44 100644
>> --- a/include/hw/s390x/event-facility.h
>> +++ b/include/hw/s390x/event-facility.h
>> @@ -169,10 +169,19 @@ typedef struct ReadEventData {
>>      };
>>  } QEMU_PACKED ReadEventData;
>> 
>> +typedef struct ControlProgramId {
>> +    uint8_t system_type[8];
>> +    uint8_t system_name[8];
>> +    uint64_t system_level;
>> +    uint8_t sysplex_name[8];
>> +    uint64_t timestamp;
>> +} QEMU_PACKED ControlProgramId;
>> +
>>  struct SCLPEvent {
>>      DeviceState qdev;
>>      bool event_pending;
>>      char *name;
>> +    ControlProgramId cpi;
> 
> I don't think this should go into SCLPEvent.
> Rather SCLPEventFacility or SCLPDevice. Otherwise all events,
> so also quiesce and cpu_hotplug have a cpi field.
> 
ok, that is correct.

I gave it a try by moving ControlProgramId to SCLPDevice. With this, the 
migration data is stored in dc->vmsd of TYPE_SCLP as shown below.

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 5945c9b1d8..4d6d5bb857 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -424,6 +424,29 @@ static void sclp_init(Object *obj)
      sclp_memory_init(sclp);
  }

+static const VMStateDescription vmstate_control_program_id = {
+    .name = "s390_control_program_id",
+    .version_id = 0,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT8_ARRAY(system_type, ControlProgramId, 8),
+        VMSTATE_UINT8_ARRAY(system_name, ControlProgramId, 8),
+        VMSTATE_UINT64(system_level, ControlProgramId),
+        VMSTATE_UINT8_ARRAY(sysplex_name, ControlProgramId, 8),
+        VMSTATE_UINT64(timestamp, ControlProgramId),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_sclpcpi = {
+    .name = "s390_sclpcpi",
+    .version_id = 0,
+    .fields = (const VMStateField[]) {
+        VMSTATE_STRUCT(cpi, SCLPDevice, 0, vmstate_control_program_id,
+                       ControlProgramId),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
  static void sclp_class_init(ObjectClass *oc, void *data)
  {
      SCLPDeviceClass *sc = SCLP_CLASS(oc);
@@ -438,6 +461,7 @@ static void sclp_class_init(ObjectClass *oc, void 
*data)
       * which is a non-pluggable sysbus device
       */
      dc->user_creatable = false;
+    dc->vmsd =  &vmstate_sclpcpi;

      sc->read_SCP_info = read_SCP_info;
      sc->read_cpu_info = sclp_read_cpu_info;

@Thomas Huth: Is this ok?, thank you.


>>  };
> 
> [...]

-- 
Mit freundlichen Grüßen / Kind regards
Shalini Chellathurai Saroja
Software Developer
Linux on IBM Z & KVM Development
IBM Deutschland Research & Development GmbH
Dept 1419, Schoenaicher Str. 220, 71032 Boeblingen
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht 
Stuttgart, HRB 243294


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/4] hw/s390x: add Control-Program Identification to QOM
@ 2025-04-02  9:06 Shalini Chellathurai Saroja
  0 siblings, 0 replies; 7+ messages in thread
From: Shalini Chellathurai Saroja @ 2025-04-02  9:06 UTC (permalink / raw)
  To: Thomas Huth
  Cc: qemu-s390x mailing list, Daniel Berrange, qemu-devel mailing list,
	Nina Schoetterl-Glausch, Hendrik Brueckner

On 2025-04-02 07:58, Thomas Huth wrote:
> On 31/03/2025 16.00, Shalini Chellathurai Saroja wrote:
>> Add Control-Program Identification data to the QEMU Object
>> Model (QOM), along with the timestamp in which the data was received.
>> 
>> Example:
>> virsh # qemu-monitor-command vm --pretty '{
>> "execute": "qom-get",
>> "arguments": {
>> "path": "/machine/sclp/s390-sclp-event-facility/sclpcpi",
>> "property": "control-program-id" }}'
>> {
>>    "return": {
>>      "timestamp": 1742390410685762000,
>>      "system-level": 74872343805430528,
>>      "sysplex-name": "PLEX ",
>>      "system-name": "TESTVM  ",
>>      "system-type": "LINUX   "
>>    },
>>    "id": "libvirt-15"
>> }
>> 
>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> ---
>>   hw/s390x/sclpcpi.c                | 38 ++++++++++++++++++++
>>   include/hw/s390x/event-facility.h |  9 +++++
>>   qapi/machine.json                 | 58 
>> +++++++++++++++++++++++++++++++
>>   3 files changed, 105 insertions(+)
>> 
>> diff --git a/include/hw/s390x/event-facility.h 
>> b/include/hw/s390x/event-facility.h
>> index f445d2f9f5..39e589ed44 100644
>> --- a/include/hw/s390x/event-facility.h
>> +++ b/include/hw/s390x/event-facility.h
>> @@ -169,10 +169,19 @@ typedef struct ReadEventData {
>>       };
>>   } QEMU_PACKED ReadEventData;
>>   +typedef struct ControlProgramId {
>> +    uint8_t system_type[8];
>> +    uint8_t system_name[8];
>> +    uint64_t system_level;
>> +    uint8_t sysplex_name[8];
>> +    uint64_t timestamp;
>> +} QEMU_PACKED ControlProgramId;
> 
> Do we need the QEMU_PACKED here? The members seem naturally aligned,
> and the struct does not seem to be involved in the communication with
> the guest directly, so if you don't need it, please drop the
> QEMU_PACKED here.
> (without PACKED, the compiler can optimize the code in a better way,
> and this causes less trouble on exotic platforms like SPARC that
> cannot access unaligned memory addresses)

Hello Thomas,

Thank you for the explanation. I will drop the QEMU_PACKED.

> 
>>   struct SCLPEvent {
>>       DeviceState qdev;
>>       bool event_pending;
>>       char *name;
>> +    ControlProgramId cpi;
>>   };
>>     struct SCLPEventClass {
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index a6b8795b09..cd2bcd2d13 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -1898,3 +1898,61 @@
>>   { 'command': 'x-query-interrupt-controllers',
>>     'returns': 'HumanReadableText',
>>     'features': [ 'unstable' ]}
>> +
>> +##
>> +# @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.
> 
> You've got the same information in the comment at the beginning of the
> sclpcpi.c file already ... maybe it would be good to have it in one
> place only to avoid double maintenance in case it needs to be changed
> in the future. I'd suggest to change the comment at the beginning of
> sclpcpi.c to say something like "for a detailed description of the
> contents of the CPI, please see the S390ControlProgramId QOM-type
> description." or something similar?

ok, I will do that.

> 
>  Thomas

-- 
Mit freundlichen Grüßen / Kind regards
Shalini Chellathurai Saroja
Software Developer
Linux on IBM Z & KVM Development
IBM Deutschland Research & Development GmbH
Dept 1419, Schoenaicher Str. 220, 71032 Boeblingen
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht 
Stuttgart, HRB 243294


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH v3 0/4] *** Add SCLP event type CPI ***
@ 2025-03-31 14:00 Shalini Chellathurai Saroja
  2025-03-31 14:00 ` [PATCH v3 2/4] hw/s390x: add Control-Program Identification to QOM Shalini Chellathurai Saroja
  0 siblings, 1 reply; 7+ messages in thread
From: Shalini Chellathurai Saroja @ 2025-03-31 14:00 UTC (permalink / raw)
  To: qemu-s390x mailing list, Thomas Huth, Daniel Berrange
  Cc: qemu-devel mailing list, Nina Schoetterl-Glausch,
	Hendrik Brueckner, Shalini Chellathurai Saroja

Implement the Service-Call Logical Processor (SCLP) event
type Control-Program Identification (CPI) in QEMU.

Changed since v2:
- Add SPDX license tag in the new file hw/s390x/sclpcpi.c
- Store the control-program Identification data in the sclpcpi device
- Update the description of CPI attributes
- Use ldq_be_p() intead of be64_to_cpu() 	
- Return the CPI attribute system-level as an integer in QMP
- Add compat handling for backward migration
- Other minor changes

Shalini Chellathurai Saroja (4):
  hw/s390x: add SCLP event type CPI
  hw/s390x: add Control-Program Identification to QOM
  hw/s390x: support migration of CPI data
  hw/s390x: compat handling for backward migration

 hw/s390x/event-facility.c         |  28 ++++-
 hw/s390x/meson.build              |   1 +
 hw/s390x/s390-virtio-ccw.c        |   1 +
 hw/s390x/sclpcpi.c                | 196 ++++++++++++++++++++++++++++++
 include/hw/s390x/event-facility.h |  12 ++
 qapi/machine.json                 |  58 +++++++++
 6 files changed, 295 insertions(+), 1 deletion(-)
 create mode 100644 hw/s390x/sclpcpi.c

-- 
2.47.0



^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-04-10  9:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-03 14:33 [PATCH v3 2/4] hw/s390x: add Control-Program Identification to QOM Shalini Chellathurai Saroja
2025-04-09  5:30 ` Thomas Huth
2025-04-10  9:30   ` Shalini Chellathurai Saroja
  -- strict thread matches above, loose matches on Subject: below --
2025-04-02  9:06 Shalini Chellathurai Saroja
2025-03-31 14:00 [PATCH v3 0/4] *** Add SCLP event type CPI *** Shalini Chellathurai Saroja
2025-03-31 14:00 ` [PATCH v3 2/4] hw/s390x: add Control-Program Identification to QOM Shalini Chellathurai Saroja
2025-04-01 13:55   ` Nina Schoetterl-Glausch
2025-04-02  5:58   ` Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).