Re: [PATCH v3 4/4] hw/s390x: compat handling for backward migration

[Shalini Chellathurai Saroja <shalini@linux.ibm.com>]

On 2025-04-02 09:52, Thomas Huth wrote:
> On 31/03/2025 16.00, Shalini Chellathurai Saroja wrote:
>> Add Control-Program Identification (CPI) device to QOM only when the 
>> virtual
>> machine supports CPI. CPI is supported from "s390-ccw-virtio-10.0" 
>> machine
>> and higher.
>> 
>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> ---
>>   hw/s390x/event-facility.c  | 27 ++++++++++++++++++++++-----
>>   hw/s390x/s390-virtio-ccw.c |  1 +
>>   2 files changed, 23 insertions(+), 5 deletions(-)
>> 
>> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
>> index c0fb6e098c..cb23bbc54b 100644
>> --- a/hw/s390x/event-facility.c
>> +++ b/hw/s390x/event-facility.c
>> @@ -22,6 +22,7 @@
>>   #include "hw/s390x/sclp.h"
>>   #include "migration/vmstate.h"
>>   #include "hw/s390x/event-facility.h"
>> +#include "hw/qdev-properties.h"
>>     typedef struct SCLPEventsBus {
>>       BusState qbus;
>> @@ -54,6 +55,7 @@ struct SCLPEventFacility {
>>       bool allow_all_mask_sizes;
>>       /* length of the receive mask */
>>       uint16_t mask_length;
>> +    bool use_cpi;
>>   };
>>     /* return true if any child has event pending set */
>> @@ -455,11 +457,20 @@ static void realize_event_facility(DeviceState 
>> *dev, Error **errp)
>>           qdev_unrealize(DEVICE(&event_facility->quiesce));
>>           return;
>>       }
>> -    if (!qdev_realize(DEVICE(&event_facility->cpi),
>> -                      BUS(&event_facility->sbus), errp)) {
>> -        qdev_unrealize(DEVICE(&event_facility->quiesce));
>> -        qdev_unrealize(DEVICE(&event_facility->cpu_hotplug));
>> -        return;
>> +    /*
>> +     * Add sclpcpi device to QOM only when the virtual machine 
>> supports
>> +     * Control-Program Identification. It is supported by 
>> "s390-ccw-virtio-10.0"
>> +     * machine and higher.
>> +     */
>> +    if (!event_facility->use_cpi) {
>> +        object_unparent(OBJECT(&event_facility->cpi));
>> +    } else {
>> +        if (!qdev_realize(DEVICE(&event_facility->cpi),
>> +                          BUS(&event_facility->sbus), errp)) {
>> +            qdev_unrealize(DEVICE(&event_facility->quiesce));
>> +            qdev_unrealize(DEVICE(&event_facility->cpu_hotplug));
>> +            return;
>> +        }
> 
> Hmm, first doing object_initialize_child() in init_event_facility()
> and then unparenting it here again in case we are running with an
> older machine type is a little bit ugly. I wonder whether it would be
> nicer to add the QOM object from ccw_init() init instead, similar to
> what we do with the SCLP-console in s390_create_sclpconsole() ? If
> you've got some spare minutes, could you please give it a try whether
> that looks nicer?
> 

Hello Thomas,

Sure. Did you mean like the code below?, if yes, the use_cpi is always 
true when adding the sclpcpi device from ccw_init(). The use_cpi is set 
to false at a later point, when the machine type is 9.2 or older. This 
means the sclpcpi device is always added, the output and the code are 
provided below. Please let me know how to proceed, thank you very much.

virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get", 
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility", 
"property":"use-cpi"}}'
{
   "return": false,
   "id": "libvirt-16"
}
virsh # qemu-monitor-command vm --pretty '{"execute":"qom-get", 
"arguments":{"path":"/machine/sclp/s390-sclp-event-facility/sclpcpi", 
"property":"control-program-id"}}'
{
   "return": {
     "timestamp": 1743681889538425000,
     "system-level": 74872343805430528,
     "sysplex-name": "        ",
     "system-name": "        ",
     "system-type": "LINUX   "
   },
   "id": "libvirt-17"
}

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index cb23bbc54b..15d9f94845 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -76,6 +76,11 @@ static bool event_pending(SCLPEventFacility *ef)
      return false;
  }

