Re: [PATCH v4 1/4] hw/s390x: add SCLP event type CPI

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

On Mon, 2025-05-05 at 08:55 +0200, Shalini Chellathurai Saroja wrote:
> On 2025-04-28 11:22, Janis Schoetterl-Glausch wrote:
> > On Thu, 2025-04-10 at 17:09 +0200, Shalini Chellathurai Saroja wrote:
> > > Implement the Service-Call Logical Processor (SCLP) event
> > > type Control-Program Identification (CPI) in QEMU. This
> > > event is used to send CPI identifiers from the guest to the
> > > host. The CPI identifiers are: system type, system name,
> > > system level and sysplex name.
> > > 
> > > System type: operating system of the guest (e.g. "LINUX").
> > > System name: user configurable name of the guest (e.g. "TESTVM").
> > > System level: distribution and kernel version, if the system type is 
> > > Linux
> > > (e.g. 0x50e00).
> > > Sysplex name: name of the cluster which the guest belongs to (if any)
> > > (e.g. "PLEX").
> > > 
> > > Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> > > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >  hw/s390x/event-facility.c         |  2 +
> > >  hw/s390x/meson.build              |  1 +
> > >  hw/s390x/s390-virtio-ccw.c        | 14 +++++
> > >  hw/s390x/sclpcpi.c                | 92 
> > > +++++++++++++++++++++++++++++++
> > >  include/hw/s390x/event-facility.h | 13 +++++
> > >  5 files changed, 122 insertions(+)
> > >  create mode 100644 hw/s390x/sclpcpi.c
> > > 
> > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > > index 2b0332c20e..60237b8581 100644
> > > --- a/hw/s390x/event-facility.c
> > > +++ b/hw/s390x/event-facility.c

[...]

> It is possible to add the identifiers directly as the properties of 
> sclpcpi (eg. system type as shown below) and remove the 
> control-program-id.

This is what I meant, drop it from qom.
> 
> virsh # qemu-monitor-command vm --pretty 
> '{"execute":"qom-list","arguments": {"path": 
> "/machine/sclp/s390-sclp-event-facility/sclpcpi"}}'
> {
>    "return": [
>      {
>        "name": "type",
>        "type": "string"
>      },
>      {
>        "name": "parent_bus",
>        "type": "link<bus>"
>      },
>      {
>        "name": "realized",
>        "type": "bool"
>      },
>      {
>        "name": "hotplugged",
>        "type": "bool"
>      },
>      {
>        "name": "hotpluggable",
>        "type": "bool"
>      },
>      {
>        "name": "system_type",
>        "type": "string"
>      },
>      {
>        "name": "control-program-id",
>        "type": "S390ControlProgramId"
>      }
>    ],
>    "id": "libvirt-16"
> }
> 
> virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get", 
> "arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi", 
> "property":"system_type"}}'
> {
>    "return": "LINUX   ",
>    "id": "libvirt-17"
> }
> 
> However, Hendrik Brückner suggested to group the identifiers together 
> during the initial discussion of this line item. So, I would prefer to 

They are grouped together under "sclpcpi", no?

> leave this as it is. Please let me know if you still think otherwise.

I don't have a strong opinion on this, just wanted to mention it and see what other people say.
> 
> > > +}
> > 
> > [...]
> > 
> > > diff --git a/include/hw/s390x/event-facility.h 
> > > b/include/hw/s390x/event-facility.h
> > > index ff874e792d..ef469e62ae 100644
> > > --- a/include/hw/s390x/event-facility.h
> > > +++ b/include/hw/s390x/event-facility.h
> > 
> > [...]
> >    I.
> > > +struct SCLPEventCPI {
> > > +    DeviceState qdev;
> > > +    SCLPEvent event;
> > 
> > I think you should just inherit from SCLPEvent, which in turn inherits
> > from DeviceState.
> > So without the qdev.
> > 
> 
> Ok, I will do so. Thank you very much for the review.
> 
> > > +};
> > > +
> > >  #define TYPE_SCLP_EVENT_FACILITY "s390-sclp-event-facility"
> > >  typedef struct SCLPEventFacility SCLPEventFacility;
> > >  typedef struct SCLPEventFacilityClass SCLPEventFacilityClass;
> 

-- 
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