[Qemu-devel] [PATCH 0/7] s390x/kvm: pending patches

[Qemu-devel] [PATCH 0/7] s390x/kvm: pending patches

[Christian Borntraeger]

This is what I have queued for s390x.

Cornelia Huck (1):
  s390x/MAINTAINERS: Update my email address

Dong Jia Shi (1):
  s390x/3270: fix instruction interception handler

Halil Pasic (3):
  s390x: vmstatify config migration for virtio-ccw
  s390x: fix error propagation in kvm-flic's realize
  s390x: fix realize inheritance for kvm-flic

QingFeng Hao (1):
  virtio-scsi-ccw: use ioeventfd even when KVM is disabled

Viktor Mihajlovski (1):
  s390x: return unavailable features via query-cpu-definitions

 MAINTAINERS                  |   8 +-
 hw/intc/s390_flic.c          |  32 +++-
 hw/intc/s390_flic_kvm.c      |  31 +++-
 hw/s390x/3270-ccw.c          |   1 +
 hw/s390x/ccw-device.c        |  10 ++
 hw/s390x/ccw-device.h        |   4 +
 hw/s390x/css.c               | 378 +++++++++++++++++++++++++------------------
 hw/s390x/virtio-ccw.c        | 160 +++++++++---------
 include/hw/s390x/css.h       |  12 +-
 include/hw/s390x/s390_flic.h |   5 +
 target/s390x/cpu.h           |   6 +-
 target/s390x/cpu_models.c    |  62 ++++++-
 12 files changed, 456 insertions(+), 253 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/7] s390x: vmstatify config migration for virtio-ccw

[Christian Borntraeger]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for
flexibility (extending using subsections) and for fun.

To achieve this we need to hack the config_vector, which is VirtIODevice
(that is common virtio) state, in the middle of the VirtioCcwDevice state
representation.  This is somewhat ugly, but we have no choice because the
stream format needs to be preserved.

Almost no changes in behavior. Exception is everything that comes with
vmstate like extra bookkeeping about what's in the stream, and maybe some
extra checks and better error reporting.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Message-Id: <20170703213414.94298-1-pasic@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/intc/s390_flic.c          |  28 ++++
 hw/s390x/ccw-device.c        |  10 ++
 hw/s390x/ccw-device.h        |   4 +
 hw/s390x/css.c               | 378 +++++++++++++++++++++++++------------------
 hw/s390x/virtio-ccw.c        | 158 +++++++++---------
 include/hw/s390x/css.h       |  12 +-
 include/hw/s390x/s390_flic.h |   5 +
 7 files changed, 358 insertions(+), 237 deletions(-)

diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index a26e906..a99a350 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -17,6 +17,7 @@
 #include "trace.h"
 #include "hw/qdev.h"
 #include "qapi/error.h"
+#include "hw/s390x/s390-virtio-ccw.h"
 
 S390FLICState *s390_get_flic(void)
 {
@@ -136,3 +137,30 @@ static void qemu_s390_flic_register_types(void)
 }
 
 type_init(qemu_s390_flic_register_types)
+
+const VMStateDescription vmstate_adapter_info = {
+    .name = "s390_adapter_info",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(ind_offset, AdapterInfo),
+        /*
+         * We do not have to migrate neither the id nor the addresses.
+         * The id is set by css_register_io_adapter and the addresses
+         * are set based on the IndAddr objects after those get mapped.
+         */
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+const VMStateDescription vmstate_adapter_routes = {
+
+    .name = "s390_adapter_routes",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT(adapter, AdapterRoutes, 1, vmstate_adapter_info,
+                       AdapterInfo),
+        VMSTATE_END_OF_LIST()
+    }
+};
diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index fb8d640..f9bfa15 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -50,6 +50,16 @@ static void ccw_device_class_init(ObjectClass *klass, void *data)
     dc->props = ccw_device_properties;
 }
 
