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

[Thomas Huth <thuth@redhat.com>]

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

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