[Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Halil Pasic]

Like for v2 the scope of this patch series is now limited to decoupling
channel subsystem migration from the migration of virtio-ccw proxies.

There wasn't a whole lot of criticism regarding v2, so very little
changed since then. All issues identified in the previous round were
addressed as agreed upon. I hope this series is now good for 2.10.

@Dave You helped me quite a lot with these, thanks a lot! I could not
find any r-b's or similar form you so if you would like to take the
well deserved credit for your work...

v2 --> v3:
* rebased onto current master, since 's390x: vmstatify config migration
  for virtio-ccw' is already there we don't need it here any more
* added ack's and r-b'
* got rid of loads of nits (thanks Connie and Thomas)
* added explanation why certain members are not migrated

v1 --> v2:
* Split out the vmystatify virtio-ccw part, reorganize
* Use WITH_TMP instead of one-shot VMStateInfo's

Halil Pasic (6):
  s390x: add helper get_machine_class
  s390x: add css_migration_enabled to machine class
  s390x/css: add missing css state conditionally
  s390x/css: add ORB to SubchDev
  s390x/css: activate ChannelSubSys migration
  s390x/css: use SubchDev.orb

 hw/intc/s390_flic.c                |  20 ++++++
 hw/s390x/css.c                     | 144 +++++++++++++++++++++++++++++++++----
 hw/s390x/s390-virtio-ccw.c         |  58 +++++++++------
 include/hw/s390x/css.h             |  11 ++-
 include/hw/s390x/s390-virtio-ccw.h |   7 ++
 5 files changed, 199 insertions(+), 41 deletions(-)

-- 
2.11.2

[Qemu-devel] [PATCH v3 1/6] s390x: add helper get_machine_class

[Halil Pasic]

We will need the machine class at machine initialization time, so the
usual way via qdev won't do. Let's cache the machine class and also use
the default values of the base machine for capability discovery.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 41ca6668e2..1eb17ad184 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -252,36 +252,35 @@ static inline void machine_set_dea_key_wrap(Object *obj, bool value,
     ms->dea_key_wrap = value;
 }
 
