[Qemu-devel] [PATCH for-2.10 00/10] s390x queue for 2.10

[Qemu-devel] [PATCH for-2.10 00/10] s390x queue for 2.10

[Cornelia Huck]

Here's the first batch of s390x changes for 2.10:
- the new compat machine
- several cleanups and optimizations
- introspection for css ids

More to come; for example, the 3270 support I'd still like some
feedback on (I'll resend a rebased version).

Cornelia Huck (1):
  s390x: introduce 2.10 compat machine

Danil Antonov (2):
  s390x/kvm: make printf always compile in debug output
  s390x/pci: make printf always compile in debug output

Dong Jia Shi (3):
  s390x/css: introduce read-only property type for device ids
  s390x/css: provide introspection for virtual subchannel and device
    busid
  s390x/css: consolidate the devno property for ccw devices

Fei Li (4):
  s390x: use enum for adapter type and standardize its naming
  s390x: initialize flic before I/O subsystems
  s390x/flic: cache flic in s390_get_flic
  s390x: register I/O adapters per ISC during init

 hw/intc/s390_flic.c        |  9 +++--
 hw/s390x/ccw-device.c      | 40 ++++++++++++++++++++
 hw/s390x/ccw-device.h      |  9 ++++-
 hw/s390x/css-bridge.c      |  3 ++
 hw/s390x/css.c             | 92 ++++++++++++++++++++++++++++++----------------
 hw/s390x/s390-pci-bus.c    | 19 +++++++---
 hw/s390x/s390-pci-bus.h    |  1 -
 hw/s390x/s390-pci-inst.c   | 26 +++++++------
 hw/s390x/s390-virtio-ccw.c | 20 +++++++++-
 hw/s390x/virtio-ccw.c      | 55 ++++++++++++++-------------
 include/hw/compat.h        |  3 ++
 include/hw/s390x/css.h     | 19 ++++++++--
 target/s390x/kvm.c         | 16 ++++----
 13 files changed, 219 insertions(+), 93 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 01/10] s390x: introduce 2.10 compat machine