+static bool use_cpi(SCLPEventFacility *ef)
+{
+    return ef->use_cpi;
+}
+
  static sccb_mask_t get_host_send_mask(SCLPEventFacility *ef)
  {
      sccb_mask_t mask;
@@ -438,10 +443,6 @@ static void init_event_facility(Object *obj)
      object_initialize_child(obj, TYPE_SCLP_CPU_HOTPLUG,
                              &event_facility->cpu_hotplug,
                              TYPE_SCLP_CPU_HOTPLUG);
-
-    object_initialize_child(obj, TYPE_SCLP_CPI,
-                            &event_facility->cpi,
-                            TYPE_SCLP_CPI);
  }

  static void realize_event_facility(DeviceState *dev, Error **errp)
@@ -457,21 +458,6 @@ static void realize_event_facility(DeviceState 
*dev, Error **errp)
          qdev_unrealize(DEVICE(&event_facility->quiesce));
          return;
      }
-    /*
-     * Add sclpcpi device to QOM only when the virtual machine supports
-     * Control-Program Identification. It is supported by 
"s390-ccw-virtio-10.0"
-     * machine and higher.
-     */
-    if (!event_facility->use_cpi) {
-        object_unparent(OBJECT(&event_facility->cpi));
-    } else {
-        if (!qdev_realize(DEVICE(&event_facility->cpi),
-                          BUS(&event_facility->sbus), errp)) {
-            qdev_unrealize(DEVICE(&event_facility->quiesce));
-            qdev_unrealize(DEVICE(&event_facility->cpu_hotplug));
-            return;
-        }
  }

  static void reset_event_facility(DeviceState *dev)
@@ -499,6 +485,7 @@ static void init_event_facility_class(ObjectClass 
*klass, void *data)
      set_bit(DEVICE_CATEGORY_MISC, dc->categories);
      k->command_handler = command_handler;
      k->event_pending = event_pending;
+    k->use_cpi = use_cpi;
  }

  static const TypeInfo sclp_event_facility_info = {
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index c1001322e0..f077ecaee1 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -260,6 +260,21 @@ static void s390_create_sclpconsole(SCLPDevice 
*sclp,
      qdev_realize_and_unref(dev, ev_fac_bus, &error_fatal);
  }

+static void s390_create_sclpcpi(SCLPDevice *sclp)
+{
+    SCLPEventFacility *ef = sclp->event_facility;
+    SCLPEventFacilityClass *efc = EVENT_FACILITY_GET_CLASS(ef);
+    BusState *ev_fac_bus = sclp_get_event_facility_bus(ef);
+    DeviceState *dev;
+
+    if(efc->use_cpi) {
+        dev = qdev_new(TYPE_SCLP_CPI);
+        object_property_add_child(OBJECT(ef), "sclpcpi", OBJECT(dev));
+        object_unref(OBJECT(dev));
+        qdev_realize_and_unref(dev, ev_fac_bus, &error_fatal);
+    }
+}
+
  static void ccw_init(MachineState *machine)
  {
      MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -323,6 +338,10 @@ static void ccw_init(MachineState *machine)

      /* init the TOD clock */
      s390_init_tod();
+
+    /* init SCLP event Control-Program Identification */
+    s390_create_sclpcpi(ms->sclp);
+
  }

  static void s390_cpu_plug(HotplugHandler *hotplug_dev,
diff --git a/include/hw/s390x/event-facility.h 
b/include/hw/s390x/event-facility.h
index f445d2f9f5..ba20161023 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -204,6 +204,7 @@ struct SCLPEventFacilityClass {
      SysBusDeviceClass parent_class;
      void (*command_handler)(SCLPEventFacility *ef, SCCB *sccb, uint64_t 
code);
      bool (*event_pending)(SCLPEventFacility *ef);
+    bool (*use_cpi)(SCLPEventFacility *ef);
  };

  BusState *sclp_get_event_facility_bus(SCLPEventFacility *ef);


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