-bool ri_allowed(void)
-{
-    if (kvm_enabled()) {
-        MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
-        if (object_class_dynamic_cast(OBJECT_CLASS(mc),
-                                      TYPE_S390_CCW_MACHINE)) {
-            S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+static S390CcwMachineClass *current_mc;
 
-            return s390mc->ri_allowed;
-        }
+static S390CcwMachineClass *get_machine_class(void)
+{
+    if (unlikely(!current_mc)) {
         /*
-         * Make sure the "none" machine can have ri, otherwise it won't * be
-         * unlocked in KVM and therefore the host CPU model might be wrong.
-         */
-        return true;
+        * No s390 ccw machine was instantiated, we are likely to
+        * be called for the 'none' machine. The properties will
+        * have their after-initialization values.
+        */
+        current_mc = S390_MACHINE_CLASS(
+                     object_class_by_name(TYPE_S390_CCW_MACHINE));
     }
-    return 0;
+    return current_mc;
 }
 
-bool cpu_model_allowed(void)
+bool ri_allowed(void)
 {
-    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
-    if (object_class_dynamic_cast(OBJECT_CLASS(mc),
-                                  TYPE_S390_CCW_MACHINE)) {
-        S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
-
-        return s390mc->cpu_model_allowed;
+    if (!kvm_enabled()) {
+        return false;
     }
-    /* allow CPU model qmp queries with the "none" machine */
-    return true;
+    /* for "none" machine this results in true */
+    return get_machine_class()->ri_allowed;
+}
+
+bool cpu_model_allowed(void)
+{
+    /* for "none" machine this results in true */
+    return get_machine_class()->cpu_model_allowed;
 }
 
 static char *machine_get_loadparm(Object *obj, Error **errp)
@@ -391,6 +390,7 @@ static const TypeInfo ccw_machine_info = {
     static void ccw_machine_##suffix##_instance_init(Object *obj)             \
     {                                                                         \
         MachineState *machine = MACHINE(obj);                                 \
+        current_mc = S390_MACHINE_CLASS(MACHINE_GET_CLASS(machine));          \
         ccw_machine_##suffix##_instance_options(machine);                     \
     }                                                                         \
     static const TypeInfo ccw_machine_##suffix##_info = {                     \
-- 
2.11.2

[Qemu-devel] [PATCH v3 2/6] s390x: add css_migration_enabled to machine class

[Halil Pasic]

Currently the migration of the channel subsystem (css) is only partial
and is done by the virtio ccw proxies -- the only migratable css devices
existing at the moment.

With the current work on emulated and passthrough devices we need to
decouple the migration of the channel subsystem state from virtio ccw,
and have a separate section for it. A new section  however necessarily
breaks the migration compatibility.

So let us introduce a switch at the machine class, and put it in 'off'
state for now. We will turn the switch 'on' for future machines once all
preparations are met. For compatibility  machines the switch will stay
'off'.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c         | 13 +++++++++++++
 include/hw/s390x/s390-virtio-ccw.h |  7 +++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1eb17ad184..751febb87a 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -206,6 +206,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
 
     s390mc->ri_allowed = true;
     s390mc->cpu_model_allowed = true;
+    s390mc->css_migration_enabled = false; /* TODO: set to true */
     mc->init = ccw_init;
     mc->reset = s390_machine_reset;
     mc->hot_add_cpu = s390_hot_add_cpu;
@@ -375,6 +376,11 @@ static const TypeInfo ccw_machine_info = {
     },
 };
 
+bool css_migration_enabled(void)
+{
+    return get_machine_class()->css_migration_enabled;
+}
+
 #define DEFINE_CCW_MACHINE(suffix, verstr, latest)                            \
     static void ccw_machine_##suffix##_class_init(ObjectClass *oc,            \
                                                   void *data)                 \
@@ -476,6 +482,10 @@ static const TypeInfo ccw_machine_info = {
 
 static void ccw_machine_2_10_instance_options(MachineState *machine)
 {
+    /*
+     * TODO Once preparations are done register vmstate for the css if
+     * css_migration_enabled().
+     */
 }
 
 static void ccw_machine_2_10_class_options(MachineClass *mc)
@@ -490,8 +500,11 @@ static void ccw_machine_2_9_instance_options(MachineState *machine)
 
 static void ccw_machine_2_9_class_options(MachineClass *mc)
 {
+    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
     ccw_machine_2_10_class_options(mc);
     SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
+    s390mc->css_migration_enabled = false;
 }
 DEFINE_CCW_MACHINE(2_9, "2.9", false);
 
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 3027555f6d..ab88d49d10 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -39,6 +39,7 @@ typedef struct S390CcwMachineClass {
     /*< public >*/
     bool ri_allowed;
     bool cpu_model_allowed;
+    bool css_migration_enabled;
 } S390CcwMachineClass;
 
 /* runtime-instrumentation allowed by the machine */
@@ -46,4 +47,10 @@ bool ri_allowed(void);
 /* cpu model allowed by the machine */
 bool cpu_model_allowed(void);
 
+/**
+ * Returns true if (vmstate based) migration of the channel subsystem
+ * is enabled, false if it is disabled.
+ */
+bool css_migration_enabled(void);
+
 #endif
-- 
2.11.2

[Qemu-devel] [PATCH v3 3/6] s390x/css: add missing css state conditionally

[Halil Pasic]

Although we have recently vmstatified the migration of some css
infrastructure,  for some css entities there is still state to be
migrated left, because the focus was keeping migration stream
compatibility (that is basically everything as-is).

Let us add vmstate helpers and extend existing vmstate descriptions so
that we have everything we need. Let us guard the added state via
css_migration_enabled, so we keep the compatible behavior if css
migration is disabled.

Let's also annotate the bits which do not need to be migrated for better
readability.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 hw/intc/s390_flic.c | 20 +++++++++++++++
 hw/s390x/css.c      | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)

diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index 837158bdaf..1678039d6b 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -138,6 +138,22 @@ static void qemu_s390_flic_register_types(void)
 
 type_init(qemu_s390_flic_register_types)
 
+static bool adapter_info_so_needed(void *opaque)
+{
+    return css_migration_enabled();
+}
+
+const VMStateDescription vmstate_adapter_info_so = {
+    .name = "s390_adapter_info/summary_offset",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = adapter_info_so_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(summary_offset, AdapterInfo),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_adapter_info = {
     .name = "s390_adapter_info",
     .version_id = 1,
@@ -151,6 +167,10 @@ const VMStateDescription vmstate_adapter_info = {
          */
         VMSTATE_END_OF_LIST()
     },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_adapter_info_so,
+        NULL
+    }
 };
 
 const VMStateDescription vmstate_adapter_routes = {
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index d67fffae30..4747589d90 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -29,12 +29,45 @@ typedef struct CrwContainer {
     QTAILQ_ENTRY(CrwContainer) sibling;
 } CrwContainer;
 
+static const VMStateDescription vmstate_crw = {
+    .name = "s390_crw",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16(flags, CRW),
+        VMSTATE_UINT16(rsid, CRW),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static const VMStateDescription vmstate_crw_container = {
+    .name = "s390_crw_container",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT(crw, CrwContainer, 0, vmstate_crw, CRW),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 typedef struct ChpInfo {
     uint8_t in_use;
     uint8_t type;
     uint8_t is_virtual;
 } ChpInfo;
 
+static const VMStateDescription vmstate_chp_info = {
+    .name = "s390_chp_info",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(in_use, ChpInfo),
+        VMSTATE_UINT8(type, ChpInfo),
+        VMSTATE_UINT8(is_virtual, ChpInfo),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 typedef struct SubchSet {
     SubchDev *sch[MAX_SCHID + 1];
     unsigned long schids_used[BITS_TO_LONGS(MAX_SCHID + 1)];
@@ -221,6 +254,19 @@ typedef struct CssImage {
     ChpInfo chpids[MAX_CHPID + 1];
 } CssImage;
 
+static const VMStateDescription vmstate_css_img = {
+    .name = "s390_css_img",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        /* Subchannel sets have no relevant state. */
+        VMSTATE_STRUCT_ARRAY(chpids, CssImage, MAX_CHPID + 1, 0,
+                             vmstate_chp_info, ChpInfo),
+        VMSTATE_END_OF_LIST()
+    }
+
+};
+
 typedef struct IoAdapter {
     uint32_t id;
     uint8_t type;
@@ -238,10 +284,34 @@ typedef struct ChannelSubSys {
     uint64_t chnmon_area;
     CssImage *css[MAX_CSSID + 1];
     uint8_t default_cssid;
+    /* don't migrate, see css_register_io_adapters */
     IoAdapter *io_adapters[CSS_IO_ADAPTER_TYPE_NUMS][MAX_ISC + 1];
+    /* don't migrate, see get_indicator and IndAddrPtrTmp */
     QTAILQ_HEAD(, IndAddr) indicator_addresses;
 } ChannelSubSys;
 
+static const VMStateDescription vmstate_css = {
+    .name = "s390_css",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_QTAILQ_V(pending_crws, ChannelSubSys, 1, vmstate_crw_container,
+                         CrwContainer, sibling),
+        VMSTATE_BOOL(sei_pending, ChannelSubSys),
+        VMSTATE_BOOL(do_crw_mchk, ChannelSubSys),
+        VMSTATE_BOOL(crws_lost, ChannelSubSys),
+        /* These were kind of migrated by virtio */
+        VMSTATE_UINT8(max_cssid, ChannelSubSys),
+        VMSTATE_UINT8(max_ssid, ChannelSubSys),
+        VMSTATE_BOOL(chnmon_active, ChannelSubSys),
+        VMSTATE_UINT64(chnmon_area, ChannelSubSys),
+        VMSTATE_ARRAY_OF_POINTER_TO_STRUCT(css, ChannelSubSys, MAX_CSSID + 1,
+                0, vmstate_css_img, CssImage),
+        VMSTATE_UINT8(default_cssid, ChannelSubSys),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static ChannelSubSys channel_subsys = {
     .pending_crws = QTAILQ_HEAD_INITIALIZER(channel_subsys.pending_crws),
     .do_crw_mchk = true,
@@ -281,6 +351,10 @@ static int subch_dev_post_load(void *opaque, int version_id)
         css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s);
     }
 
+    if (css_migration_enabled()) {
+        /* No compat voodoo to do ;) */
+        return 0;
+    }
     /*
      * Hack alert. If we don't migrate the channel subsystem status
      * we still need to find out if the guest enabled mss/mcss-e.
-- 
2.11.2

[Qemu-devel] [PATCH v3 4/6] s390x/css: add ORB to SubchDev

[Halil Pasic]

Since we are going to need a migration compatibility breaking change to
activate ChannelSubSys migration let us use the opportunity to introduce
ORB to the SubchDev before that (otherwise we would need separate
handling e.g. a compat property).

The ORB will be useful for implementing IDA, or async handling of
subchannel work.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Guenther Hutzl <hutzl@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css.c         | 35 +++++++++++++++++++++++++++++++++++
 include/hw/s390x/css.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 4747589d90..20b5fbed68 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -165,6 +165,36 @@ static const VMStateDescription vmstate_sense_id = {
     }
 };
 
+static const VMStateDescription vmstate_orb = {
+    .name = "s390_orb",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(intparm, ORB),
+        VMSTATE_UINT16(ctrl0, ORB),
+        VMSTATE_UINT8(lpm, ORB),
+        VMSTATE_UINT8(ctrl1, ORB),
+        VMSTATE_UINT32(cpa, ORB),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static bool vmstate_schdev_orb_needed(void *opaque)
+{
+    return css_migration_enabled();
+}
+
+static const VMStateDescription vmstate_schdev_orb = {
+    .name = "s390_subch_dev/orb",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = vmstate_schdev_orb_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT(orb, SubchDev, 1, vmstate_orb, ORB),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int subch_dev_post_load(void *opaque, int version_id);
 static void subch_dev_pre_save(void *opaque);
 
@@ -193,6 +223,10 @@ const VMStateDescription vmstate_subch_dev = {
         VMSTATE_BOOL(ccw_fmt_1, SubchDev),
         VMSTATE_UINT8(ccw_no_data_cnt, SubchDev),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * []) {
+        &vmstate_schdev_orb,
+        NULL
     }
 };
 
@@ -1342,6 +1376,7 @@ int css_do_ssch(SubchDev *sch, ORB *orb)
     if (channel_subsys.chnmon_active) {
         css_update_chnmon(sch);
     }
+    sch->orb = *orb;
     sch->channel_prog = orb->cpa;
     /* Trigger the start function. */
     s->ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND);
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index eb0e26f258..5d302223e6 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -89,6 +89,7 @@ struct SubchDev {
     bool thinint_active;
     uint8_t ccw_no_data_cnt;
     uint16_t migrated_schid; /* used for missmatch detection */
+    ORB orb;
     /* transport-provided data: */
     int (*ccw_cb) (SubchDev *, CCW1);
     void (*disable_cb)(SubchDev *);
-- 
2.11.2

[Qemu-devel] [PATCH v3 5/6] s390x/css: activate ChannelSubSys migration

[Halil Pasic]

Turn on migration for the channel subsystem for the next machine.  For
legacy machines we still have to do things the old way.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css.c             | 5 +++++
 hw/s390x/s390-virtio-ccw.c | 9 ++++-----
 include/hw/s390x/css.h     | 4 ++++
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 20b5fbed68..b89b60751a 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -407,6 +407,11 @@ static int subch_dev_post_load(void *opaque, int version_id)
     return 0;
 }
 
+void css_register_vmstate(void)
+{
+    vmstate_register(NULL, 0, &vmstate_css, &channel_subsys);
+}
+
 IndAddr *get_indicator(hwaddr ind_addr, int len)
 {
     IndAddr *indicator;
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 751febb87a..20e3f06519 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -206,7 +206,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
 
     s390mc->ri_allowed = true;
     s390mc->cpu_model_allowed = true;
-    s390mc->css_migration_enabled = false; /* TODO: set to true */
+    s390mc->css_migration_enabled = true;
     mc->init = ccw_init;
     mc->reset = s390_machine_reset;
     mc->hot_add_cpu = s390_hot_add_cpu;
@@ -482,10 +482,9 @@ bool css_migration_enabled(void)
 
 static void ccw_machine_2_10_instance_options(MachineState *machine)
 {
-    /*
-     * TODO Once preparations are done register vmstate for the css if
-     * css_migration_enabled().
-     */
+    if (css_migration_enabled()) {
+        css_register_vmstate();
+    }
 }
 
 static void ccw_machine_2_10_class_options(MachineClass *mc)
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index 5d302223e6..949e0f0a28 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -226,4 +226,8 @@ extern PropertyInfo css_devid_ro_propinfo;
  */
 SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
                          Error **errp);
+
+/** Turn on css migration */
+void css_register_vmstate(void);
+
 #endif
-- 
2.11.2

[Qemu-devel] [PATCH v3 6/6] s390x/css: use SubchDev.orb

[Halil Pasic]

Instead of passing around a pointer to ORB let us simplify some
function signatures by using the previously introduced ORB saved at the
subchannel (SubchDev).

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css.c         | 30 +++++++++++++++---------------
 include/hw/s390x/css.h |  6 +++---
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index b89b60751a..abc77f7d4c 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -865,7 +865,7 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr,
     return ret;
 }
 
-static void sch_handle_start_func_virtual(SubchDev *sch, ORB *orb)
+static void sch_handle_start_func_virtual(SubchDev *sch)
 {
 
     PMCW *p = &sch->curr_status.pmcw;
@@ -879,10 +879,10 @@ static void sch_handle_start_func_virtual(SubchDev *sch, ORB *orb)
 
     if (!(s->ctrl & SCSW_ACTL_SUSP)) {
         /* Start Function triggered via ssch, i.e. we have an ORB */
+        ORB *orb = &sch->orb;
         s->cstat = 0;
         s->dstat = 0;
         /* Look at the orb and try to execute the channel program. */
-        assert(orb != NULL); /* resume does not pass an orb */
         p->intparm = orb->intparm;
         if (!(orb->lpm & path)) {
             /* Generate a deferred cc 3 condition. */
@@ -896,8 +896,7 @@ static void sch_handle_start_func_virtual(SubchDev *sch, ORB *orb)
         sch->ccw_no_data_cnt = 0;
         suspend_allowed = !!(orb->ctrl0 & ORB_CTRL0_MASK_SPND);
     } else {
-        /* Start Function resumed via rsch, i.e. we don't have an
-         * ORB */
+        /* Start Function resumed via rsch */
         s->ctrl &= ~(SCSW_ACTL_SUSP | SCSW_ACTL_RESUME_PEND);
         /* The channel program had been suspended before. */
         suspend_allowed = true;
@@ -967,13 +966,14 @@ static void sch_handle_start_func_virtual(SubchDev *sch, ORB *orb)
 
 }
 
-static int sch_handle_start_func_passthrough(SubchDev *sch, ORB *orb)
+static int sch_handle_start_func_passthrough(SubchDev *sch)
 {
 
     PMCW *p = &sch->curr_status.pmcw;
     SCSW *s = &sch->curr_status.scsw;
     int ret;
 
+    ORB *orb = &sch->orb;
     if (!(s->ctrl & SCSW_ACTL_SUSP)) {
         assert(orb != NULL);
         p->intparm = orb->intparm;
@@ -1018,7 +1018,7 @@ static int sch_handle_start_func_passthrough(SubchDev *sch, ORB *orb)
  * read/writes) asynchronous later on if we start supporting more than
  * our current very simple devices.
  */
-int do_subchannel_work_virtual(SubchDev *sch, ORB *orb)
+int do_subchannel_work_virtual(SubchDev *sch)
 {
 
     SCSW *s = &sch->curr_status.scsw;
@@ -1029,7 +1029,7 @@ int do_subchannel_work_virtual(SubchDev *sch, ORB *orb)
         sch_handle_halt_func(sch);
     } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
         /* Triggered by both ssch and rsch. */
-        sch_handle_start_func_virtual(sch, orb);
+        sch_handle_start_func_virtual(sch);
     } else {
         /* Cannot happen. */
         return 0;
@@ -1038,7 +1038,7 @@ int do_subchannel_work_virtual(SubchDev *sch, ORB *orb)
     return 0;
 }
 
-int do_subchannel_work_passthrough(SubchDev *sch, ORB *orb)
+int do_subchannel_work_passthrough(SubchDev *sch)
 {
     int ret;
     SCSW *s = &sch->curr_status.scsw;
@@ -1052,7 +1052,7 @@ int do_subchannel_work_passthrough(SubchDev *sch, ORB *orb)
         sch_handle_halt_func(sch);
         ret = 0;
     } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
-        ret = sch_handle_start_func_passthrough(sch, orb);
+        ret = sch_handle_start_func_passthrough(sch);
     } else {
         /* Cannot happen. */
         return -ENODEV;
@@ -1061,10 +1061,10 @@ int do_subchannel_work_passthrough(SubchDev *sch, ORB *orb)
     return ret;
 }
 
-static int do_subchannel_work(SubchDev *sch, ORB *orb)
+static int do_subchannel_work(SubchDev *sch)
 {
     if (sch->do_subchannel_work) {
-        return sch->do_subchannel_work(sch, orb);
+        return sch->do_subchannel_work(sch);
     } else {
         return -EINVAL;
     }
@@ -1271,7 +1271,7 @@ int css_do_csch(SubchDev *sch)
     s->ctrl &= ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL);
     s->ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_ACTL_CLEAR_PEND;
 
-    do_subchannel_work(sch, NULL);
+    do_subchannel_work(sch);
     ret = 0;
 
 out:
@@ -1312,7 +1312,7 @@ int css_do_hsch(SubchDev *sch)
     }
     s->ctrl |= SCSW_ACTL_HALT_PEND;
 
-    do_subchannel_work(sch, NULL);
+    do_subchannel_work(sch);
     ret = 0;
 
 out:
@@ -1387,7 +1387,7 @@ int css_do_ssch(SubchDev *sch, ORB *orb)
     s->ctrl |= (SCSW_FCTL_START_FUNC | SCSW_ACTL_START_PEND);
     s->flags &= ~SCSW_FLAGS_MASK_PNO;
 
-    ret = do_subchannel_work(sch, orb);
+    ret = do_subchannel_work(sch);
 
 out:
     return ret;
@@ -1666,7 +1666,7 @@ int css_do_rsch(SubchDev *sch)
     }
 
     s->ctrl |= SCSW_ACTL_RESUME_PEND;
-    do_subchannel_work(sch, NULL);
+    do_subchannel_work(sch);
     ret = 0;
 
 out:
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index 949e0f0a28..165ca2f4e9 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -93,7 +93,7 @@ struct SubchDev {
     /* transport-provided data: */
     int (*ccw_cb) (SubchDev *, CCW1);
     void (*disable_cb)(SubchDev *);
-    int (*do_subchannel_work) (SubchDev *, ORB *);
+    int (*do_subchannel_work) (SubchDev *);
     SenseId id;
     void *driver_data;
 };
@@ -157,8 +157,8 @@ void css_generate_css_crws(uint8_t cssid);
 void css_clear_sei_pending(void);
 void css_adapter_interrupt(uint8_t isc);
 int s390_ccw_cmd_request(ORB *orb, SCSW *scsw, void *data);
-int do_subchannel_work_virtual(SubchDev *sub, ORB *orb);
-int do_subchannel_work_passthrough(SubchDev *sub, ORB *orb);
+int do_subchannel_work_virtual(SubchDev *sub);
+int do_subchannel_work_passthrough(SubchDev *sub);
 
 typedef enum {
     CSS_IO_ADAPTER_VIRTIO = 0,
-- 
2.11.2

Re: [Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Cornelia Huck]

On Tue, 11 Jul 2017 16:54:35 +0200
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> Like for v2 the scope of this patch series is now limited to decoupling
> channel subsystem migration from the migration of virtio-ccw proxies.
> 
> There wasn't a whole lot of criticism regarding v2, so very little
> changed since then. All issues identified in the previous round were
> addressed as agreed upon. I hope this series is now good for 2.10.

I did a quick readthrough again and I agree that this series is good
for 2.10.

> 
> @Dave You helped me quite a lot with these, thanks a lot! I could not
> find any r-b's or similar form you so if you would like to take the
> well deserved credit for your work...
> 
> v2 --> v3:
> * rebased onto current master, since 's390x: vmstatify config migration
>   for virtio-ccw' is already there we don't need it here any more
> * added ack's and r-b'
> * got rid of loads of nits (thanks Connie and Thomas)
> * added explanation why certain members are not migrated
> 
> v1 --> v2:
> * Split out the vmystatify virtio-ccw part, reorganize
> * Use WITH_TMP instead of one-shot VMStateInfo's
> 
> Halil Pasic (6):
>   s390x: add helper get_machine_class
>   s390x: add css_migration_enabled to machine class
>   s390x/css: add missing css state conditionally
>   s390x/css: add ORB to SubchDev
>   s390x/css: activate ChannelSubSys migration
>   s390x/css: use SubchDev.orb
> 
>  hw/intc/s390_flic.c                |  20 ++++++
>  hw/s390x/css.c                     | 144 +++++++++++++++++++++++++++++++++----
>  hw/s390x/s390-virtio-ccw.c         |  58 +++++++++------
>  include/hw/s390x/css.h             |  11 ++-
>  include/hw/s390x/s390-virtio-ccw.h |   7 ++
>  5 files changed, 199 insertions(+), 41 deletions(-)
> 

Re: [Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Christian Borntraeger]

On 07/11/2017 04:54 PM, Halil Pasic wrote:
> Like for v2 the scope of this patch series is now limited to decoupling
> channel subsystem migration from the migration of virtio-ccw proxies.
> 
> There wasn't a whole lot of criticism regarding v2, so very little
> changed since then. All issues identified in the previous round were
> addressed as agreed upon. I hope this series is now good for 2.10.
> 
> @Dave You helped me quite a lot with these, thanks a lot! I could not
> find any r-b's or similar form you so if you would like to take the
> well deserved credit for your work...
> 
> v2 --> v3:
> * rebased onto current master, since 's390x: vmstatify config migration
>   for virtio-ccw' is already there we don't need it here any more
> * added ack's and r-b'
> * got rid of loads of nits (thanks Connie and Thomas)
> * added explanation why certain members are not migrated
> 
> v1 --> v2:
> * Split out the vmystatify virtio-ccw part, reorganize
> * Use WITH_TMP instead of one-shot VMStateInfo's
> 
> Halil Pasic (6):
>   s390x: add helper get_machine_class
>   s390x: add css_migration_enabled to machine class
>   s390x/css: add missing css state conditionally
>   s390x/css: add ORB to SubchDev
>   s390x/css: activate ChannelSubSys migration
>   s390x/css: use SubchDev.orb
> 
>  hw/intc/s390_flic.c                |  20 ++++++
>  hw/s390x/css.c                     | 144 +++++++++++++++++++++++++++++++++----
>  hw/s390x/s390-virtio-ccw.c         |  58 +++++++++------
>  include/hw/s390x/css.h             |  11 ++-
>  include/hw/s390x/s390-virtio-ccw.h |   7 ++
>  5 files changed, 199 insertions(+), 41 deletions(-)

Can you clarify what test you have done regarding compatibility migrations to 2.9 and earlier?

Re: [Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Halil Pasic]

On 07/12/2017 10:01 AM, Christian Borntraeger wrote:
> On 07/11/2017 04:54 PM, Halil Pasic wrote:
>> Like for v2 the scope of this patch series is now limited to decoupling
>> channel subsystem migration from the migration of virtio-ccw proxies.
>>
>> There wasn't a whole lot of criticism regarding v2, so very little
>> changed since then. All issues identified in the previous round were
>> addressed as agreed upon. I hope this series is now good for 2.10.
>>
>> @Dave You helped me quite a lot with these, thanks a lot! I could not
>> find any r-b's or similar form you so if you would like to take the
>> well deserved credit for your work...
>>
>> v2 --> v3:
>> * rebased onto current master, since 's390x: vmstatify config migration
>>   for virtio-ccw' is already there we don't need it here any more
>> * added ack's and r-b'
>> * got rid of loads of nits (thanks Connie and Thomas)
>> * added explanation why certain members are not migrated
>>
>> v1 --> v2:
>> * Split out the vmystatify virtio-ccw part, reorganize
>> * Use WITH_TMP instead of one-shot VMStateInfo's
>>
>> Halil Pasic (6):
>>   s390x: add helper get_machine_class
>>   s390x: add css_migration_enabled to machine class
>>   s390x/css: add missing css state conditionally
>>   s390x/css: add ORB to SubchDev
>>   s390x/css: activate ChannelSubSys migration
>>   s390x/css: use SubchDev.orb
>>
>>  hw/intc/s390_flic.c                |  20 ++++++
>>  hw/s390x/css.c                     | 144 +++++++++++++++++++++++++++++++++----
>>  hw/s390x/s390-virtio-ccw.c         |  58 +++++++++------
>>  include/hw/s390x/css.h             |  11 ++-
>>  include/hw/s390x/s390-virtio-ccw.h |   7 ++
>>  5 files changed, 199 insertions(+), 41 deletions(-)
> 
> Can you clarify what test you have done regarding compatibility migrations to 2.9 and earlier?
> 

Hi Christian!

Again the same. I was migrating a guests with just some virtio-blk devices
between my new qemu (with this patch set on top of master) and itself with
default machine. And then between qemu with machine virtio-ccw-2.5 and a
2.5 binary with machine virtio-ccw-2.5 (2.5 the binary is some old
"internal driver"). The old and the new binary where tested in both (source
and target) roles. I've verified that the disks are still working after
migration.

Given the nature of the changes I didn't do a great number of repeated tests,
as was doing all the migrations within the same host via HMP command.

If one has an appropriate automated setup (I don't) I would welcome some
more testing (and be happy to add a Tested-by), but I don't think it is
strictly necessary.

Regards,
Halil

Re: [Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Christian Borntraeger]

On 07/12/2017 01:06 PM, Halil Pasic wrote:
> 
> 
> On 07/12/2017 10:01 AM, Christian Borntraeger wrote:
>> On 07/11/2017 04:54 PM, Halil Pasic wrote:
>>> Like for v2 the scope of this patch series is now limited to decoupling
>>> channel subsystem migration from the migration of virtio-ccw proxies.
>>>
>>> There wasn't a whole lot of criticism regarding v2, so very little
>>> changed since then. All issues identified in the previous round were
>>> addressed as agreed upon. I hope this series is now good for 2.10.
>>>
>>> @Dave You helped me quite a lot with these, thanks a lot! I could not
>>> find any r-b's or similar form you so if you would like to take the
>>> well deserved credit for your work...
>>>
>>> v2 --> v3:
>>> * rebased onto current master, since 's390x: vmstatify config migration
>>>   for virtio-ccw' is already there we don't need it here any more
>>> * added ack's and r-b'
>>> * got rid of loads of nits (thanks Connie and Thomas)
>>> * added explanation why certain members are not migrated
>>>
>>> v1 --> v2:
>>> * Split out the vmystatify virtio-ccw part, reorganize
>>> * Use WITH_TMP instead of one-shot VMStateInfo's
>>>
>>> Halil Pasic (6):
>>>   s390x: add helper get_machine_class
>>>   s390x: add css_migration_enabled to machine class
>>>   s390x/css: add missing css state conditionally
>>>   s390x/css: add ORB to SubchDev
>>>   s390x/css: activate ChannelSubSys migration
>>>   s390x/css: use SubchDev.orb
>>>
>>>  hw/intc/s390_flic.c                |  20 ++++++
>>>  hw/s390x/css.c                     | 144 +++++++++++++++++++++++++++++++++----
>>>  hw/s390x/s390-virtio-ccw.c         |  58 +++++++++------
>>>  include/hw/s390x/css.h             |  11 ++-
>>>  include/hw/s390x/s390-virtio-ccw.h |   7 ++
>>>  5 files changed, 199 insertions(+), 41 deletions(-)
>>
>> Can you clarify what test you have done regarding compatibility migrations to 2.9 and earlier?
>>
> 
> Hi Christian!
> 
> Again the same. I was migrating a guests with just some virtio-blk devices
> between my new qemu (with this patch set on top of master) and itself with
> default machine. And then between qemu with machine virtio-ccw-2.5 and a
> 2.5 binary with machine virtio-ccw-2.5 (2.5 the binary is some old
> "internal driver"). The old and the new binary where tested in both (source
> and target) roles. I've verified that the disks are still working after
> migration.
> 
> Given the nature of the changes I didn't do a great number of repeated tests,
> as was doing all the migrations within the same host via HMP command.
> 
> If one has an appropriate automated setup (I don't) I would welcome some
> more testing (and be happy to add a Tested-by), but I don't think it is
> strictly necessary.

No thats fine. I just want to have this kind of testing done by the developer.
Thank you for doing so. If you have done it, you can always say it in your
cover letter

Re: [Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Christian Borntraeger]

On 07/11/2017 04:54 PM, Halil Pasic wrote:
> Like for v2 the scope of this patch series is now limited to decoupling
> channel subsystem migration from the migration of virtio-ccw proxies.
> 
> There wasn't a whole lot of criticism regarding v2, so very little
> changed since then. All issues identified in the previous round were
> addressed as agreed upon. I hope this series is now good for 2.10.
> 
> @Dave You helped me quite a lot with these, thanks a lot! I could not
> find any r-b's or similar form you so if you would like to take the
> well deserved credit for your work...
> 
> v2 --> v3:
> * rebased onto current master, since 's390x: vmstatify config migration
>   for virtio-ccw' is already there we don't need it here any more
> * added ack's and r-b'
> * got rid of loads of nits (thanks Connie and Thomas)
> * added explanation why certain members are not migrated
> 
> v1 --> v2:
> * Split out the vmystatify virtio-ccw part, reorganize
> * Use WITH_TMP instead of one-shot VMStateInfo's
> 
> Halil Pasic (6):
>   s390x: add helper get_machine_class
>   s390x: add css_migration_enabled to machine class
>   s390x/css: add missing css state conditionally
>   s390x/css: add ORB to SubchDev
>   s390x/css: activate ChannelSubSys migration
>   s390x/css: use SubchDev.orb
> 
>  hw/intc/s390_flic.c                |  20 ++++++
>  hw/s390x/css.c                     | 144 +++++++++++++++++++++++++++++++++----
>  hw/s390x/s390-virtio-ccw.c         |  58 +++++++++------
>  include/hw/s390x/css.h             |  11 ++-
>  include/hw/s390x/s390-virtio-ccw.h |   7 ++
>  5 files changed, 199 insertions(+), 41 deletions(-)
> 

applied to s390-next. Thanks

Re: [Qemu-devel] [PATCH v3 0/6] migration: s390x css migration

[Dr. David Alan Gilbert]

* Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> Like for v2 the scope of this patch series is now limited to decoupling
> channel subsystem migration from the migration of virtio-ccw proxies.
> 
> There wasn't a whole lot of criticism regarding v2, so very little
> changed since then. All issues identified in the previous round were
> addressed as agreed upon. I hope this series is now good for 2.10.
> 
> @Dave You helped me quite a lot with these, thanks a lot! I could not
> find any r-b's or similar form you so if you would like to take the
> well deserved credit for your work...

No problem, as long as we've got a happy migration that's enough,
and we've finally got VMSTATE_QTAILQ used as well.

Dave

> v2 --> v3:
> * rebased onto current master, since 's390x: vmstatify config migration
>   for virtio-ccw' is already there we don't need it here any more
> * added ack's and r-b'
> * got rid of loads of nits (thanks Connie and Thomas)
> * added explanation why certain members are not migrated
> 
> v1 --> v2:
> * Split out the vmystatify virtio-ccw part, reorganize
> * Use WITH_TMP instead of one-shot VMStateInfo's
> 
> Halil Pasic (6):
>   s390x: add helper get_machine_class
>   s390x: add css_migration_enabled to machine class
>   s390x/css: add missing css state conditionally
>   s390x/css: add ORB to SubchDev
>   s390x/css: activate ChannelSubSys migration
>   s390x/css: use SubchDev.orb
> 
>  hw/intc/s390_flic.c                |  20 ++++++
>  hw/s390x/css.c                     | 144 +++++++++++++++++++++++++++++++++----
>  hw/s390x/s390-virtio-ccw.c         |  58 +++++++++------
>  include/hw/s390x/css.h             |  11 ++-
>  include/hw/s390x/s390-virtio-ccw.h |   7 ++
>  5 files changed, 199 insertions(+), 41 deletions(-)
> 
> -- 
> 2.11.2
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 5/6] s390x/css: activate ChannelSubSys migration

[Thomas Huth]

On 11.07.2017 16:54, Halil Pasic wrote:
> Turn on migration for the channel subsystem for the next machine.  For
> legacy machines we still have to do things the old way.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/css.c             | 5 +++++
>  hw/s390x/s390-virtio-ccw.c | 9 ++++-----
>  include/hw/s390x/css.h     | 4 ++++
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 20b5fbed68..b89b60751a 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -407,6 +407,11 @@ static int subch_dev_post_load(void *opaque, int version_id)
>      return 0;
>  }
>  
> +void css_register_vmstate(void)
> +{
> +    vmstate_register(NULL, 0, &vmstate_css, &channel_subsys);
> +}
> +
>  IndAddr *get_indicator(hwaddr ind_addr, int len)
>  {
>      IndAddr *indicator;
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 751febb87a..20e3f06519 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -206,7 +206,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>  
>      s390mc->ri_allowed = true;
>      s390mc->cpu_model_allowed = true;
> -    s390mc->css_migration_enabled = false; /* TODO: set to true */
> +    s390mc->css_migration_enabled = true;
>      mc->init = ccw_init;
>      mc->reset = s390_machine_reset;
>      mc->hot_add_cpu = s390_hot_add_cpu;
> @@ -482,10 +482,9 @@ bool css_migration_enabled(void)
>  
>  static void ccw_machine_2_10_instance_options(MachineState *machine)
>  {
> -    /*
> -     * TODO Once preparations are done register vmstate for the css if
> -     * css_migration_enabled().
> -     */
> +    if (css_migration_enabled()) {
> +        css_register_vmstate();
> +    }
>  }

The location for the above hunk seems to be quite unfortunate: For the
s390-ccw-virtio-2.11 machine, we now do not call css_register_vmstate()
anymore.

Maybe this should rather go into ccw_init() instead?

 Thomas

Re: [Qemu-devel] [PATCH v3 5/6] s390x/css: activate ChannelSubSys migration

[Cornelia Huck]

On Tue, 3 Oct 2017 13:58:29 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 11.07.2017 16:54, Halil Pasic wrote:
> > Turn on migration for the channel subsystem for the next machine.  For
> > legacy machines we still have to do things the old way.
> > 
> > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> > Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  hw/s390x/css.c             | 5 +++++
> >  hw/s390x/s390-virtio-ccw.c | 9 ++++-----
> >  include/hw/s390x/css.h     | 4 ++++
> >  3 files changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> > index 20b5fbed68..b89b60751a 100644
> > --- a/hw/s390x/css.c
> > +++ b/hw/s390x/css.c
> > @@ -407,6 +407,11 @@ static int subch_dev_post_load(void *opaque, int version_id)
> >      return 0;
> >  }
> >  
> > +void css_register_vmstate(void)
> > +{
> > +    vmstate_register(NULL, 0, &vmstate_css, &channel_subsys);
> > +}
> > +
> >  IndAddr *get_indicator(hwaddr ind_addr, int len)
> >  {
> >      IndAddr *indicator;
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index 751febb87a..20e3f06519 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -206,7 +206,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
> >  
> >      s390mc->ri_allowed = true;
> >      s390mc->cpu_model_allowed = true;
> > -    s390mc->css_migration_enabled = false; /* TODO: set to true */
> > +    s390mc->css_migration_enabled = true;
> >      mc->init = ccw_init;
> >      mc->reset = s390_machine_reset;
> >      mc->hot_add_cpu = s390_hot_add_cpu;
> > @@ -482,10 +482,9 @@ bool css_migration_enabled(void)
> >  
> >  static void ccw_machine_2_10_instance_options(MachineState *machine)
> >  {
> > -    /*
> > -     * TODO Once preparations are done register vmstate for the css if
> > -     * css_migration_enabled().
> > -     */
> > +    if (css_migration_enabled()) {
> > +        css_register_vmstate();
> > +    }
> >  }  
> 
> The location for the above hunk seems to be quite unfortunate: For the
> s390-ccw-virtio-2.11 machine, we now do not call css_register_vmstate()
> anymore.

Hm, you're right... only setting the boolean should be done in the
compat callbacks.

> 
> Maybe this should rather go into ccw_init() instead?

Sounds reasonable. Halil, can you please check (and send a patch if it
makes sense?)

Re: [Qemu-devel] [PATCH v3 5/6] s390x/css: activate ChannelSubSys migration

[Halil Pasic]

On 10/04/2017 10:16 AM, Cornelia Huck wrote:
> On Tue, 3 Oct 2017 13:58:29 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 11.07.2017 16:54, Halil Pasic wrote:
>>> Turn on migration for the channel subsystem for the next machine.  For
>>> legacy machines we still have to do things the old way.
>>>
>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>>> ---
>>>  hw/s390x/css.c             | 5 +++++
>>>  hw/s390x/s390-virtio-ccw.c | 9 ++++-----
>>>  include/hw/s390x/css.h     | 4 ++++
>>>  3 files changed, 13 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>>> index 20b5fbed68..b89b60751a 100644
>>> --- a/hw/s390x/css.c
>>> +++ b/hw/s390x/css.c
>>> @@ -407,6 +407,11 @@ static int subch_dev_post_load(void *opaque, int version_id)
>>>      return 0;
>>>  }
>>>  
>>> +void css_register_vmstate(void)
>>> +{
>>> +    vmstate_register(NULL, 0, &vmstate_css, &channel_subsys);
>>> +}
>>> +
>>>  IndAddr *get_indicator(hwaddr ind_addr, int len)
>>>  {
>>>      IndAddr *indicator;
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 751febb87a..20e3f06519 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -206,7 +206,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
>>>  
>>>      s390mc->ri_allowed = true;
>>>      s390mc->cpu_model_allowed = true;
>>> -    s390mc->css_migration_enabled = false; /* TODO: set to true */
>>> +    s390mc->css_migration_enabled = true;
>>>      mc->init = ccw_init;
>>>      mc->reset = s390_machine_reset;
>>>      mc->hot_add_cpu = s390_hot_add_cpu;
>>> @@ -482,10 +482,9 @@ bool css_migration_enabled(void)
>>>  
>>>  static void ccw_machine_2_10_instance_options(MachineState *machine)
>>>  {
>>> -    /*
>>> -     * TODO Once preparations are done register vmstate for the css if
>>> -     * css_migration_enabled().
>>> -     */
>>> +    if (css_migration_enabled()) {
>>> +        css_register_vmstate();
>>> +    }
>>>  }  
>>
>> The location for the above hunk seems to be quite unfortunate: For the
>> s390-ccw-virtio-2.11 machine, we now do not call css_register_vmstate()
>> anymore.
> 
> Hm, you're right... only setting the boolean should be done in the
> compat callbacks.

Nod.

> 
>>
>> Maybe this should rather go into ccw_init() instead?
> 
> Sounds reasonable. Halil, can you please check (and send a patch if it
> makes sense?)

Makes sense. I will send a patch today.

Halil
>