[Cornelia Huck]

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 17 ++++++++++++++++-
 include/hw/compat.h        |  3 +++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 40914fde6f..5ac315ac78 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -336,6 +336,9 @@ static const TypeInfo ccw_machine_info = {
     }                                                                         \
     type_init(ccw_machine_register_##suffix)
 
+#define CCW_COMPAT_2_9 \
+        HW_COMPAT_2_9
+
 #define CCW_COMPAT_2_8 \
         HW_COMPAT_2_8 \
         {\
@@ -402,14 +405,26 @@ static const TypeInfo ccw_machine_info = {
             .value    = "0",\
         },
 
+static void ccw_machine_2_10_instance_options(MachineState *machine)
+{
+}
+
+static void ccw_machine_2_10_class_options(MachineClass *mc)
+{
+}
+DEFINE_CCW_MACHINE(2_10, "2.10", true);
+
 static void ccw_machine_2_9_instance_options(MachineState *machine)
 {
+    ccw_machine_2_10_instance_options(machine);
 }
 
 static void ccw_machine_2_9_class_options(MachineClass *mc)
 {
+    ccw_machine_2_10_class_options(mc);
+    SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_9);
 }
-DEFINE_CCW_MACHINE(2_9, "2.9", true);
+DEFINE_CCW_MACHINE(2_9, "2.9", false);
 
 static void ccw_machine_2_8_instance_options(MachineState *machine)
 {
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 5d5be91daf..846b90eb67 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,9 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
+#define HW_COMPAT_2_9 \
+    /* empty */
+
 #define HW_COMPAT_2_8 \
     {\
         .driver   = "fw_cfg_mem",\
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 02/10] s390x/kvm: make printf always compile in debug output

[Cornelia Huck]

From: Danil Antonov <g.danil.anto@gmail.com>

Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.

Signed-off-by: Danil Antonov <g.danil.anto@gmail.com>
Message-Id: <CA+KKJYAhsuTodm3s2rK65hR=-Xi5+Z7Q+M2nJYZQf2wa44HfOg@mail.gmail.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 target/s390x/kvm.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index ac47154b83..1a249d8359 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -47,16 +47,16 @@
 #include "exec/memattrs.h"
 #include "hw/s390x/s390-virtio-ccw.h"
 
-/* #define DEBUG_KVM */
-
-#ifdef DEBUG_KVM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
+#ifndef DEBUG_KVM
+#define DEBUG_KVM  0
 #endif
 
+#define DPRINTF(fmt, ...) do {                \
+    if (DEBUG_KVM) {                          \
+        fprintf(stderr, fmt, ## __VA_ARGS__); \
+    }                                         \
+} while (0);
+
 #define kvm_vm_check_mem_attr(s, attr) \
     kvm_vm_check_attr(s, KVM_S390_VM_MEM_CTRL, attr)
 
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 03/10] s390x/pci: make printf always compile in debug output

[Cornelia Huck]

From: Danil Antonov <g.danil.anto@gmail.com>

Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
This will ensure that printf function will always compile even if debug
output is turned off and, in turn, will prevent bitrot of the format
strings.

Signed-off-by: Danil Antonov <g.danil.anto@gmail.com>
Message-Id: <CA+KKJYBi31Bs7DtVdzZdwG2t+u5+FGiAhQpd3pqJzUX1O8Cprg@mail.gmail.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-pci-bus.c  | 16 ++++++++++------
 hw/s390x/s390-pci-inst.c | 16 ++++++++++------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 69b0291e8a..0f62363434 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -24,14 +24,18 @@
 #include "qemu/error-report.h"
 
 /* #define DEBUG_S390PCI_BUS */
-#ifdef DEBUG_S390PCI_BUS
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
+
+#ifndef DEBUG_S390PCI_BUS
+#define DEBUG_S390PCI_BUS  0
 #endif
 
+#define DPRINTF(fmt, ...)                                         \
+    do {                                                          \
+        if (DEBUG_S390PCI_BUS) {                                  \
+            fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); \
+        }                                                         \
+    } while (0)
+
 S390pciState *s390_get_phb(void)
 {
     static S390pciState *phb;
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index d2a8c0a083..763eebd67f 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -21,14 +21,18 @@
 #include "sysemu/hw_accel.h"
 
 /* #define DEBUG_S390PCI_INST */
-#ifdef DEBUG_S390PCI_INST
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, "s390pci-inst: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
+
+#ifndef DEBUG_S390PCI_INST
+#define DEBUG_S390PCI_INST  0
 #endif
 
+#define DPRINTF(fmt, ...)                                          \
+    do {                                                           \
+        if (DEBUG_S390PCI_INST) {                                  \
+            fprintf(stderr, "s390pci-inst: " fmt, ## __VA_ARGS__); \
+        }                                                          \
+    } while (0)
+
 static void s390_set_status_code(CPUS390XState *env,
                                  uint8_t r, uint64_t status_code)
 {
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 04/10] s390x/css: introduce read-only property type for device ids

[Cornelia Huck]

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

Let's introduce a read-only property type that handles device ids of the
CssDevId type used for channel devices for future use. e.g. exposing the
busid of an I/O subchannel that is assigned to a ccw device.

Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css.c         | 7 +++++++
 include/hw/s390x/css.h | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 37caa98195..f966ce2d15 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -1894,6 +1894,13 @@ PropertyInfo css_devid_propinfo = {
     .set = set_css_devid,
 };
 
+PropertyInfo css_devid_ro_propinfo = {
+    .name = "str",
+    .description = "Read-only identifier of an I/O device in the channel "
+                   "subsystem, example: fe.1.23ab",
+    .get = get_css_devid,
+};
+
 SubchDev *css_create_virtual_sch(CssDevId bus_id, Error **errp)
 {
     uint16_t schid = 0;
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index c96c862057..e88f24b868 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -172,6 +172,11 @@ extern PropertyInfo css_devid_propinfo;
 #define DEFINE_PROP_CSS_DEV_ID(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, css_devid_propinfo, CssDevId)
 
+extern PropertyInfo css_devid_ro_propinfo;
+
+#define DEFINE_PROP_CSS_DEV_ID_RO(_n, _s, _f) \
+    DEFINE_PROP(_n, _s, _f, css_devid_ro_propinfo, CssDevId)
+
 /**
  * Create a subchannel for the given bus id.
  *
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 05/10] s390x/css: provide introspection for virtual subchannel and device busid

[Cornelia Huck]

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

Expose the busids of the virtual I/O subchannel and the virtual CCW
device to ease debugging. This is needed because:
1. subchannel id are assigned dynamically, and cannot be set from
   outside.
2. device busid could possibly be auto generated.

An example of using HMP to retrieve the property values of a
virtio-balloon-ccw device looks like:

[root@localhost ~]# lscss -d 0.0.0004
Device   Subchan.  DevType CU Type Use  PIM PAM POM  CHPIDs
----------------------------------------------------------------------
0.0.0004 0.0.0003  0000/00 3832/05 yes  80  80  ff   00000000 00000000

(qemu) info qtree
... ...
      dev: virtio-balloon-ccw, id "balloon0"
        devno = "<unset>"
        ioeventfd = true
        max_revision = 2 (0x2)
        dev_id = "fe.0.0004"
        subch_id = "fe.0.0003"
... ...

After migration, if we have the same device that shows up on a
different subchannel, we must re-fill the subch_id of the ccw
device with the new schid, or the subch_id will have an old wrong
schid value. So this also re-fills the subch_id after migration.

While we are at it, also neaten the related error handling a bit.

Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/ccw-device.c | 39 +++++++++++++++++++++++++++++++++++++++
 hw/s390x/ccw-device.h |  7 +++++++
 hw/s390x/virtio-ccw.c | 28 ++++++++++++++++++++++------
 3 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index 28ea20440e..8fe3cf49ef 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -11,11 +11,50 @@
 #include "qemu/osdep.h"
 #include "ccw-device.h"
 
+static void ccw_device_refill_ids(CcwDevice *dev)
+{
+    SubchDev *sch = dev->sch;
+
+    assert(sch);
+
+    dev->dev_id.cssid = sch->cssid;
+    dev->dev_id.ssid = sch->ssid;
+    dev->dev_id.devid = sch->devno;
+    dev->dev_id.valid = true;
+
+    dev->subch_id.cssid = sch->cssid;
+    dev->subch_id.ssid = sch->ssid;
+    dev->subch_id.devid = sch->schid;
+    dev->subch_id.valid = true;
+}
+
+static void ccw_device_realize(CcwDevice *dev, Error **errp)
+{
+    ccw_device_refill_ids(dev);
+}
+
+static Property ccw_device_properties[] = {
+    DEFINE_PROP_CSS_DEV_ID_RO("dev_id", CcwDevice, dev_id),
+    DEFINE_PROP_CSS_DEV_ID_RO("subch_id", CcwDevice, subch_id),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ccw_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    CCWDeviceClass *k = CCW_DEVICE_CLASS(klass);
+
+    k->realize = ccw_device_realize;
+    k->refill_ids = ccw_device_refill_ids;
+    dc->props = ccw_device_properties;
+}
+
 static const TypeInfo ccw_device_info = {
     .name = TYPE_CCW_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(CcwDevice),
     .class_size = sizeof(CCWDeviceClass),
+    .class_init = ccw_device_class_init,
     .abstract = true,
 };
 
diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 59ba01b6c5..48700fe23a 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -19,12 +19,19 @@ typedef struct CcwDevice {
     DeviceState parent_obj;
     SubchDev *sch;
     /* <cssid>.<ssid>.<device number> */
+    /* The user-set busid of the virtual ccw device. */
     CssDevId bus_id;
+    /* The actual busid of the virtual ccw device. */
+    CssDevId dev_id;
+    /* The actual busid of the virtual subchannel. */
+    CssDevId subch_id;
 } CcwDevice;
 
 typedef struct CCWDeviceClass {
     DeviceClass parent_class;
     void (*unplug)(HotplugHandler *, DeviceState *, Error **);
+    void (*realize)(CcwDevice *, Error **);
+    void (*refill_ids)(CcwDevice *);
 } CCWDeviceClass;
 
 static inline CcwDevice *to_ccw_dev_fast(DeviceState *d)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 00b3bde4e9..4e59e34d74 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -680,6 +680,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
 {
     VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
     CcwDevice *ccw_dev = CCW_DEVICE(dev);
+    CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
     SubchDev *sch = css_create_virtual_sch(ccw_dev->bus_id, errp);
     Error *err = NULL;
 
@@ -689,8 +690,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
     if (!virtio_ccw_rev_max(dev) && dev->force_revision_1) {
         error_setg(&err, "Invalid value of property max_rev "
                    "(is %d expected >= 1)", virtio_ccw_rev_max(dev));
-        error_propagate(errp, err);
-        return;
+        goto out_err;
     }
 
     sch->driver_data = dev;
@@ -713,13 +713,24 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
 
     if (k->realize) {
         k->realize(dev, &err);
+        if (err) {
+            goto out_err;
+        }
     }
+
+    ck->realize(ccw_dev, &err);
     if (err) {
-        error_propagate(errp, err);
-        css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
-        ccw_dev->sch = NULL;
-        g_free(sch);
+        goto out_err;
     }
+
+    return;
+
+out_err:
+    error_propagate(errp, err);
+    css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
+    ccw_dev->sch = NULL;
+    g_free(sch);
+    return;
 }
 
 static int virtio_ccw_exit(VirtioCcwDevice *dev)
@@ -1261,12 +1272,17 @@ 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;
 
     s->driver_data = dev;
     subch_device_load(s, f);
+    /* 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);
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 06/10] s390x/css: consolidate the devno property for ccw devices

[Cornelia Huck]

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

'devno' should rather be a property of the ccw device, instead of a
property of a specific virtio-ccw device. Let's consolidate it.

While we are at here, also rename CcwDevice.bus_id to CcwDevice.devno to
make things clearer.

Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/ccw-device.c |  1 +
 hw/s390x/ccw-device.h |  2 +-
 hw/s390x/virtio-ccw.c | 14 ++------------
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index 8fe3cf49ef..fb8d640a7e 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -34,6 +34,7 @@ static void ccw_device_realize(CcwDevice *dev, Error **errp)
 }
 
 static Property ccw_device_properties[] = {
+    DEFINE_PROP_CSS_DEV_ID("devno", CcwDevice, devno),
     DEFINE_PROP_CSS_DEV_ID_RO("dev_id", CcwDevice, dev_id),
     DEFINE_PROP_CSS_DEV_ID_RO("subch_id", CcwDevice, subch_id),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 48700fe23a..89c8e5dff7 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -20,7 +20,7 @@ typedef struct CcwDevice {
     SubchDev *sch;
     /* <cssid>.<ssid>.<device number> */
     /* The user-set busid of the virtual ccw device. */
-    CssDevId bus_id;
+    CssDevId devno;
     /* The actual busid of the virtual ccw device. */
     CssDevId dev_id;
     /* The actual busid of the virtual subchannel. */
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 4e59e34d74..a81e27f7fd 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -681,7 +681,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
     VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
     CcwDevice *ccw_dev = CCW_DEVICE(dev);
     CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
-    SubchDev *sch = css_create_virtual_sch(ccw_dev->bus_id, errp);
+    SubchDev *sch = css_create_virtual_sch(ccw_dev->devno, errp);
     Error *err = NULL;
 
     if (!sch) {
@@ -705,7 +705,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
 
     trace_virtio_ccw_new_device(
         sch->cssid, sch->ssid, sch->schid, sch->devno,
-        ccw_dev->bus_id.valid ? "user-configured" : "auto-configured");
+        ccw_dev->devno.valid ? "user-configured" : "auto-configured");
 
     if (!kvm_eventfds_enabled()) {
         dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
@@ -1370,7 +1370,6 @@ static void virtio_ccw_device_unplugged(DeviceState *d)
 /**************** Virtio-ccw Bus Device Descriptions *******************/
 
 static Property virtio_ccw_net_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1399,7 +1398,6 @@ static const TypeInfo virtio_ccw_net = {
 };
 
 static Property virtio_ccw_blk_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1428,7 +1426,6 @@ static const TypeInfo virtio_ccw_blk = {
 };
 
 static Property virtio_ccw_serial_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1457,7 +1454,6 @@ static const TypeInfo virtio_ccw_serial = {
 };
 
 static Property virtio_ccw_balloon_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1486,7 +1482,6 @@ static const TypeInfo virtio_ccw_balloon = {
 };
 
 static Property virtio_ccw_scsi_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1516,7 +1511,6 @@ static const TypeInfo virtio_ccw_scsi = {
 
 #ifdef CONFIG_VHOST_SCSI
 static Property vhost_ccw_scsi_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
                        VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
@@ -1554,7 +1548,6 @@ static void virtio_ccw_rng_instance_init(Object *obj)
 }
 
 static Property virtio_ccw_rng_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1583,7 +1576,6 @@ static const TypeInfo virtio_ccw_rng = {
 };
 
 static Property virtio_ccw_crypto_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1710,7 +1702,6 @@ static const TypeInfo virtio_ccw_bus_info = {
 
 #ifdef CONFIG_VIRTFS
 static Property virtio_ccw_9p_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
             VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
@@ -1759,7 +1750,6 @@ static const TypeInfo virtio_ccw_9p_info = {
 #ifdef CONFIG_VHOST_VSOCK
 
 static Property vhost_vsock_ccw_properties[] = {
-    DEFINE_PROP_CSS_DEV_ID("devno", VirtioCcwDevice, parent_obj.bus_id),
     DEFINE_PROP_UINT32("max_revision", VirtioCcwDevice, max_rev,
                        VIRTIO_CCW_MAX_REV),
     DEFINE_PROP_END_OF_LIST(),
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 07/10] s390x: use enum for adapter type and standardize its naming

[Cornelia Huck]

From: Fei Li <sherrylf@linux.vnet.ibm.com>

Let's use an enum for io adapter type, and standardize its naming to
CSS_IO_ADAPTER_* by changing S390_PCIPT_ADAPTER to CSS_IO_ADAPTER_PCI.

Signed-off-by: Fei Li <sherrylf@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css.c           | 2 +-
 hw/s390x/s390-pci-bus.h  | 1 -
 hw/s390x/s390-pci-inst.c | 2 +-
 include/hw/s390x/css.h   | 9 +++++++--
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index f966ce2d15..1b242c1fb7 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -155,7 +155,7 @@ int css_create_css_image(uint8_t cssid, bool default_image)
     return 0;
 }
 
-int css_register_io_adapter(uint8_t type, uint8_t isc, bool swap,
+int css_register_io_adapter(CssIoAdapterType type, uint8_t isc, bool swap,
                             bool maskable, uint32_t *id)
 {
     IoAdapter *adapter;
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index dcbf4820c9..cf142a3e68 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -30,7 +30,6 @@
 #define FH_MASK_INDEX    0x0000ffff
 #define FH_SHM_VFIO      0x00010000
 #define FH_SHM_EMUL      0x00020000
-#define S390_PCIPT_ADAPTER 2
 #define ZPCI_MAX_FID 0xffffffff
 #define ZPCI_MAX_UID 0xffff
 #define UID_UNDEFINED 0
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 763eebd67f..214fde7ac3 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -736,7 +736,7 @@ static int reg_irqs(CPUS390XState *env, S390PCIBusDevice *pbdev, ZpciFib fib)
 {
     int ret, len;
 
-    ret = css_register_io_adapter(S390_PCIPT_ADAPTER,
+    ret = css_register_io_adapter(CSS_IO_ADAPTER_PCI,
                                   FIB_DATA_ISC(ldl_p(&fib.data)), true, false,
                                   &pbdev->routes.adapter.adapter_id);
     assert(ret == 0);
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index e88f24b868..cdc73fe0aa 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -124,8 +124,13 @@ void css_generate_css_crws(uint8_t cssid);
 void css_clear_sei_pending(void);
 void css_adapter_interrupt(uint8_t isc);
 
-#define CSS_IO_ADAPTER_VIRTIO 1
-int css_register_io_adapter(uint8_t type, uint8_t isc, bool swap,
+typedef enum {
+    CSS_IO_ADAPTER_VIRTIO = 0,
+    CSS_IO_ADAPTER_PCI = 1,
+    CSS_IO_ADAPTER_TYPE_NUMS,
+} CssIoAdapterType;
+
+int css_register_io_adapter(CssIoAdapterType type, uint8_t isc, bool swap,
                             bool maskable, uint32_t *id);
 
 #ifndef CONFIG_USER_ONLY
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 08/10] s390x: initialize flic before I/O subsystems

[Cornelia Huck]

From: Fei Li <sherrylf@linux.vnet.ibm.com>

Let's have a flic before we move on to initialize more specific
subsystems that make use of it.

Signed-off-by: Fei Li <sherrylf@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5ac315ac78..04bd0ebe40 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -113,12 +113,13 @@ static void ccw_init(MachineState *machine)
     s390_sclp_init();
     s390_memory_init(machine->ram_size);
 
+    s390_flic_init();
+
     /* get a BUS */
     css_bus = virtual_css_bus_init();
     s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline,
                       machine->initrd_filename, "s390-ccw.img",
                       "s390-netboot.img", true);
-    s390_flic_init();
 
     dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
     object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 09/10] s390x/flic: cache flic in s390_get_flic

[Cornelia Huck]

From: Fei Li <sherrylf@linux.vnet.ibm.com>

s390_get_flic() is called many times to obtain the flic. This wastes a
lot of time as it calls object_resolve_path() every time. Let's cache
S390FLICState by defining it as static.

Signed-off-by: Fei Li <sherrylf@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/intc/s390_flic.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index bef4caf980..711c11454f 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -21,11 +21,14 @@
 
 S390FLICState *s390_get_flic(void)
 {
-    S390FLICState *fs;
+    static S390FLICState *fs;
 
-    fs = S390_FLIC_COMMON(object_resolve_path(TYPE_KVM_S390_FLIC, NULL));
     if (!fs) {
-        fs = S390_FLIC_COMMON(object_resolve_path(TYPE_QEMU_S390_FLIC, NULL));
+        fs = S390_FLIC_COMMON(object_resolve_path(TYPE_KVM_S390_FLIC, NULL));
+        if (!fs) {
+            fs = S390_FLIC_COMMON(object_resolve_path(TYPE_QEMU_S390_FLIC,
+                                                      NULL));
+        }
     }
     return fs;
 }
-- 
2.11.0

[Qemu-devel] [PATCH for-2.10 10/10] s390x: register I/O adapters per ISC during init

[Cornelia Huck]

From: Fei Li <sherrylf@linux.vnet.ibm.com>

The I/O adapters should exist as soon as the bus/infrastructure
exists, and not only when the guest is actually trying to do something
with them. While the lazy allocation was not wrong, allocating at init
time is cleaner, both for the architecture and the code. Let's adjust
this by having each device type (currently for PCI and virtio-ccw)
register the adapters for each ISC (as now we don't know which ISC the
guest will use) as soon as it initializes.

Use a two-dimensional array io_adapters[type][isc] to store adapters
in ChannelSubSys, so that we can conveniently get the adapter id by
the helper function css_get_adapter_id(type, isc).

Signed-off-by: Fei Li <sherrylf@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css-bridge.c    |  3 ++
 hw/s390x/css.c           | 85 ++++++++++++++++++++++++++++++------------------
 hw/s390x/s390-pci-bus.c  |  3 ++
 hw/s390x/s390-pci-inst.c | 10 +++---
 hw/s390x/virtio-ccw.c    | 13 ++++----
 include/hw/s390x/css.h   |  7 ++--
 6 files changed, 74 insertions(+), 47 deletions(-)

diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index 9a7f7ee60c..b54ac01d37 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -107,6 +107,9 @@ VirtualCssBus *virtual_css_bus_init(void)
     /* Enable hotplugging */
     qbus_set_hotplug_handler(bus, dev, &error_abort);
 
+    css_register_io_adapters(CSS_IO_ADAPTER_VIRTIO, true, false,
+                             &error_abort);
+
     return cbus;
  }
 
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 1b242c1fb7..c03bb20bc9 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -47,7 +47,6 @@ typedef struct IoAdapter {
     uint32_t id;
     uint8_t type;
     uint8_t isc;
-    QTAILQ_ENTRY(IoAdapter) sibling;
 } IoAdapter;
 
 typedef struct ChannelSubSys {
@@ -61,7 +60,7 @@ typedef struct ChannelSubSys {
     uint64_t chnmon_area;
     CssImage *css[MAX_CSSID + 1];
     uint8_t default_cssid;
-    QTAILQ_HEAD(, IoAdapter) io_adapters;
+    IoAdapter *io_adapters[CSS_IO_ADAPTER_TYPE_NUMS][MAX_ISC + 1];
     QTAILQ_HEAD(, IndAddr) indicator_addresses;
 } ChannelSubSys;
 
@@ -72,7 +71,6 @@ static ChannelSubSys channel_subsys = {
     .do_crw_mchk = true,
     .crws_lost = false,
     .chnmon_active = false,
-    .io_adapters = QTAILQ_HEAD_INITIALIZER(channel_subsys.io_adapters),
     .indicator_addresses =
         QTAILQ_HEAD_INITIALIZER(channel_subsys.indicator_addresses),
 };
@@ -155,44 +153,67 @@ int css_create_css_image(uint8_t cssid, bool default_image)
     return 0;
 }
 
-int css_register_io_adapter(CssIoAdapterType type, uint8_t isc, bool swap,
-                            bool maskable, uint32_t *id)
+uint32_t css_get_adapter_id(CssIoAdapterType type, uint8_t isc)
 {
+    if (type >= CSS_IO_ADAPTER_TYPE_NUMS || isc > MAX_ISC ||
+        !channel_subsys.io_adapters[type][isc]) {
+        return -1;
+    }
+
+    return channel_subsys.io_adapters[type][isc]->id;
+}
+
+/**
+ * css_register_io_adapters: Register I/O adapters per ISC during init
+ *
+ * @swap: an indication if byte swap is needed.
+ * @maskable: an indication if the adapter is subject to the mask operation.
+ * @errp: location to store error information.
+ */
+void css_register_io_adapters(CssIoAdapterType type, bool swap, bool maskable,
+                              Error **errp)
+{
+    uint32_t id;
+    int ret, isc;
     IoAdapter *adapter;
-    bool found = false;
-    int ret;
     S390FLICState *fs = s390_get_flic();
     S390FLICStateClass *fsc = S390_FLIC_COMMON_GET_CLASS(fs);
 
-    *id = 0;
-    QTAILQ_FOREACH(adapter, &channel_subsys.io_adapters, sibling) {
-        if ((adapter->type == type) && (adapter->isc == isc)) {
-            *id = adapter->id;
-            found = true;
-            ret = 0;
+    /*
+     * Disallow multiple registrations for the same device type.
+     * Report an error if registering for an already registered type.
+     */
+    if (channel_subsys.io_adapters[type][0]) {
+        error_setg(errp, "Adapters for type %d already registered", type);
+    }
+
+    for (isc = 0; isc <= MAX_ISC; isc++) {
+        id = (type << 3) | isc;
+        ret = fsc->register_io_adapter(fs, id, isc, swap, maskable);
+        if (ret == 0) {
+            adapter = g_new0(IoAdapter, 1);
+            adapter->id = id;
+            adapter->isc = isc;
+            adapter->type = type;
+            channel_subsys.io_adapters[type][isc] = adapter;
+        } else {
+            error_setg_errno(errp, -ret, "Unexpected error %d when "
+                             "registering adapter %d", ret, id);
             break;
         }
-        if (adapter->id >= *id) {
-            *id = adapter->id + 1;
-        }
-    }
-    if (found) {
-        goto out;
     }
-    adapter = g_new0(IoAdapter, 1);
-    ret = fsc->register_io_adapter(fs, *id, isc, swap, maskable);
-    if (ret == 0) {
-        adapter->id = *id;
-        adapter->isc = isc;
-        adapter->type = type;
-        QTAILQ_INSERT_TAIL(&channel_subsys.io_adapters, adapter, sibling);
-    } else {
-        g_free(adapter);
-        fprintf(stderr, "Unexpected error %d when registering adapter %d\n",
-                ret, *id);
+
+    /*
+     * No need to free registered adapters in kvm: kvm will clean up
+     * when the machine goes away.
+     */
+    if (ret) {
+        for (isc--; isc >= 0; isc--) {
+            g_free(channel_subsys.io_adapters[type][isc]);
+            channel_subsys.io_adapters[type][isc] = NULL;
+        }
     }
-out:
-    return ret;
+
 }
 
 static void css_clear_io_interrupt(uint16_t subchannel_id,
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 0f62363434..8b456fad6c 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -583,6 +583,9 @@ static int s390_pcihost_init(SysBusDevice *dev)
     s->bus_no = 0;
     QTAILQ_INIT(&s->pending_sei);
     QTAILQ_INIT(&s->zpci_devs);
+
+    css_register_io_adapters(CSS_IO_ADAPTER_PCI, true, false, &error_abort);
+
     return 0;
 }
 
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 214fde7ac3..8bd5a564db 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -735,12 +735,10 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
 static int reg_irqs(CPUS390XState *env, S390PCIBusDevice *pbdev, ZpciFib fib)
 {
     int ret, len;
+    uint8_t isc = FIB_DATA_ISC(ldl_p(&fib.data));
 
-    ret = css_register_io_adapter(CSS_IO_ADAPTER_PCI,
-                                  FIB_DATA_ISC(ldl_p(&fib.data)), true, false,
-                                  &pbdev->routes.adapter.adapter_id);
-    assert(ret == 0);
-
+    pbdev->routes.adapter.adapter_id = css_get_adapter_id(
+                                       CSS_IO_ADAPTER_PCI, isc);
     pbdev->summary_ind = get_indicator(ldq_p(&fib.aisb), sizeof(uint64_t));
     len = BITS_TO_LONGS(FIB_DATA_NOI(ldl_p(&fib.data))) * sizeof(unsigned long);
     pbdev->indicator = get_indicator(ldq_p(&fib.aibv), len);
@@ -759,7 +757,7 @@ static int reg_irqs(CPUS390XState *env, S390PCIBusDevice *pbdev, ZpciFib fib)
     pbdev->routes.adapter.summary_offset = FIB_DATA_AISBO(ldl_p(&fib.data));
     pbdev->routes.adapter.ind_addr = ldq_p(&fib.aibv);
     pbdev->routes.adapter.ind_offset = FIB_DATA_AIBVO(ldl_p(&fib.data));
-    pbdev->isc = FIB_DATA_ISC(ldl_p(&fib.data));
+    pbdev->isc = isc;
     pbdev->noi = FIB_DATA_NOI(ldl_p(&fib.data));
     pbdev->sum = FIB_DATA_SUM(ldl_p(&fib.data));
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index a81e27f7fd..823c90ad6e 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -616,10 +616,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
                 dev->routes.adapter.ind_offset = ind_bit;
                 dev->routes.adapter.summary_offset = 7;
                 cpu_physical_memory_unmap(thinint, hw_len, 0, hw_len);
-                ret = css_register_io_adapter(CSS_IO_ADAPTER_VIRTIO,
-                                              dev->thinint_isc, true, false,
-                                              &dev->routes.adapter.adapter_id);
-                assert(ret == 0);
+                dev->routes.adapter.adapter_id = css_get_adapter_id(
+                                                 CSS_IO_ADAPTER_VIRTIO,
+                                                 dev->thinint_isc);
                 sch->thinint_active = ((dev->indicators != NULL) &&
                                        (dev->summary_indicator != NULL));
                 sch->curr_status.scsw.count = ccw.count - len;
@@ -1309,9 +1308,9 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
     dev->thinint_isc = qemu_get_byte(f);
     dev->revision = qemu_get_be32(f);
     if (s->thinint_active) {
-        return css_register_io_adapter(CSS_IO_ADAPTER_VIRTIO,
-                                       dev->thinint_isc, true, false,
-                                       &dev->routes.adapter.adapter_id);
+        dev->routes.adapter.adapter_id = css_get_adapter_id(
+                                         CSS_IO_ADAPTER_VIRTIO,
+                                         dev->thinint_isc);
     }
 
     return 0;
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index cdc73fe0aa..f1f0d7f07a 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -23,6 +23,8 @@
 #define MAX_CSSID 255
 #define MAX_CHPID 255
 
+#define MAX_ISC 7
+
 #define MAX_CIWS 62
 
 #define VIRTUAL_CSSID 0xfe
@@ -130,8 +132,9 @@ typedef enum {
     CSS_IO_ADAPTER_TYPE_NUMS,
 } CssIoAdapterType;
 
-int css_register_io_adapter(CssIoAdapterType type, uint8_t isc, bool swap,
-                            bool maskable, uint32_t *id);
+uint32_t css_get_adapter_id(CssIoAdapterType type, uint8_t isc);
+void css_register_io_adapters(CssIoAdapterType type, bool swap, bool maskable,
+                              Error **errp);
 
 #ifndef CONFIG_USER_ONLY
 SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t ssid,
-- 
2.11.0

Re: [Qemu-devel] [PATCH for-2.10 03/10] s390x/pci: make printf always compile in debug output

[Thomas Huth]

On 06.04.2017 13:16, Cornelia Huck wrote:
> From: Danil Antonov <g.danil.anto@gmail.com>
> 
> Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
> This will ensure that printf function will always compile even if debug
> output is turned off and, in turn, will prevent bitrot of the format
> strings.
> 
> Signed-off-by: Danil Antonov <g.danil.anto@gmail.com>
> Message-Id: <CA+KKJYBi31Bs7DtVdzZdwG2t+u5+FGiAhQpd3pqJzUX1O8Cprg@mail.gmail.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/s390-pci-bus.c  | 16 ++++++++++------
>  hw/s390x/s390-pci-inst.c | 16 ++++++++++------
>  2 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 69b0291e8a..0f62363434 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -24,14 +24,18 @@
>  #include "qemu/error-report.h"
>  
>  /* #define DEBUG_S390PCI_BUS */

This comment is now somewhat misleading. If you uncomment this "#define
DEBUG_S390PCI_BUS" (without adding a "1" at the end), you end up with
some empty "if ()" statements now, i.e. compilation failures. So I'd
like to suggest to simply remove the above line.

> -#ifdef DEBUG_S390PCI_BUS
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> +
> +#ifndef DEBUG_S390PCI_BUS
> +#define DEBUG_S390PCI_BUS  0
>  #endif
>  
> +#define DPRINTF(fmt, ...)                                         \
> +    do {                                                          \
> +        if (DEBUG_S390PCI_BUS) {                                  \
> +            fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); \
> +        }                                                         \
> +    } while (0)
> +
>  S390pciState *s390_get_phb(void)
>  {
>      static S390pciState *phb;
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index d2a8c0a083..763eebd67f 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -21,14 +21,18 @@
>  #include "sysemu/hw_accel.h"
>  
>  /* #define DEBUG_S390PCI_INST */

dito

> -#ifdef DEBUG_S390PCI_INST
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, "s390pci-inst: " fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> +
> +#ifndef DEBUG_S390PCI_INST
> +#define DEBUG_S390PCI_INST  0
>  #endif
>  
> +#define DPRINTF(fmt, ...)                                          \
> +    do {                                                           \
> +        if (DEBUG_S390PCI_INST) {                                  \
> +            fprintf(stderr, "s390pci-inst: " fmt, ## __VA_ARGS__); \
> +        }                                                          \
> +    } while (0)
> +
>  static void s390_set_status_code(CPUS390XState *env,
>                                   uint8_t r, uint64_t status_code)
>  {
> 

 Thomas

Re: [Qemu-devel] [PATCH for-2.10 05/10] s390x/css: provide introspection for virtual subchannel and device busid

[Thomas Huth]

On 06.04.2017 13:16, Cornelia Huck wrote:
> From: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> 
> Expose the busids of the virtual I/O subchannel and the virtual CCW
> device to ease debugging. This is needed because:
> 1. subchannel id are assigned dynamically, and cannot be set from
>    outside.
> 2. device busid could possibly be auto generated.
> 
> An example of using HMP to retrieve the property values of a
> virtio-balloon-ccw device looks like:
> 
> [root@localhost ~]# lscss -d 0.0.0004
> Device   Subchan.  DevType CU Type Use  PIM PAM POM  CHPIDs
> ----------------------------------------------------------------------
> 0.0.0004 0.0.0003  0000/00 3832/05 yes  80  80  ff   00000000 00000000
> 
> (qemu) info qtree
> ... ...
>       dev: virtio-balloon-ccw, id "balloon0"
>         devno = "<unset>"
>         ioeventfd = true
>         max_revision = 2 (0x2)
>         dev_id = "fe.0.0004"
>         subch_id = "fe.0.0003"
> ... ...
> 
> After migration, if we have the same device that shows up on a
> different subchannel, we must re-fill the subch_id of the ccw
> device with the new schid, or the subch_id will have an old wrong
> schid value. So this also re-fills the subch_id after migration.
> 
> While we are at it, also neaten the related error handling a bit.
> 
> Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/ccw-device.c | 39 +++++++++++++++++++++++++++++++++++++++
>  hw/s390x/ccw-device.h |  7 +++++++
>  hw/s390x/virtio-ccw.c | 28 ++++++++++++++++++++++------
>  3 files changed, 68 insertions(+), 6 deletions(-)
[...]
>  static inline CcwDevice *to_ccw_dev_fast(DeviceState *d)
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 00b3bde4e9..4e59e34d74 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -680,6 +680,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>  {
>      VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
>      CcwDevice *ccw_dev = CCW_DEVICE(dev);
> +    CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
>      SubchDev *sch = css_create_virtual_sch(ccw_dev->bus_id, errp);
>      Error *err = NULL;
>  
> @@ -689,8 +690,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>      if (!virtio_ccw_rev_max(dev) && dev->force_revision_1) {
>          error_setg(&err, "Invalid value of property max_rev "
>                     "(is %d expected >= 1)", virtio_ccw_rev_max(dev));
> -        error_propagate(errp, err);
> -        return;
> +        goto out_err;
>      }
>  
>      sch->driver_data = dev;
> @@ -713,13 +713,24 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>  
>      if (k->realize) {
>          k->realize(dev, &err);
> +        if (err) {
> +            goto out_err;
> +        }
>      }
> +
> +    ck->realize(ccw_dev, &err);
>      if (err) {
> -        error_propagate(errp, err);
> -        css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
> -        ccw_dev->sch = NULL;
> -        g_free(sch);
> +        goto out_err;
>      }
> +
> +    return;
> +
> +out_err:
> +    error_propagate(errp, err);
> +    css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
> +    ccw_dev->sch = NULL;
> +    g_free(sch);
> +    return;
>  }

Cosmetic nit: Remove the unnecessary "return;" statement right before
the closing curly bracket.

 Thomas

Re: [Qemu-devel] [PATCH for-2.10 03/10] s390x/pci: make printf always compile in debug output

[Cornelia Huck]

On Thu, 6 Apr 2017 14:15:45 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 06.04.2017 13:16, Cornelia Huck wrote:
> > From: Danil Antonov <g.danil.anto@gmail.com>
> > 
> > Wrapped printf calls inside debug macros (DPRINTF) in `if` statement.
> > This will ensure that printf function will always compile even if debug
> > output is turned off and, in turn, will prevent bitrot of the format
> > strings.
> > 
> > Signed-off-by: Danil Antonov <g.danil.anto@gmail.com>
> > Message-Id: <CA+KKJYBi31Bs7DtVdzZdwG2t+u5+FGiAhQpd3pqJzUX1O8Cprg@mail.gmail.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  hw/s390x/s390-pci-bus.c  | 16 ++++++++++------
> >  hw/s390x/s390-pci-inst.c | 16 ++++++++++------
> >  2 files changed, 20 insertions(+), 12 deletions(-)
> > 
> > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > index 69b0291e8a..0f62363434 100644
> > --- a/hw/s390x/s390-pci-bus.c
> > +++ b/hw/s390x/s390-pci-bus.c
> > @@ -24,14 +24,18 @@
> >  #include "qemu/error-report.h"
> >  
> >  /* #define DEBUG_S390PCI_BUS */
> 
> This comment is now somewhat misleading. If you uncomment this "#define
> DEBUG_S390PCI_BUS" (without adding a "1" at the end), you end up with
> some empty "if ()" statements now, i.e. compilation failures. So I'd
> like to suggest to simply remove the above line.

Yeah, makes sense, and also matches what is done in the s390x/kvm patch.
I'll merge that in.

> 
> > -#ifdef DEBUG_S390PCI_BUS
> > -#define DPRINTF(fmt, ...) \
> > -    do { fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); } while (0)
> > -#else
> > -#define DPRINTF(fmt, ...) \
> > -    do { } while (0)
> > +
> > +#ifndef DEBUG_S390PCI_BUS
> > +#define DEBUG_S390PCI_BUS  0
> >  #endif
> >  
> > +#define DPRINTF(fmt, ...)                                         \
> > +    do {                                                          \
> > +        if (DEBUG_S390PCI_BUS) {                                  \
> > +            fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); \
> > +        }                                                         \
> > +    } while (0)
> > +

Re: [Qemu-devel] [PATCH for-2.10 05/10] s390x/css: provide introspection for virtual subchannel and device busid

[Cornelia Huck]

On Thu, 6 Apr 2017 14:19:10 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 06.04.2017 13:16, Cornelia Huck wrote:

> > +out_err:
> > +    error_propagate(errp, err);
> > +    css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
> > +    ccw_dev->sch = NULL;
> > +    g_free(sch);
> > +    return;
> >  }
> 
> Cosmetic nit: Remove the unnecessary "return;" statement right before
> the closing curly bracket.

Yup, will merge in.