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

[Hendrik Brueckner <brueckner@linux.ibm.com>]

On Tue, May 06, 2025 at 02:16:18PM +0200, Shalini Chellathurai Saroja wrote:
> On 2025-05-06 09:48, Thomas Huth wrote:
> > On 06/05/2025 08.48, Nina Schoetterl-Glausch wrote:
> > > 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.
> 
> Ok.
> 
> > > > 
> > > > 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?
> 
> Yes.
> 
> > > 
> > > > 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.
> > 
> > I guess it mostly depends on whether there are future plans for using
> > the QOM type elsewhere. If this type is supposed to be used in other
> > QAPI calls in the future, it makes sense to keep it separate now. If
> > there are no such plans, it's maybe easier to integrate the values
> > into sclpcpi directly. Hendrik, any opinion from your side?
> > 
> >  Thomas
> 
> Hello Hendrik,
> 
> I have provided the output of both the options below. Please let me know
> which option do you prefer.
> 
> I have incorporated the other comments. So I will be able to send V5 after
> your response. Thank you very much!
> 
> Option 1: Control-program identifiers set as the properties of sclpcpi:
> 
> virsh # qemu-monitor-command vm --pretty '{"execute":"qom-list","arguments":
> {"path": "/machine/sclp/s390-sclp-event-facility/sclpcpi"}}'
> {
>   "return": [
>     {
> [...]
>     },
>     {
>       "name": "system_level",
>       "type": "uint64"
>     },
>     {
>       "name": "system_name",
>       "type": "string"
>     },
>     {
>       "name": "system_type",
>       "type": "string"
>     },
>     {
>       "name": "timestamp",
>       "type": "uint64"
>     },
>     {
>       "name": "sysplex_name",
>       "type": "string"
>     }
>   ],
>   "id": "libvirt-14"
> }

I would vote for Option 1 as this seems to be a bit more natural from
an object model.  There is not much impact for the upcoming changes
to consume this information.