+const VMStateDescription vmstate_ccw_dev = {
+    .name = "s390_ccw_dev",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT_POINTER(sch, CcwDevice, vmstate_subch_dev, SubchDev),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const TypeInfo ccw_device_info = {
     .name = TYPE_CCW_DEVICE,
     .parent = TYPE_DEVICE,
diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 89c8e5d..4e6af28 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -27,6 +27,10 @@ typedef struct CcwDevice {
     CssDevId subch_id;
 } CcwDevice;
 
+extern const VMStateDescription vmstate_ccw_dev;
+#define VMSTATE_CCW_DEVICE(_field, _state)                     \
+    VMSTATE_STRUCT(_field, _state, 1, vmstate_ccw_dev, CcwDevice)
+
 typedef struct CCWDeviceClass {
     DeviceClass parent_class;
     void (*unplug)(HotplugHandler *, DeviceState *, Error **);
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 599805d..d67fffa 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -22,6 +22,7 @@
 #include "hw/s390x/css.h"
 #include "trace.h"
 #include "hw/s390x/s390_flic.h"
+#include "hw/s390x/s390-virtio-ccw.h"
 
 typedef struct CrwContainer {
     CRW crw;
@@ -40,6 +41,181 @@ typedef struct SubchSet {
     unsigned long devnos_used[BITS_TO_LONGS(MAX_SCHID + 1)];
 } SubchSet;
 
+static const VMStateDescription vmstate_scsw = {
+    .name = "s390_scsw",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16(flags, SCSW),
+        VMSTATE_UINT16(ctrl, SCSW),
+        VMSTATE_UINT32(cpa, SCSW),
+        VMSTATE_UINT8(dstat, SCSW),
+        VMSTATE_UINT8(cstat, SCSW),
+        VMSTATE_UINT16(count, SCSW),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_pmcw = {
+    .name = "s390_pmcw",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(intparm, PMCW),
+        VMSTATE_UINT16(flags, PMCW),
+        VMSTATE_UINT16(devno, PMCW),
+        VMSTATE_UINT8(lpm, PMCW),
+        VMSTATE_UINT8(pnom, PMCW),
+        VMSTATE_UINT8(lpum, PMCW),
+        VMSTATE_UINT8(pim, PMCW),
+        VMSTATE_UINT16(mbi, PMCW),
+        VMSTATE_UINT8(pom, PMCW),
+        VMSTATE_UINT8(pam, PMCW),
+        VMSTATE_UINT8_ARRAY(chpid, PMCW, 8),
+        VMSTATE_UINT32(chars, PMCW),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_schib = {
+    .name = "s390_schib",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT(pmcw, SCHIB, 0, vmstate_pmcw, PMCW),
+        VMSTATE_STRUCT(scsw, SCHIB, 0, vmstate_scsw, SCSW),
+        VMSTATE_UINT64(mba, SCHIB),
+        VMSTATE_UINT8_ARRAY(mda, SCHIB, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
+static const VMStateDescription vmstate_ccw1 = {
+    .name = "s390_ccw1",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(cmd_code, CCW1),
+        VMSTATE_UINT8(flags, CCW1),
+        VMSTATE_UINT16(count, CCW1),
+        VMSTATE_UINT32(cda, CCW1),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_ciw = {
+    .name = "s390_ciw",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(type, CIW),
+        VMSTATE_UINT8(command, CIW),
+        VMSTATE_UINT16(count, CIW),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_sense_id = {
+    .name = "s390_sense_id",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(reserved, SenseId),
+        VMSTATE_UINT16(cu_type, SenseId),
+        VMSTATE_UINT8(cu_model, SenseId),
+        VMSTATE_UINT16(dev_type, SenseId),
+        VMSTATE_UINT8(dev_model, SenseId),
+        VMSTATE_UINT8(unused, SenseId),
+        VMSTATE_STRUCT_ARRAY(ciw, SenseId, MAX_CIWS, 0, vmstate_ciw, CIW),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static int subch_dev_post_load(void *opaque, int version_id);
+static void subch_dev_pre_save(void *opaque);
+
+const char err_hint_devno[] = "Devno mismatch, tried to load wrong section!"
+    " Likely reason: some sequences of plug and unplug  can break"
+    " migration for machine versions prior to  2.7 (known design flaw).";
+
+const VMStateDescription vmstate_subch_dev = {
+    .name = "s390_subch_dev",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = subch_dev_post_load,
+    .pre_save = subch_dev_pre_save,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8_EQUAL(cssid, SubchDev, "Bug!"),
+        VMSTATE_UINT8_EQUAL(ssid, SubchDev, "Bug!"),
+        VMSTATE_UINT16(migrated_schid, SubchDev),
+        VMSTATE_UINT16_EQUAL(devno, SubchDev, err_hint_devno),
+        VMSTATE_BOOL(thinint_active, SubchDev),
+        VMSTATE_STRUCT(curr_status, SubchDev, 0, vmstate_schib, SCHIB),
+        VMSTATE_UINT8_ARRAY(sense_data, SubchDev, 32),
+        VMSTATE_UINT64(channel_prog, SubchDev),
+        VMSTATE_STRUCT(last_cmd, SubchDev, 0, vmstate_ccw1, CCW1),
+        VMSTATE_BOOL(last_cmd_valid, SubchDev),
+        VMSTATE_STRUCT(id, SubchDev, 0, vmstate_sense_id, SenseId),
+        VMSTATE_BOOL(ccw_fmt_1, SubchDev),
+        VMSTATE_UINT8(ccw_no_data_cnt, SubchDev),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+typedef struct IndAddrPtrTmp {
+    IndAddr **parent;
+    uint64_t addr;
+    int32_t len;
+} IndAddrPtrTmp;
+
+static int post_load_ind_addr(void *opaque, int version_id)
+{
+    IndAddrPtrTmp *ptmp = opaque;
+    IndAddr **ind_addr = ptmp->parent;
+
+    if (ptmp->len != 0) {
+        *ind_addr = get_indicator(ptmp->addr, ptmp->len);
+    } else {
+        *ind_addr = NULL;
+    }
+    return 0;
+}
+
+static void pre_save_ind_addr(void *opaque)
+{
+    IndAddrPtrTmp *ptmp = opaque;
+    IndAddr *ind_addr = *(ptmp->parent);
+
+    if (ind_addr != NULL) {
+        ptmp->len = ind_addr->len;
+        ptmp->addr = ind_addr->addr;
+    } else {
+        ptmp->len = 0;
+        ptmp->addr = 0L;
+    }
+}
+
+const VMStateDescription vmstate_ind_addr_tmp = {
+    .name = "s390_ind_addr_tmp",
+    .pre_save = pre_save_ind_addr,
+    .post_load = post_load_ind_addr,
+
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(len, IndAddrPtrTmp),
+        VMSTATE_UINT64(addr, IndAddrPtrTmp),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+const VMStateDescription vmstate_ind_addr = {
+    .name = "s390_ind_addr_tmp",
+    .fields = (VMStateField[]) {
+        VMSTATE_WITH_TMP(IndAddr*, IndAddrPtrTmp, vmstate_ind_addr_tmp),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 typedef struct CssImage {
     SubchSet *sch_set[MAX_SSID + 1];
     ChpInfo chpids[MAX_CHPID + 1];
@@ -77,6 +253,52 @@ static ChannelSubSys channel_subsys = {
         QTAILQ_HEAD_INITIALIZER(channel_subsys.indicator_addresses),
 };
 
+static void subch_dev_pre_save(void *opaque)
+{
+    SubchDev *s = opaque;
+
+    /* Prepare remote_schid for save */
+    s->migrated_schid = s->schid;
+}
+
+static int subch_dev_post_load(void *opaque, int version_id)
+{
+
+    SubchDev *s = opaque;
+
+    /* Re-assign the subchannel to remote_schid if necessary */
+    if (s->migrated_schid != s->schid) {
+        if (css_find_subch(true, s->cssid, s->ssid, s->schid) == s) {
+            /*
+             * Cleanup the slot before moving to s->migrated_schid provided
+             * it still belongs to us, i.e. it was not changed by previous
+             * invocation of this function.
+             */
+            css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, NULL);
+        }
+        /* It's OK to re-assign without a prior de-assign. */
+        s->schid = s->migrated_schid;
+        css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s);
+    }
+
+    /*
+     * Hack alert. If we don't migrate the channel subsystem status
+     * we still need to find out if the guest enabled mss/mcss-e.
+     * If the subchannel is enabled, it certainly was able to access it,
+     * so adjust the max_ssid/max_cssid values for relevant ssid/cssid
+     * values. This is not watertight, but better than nothing.
+     */
+    if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) {
+        if (s->ssid) {
+            channel_subsys.max_ssid = MAX_SSID;
+        }
+        if (s->cssid != channel_subsys.default_cssid) {
+            channel_subsys.max_cssid = MAX_CSSID;
+        }
+    }
+    return 0;
+}
+
 IndAddr *get_indicator(hwaddr ind_addr, int len)
 {
     IndAddr *indicator;
@@ -1747,162 +1969,6 @@ int css_enable_mss(void)
     return 0;
 }
 
-void subch_device_save(SubchDev *s, QEMUFile *f)
-{
-    int i;
-
-    qemu_put_byte(f, s->cssid);
-    qemu_put_byte(f, s->ssid);
-    qemu_put_be16(f, s->schid);
-    qemu_put_be16(f, s->devno);
-    qemu_put_byte(f, s->thinint_active);
-    /* SCHIB */
-    /*     PMCW */
-    qemu_put_be32(f, s->curr_status.pmcw.intparm);
-    qemu_put_be16(f, s->curr_status.pmcw.flags);
-    qemu_put_be16(f, s->curr_status.pmcw.devno);
-    qemu_put_byte(f, s->curr_status.pmcw.lpm);
-    qemu_put_byte(f, s->curr_status.pmcw.pnom);
-    qemu_put_byte(f, s->curr_status.pmcw.lpum);
-    qemu_put_byte(f, s->curr_status.pmcw.pim);
-    qemu_put_be16(f, s->curr_status.pmcw.mbi);
-    qemu_put_byte(f, s->curr_status.pmcw.pom);
-    qemu_put_byte(f, s->curr_status.pmcw.pam);
-    qemu_put_buffer(f, s->curr_status.pmcw.chpid, 8);
-    qemu_put_be32(f, s->curr_status.pmcw.chars);
-    /*     SCSW */
-    qemu_put_be16(f, s->curr_status.scsw.flags);
-    qemu_put_be16(f, s->curr_status.scsw.ctrl);
-    qemu_put_be32(f, s->curr_status.scsw.cpa);
-    qemu_put_byte(f, s->curr_status.scsw.dstat);
-    qemu_put_byte(f, s->curr_status.scsw.cstat);
-    qemu_put_be16(f, s->curr_status.scsw.count);
-    qemu_put_be64(f, s->curr_status.mba);
-    qemu_put_buffer(f, s->curr_status.mda, 4);
-    /* end SCHIB */
-    qemu_put_buffer(f, s->sense_data, 32);
-    qemu_put_be64(f, s->channel_prog);
-    /* last cmd */
-    qemu_put_byte(f, s->last_cmd.cmd_code);
-    qemu_put_byte(f, s->last_cmd.flags);
-    qemu_put_be16(f, s->last_cmd.count);
-    qemu_put_be32(f, s->last_cmd.cda);
-    qemu_put_byte(f, s->last_cmd_valid);
-    qemu_put_byte(f, s->id.reserved);
-    qemu_put_be16(f, s->id.cu_type);
-    qemu_put_byte(f, s->id.cu_model);
-    qemu_put_be16(f, s->id.dev_type);
-    qemu_put_byte(f, s->id.dev_model);
-    qemu_put_byte(f, s->id.unused);
-    for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) {
-        qemu_put_byte(f, s->id.ciw[i].type);
-        qemu_put_byte(f, s->id.ciw[i].command);
-        qemu_put_be16(f, s->id.ciw[i].count);
-    }
-    qemu_put_byte(f, s->ccw_fmt_1);
-    qemu_put_byte(f, s->ccw_no_data_cnt);
-}
-
-int subch_device_load(SubchDev *s, QEMUFile *f)
-{
-    SubchDev *old_s;
-    Error *err = NULL;
-    uint16_t old_schid = s->schid;
-    uint16_t old_devno = s->devno;
-    int i;
-
-    s->cssid = qemu_get_byte(f);
-    s->ssid = qemu_get_byte(f);
-    s->schid = qemu_get_be16(f);
-    s->devno = qemu_get_be16(f);
-    if (s->devno != old_devno) {
-        /* Only possible if machine < 2.7 (no css_dev_path) */
-
-        error_setg(&err, "%x != %x", old_devno,  s->devno);
-        error_append_hint(&err, "Devno mismatch, tried to load wrong section!"
-                          " Likely reason: some sequences of plug and unplug"
-                          " can break migration for machine versions prior to"
-                          " 2.7 (known design flaw).\n");
-        error_report_err(err);
-        return -EINVAL;
-    }
-    /* Re-assign subch. */
-    if (old_schid != s->schid) {
-        old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid];
-        /*
-         * (old_s != s) means that some other device has its correct
-         * subchannel already assigned (in load).
-         */
-        if (old_s == s) {
-            css_subch_assign(s->cssid, s->ssid, old_schid, s->devno, NULL);
-        }
-        /* It's OK to re-assign without a prior de-assign. */
-        css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s);
-    }
-    s->thinint_active = qemu_get_byte(f);
-    /* SCHIB */
-    /*     PMCW */
-    s->curr_status.pmcw.intparm = qemu_get_be32(f);
-    s->curr_status.pmcw.flags = qemu_get_be16(f);
-    s->curr_status.pmcw.devno = qemu_get_be16(f);
-    s->curr_status.pmcw.lpm = qemu_get_byte(f);
-    s->curr_status.pmcw.pnom  = qemu_get_byte(f);
-    s->curr_status.pmcw.lpum = qemu_get_byte(f);
-    s->curr_status.pmcw.pim = qemu_get_byte(f);
-    s->curr_status.pmcw.mbi = qemu_get_be16(f);
-    s->curr_status.pmcw.pom = qemu_get_byte(f);
-    s->curr_status.pmcw.pam = qemu_get_byte(f);
-    qemu_get_buffer(f, s->curr_status.pmcw.chpid, 8);
-    s->curr_status.pmcw.chars = qemu_get_be32(f);
-    /*     SCSW */
-    s->curr_status.scsw.flags = qemu_get_be16(f);
-    s->curr_status.scsw.ctrl = qemu_get_be16(f);
-    s->curr_status.scsw.cpa = qemu_get_be32(f);
-    s->curr_status.scsw.dstat = qemu_get_byte(f);
-    s->curr_status.scsw.cstat = qemu_get_byte(f);
-    s->curr_status.scsw.count = qemu_get_be16(f);
-    s->curr_status.mba = qemu_get_be64(f);
-    qemu_get_buffer(f, s->curr_status.mda, 4);
-    /* end SCHIB */
-    qemu_get_buffer(f, s->sense_data, 32);
-    s->channel_prog = qemu_get_be64(f);
-    /* last cmd */
-    s->last_cmd.cmd_code = qemu_get_byte(f);
-    s->last_cmd.flags = qemu_get_byte(f);
-    s->last_cmd.count = qemu_get_be16(f);
-    s->last_cmd.cda = qemu_get_be32(f);
-    s->last_cmd_valid = qemu_get_byte(f);
-    s->id.reserved = qemu_get_byte(f);
-    s->id.cu_type = qemu_get_be16(f);
-    s->id.cu_model = qemu_get_byte(f);
-    s->id.dev_type = qemu_get_be16(f);
-    s->id.dev_model = qemu_get_byte(f);
-    s->id.unused = qemu_get_byte(f);
-    for (i = 0; i < ARRAY_SIZE(s->id.ciw); i++) {
-        s->id.ciw[i].type = qemu_get_byte(f);
-        s->id.ciw[i].command = qemu_get_byte(f);
-        s->id.ciw[i].count = qemu_get_be16(f);
-    }
-    s->ccw_fmt_1 = qemu_get_byte(f);
-    s->ccw_no_data_cnt = qemu_get_byte(f);
-    /*
-     * Hack alert. We don't migrate the channel subsystem status (no
-     * device!), but we need to find out if the guest enabled mss/mcss-e.
-     * If the subchannel is enabled, it certainly was able to access it,
-     * so adjust the max_ssid/max_cssid values for relevant ssid/cssid
-     * values. This is not watertight, but better than nothing.
-     */
-    if (s->curr_status.pmcw.flags & PMCW_FLAGS_MASK_ENA) {
-        if (s->ssid) {
-            channel_subsys.max_ssid = MAX_SSID;
-        }
-        if (s->cssid != channel_subsys.default_cssid) {
-            channel_subsys.max_cssid = MAX_CSSID;
-        }
-    }
-    return 0;
-}
-
 void css_reset_sch(SubchDev *sch)
 {
     PMCW *p = &sch->curr_status.pmcw;
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb..f0e7fc8 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -34,9 +34,87 @@
 #include "virtio-ccw.h"
 #include "trace.h"
 #include "hw/s390x/css-bridge.h"
+#include "hw/s390x/s390-virtio-ccw.h"
 
 #define NR_CLASSIC_INDICATOR_BITS 64
 
+static int virtio_ccw_dev_post_load(void *opaque, int version_id)
+{
+    VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(opaque);
+    CcwDevice *ccw_dev = CCW_DEVICE(dev);
+    CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
+
+    ccw_dev->sch->driver_data = dev;
+    if (ccw_dev->sch->thinint_active) {
+        dev->routes.adapter.adapter_id = css_get_adapter_id(
+                                         CSS_IO_ADAPTER_VIRTIO,
+                                         dev->thinint_isc);
+    }
+    /* Re-fill subch_id after loading the subchannel states.*/
+    if (ck->refill_ids) {
+        ck->refill_ids(ccw_dev);
+    }
+    return 0;
+}
+
+typedef struct VirtioCcwDeviceTmp {
+    VirtioCcwDevice *parent;
+    uint16_t config_vector;
+} VirtioCcwDeviceTmp;
+
+static void virtio_ccw_dev_tmp_pre_save(void *opaque)
+{
+    VirtioCcwDeviceTmp *tmp = opaque;
+    VirtioCcwDevice *dev = tmp->parent;
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+
+    tmp->config_vector = vdev->config_vector;
+}
+
+static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id)
+{
+    VirtioCcwDeviceTmp *tmp = opaque;
+    VirtioCcwDevice *dev = tmp->parent;
+    VirtIODevice *vdev = virtio_bus_get_device(&dev->bus);
+
+    vdev->config_vector = tmp->config_vector;
+    return 0;
+}
+
+const VMStateDescription vmstate_virtio_ccw_dev_tmp = {
+    .name = "s390_virtio_ccw_dev_tmp",
+    .pre_save = virtio_ccw_dev_tmp_pre_save,
+    .post_load = virtio_ccw_dev_tmp_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+const VMStateDescription vmstate_virtio_ccw_dev = {
+    .name = "s390_virtio_ccw_dev",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = virtio_ccw_dev_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice),
+        VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice),
+        VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice),
+        VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice),
+        /*
+         * Ugly hack because VirtIODevice does not migrate itself.
+         * This also makes legacy via vmstate_save_state possible.
+         */
+        VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp,
+                         vmstate_virtio_ccw_dev_tmp),
+        VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes,
+                       AdapterRoutes),
+        VMSTATE_UINT8(thinint_isc, VirtioCcwDevice),
+        VMSTATE_INT32(revision, VirtioCcwDevice),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size,
                                VirtioCcwDevice *dev);
 
@@ -1239,89 +1317,13 @@ static int virtio_ccw_load_queue(DeviceState *d, int n, QEMUFile *f)
 static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
-    CcwDevice *ccw_dev = CCW_DEVICE(d);
-    SubchDev *s = ccw_dev->sch;
-    VirtIODevice *vdev = virtio_ccw_get_vdev(s);
-
-    subch_device_save(s, f);
-    if (dev->indicators != NULL) {
-        qemu_put_be32(f, dev->indicators->len);
-        qemu_put_be64(f, dev->indicators->addr);
-    } else {
-        qemu_put_be32(f, 0);
-        qemu_put_be64(f, 0UL);
-    }
-    if (dev->indicators2 != NULL) {
-        qemu_put_be32(f, dev->indicators2->len);
-        qemu_put_be64(f, dev->indicators2->addr);
-    } else {
-        qemu_put_be32(f, 0);
-        qemu_put_be64(f, 0UL);
-    }
-    if (dev->summary_indicator != NULL) {
-        qemu_put_be32(f, dev->summary_indicator->len);
-        qemu_put_be64(f, dev->summary_indicator->addr);
-    } else {
-        qemu_put_be32(f, 0);
-        qemu_put_be64(f, 0UL);
-    }
-    qemu_put_be16(f, vdev->config_vector);
-    qemu_put_be64(f, dev->routes.adapter.ind_offset);
-    qemu_put_byte(f, dev->thinint_isc);
-    qemu_put_be32(f, dev->revision);
+    vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL);
 }
 
 static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
-    CcwDevice *ccw_dev = CCW_DEVICE(d);
-    CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
-    SubchDev *s = ccw_dev->sch;
-    VirtIODevice *vdev = virtio_ccw_get_vdev(s);
-    int len;
-    int ret;
-
-    s->driver_data = dev;
-    ret = subch_device_load(s, f);
-    if (ret) {
-        return ret;
-    }
-    /* Re-fill subch_id after loading the subchannel states.*/
-    if (ck->refill_ids) {
-        ck->refill_ids(ccw_dev);
-    }
-    len = qemu_get_be32(f);
-    if (len != 0) {
-        dev->indicators = get_indicator(qemu_get_be64(f), len);
-    } else {
-        qemu_get_be64(f);
-        dev->indicators = NULL;
-    }
-    len = qemu_get_be32(f);
-    if (len != 0) {
-        dev->indicators2 = get_indicator(qemu_get_be64(f), len);
-    } else {
-        qemu_get_be64(f);
-        dev->indicators2 = NULL;
-    }
-    len = qemu_get_be32(f);
-    if (len != 0) {
-        dev->summary_indicator = get_indicator(qemu_get_be64(f), len);
-    } else {
-        qemu_get_be64(f);
-        dev->summary_indicator = NULL;
-    }
-    qemu_get_be16s(f, &vdev->config_vector);
-    dev->routes.adapter.ind_offset = qemu_get_be64(f);
-    dev->thinint_isc = qemu_get_byte(f);
-    dev->revision = qemu_get_be32(f);
-    if (s->thinint_active) {
-        dev->routes.adapter.adapter_id = css_get_adapter_id(
-                                         CSS_IO_ADAPTER_VIRTIO,
-                                         dev->thinint_isc);
-    }
-
-    return 0;
+    return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1);
 }
 
 static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp)
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index 596a2f2..eb0e26f 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -88,6 +88,7 @@ struct SubchDev {
     bool ccw_fmt_1;
     bool thinint_active;
     uint8_t ccw_no_data_cnt;
+    uint16_t migrated_schid; /* used for missmatch detection */
     /* transport-provided data: */
     int (*ccw_cb) (SubchDev *, CCW1);
     void (*disable_cb)(SubchDev *);
@@ -96,6 +97,8 @@ struct SubchDev {
     void *driver_data;
 };
 
+extern const VMStateDescription vmstate_subch_dev;
+
 /*
  * Identify a device within the channel subsystem.
  * Note that this can be used to identify either the subchannel or
@@ -118,18 +121,21 @@ typedef struct IndAddr {
     hwaddr addr;
     uint64_t map;
     unsigned long refcnt;
-    int len;
+    int32_t len;
     QTAILQ_ENTRY(IndAddr) sibling;
 } IndAddr;
 
+extern const VMStateDescription vmstate_ind_addr;
+
+#define VMSTATE_PTR_TO_IND_ADDR(_f, _s)                                   \
+    VMSTATE_STRUCT(_f, _s, 1, vmstate_ind_addr, IndAddr*)
+
 IndAddr *get_indicator(hwaddr ind_addr, int len);
 void release_indicator(AdapterInfo *adapter, IndAddr *indicator);
 int map_indicator(AdapterInfo *adapter, IndAddr *indicator);
 
 typedef SubchDev *(*css_subch_cb_func)(uint8_t m, uint8_t cssid, uint8_t ssid,
                                        uint16_t schid);
-void subch_device_save(SubchDev *s, QEMUFile *f);
-int subch_device_load(SubchDev *s, QEMUFile *f);
 int css_create_css_image(uint8_t cssid, bool default_image);
 bool css_devno_used(uint8_t cssid, uint8_t ssid, uint16_t devno);
 void css_subch_assign(uint8_t cssid, uint8_t ssid, uint16_t schid,
diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index f9e6890..caa6fc6 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -31,6 +31,11 @@ typedef struct AdapterRoutes {
     int gsi[ADAPTER_ROUTES_MAX_GSI];
 } AdapterRoutes;
 
+extern const VMStateDescription vmstate_adapter_routes;
+
+#define VMSTATE_ADAPTER_ROUTES(_f, _s) \
+    VMSTATE_STRUCT(_f, _s, 1, vmstate_adapter_routes, AdapterRoutes)
+
 #define TYPE_S390_FLIC_COMMON "s390-flic"
 #define S390_FLIC_COMMON(obj) \
     OBJECT_CHECK(S390FLICState, (obj), TYPE_S390_FLIC_COMMON)
-- 
2.7.4

[Qemu-devel] [PATCH 2/7] s390x/3270: fix instruction interception handler

[Christian Borntraeger]

From: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>

Commit bab482d7405f ("s390x/css: ccw translation infrastructure")
introduced instruction interception handler for different types of
subchannels. For emulated 3270 devices, we should assign the virtual
subchannel handler to them during device realization process, or 3270
will not work.

Fixes: bab482d7405f ("s390x/css: ccw translation infrastructure")

Reviewed-by: Jing Liu <liujbjl@linux.vnet.ibm.com>
Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/3270-ccw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/s390x/3270-ccw.c b/hw/s390x/3270-ccw.c
index 6e6eee4..1554aa2 100644
--- a/hw/s390x/3270-ccw.c
+++ b/hw/s390x/3270-ccw.c
@@ -126,6 +126,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds, Error **errp)
     sch->id.cu_type = EMULATED_CCW_3270_CU_TYPE;
     css_sch_build_virtual_schib(sch, (uint8_t)chpid,
                                 EMULATED_CCW_3270_CHPID_TYPE);
+    sch->do_subchannel_work = do_subchannel_work_virtual;
     sch->ccw_cb = emulated_ccw_3270_cb;
 
     ck->init(dev, &err);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/7] s390x/3270: fix instruction interception handler

[Cornelia Huck]

On Tue,  4 Jul 2017 16:07:54 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> 
> Commit bab482d7405f ("s390x/css: ccw translation infrastructure")
> introduced instruction interception handler for different types of
> subchannels. For emulated 3270 devices, we should assign the virtual
> subchannel handler to them during device realization process, or 3270
> will not work.
> 
> Fixes: bab482d7405f ("s390x/css: ccw translation infrastructure")
> 
> Reviewed-by: Jing Liu <liujbjl@linux.vnet.ibm.com>
> Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390x/3270-ccw.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/s390x/3270-ccw.c b/hw/s390x/3270-ccw.c
> index 6e6eee4..1554aa2 100644
> --- a/hw/s390x/3270-ccw.c
> +++ b/hw/s390x/3270-ccw.c
> @@ -126,6 +126,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds, Error **errp)
>      sch->id.cu_type = EMULATED_CCW_3270_CU_TYPE;
>      css_sch_build_virtual_schib(sch, (uint8_t)chpid,
>                                  EMULATED_CCW_3270_CHPID_TYPE);
> +    sch->do_subchannel_work = do_subchannel_work_virtual;
>      sch->ccw_cb = emulated_ccw_3270_cb;
>  
>      ck->init(dev, &err);

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize

[Christian Borntraeger]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

>From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
implement floating-interrupt controller device", 2013-07-16) the kvm-flic
is not making realize fail properly in case it's impossible to create the
KVM device which basically serves as a backend and is absolutely
essential for having an operational kvm-flic.

Let's fix this by making sure we do proper error propagation in realize.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/intc/s390_flic_kvm.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index b4c61d8..bea3997 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -15,6 +15,7 @@
 #include "cpu.h"
 #include <sys/ioctl.h>
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "hw/sysbus.h"
 #include "sysemu/kvm.h"
 #include "hw/s390x/s390_flic.h"
@@ -397,18 +398,22 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
     struct kvm_create_device cd = {0};
     struct kvm_device_attr test_attr = {0};
     int ret;
+    Error *errp_local = NULL;
 
     flic_state->fd = -1;
     if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
+        error_setg_errno(&errp_local, errno, "KVM is missing capability"
+                         " KVM_CAP_DEVICE_CTRL");
         trace_flic_no_device_api(errno);
-        return;
+        goto fail;
     }
 
     cd.type = KVM_DEV_TYPE_FLIC;
     ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
     if (ret < 0) {
-        trace_flic_create_device(errno);
-        return;
+        error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
+        trace_flic_no_device_api(errno);
+        goto fail;
     }
     flic_state->fd = cd.fd;
 
@@ -417,6 +422,9 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
     flic_state->clear_io_supported = !ioctl(flic_state->fd,
                                             KVM_HAS_DEVICE_ATTR, test_attr);
 
+    return;
+fail:
+    error_propagate(errp, errp_local);
 }
 
 static void kvm_s390_flic_reset(DeviceState *dev)
-- 
2.7.4

Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize

[Cornelia Huck]

On Tue,  4 Jul 2017 16:07:55 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> 
> From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
> implement floating-interrupt controller device", 2013-07-16) the kvm-flic
> is not making realize fail properly in case it's impossible to create the
> KVM device which basically serves as a backend and is absolutely
> essential for having an operational kvm-flic.
> 
> Let's fix this by making sure we do proper error propagation in realize.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/intc/s390_flic_kvm.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 

(...)

>      cd.type = KVM_DEV_TYPE_FLIC;
>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>      if (ret < 0) {
> -        trace_flic_create_device(errno);
> -        return;
> +        error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
> +        trace_flic_no_device_api(errno);

Err... this should still be trace_flic_create_device(), no?

> +        goto fail;
>      }
>      flic_state->fd = cd.fd;

Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize

[Halil Pasic]

On 07/04/2017 04:31 PM, Cornelia Huck wrote:
> On Tue,  4 Jul 2017 16:07:55 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> From: Halil Pasic <pasic@linux.vnet.ibm.com>
>>
>> From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
>> implement floating-interrupt controller device", 2013-07-16) the kvm-flic
>> is not making realize fail properly in case it's impossible to create the
>> KVM device which basically serves as a backend and is absolutely
>> essential for having an operational kvm-flic.
>>
>> Let's fix this by making sure we do proper error propagation in realize.
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  hw/intc/s390_flic_kvm.c | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
> 
> (...)
> 
>>      cd.type = KVM_DEV_TYPE_FLIC;
>>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>>      if (ret < 0) {
>> -        trace_flic_create_device(errno);
>> -        return;
>> +        error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
>> +        trace_flic_no_device_api(errno);
> 
> Err... this should still be trace_flic_create_device(), no?

I'm afraid you are right! Probably a copy paste error :/

> 
>> +        goto fail;
>>      }
>>      flic_state->fd = cd.fd;
> 

Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize

[Halil Pasic]

On 07/04/2017 04:46 PM, Halil Pasic wrote:
> 
> 
> On 07/04/2017 04:31 PM, Cornelia Huck wrote:
>> On Tue,  4 Jul 2017 16:07:55 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> From: Halil Pasic <pasic@linux.vnet.ibm.com>
>>>
>>> From the moment it was introduced by commit a2875e6f98 ("s390x/kvm:
>>> implement floating-interrupt controller device", 2013-07-16) the kvm-flic
>>> is not making realize fail properly in case it's impossible to create the
>>> KVM device which basically serves as a backend and is absolutely
>>> essential for having an operational kvm-flic.
>>>
>>> Let's fix this by making sure we do proper error propagation in realize.
>>>
>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>>> Fixes: a2875e6f98 "s390x/kvm: implement floating-interrupt controller device"
>>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>>> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  hw/intc/s390_flic_kvm.c | 14 +++++++++++---
>>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>>
>>
>> (...)
>>
>>>      cd.type = KVM_DEV_TYPE_FLIC;
>>>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>>>      if (ret < 0) {
>>> -        trace_flic_create_device(errno);
>>> -        return;
>>> +        error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
>>> +        trace_flic_no_device_api(errno);
>>
>> Err... this should still be trace_flic_create_device(), no?
> 
> I'm afraid you are right! Probably a copy paste error :/
> 

Do you think the traces are still appropriate once we have
proper error propagation?

I did not feel comfortable removing them but thinking again,
than might be the thing to do.

@Christian:
I think we should really fix this the one way or the other.
Can you tell me what is the proper procedure?

>>
>>> +        goto fail;
>>>      }
>>>      flic_state->fd = cd.fd;
>>
> 
> 

Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize

[Cornelia Huck]

On Tue, 4 Jul 2017 17:08:52 +0200
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> >>>      cd.type = KVM_DEV_TYPE_FLIC;
> >>>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
> >>>      if (ret < 0) {
> >>> -        trace_flic_create_device(errno);
> >>> -        return;
> >>> +        error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
> >>> +        trace_flic_no_device_api(errno);  
> >>
> >> Err... this should still be trace_flic_create_device(), no?  
> > 
> > I'm afraid you are right! Probably a copy paste error :/
> >   
> 
> Do you think the traces are still appropriate once we have
> proper error propagation?

They add less value, but I'd just keep them.

> 
> I did not feel comfortable removing them but thinking again,
> than might be the thing to do.
> 
> @Christian:
> I think we should really fix this the one way or the other.
> Can you tell me what is the proper procedure?

I'd vote for just fixing the trace event. Smaller change, and we can
revisit this later.

Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize

[Christian Borntraeger]

On 07/04/2017 06:59 PM, Cornelia Huck wrote:
> On Tue, 4 Jul 2017 17:08:52 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
>>>>>      cd.type = KVM_DEV_TYPE_FLIC;
>>>>>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>>>>>      if (ret < 0) {
>>>>> -        trace_flic_create_device(errno);
>>>>> -        return;
>>>>> +        error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
>>>>> +        trace_flic_no_device_api(errno);  
>>>>
>>>> Err... this should still be trace_flic_create_device(), no?  
>>>
>>> I'm afraid you are right! Probably a copy paste error :/
>>>   
>>
>> Do you think the traces are still appropriate once we have
>> proper error propagation?
> 
> They add less value, but I'd just keep them.
> 
>>
>> I did not feel comfortable removing them but thinking again,
>> than might be the thing to do.
>>
>> @Christian:
>> I think we should really fix this the one way or the other.
>> Can you tell me what is the proper procedure?
> 
> I'd vote for just fixing the trace event. Smaller change, and we can
> revisit this later.
> 

+1

[Qemu-devel] [PATCH 1/1] s390x: fixup for "fix error propagation in kvm-flic..."

[Halil Pasic]

My patch "s390x: fix error propagation in kvm-flic's realize" accidentally
replaced with. That's wrong! So please apply this fixup before including
that patch into mainline (or any other repo).

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---

Hope this is appropriate.
---
 hw/intc/s390_flic_kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 88f1555121..a587ace3df 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -467,7 +467,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
     ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
     if (ret < 0) {
         error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
-        trace_flic_no_device_api(errno);
+        trace_flic_create_device(errno);
         goto fail;
     }
     flic_state->fd = cd.fd;
-- 
2.11.2

Re: [Qemu-devel] [PATCH 1/1] s390x: fixup for "fix error propagation in kvm-flic..."

[Cornelia Huck]

On Wed,  5 Jul 2017 15:54:07 +0200
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> My patch "s390x: fix error propagation in kvm-flic's realize" accidentally
> replaced with. That's wrong! So please apply this fixup before including

This sentence is missing something ;)

> that patch into mainline (or any other repo).
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
> 
> Hope this is appropriate.

git actually has a 'fixup!' handling mechanism.

> ---
>  hw/intc/s390_flic_kvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> index 88f1555121..a587ace3df 100644
> --- a/hw/intc/s390_flic_kvm.c
> +++ b/hw/intc/s390_flic_kvm.c
> @@ -467,7 +467,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>      if (ret < 0) {
>          error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
> -        trace_flic_no_device_api(errno);
> +        trace_flic_create_device(errno);
>          goto fail;
>      }
>      flic_state->fd = cd.fd;

Acked-by: Cornelia Huck <cohuck@redhat.com>

Christian, will you handle this?

Re: [Qemu-devel] [PATCH 1/1] s390x: fixup for "fix error propagation in kvm-flic..."

[Christian Borntraeger]

On 07/05/2017 05:29 PM, Cornelia Huck wrote:
> On Wed,  5 Jul 2017 15:54:07 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
>> My patch "s390x: fix error propagation in kvm-flic's realize" accidentally
>> replaced with. That's wrong! So please apply this fixup before including
> 
> This sentence is missing something ;)
> 
>> that patch into mainline (or any other repo).
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> ---
>>
>> Hope this is appropriate.
> 
> git actually has a 'fixup!' handling mechanism.
> 
>> ---
>>  hw/intc/s390_flic_kvm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
>> index 88f1555121..a587ace3df 100644
>> --- a/hw/intc/s390_flic_kvm.c
>> +++ b/hw/intc/s390_flic_kvm.c
>> @@ -467,7 +467,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>>      if (ret < 0) {
>>          error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
>> -        trace_flic_no_device_api(errno);
>> +        trace_flic_create_device(errno);
>>          goto fail;
>>      }
>>      flic_state->fd = cd.fd;
> 
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> 
> Christian, will you handle this?

Yes, I will fold this into the original fix and send a pull request for the whole patch list.
Thanks for your review :-)

Re: [Qemu-devel] [PATCH 1/1] s390x: fixup for "fix error propagation in kvm-flic..."

[Halil Pasic]

On 07/05/2017 06:27 PM, Christian Borntraeger wrote:
> On 07/05/2017 05:29 PM, Cornelia Huck wrote:
>> On Wed,  5 Jul 2017 15:54:07 +0200
>> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
>>
>>> My patch "s390x: fix error propagation in kvm-flic's realize" accidentally
>>> replaced with. That's wrong! So please apply this fixup before including
>>
>> This sentence is missing something ;)
>>
>>> that patch into mainline (or any other repo).
>>>
>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>>> ---
>>>
>>> Hope this is appropriate.
>>
>> git actually has a 'fixup!' handling mechanism.
>>
>>> ---
>>>  hw/intc/s390_flic_kvm.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
>>> index 88f1555121..a587ace3df 100644
>>> --- a/hw/intc/s390_flic_kvm.c
>>> +++ b/hw/intc/s390_flic_kvm.c
>>> @@ -467,7 +467,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>>>      ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
>>>      if (ret < 0) {
>>>          error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
>>> -        trace_flic_no_device_api(errno);
>>> +        trace_flic_create_device(errno);
>>>          goto fail;
>>>      }
>>>      flic_state->fd = cd.fd;
>>
>> Acked-by: Cornelia Huck <cohuck@redhat.com>
>>
>> Christian, will you handle this?
> 
> Yes, I will fold this into the original fix and send a pull request for the whole patch list.
> Thanks for your review :-)
> 

Thanks Christian, Connie! I will try to remember git checkin --fixup (I was
not aware of this feature) next time, and of course try to avoid next time
from happening in the first.

Regards,
Halil

[Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic

[Christian Borntraeger]

From: Halil Pasic <pasic@linux.vnet.ibm.com>

Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
2016-12-09) introduces a common realize (intended to be common for all
the subclasses) for flic, but fails to make sure the kvm-flic which had
it's own is actually calling this common realize.

This omission fortunately does not result in a grave problem. The common
realize was only supposed to catch a possible programming mistake by
validating a value of a property set via the compat machine macros. Since
there was no programming mistake we don't need this fixed for stable.

Let's fix this problem by making sure kvm flic honors the realize of its
parent class.

Let us also improve on the error message we would hypothetically emit
when the validation fails.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/intc/s390_flic.c     |  4 ++--
 hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index a99a350..837158b 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp)
     uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
 
     if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
-        error_setg(errp, "flic adapter_routes_max_batch too big"
-                   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
+        error_setg(errp, "flic property adapter_routes_max_batch too big"
+                   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
     }
 }
 
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index bea3997..535d99d 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate = {
     }
 };
 
+typedef struct KVMS390FLICStateClass {
+    S390FLICStateClass parent_class;
+    DeviceRealize parent_realize;
+} KVMS390FLICStateClass;
+
+#define KVM_S390_FLIC_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
+
+#define KVM_S390_FLIC_CLASS(klass) \
+    OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
+
 static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
 {
     KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
@@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
     int ret;
     Error *errp_local = NULL;
 
+    KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
+    if (errp_local) {
+        goto fail;
+    }
     flic_state->fd = -1;
     if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
         error_setg_errno(&errp_local, errno, "KVM is missing capability"
@@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
     S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
 
+    KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
     dc->realize = kvm_s390_flic_realize;
     dc->vmsd = &kvm_s390_flic_vmstate;
     dc->reset = kvm_s390_flic_reset;
@@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
     .name          = TYPE_KVM_S390_FLIC,
     .parent        = TYPE_S390_FLIC_COMMON,
     .instance_size = sizeof(KVMS390FLICState),
+    .class_size    = sizeof(KVMS390FLICStateClass),
     .class_init    = kvm_s390_flic_class_init,
 };
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic

[Cornelia Huck]

On Tue,  4 Jul 2017 16:07:56 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> 
> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
> 2016-12-09) introduces a common realize (intended to be common for all
> the subclasses) for flic, but fails to make sure the kvm-flic which had
> it's own is actually calling this common realize.

s/it's/its/

> 
> This omission fortunately does not result in a grave problem. The common
> realize was only supposed to catch a possible programming mistake by
> validating a value of a property set via the compat machine macros. Since
> there was no programming mistake we don't need this fixed for stable.
> 
> Let's fix this problem by making sure kvm flic honors the realize of its
> parent class.
> 
> Let us also improve on the error message we would hypothetically emit
> when the validation fails.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/intc/s390_flic.c     |  4 ++--
>  hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> index a99a350..837158b 100644
> --- a/hw/intc/s390_flic.c
> +++ b/hw/intc/s390_flic.c
> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp)
>      uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
>  
>      if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
> -        error_setg(errp, "flic adapter_routes_max_batch too big"
> -                   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
> +        error_setg(errp, "flic property adapter_routes_max_batch too big"
> +                   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);

Unrelated message change?

>      }
>  }
>  
> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> index bea3997..535d99d 100644
> --- a/hw/intc/s390_flic_kvm.c
> +++ b/hw/intc/s390_flic_kvm.c
> @@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate = {
>      }
>  };
>  
> +typedef struct KVMS390FLICStateClass {
> +    S390FLICStateClass parent_class;
> +    DeviceRealize parent_realize;
> +} KVMS390FLICStateClass;
> +
> +#define KVM_S390_FLIC_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
> +
> +#define KVM_S390_FLIC_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
> +
>  static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>  {
>      KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
> @@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>      int ret;
>      Error *errp_local = NULL;
>  
> +    KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);

I usually prefer to introduce a local variable for that, but don't care
too much.

> +    if (errp_local) {
> +        goto fail;
> +    }
>      flic_state->fd = -1;
>      if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>          error_setg_errno(&errp_local, errno, "KVM is missing capability"
> @@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void *data)
>      DeviceClass *dc = DEVICE_CLASS(oc);
>      S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
>  
> +    KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;

dito

>      dc->realize = kvm_s390_flic_realize;
>      dc->vmsd = &kvm_s390_flic_vmstate;
>      dc->reset = kvm_s390_flic_reset;
> @@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
>      .name          = TYPE_KVM_S390_FLIC,
>      .parent        = TYPE_S390_FLIC_COMMON,
>      .instance_size = sizeof(KVMS390FLICState),
> +    .class_size    = sizeof(KVMS390FLICStateClass),
>      .class_init    = kvm_s390_flic_class_init,
>  };
>  

Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic

[Halil Pasic]

On 07/04/2017 04:37 PM, Cornelia Huck wrote:
> On Tue,  4 Jul 2017 16:07:56 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> From: Halil Pasic <pasic@linux.vnet.ibm.com>
>>
>> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
>> 2016-12-09) introduces a common realize (intended to be common for all
>> the subclasses) for flic, but fails to make sure the kvm-flic which had
>> it's own is actually calling this common realize.
> 
> s/it's/its/
> 

Valid. Sorry.

>>
>> This omission fortunately does not result in a grave problem. The common
>> realize was only supposed to catch a possible programming mistake by
>> validating a value of a property set via the compat machine macros. Since
>> there was no programming mistake we don't need this fixed for stable.
>>
>> Let's fix this problem by making sure kvm flic honors the realize of its
>> parent class.
>>
>> Let us also improve on the error message we would hypothetically emit
>> when the validation fails.
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  hw/intc/s390_flic.c     |  4 ++--
>>  hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
>>  2 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
>> index a99a350..837158b 100644
>> --- a/hw/intc/s390_flic.c
>> +++ b/hw/intc/s390_flic.c
>> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp)
>>      uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
>>  
>>      if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
>> -        error_setg(errp, "flic adapter_routes_max_batch too big"
>> -                   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>> +        error_setg(errp, "flic property adapter_routes_max_batch too big"
>> +                   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
> 
> Unrelated message change?
> 

I've mentioned it in the commit message. It was also introduced by the
patch I'm fixing. But yes strictly it's two different problems.


>>      }
>>  }
>>  
>> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
>> index bea3997..535d99d 100644
>> --- a/hw/intc/s390_flic_kvm.c
>> +++ b/hw/intc/s390_flic_kvm.c
>> @@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate = {
>>      }
>>  };
>>  
>> +typedef struct KVMS390FLICStateClass {
>> +    S390FLICStateClass parent_class;
>> +    DeviceRealize parent_realize;
>> +} KVMS390FLICStateClass;
>> +
>> +#define KVM_S390_FLIC_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
>> +
>> +#define KVM_S390_FLIC_CLASS(klass) \
>> +    OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
>> +
>>  static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>>  {
>>      KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
>> @@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>>      int ret;
>>      Error *errp_local = NULL;
>>  
>> +    KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
> 
> I usually prefer to introduce a local variable for that, but don't care
> too much.
> 
>> +    if (errp_local) {
>> +        goto fail;
>> +    }
>>      flic_state->fd = -1;
>>      if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>>          error_setg_errno(&errp_local, errno, "KVM is missing capability"
>> @@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void *data)
>>      DeviceClass *dc = DEVICE_CLASS(oc);
>>      S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
>>  
>> +    KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
> 
> dito
> 
>>      dc->realize = kvm_s390_flic_realize;
>>      dc->vmsd = &kvm_s390_flic_vmstate;
>>      dc->reset = kvm_s390_flic_reset;
>> @@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
>>      .name          = TYPE_KVM_S390_FLIC,
>>      .parent        = TYPE_S390_FLIC_COMMON,
>>      .instance_size = sizeof(KVMS390FLICState),
>> +    .class_size    = sizeof(KVMS390FLICStateClass),
>>      .class_init    = kvm_s390_flic_class_init,
>>  };
>>  
> 
> 

Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic

[Christian Borntraeger]

On 07/04/2017 04:51 PM, Halil Pasic wrote:
> 
> 
> On 07/04/2017 04:37 PM, Cornelia Huck wrote:
>> On Tue,  4 Jul 2017 16:07:56 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> From: Halil Pasic <pasic@linux.vnet.ibm.com>
>>>
>>> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
>>> 2016-12-09) introduces a common realize (intended to be common for all
>>> the subclasses) for flic, but fails to make sure the kvm-flic which had
>>> it's own is actually calling this common realize.
>>
>> s/it's/its/
>>
> 
> Valid. Sorry.
> 
>>>
>>> This omission fortunately does not result in a grave problem. The common
>>> realize was only supposed to catch a possible programming mistake by
>>> validating a value of a property set via the compat machine macros. Since
>>> there was no programming mistake we don't need this fixed for stable.
>>>
>>> Let's fix this problem by making sure kvm flic honors the realize of its
>>> parent class.
>>>
>>> Let us also improve on the error message we would hypothetically emit
>>> when the validation fails.
>>>
>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>>> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
>>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>>> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  hw/intc/s390_flic.c     |  4 ++--
>>>  hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
>>>  2 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
>>> index a99a350..837158b 100644
>>> --- a/hw/intc/s390_flic.c
>>> +++ b/hw/intc/s390_flic.c
>>> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp)
>>>      uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
>>>  
>>>      if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
>>> -        error_setg(errp, "flic adapter_routes_max_batch too big"
>>> -                   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>>> +        error_setg(errp, "flic property adapter_routes_max_batch too big"
>>> +                   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>>
>> Unrelated message change?
>>
> 
> I've mentioned it in the commit message. It was also introduced by the
> patch I'm fixing. But yes strictly it's two different problems.

I will only fix the patch description ( s/it's/its/) and keep the other things
unchanged. Is that fine with you?
> 
> 
>>>      }
>>>  }
>>>  
>>> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
>>> index bea3997..535d99d 100644
>>> --- a/hw/intc/s390_flic_kvm.c
>>> +++ b/hw/intc/s390_flic_kvm.c
>>> @@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate = {
>>>      }
>>>  };
>>>  
>>> +typedef struct KVMS390FLICStateClass {
>>> +    S390FLICStateClass parent_class;
>>> +    DeviceRealize parent_realize;
>>> +} KVMS390FLICStateClass;
>>> +
>>> +#define KVM_S390_FLIC_GET_CLASS(obj) \
>>> +    OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
>>> +
>>> +#define KVM_S390_FLIC_CLASS(klass) \
>>> +    OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
>>> +
>>>  static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>>>  {
>>>      KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
>>> @@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>>>      int ret;
>>>      Error *errp_local = NULL;
>>>  
>>> +    KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
>>
>> I usually prefer to introduce a local variable for that, but don't care
>> too much.
>>
>>> +    if (errp_local) {
>>> +        goto fail;
>>> +    }
>>>      flic_state->fd = -1;
>>>      if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>>>          error_setg_errno(&errp_local, errno, "KVM is missing capability"
>>> @@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc, void *data)
>>>      DeviceClass *dc = DEVICE_CLASS(oc);
>>>      S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
>>>  
>>> +    KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
>>
>> dito
>>
>>>      dc->realize = kvm_s390_flic_realize;
>>>      dc->vmsd = &kvm_s390_flic_vmstate;
>>>      dc->reset = kvm_s390_flic_reset;
>>> @@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
>>>      .name          = TYPE_KVM_S390_FLIC,
>>>      .parent        = TYPE_S390_FLIC_COMMON,
>>>      .instance_size = sizeof(KVMS390FLICState),
>>> +    .class_size    = sizeof(KVMS390FLICStateClass),
>>>      .class_init    = kvm_s390_flic_class_init,
>>>  };
>>>  
>>
>>

Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic

[Halil Pasic]

On 07/05/2017 12:20 PM, Christian Borntraeger wrote:
> On 07/04/2017 04:51 PM, Halil Pasic wrote:
>>
>>
>> On 07/04/2017 04:37 PM, Cornelia Huck wrote:
>>> On Tue,  4 Jul 2017 16:07:56 +0200
>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>
>>>> From: Halil Pasic <pasic@linux.vnet.ibm.com>
>>>>
>>>> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
>>>> 2016-12-09) introduces a common realize (intended to be common for all
>>>> the subclasses) for flic, but fails to make sure the kvm-flic which had
>>>> it's own is actually calling this common realize.
>>>
>>> s/it's/its/
>>>
>>
>> Valid. Sorry.
>>
>>>>
>>>> This omission fortunately does not result in a grave problem. The common
>>>> realize was only supposed to catch a possible programming mistake by
>>>> validating a value of a property set via the compat machine macros. Since
>>>> there was no programming mistake we don't need this fixed for stable.
>>>>
>>>> Let's fix this problem by making sure kvm flic honors the realize of its
>>>> parent class.
>>>>
>>>> Let us also improve on the error message we would hypothetically emit
>>>> when the validation fails.
>>>>
>>>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>>>> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
>>>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>>>> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> ---
>>>>  hw/intc/s390_flic.c     |  4 ++--
>>>>  hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
>>>>  2 files changed, 19 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
>>>> index a99a350..837158b 100644
>>>> --- a/hw/intc/s390_flic.c
>>>> +++ b/hw/intc/s390_flic.c
>>>> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp)
>>>>      uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
>>>>  
>>>>      if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
>>>> -        error_setg(errp, "flic adapter_routes_max_batch too big"
>>>> -                   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>>>> +        error_setg(errp, "flic property adapter_routes_max_batch too big"
>>>> +                   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>>>
>>> Unrelated message change?
>>>
>>
>> I've mentioned it in the commit message. It was also introduced by the
>> patch I'm fixing. But yes strictly it's two different problems.
> 
> I will only fix the patch description ( s/it's/its/) and keep the other things
> unchanged. Is that fine with you?

It's fine with me, but I guess you probably asked Connie. Thanks!

Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic

[Cornelia Huck]

On Wed, 5 Jul 2017 12:20:42 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 07/04/2017 04:51 PM, Halil Pasic wrote:
> > 
> > 
> > On 07/04/2017 04:37 PM, Cornelia Huck wrote:  
> >> On Tue,  4 Jul 2017 16:07:56 +0200
> >> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >>  
> >>> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> >>>
> >>> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
> >>> 2016-12-09) introduces a common realize (intended to be common for all
> >>> the subclasses) for flic, but fails to make sure the kvm-flic which had
> >>> it's own is actually calling this common realize.  
> >>
> >> s/it's/its/
> >>  
> > 
> > Valid. Sorry.
> >   
> >>>
> >>> This omission fortunately does not result in a grave problem. The common
> >>> realize was only supposed to catch a possible programming mistake by
> >>> validating a value of a property set via the compat machine macros. Since
> >>> there was no programming mistake we don't need this fixed for stable.
> >>>
> >>> Let's fix this problem by making sure kvm flic honors the realize of its
> >>> parent class.
> >>>
> >>> Let us also improve on the error message we would hypothetically emit
> >>> when the validation fails.
> >>>
> >>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> >>> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
> >>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> >>> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>> ---
> >>>  hw/intc/s390_flic.c     |  4 ++--
> >>>  hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
> >>>  2 files changed, 19 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> >>> index a99a350..837158b 100644
> >>> --- a/hw/intc/s390_flic.c
> >>> +++ b/hw/intc/s390_flic.c
> >>> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp)
> >>>      uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
> >>>  
> >>>      if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
> >>> -        error_setg(errp, "flic adapter_routes_max_batch too big"
> >>> -                   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
> >>> +        error_setg(errp, "flic property adapter_routes_max_batch too big"
> >>> +                   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);  
> >>
> >> Unrelated message change?
> >>  
> > 
> > I've mentioned it in the commit message. It was also introduced by the
> > patch I'm fixing. But yes strictly it's two different problems.  
> 
> I will only fix the patch description ( s/it's/its/) and keep the other things
> unchanged. Is that fine with you?

Yup. Let's get this done.

With the changes,

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic

[Cornelia Huck]

On Wed, 5 Jul 2017 13:11:59 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 5 Jul 2017 12:20:42 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
> > On 07/04/2017 04:51 PM, Halil Pasic wrote:  
> > > 
> > > 
> > > On 07/04/2017 04:37 PM, Cornelia Huck wrote:    
> > >> On Tue,  4 Jul 2017 16:07:56 +0200
> > >> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > >>    
> > >>> From: Halil Pasic <pasic@linux.vnet.ibm.com>
> > >>>
> > >>> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
> > >>> 2016-12-09) introduces a common realize (intended to be common for all
> > >>> the subclasses) for flic, but fails to make sure the kvm-flic which had
> > >>> it's own is actually calling this common realize.    
> > >>
> > >> s/it's/its/
> > >>    
> > > 
> > > Valid. Sorry.
> > >     
> > >>>
> > >>> This omission fortunately does not result in a grave problem. The common
> > >>> realize was only supposed to catch a possible programming mistake by
> > >>> validating a value of a property set via the compat machine macros. Since
> > >>> there was no programming mistake we don't need this fixed for stable.
> > >>>
> > >>> Let's fix this problem by making sure kvm flic honors the realize of its
> > >>> parent class.
> > >>>
> > >>> Let us also improve on the error message we would hypothetically emit
> > >>> when the validation fails.
> > >>>
> > >>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> > >>> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
> > >>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> > >>> Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
> > >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > >>> ---
> > >>>  hw/intc/s390_flic.c     |  4 ++--
> > >>>  hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
> > >>>  2 files changed, 19 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> > >>> index a99a350..837158b 100644
> > >>> --- a/hw/intc/s390_flic.c
> > >>> +++ b/hw/intc/s390_flic.c
> > >>> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp)
> > >>>      uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
> > >>>  
> > >>>      if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
> > >>> -        error_setg(errp, "flic adapter_routes_max_batch too big"
> > >>> -                   "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
> > >>> +        error_setg(errp, "flic property adapter_routes_max_batch too big"
> > >>> +                   " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);    
> > >>
> > >> Unrelated message change?
> > >>    
> > > 
> > > I've mentioned it in the commit message. It was also introduced by the
> > > patch I'm fixing. But yes strictly it's two different problems.    
> > 
> > I will only fix the patch description ( s/it's/its/) and keep the other things
> > unchanged. Is that fine with you?  
> 
> Yup. Let's get this done.
> 
> With the changes,

s/changes/change/

One coffee is obviously not enough...

> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[Qemu-devel] [PATCH 5/7] s390x/MAINTAINERS: Update my email address

[Christian Borntraeger]

From: Cornelia Huck <cohuck@redhat.com>

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <20170704092215.13742-2-cohuck@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 MAINTAINERS | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 839f7ca..4e17216 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -299,7 +299,7 @@ F: target/ppc/kvm.c
 
 S390
 M: Christian Borntraeger <borntraeger@de.ibm.com>
-M: Cornelia Huck <cornelia.huck@de.ibm.com>
+M: Cornelia Huck <cohuck@redhat.com>
 M: Alexander Graf <agraf@suse.de>
 S: Maintained
 F: target/s390x/kvm.c
@@ -778,7 +778,7 @@ F: include/hw/sparc/grlib.h
 S390 Machines
 -------------
 S390 Virtio-ccw
-M: Cornelia Huck <cornelia.huck@de.ibm.com>
+M: Cornelia Huck <cohuck@redhat.com>
 M: Christian Borntraeger <borntraeger@de.ibm.com>
 M: Alexander Graf <agraf@suse.de>
 S: Supported
@@ -1006,7 +1006,7 @@ F: hw/vfio/*
 F: include/hw/vfio/
 
 vfio-ccw
-M: Cornelia Huck <cornelia.huck@de.ibm.com>
+M: Cornelia Huck <cohuck@redhat.com>
 S: Supported
 F: hw/vfio/ccw.c
 F: hw/s390x/s390-ccw.c
@@ -1048,7 +1048,7 @@ F: tests/virtio-blk-test.c
 T: git git://github.com/stefanha/qemu.git block
 
 virtio-ccw
-M: Cornelia Huck <cornelia.huck@de.ibm.com>
+M: Cornelia Huck <cohuck@redhat.com>
 M: Christian Borntraeger <borntraeger@de.ibm.com>
 S: Supported
 F: hw/s390x/virtio-ccw.[hc]
-- 
2.7.4

[Qemu-devel] [PATCH 6/7] s390x: return unavailable features via query-cpu-definitions

[Christian Borntraeger]

From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

The response for query-cpu-definitions didn't include the
unavailable-features field, which is used by libvirt to figure
out whether a certain cpu model is usable on the host.

The unavailable features are now computed by obtaining the host CPU
model and comparing it against the known CPU models. The comparison
takes into account the generation, the GA level and the feature
bitmaps. In the case of a CPU generation/GA level mismatch
a feature called "type" is reported to be missing.

As a result, the output of virsh domcapabilities would change
from something like
 ...
     <mode name='custom' supported='yes'>
      <model usable='unknown'>z10EC-base</model>
      <model usable='unknown'>z9EC-base</model>
      <model usable='unknown'>z196.2-base</model>
      <model usable='unknown'>z900-base</model>
      <model usable='unknown'>z990</model>
 ...
to
 ...
     <mode name='custom' supported='yes'>
      <model usable='yes'>z10EC-base</model>
      <model usable='yes'>z9EC-base</model>
      <model usable='no'>z196.2-base</model>
      <model usable='yes'>z900-base</model>
      <model usable='yes'>z990</model>
 ...

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Message-Id: <1499082529-16970-1-git-send-email-mihajlov@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target/s390x/cpu_models.c | 62 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 5 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 63903c2..7cb55dc 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -283,10 +283,41 @@ void s390_cpu_list(FILE *f, fprintf_function print)
     }
 }
 
+static S390CPUModel *get_max_cpu_model(Error **errp);
+
 #ifndef CONFIG_USER_ONLY
+static void list_add_feat(const char *name, void *opaque);
+
+static void check_unavailable_features(const S390CPUModel *max_model,
+                                       const S390CPUModel *model,
+                                       strList **unavailable)
+{
+    S390FeatBitmap missing;
+
+    /* check general model compatibility */
+    if (max_model->def->gen < model->def->gen ||
+        (max_model->def->gen == model->def->gen &&
+         max_model->def->ec_ga < model->def->ec_ga)) {
+        list_add_feat("type", unavailable);
+    }
+
+    /* detect missing features if any to properly report them */
+    bitmap_andnot(missing, model->features, max_model->features,
+                  S390_FEAT_MAX);
+    if (!bitmap_empty(missing, S390_FEAT_MAX)) {
+        s390_feat_bitmap_to_ascii(missing, unavailable, list_add_feat);
+    }
+}
+
+struct CpuDefinitionInfoListData {
+    CpuDefinitionInfoList *list;
+    S390CPUModel *model;
+};
+
 static void create_cpu_model_list(ObjectClass *klass, void *opaque)
 {
-    CpuDefinitionInfoList **cpu_list = opaque;
+    struct CpuDefinitionInfoListData *cpu_list_data = opaque;
+    CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
     CpuDefinitionInfoList *entry;
     CpuDefinitionInfo *info;
     char *name = g_strdup(object_class_get_name(klass));
@@ -300,7 +331,19 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
     info->migration_safe = scc->is_migration_safe;
     info->q_static = scc->is_static;
     info->q_typename = g_strdup(object_class_get_name(klass));
-
+    /* check for unavailable features */
+    if (cpu_list_data->model) {
+        Object *obj;
+        S390CPU *sc;
+        obj = object_new(object_class_get_name(klass));
+        sc = S390_CPU(obj);
+        if (sc->model) {
+            info->has_unavailable_features = true;
+            check_unavailable_features(cpu_list_data->model, sc->model,
+                                       &info->unavailable_features);
+        }
+        object_unref(obj);
+    }
 
     entry = g_malloc0(sizeof(*entry));
     entry->value = info;
@@ -310,11 +353,20 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
-    CpuDefinitionInfoList *list = NULL;
+    struct CpuDefinitionInfoListData list_data = {
+        .list = NULL,
+    };
 
-    object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false, &list);
+    list_data.model = get_max_cpu_model(errp);
+    if (*errp) {
+        error_free(*errp);
+        *errp = NULL;
+    }
 
-    return list;
+    object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
+                         &list_data);
+
+    return list_data.list;
 }
 
 static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
-- 
2.7.4

Re: [Qemu-devel] [PATCH 6/7] s390x: return unavailable features via query-cpu-definitions

[Cornelia Huck]

On Tue,  4 Jul 2017 16:07:58 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> 
> The response for query-cpu-definitions didn't include the
> unavailable-features field, which is used by libvirt to figure
> out whether a certain cpu model is usable on the host.
> 
> The unavailable features are now computed by obtaining the host CPU
> model and comparing it against the known CPU models. The comparison
> takes into account the generation, the GA level and the feature
> bitmaps. In the case of a CPU generation/GA level mismatch
> a feature called "type" is reported to be missing.
> 
> As a result, the output of virsh domcapabilities would change
> from something like
>  ...
>      <mode name='custom' supported='yes'>
>       <model usable='unknown'>z10EC-base</model>
>       <model usable='unknown'>z9EC-base</model>
>       <model usable='unknown'>z196.2-base</model>
>       <model usable='unknown'>z900-base</model>
>       <model usable='unknown'>z990</model>
>  ...
> to
>  ...
>      <mode name='custom' supported='yes'>
>       <model usable='yes'>z10EC-base</model>
>       <model usable='yes'>z9EC-base</model>
>       <model usable='no'>z196.2-base</model>
>       <model usable='yes'>z900-base</model>
>       <model usable='yes'>z990</model>
>  ...
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
> Message-Id: <1499082529-16970-1-git-send-email-mihajlov@linux.vnet.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  target/s390x/cpu_models.c | 62 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 57 insertions(+), 5 deletions(-)

Acked-by: Cornelia Huck <cohuck@redhat.com>

[Qemu-devel] [PATCH 7/7] virtio-scsi-ccw: use ioeventfd even when KVM is disabled

[Christian Borntraeger]

From: QingFeng Hao <haoqf@linux.vnet.ibm.com>

This patch is based on a similar patch from Stefan Hajnoczi -
commit c324fd0a39c ("virtio-pci: use ioeventfd even when KVM is disabled")

Do not check kvm_eventfds_enabled() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c17777d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-scsi-ccw,iothread=iothread0 work even
when KVM is disabled.
Currently we don't have an equivalent to "memory: emulate ioeventfd"
for ccw yet, but that this doesn't hurt and qemu-iotests 068 can pass with
skipping iothread arguments.

I have tested that virtio-scsi-ccw works under tcg both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Signed-off-by: QingFeng Hao <haoqf@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <20170704132350.11874-2-haoqf@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 2 +-
 target/s390x/cpu.h    | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index f0e7fc8..e18fd26 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -789,7 +789,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
         sch->cssid, sch->ssid, sch->schid, sch->devno,
         ccw_dev->devno.valid ? "user-configured" : "auto-configured");
 
-    if (!kvm_eventfds_enabled()) {
+    if (kvm_enabled() && !kvm_eventfds_enabled()) {
         dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
     }
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 9faca04..bdb9bdb 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1264,7 +1264,11 @@ static inline int s390_assign_subch_ioeventfd(EventNotifier *notifier,
                                               uint32_t sch_id, int vq,
                                               bool assign)
 {
-    return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+    if (kvm_enabled()) {
+        return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+    } else {
+        return 0;
+    }
 }
 
 static inline void s390_crypto_reset(void)
-- 
2.7.4