[Qemu-devel] [PULL 0/5] s390x/kvm: various fixes

[Qemu-devel] [PULL 0/5] s390x/kvm: various fixes

[Christian Borntraeger]

Peter,

The following changes since commit 0748b3526e8cb78b9cd64208426bfc3d54a72b04:

  Merge remote-tracking branch 'kwolf/tags/for-upstream' into staging (2017-05-30 14:15:15 +0100)

are available in the git repository at:

  ssh://git@github.com/borntraeger/qemu.git tags/s390x-20170608

for you to fetch changes up to fbe8202ea81519a42855830059ccc8e10b58dfa5:

  s390x/cpumodel: improve defintion search without an IBC (2017-06-06 10:50:40 +0200)

----------------------------------------------------------------
s390x: misc fixes

bunch of fixes
- reject MIDA accesses for CCWs
- cpumodel fixes
- cross-build fix for bios
- migration improvements

----------------------------------------------------------------
Cornelia Huck (1):
      s390x/css: fence off MIDA

David Hildenbrand (2):
      s390x/cpumodel: take care of the cpuid format bit for KVM
      s390x/cpumodel: improve defintion search without an IBC

Greg Kurz (1):
      pc-bios/s390-ccw: use STRIP variable in Makefile

Halil Pasic (1):
      s390x/css: catch section mismatch on load

 hw/s390x/css.c            | 19 +++++++++++++++++++
 hw/s390x/virtio-ccw.c     |  6 +++++-
 include/hw/s390x/ioinst.h |  1 +
 pc-bios/s390-ccw/Makefile |  4 +++-
 target/s390x/cpu_models.c | 10 ++++++++++
 target/s390x/cpu_models.h | 10 +++++++---
 target/s390x/ioinst.c     |  4 ++++
 target/s390x/kvm.c        |  1 +
 8 files changed, 50 insertions(+), 5 deletions(-)

[Qemu-devel] [PULL 1/5] s390x/css: catch section mismatch on load

[Christian Borntraeger]

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

Prior to the virtio-ccw-2.7 machine (and commit 2a79eb1a), our virtio
devices residing under the virtual-css bus do not have qdev_path based
migration stream identifiers (because their qdev_path is NULL). The ids
are instead generated when the device is registered as a composition of
the so called idstr, which takes the vmsd name as its value, and an
instance_id, which is which is calculated as a maximal instance_id
registered with the same idstr plus one, or zero (if none was registered
previously).

That means, under certain circumstances, one device might try, and even
succeed, to load the state of a different device. This can lead to
trouble.

Let us fail the migration if the above problem is detected during load.

How to reproduce the problem:
1) start qemu-system-s390x making sure you have the following devices
   defined on your command line:
     -device virtio-rng-ccw,id=rng1,devno=fe.0.0001
     -device virtio-rng-ccw,id=rng2,devno=fe.0.0002
2) detach the devices and reattach in reverse order using the monitor:
     (qemu) device_del rng1
     (qemu) device_del rng2
     (qemu) device_add virtio-rng-ccw,id=rng2,devno=fe.0.0002
     (qemu) device_add virtio-rng-ccw,id=rng1,devno=fe.0.0001
3) save the state of the vm into a temporary file and quit QEMU:
     (qemu) migrate "exec:gzip -c > /tmp/tmp_vmstate.gz"
     (qemu) q
4) use your command line from step 1 with
     -incoming "exec:gzip -c -d /tmp/tmp_vmstate.gz"
   appended to reproduce the problem (while trying to to load the saved vm)

CC: qemu-stable@nongnu.org
Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Message-Id: <20170518111405.56947-1-pasic@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/css.c        | 14 ++++++++++++++
 hw/s390x/virtio-ccw.c |  6 +++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 1e2f26b..cc76c53 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -15,6 +15,7 @@
 #include "hw/qdev.h"
 #include "qemu/error-report.h"
 #include "qemu/bitops.h"
+#include "qemu/error-report.h"
 #include "exec/address-spaces.h"
 #include "cpu.h"
 #include "hw/s390x/ioinst.h"
@@ -1800,13 +1801,26 @@ void subch_device_save(SubchDev *s, QEMUFile *f)
 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];
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index e6a6f74..90d37cb 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1279,9 +1279,13 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
     SubchDev *s = ccw_dev->sch;
     VirtIODevice *vdev = virtio_ccw_get_vdev(s);
     int len;
+    int ret;
 
     s->driver_data = dev;
-    subch_device_load(s, f);
+    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);
-- 
2.7.4

[Qemu-devel] [PULL 2/5] s390x/css: fence off MIDA

[Christian Borntraeger]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

MIDA (modified indirect data addressing) is an optional facility, and
we (currently) don't support it. Let's post an operand exception if
the guest tries to set it in the orb and a channel program check
if it is set in a ccw, as specified in the Principles of Operation.

Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/css.c            | 5 +++++
 include/hw/s390x/ioinst.h | 1 +
 target/s390x/ioinst.c     | 4 ++++
 3 files changed, 10 insertions(+)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index cc76c53..599805d 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -433,6 +433,11 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr,
         return -EINVAL;
     }
 
+    /* We don't support MIDA. */
+    if (ccw.flags & CCW_FLAG_MIDA) {
+        return -EINVAL;
+    }
+
     if (ccw.flags & CCW_FLAG_SUSPEND) {
         return suspend_allowed ? -EINPROGRESS : -EINVAL;
     }
diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h
index c559f53..92d1565 100644
--- a/include/hw/s390x/ioinst.h
+++ b/include/hw/s390x/ioinst.h
@@ -182,6 +182,7 @@ typedef struct CCW1 {
 #define CCW_FLAG_PCI             0x08
 #define CCW_FLAG_IDA             0x04
 #define CCW_FLAG_SUSPEND         0x02
+#define CCW_FLAG_MIDA            0x01
 
 #define CCW_CMD_NOOP             0x03
 #define CCW_CMD_BASIC_SENSE      0x04
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 62a7771..d5e6b80 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -201,6 +201,10 @@ static int ioinst_orb_valid(ORB *orb)
         (orb->ctrl1 & ORB_CTRL1_MASK_INVALID)) {
         return 0;
     }
+    /* We don't support MIDA. */
+    if (orb->ctrl1 & ORB_CTRL1_MASK_MIDAW) {
+        return 0;
+    }
     if ((orb->cpa & HIGH_ORDER_BIT) != 0) {
         return 0;
     }
-- 
2.7.4

[Qemu-devel] [PULL 3/5] pc-bios/s390-ccw: use STRIP variable in Makefile

[Christian Borntraeger]

From: Greg Kurz <groug@kaod.org>

The docker-run-test-build@debian-s390x-cross target fails with:

strip --strip-unneeded s390-ccw.elf -o s390-ccw.img
strip: Unable to recognise the format of the input file `s390-ccw.elf'

The configure script defines a STRIP makefile variable whose default
value is ${cross_prefix}strip. Let's use it.

We default to using the non-prefixed strip command in case --enable-debug
or --disable-strip was passed to configure during a regular build.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <149623617700.4947.12490877660892961664.stgit@bahia.lan>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 pc-bios/s390-ccw/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 79a46b6..fb88c13 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -21,8 +21,10 @@ build-all: s390-ccw.img
 s390-ccw.elf: $(OBJECTS)
 	$(call quiet-command,$(CC) $(LDFLAGS) -o $@ $(OBJECTS),"BUILD","$(TARGET_DIR)$@")
 
+STRIP ?= strip
+
 s390-ccw.img: s390-ccw.elf
-	$(call quiet-command,strip --strip-unneeded $< -o $@,"STRIP","$(TARGET_DIR)$@")
+	$(call quiet-command,$(STRIP) --strip-unneeded $< -o $@,"STRIP","$(TARGET_DIR)$@")
 
 $(OBJECTS): Makefile
 
-- 
2.7.4

[Qemu-devel] [PULL 4/5] s390x/cpumodel: take care of the cpuid format bit for KVM

[Christian Borntraeger]

From: David Hildenbrand <david@redhat.com>

Let's also properly forward that bit. It should always be set. I
verified it under z/VM, it seems to be always set there. For now,
zKVM guests never get that bit set when the CPU model is active.

The PoP mentiones, that z800 + z900 (HW generation 7) always set this
bit to 0, so let's take care of that.

Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20170531193434.6918-2-david@redhat.com>
Acked-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target/s390x/cpu_models.c |  1 +
 target/s390x/cpu_models.h | 10 +++++++---
 target/s390x/kvm.c        |  1 +
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 8d27363..9e23535 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -740,6 +740,7 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
     /* copy over properties that can vary */
     cpu->model->lowest_ibc = max_model->lowest_ibc;
     cpu->model->cpu_id = max_model->cpu_id;
+    cpu->model->cpu_id_format = max_model->cpu_id_format;
     cpu->model->cpu_ver = max_model->cpu_ver;
 
     check_consistency(cpu->model);
diff --git a/target/s390x/cpu_models.h b/target/s390x/cpu_models.h
index 136a602..d41f8d6 100644
--- a/target/s390x/cpu_models.h
+++ b/target/s390x/cpu_models.h
@@ -46,6 +46,7 @@ typedef struct S390CPUModel {
     /* values copied from the "host" model, can change during migration */
     uint16_t lowest_ibc;    /* lowest IBC that the hardware supports */
     uint32_t cpu_id;        /* CPU id */
+    uint8_t cpu_id_format;  /* CPU id format bit */
     uint8_t cpu_ver;        /* CPU version, usually "ff" for kvm */
 } S390CPUModel;
 
@@ -54,12 +55,14 @@ typedef struct S390CPUModel {
  *
  * bits 0-7: Zeroes (ff for kvm)
  * bits 8-31: CPU ID (serial number)
- * bits 32-48: Machine type
- * bits 48-63: Zeroes
+ * bits 32-47: Machine type
+ * bit  48: CPU ID format
+ * bits 49-63: Zeroes
  */
 #define cpuid_type(x)     (((x) >> 16) & 0xffff)
 #define cpuid_id(x)       (((x) >> 32) & 0xffffff)
 #define cpuid_ver(x)      (((x) >> 56) & 0xff)
+#define cpuid_format(x)   (((x) >> 15) & 0x1)
 
 #define lowest_ibc(x)     (((uint32_t)(x) >> 16) & 0xfff)
 #define unblocked_ibc(x)  ((uint32_t)(x) & 0xfff)
@@ -92,7 +95,8 @@ static inline uint64_t s390_cpuid_from_cpu_model(const S390CPUModel *model)
 {
     return ((uint64_t)model->cpu_ver << 56) |
            ((uint64_t)model->cpu_id << 32) |
-           ((uint64_t)model->def->type << 16);
+           ((uint64_t)model->def->type << 16) |
+           (model->def->gen == 7 ? 0 : (uint64_t)model->cpu_id_format << 15);
 }
 S390CPUDef const *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
                                     S390FeatBitmap features);
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index ba1e60f..a3d0019 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2580,6 +2580,7 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         unblocked_ibc = unblocked_ibc(prop.ibc);
     }
     model->cpu_id = cpuid_id(prop.cpuid);
+    model->cpu_id_format = cpuid_format(prop.cpuid);
     model->cpu_ver = 0xff;
 
     /* get supported cpu features indicated via STFL(E) */
-- 
2.7.4

[Qemu-devel] [PULL 5/5] s390x/cpumodel: improve defintion search without an IBC

[Christian Borntraeger]

From: David Hildenbrand <david@redhat.com>

Currently, under z/VM on a 0x2827, QEMU will detect a 0x2828 if no
IBC value is provided. QEMU will simply take the last model of that HW
generation, which happens to be the BC version.

Let's improve our search for that case by selecting the latest CPU
definition that matches the CPU type. This for example will avoid
detecting an z13 as a z13s.

We might still detect a GA2 version on a GA1 system, but as we don't
have further information at hand, there isn't too much we can do about
it. The alternative of always presenting the oldest GA is not backward
compatible, e.g:
You're running on 0x2827 GA2.
Old QEMU version indicated "0x2828 GA1 == 0x2827 GA2". After you updated
QEMU, you suddenly detect "0x2827 GA1". You're previous libvirt guest
might suddenly refuse to run.

In the end presenting a newer GA level does not matter because:

1: All GAX models share the same base feature set. A GAX++ might
support "more features".
2: Without an IBC, the guest can't detect the GA version.

If we have no IBC (esp. unblocked_ibc == 0), the IBC we will present
to the guest in read_SCP_info() will be 0. The guest will not know
which GA version it has. The problem of missing IBC propagates.

If we don't have a feature of the GA++ version, also our guest won't
have it. So in summary, the guest also has no idea of its GA version.

Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20170531193434.6918-3-david@redhat.com>
Acked-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
[improve patch description by reusing mailing list discussion]
---
 target/s390x/cpu_models.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 9e23535..b6220c8 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -184,6 +184,7 @@ const S390CPUDef *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
                                     S390FeatBitmap features)
 {
     const S390CPUDef *last_compatible = NULL;
+    const S390CPUDef *matching_cpu_type = NULL;
     int i;
 
     if (!gen) {
@@ -218,8 +219,16 @@ const S390CPUDef *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
         if (def->type == type && def->ec_ga == ec_ga) {
             return def;
         }
+        /* remember if we've at least seen one with the same cpu type */
+        if (def->type == type) {
+            matching_cpu_type = def;
+        }
         last_compatible = def;
     }
+    /* prefer the model with the same cpu type, esp. don't take the BC for EC */
+    if (matching_cpu_type) {
+        return matching_cpu_type;
+    }
     return last_compatible;
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PULL 0/5] s390x/kvm: various fixes

[Christian Borntraeger]

On 06/08/2017 03:18 PM, Christian Borntraeger wrote:
> Peter,
> 
> The following changes since commit 0748b3526e8cb78b9cd64208426bfc3d54a72b04:
> 
>   Merge remote-tracking branch 'kwolf/tags/for-upstream' into staging (2017-05-30 14:15:15 +0100)
> 
> are available in the git repository at:
> 
>   ssh://git@github.com/borntraeger/qemu.git tags/s390x-20170608

This address is certainly better

  git://github.com/borntraeger/qemu.git tags/s390x-20170608



> 
> for you to fetch changes up to fbe8202ea81519a42855830059ccc8e10b58dfa5:
> 
>   s390x/cpumodel: improve defintion search without an IBC (2017-06-06 10:50:40 +0200)
> 
> ----------------------------------------------------------------
> s390x: misc fixes
> 
> bunch of fixes
> - reject MIDA accesses for CCWs
> - cpumodel fixes
> - cross-build fix for bios
> - migration improvements
> 
> ----------------------------------------------------------------
> Cornelia Huck (1):
>       s390x/css: fence off MIDA
> 
> David Hildenbrand (2):
>       s390x/cpumodel: take care of the cpuid format bit for KVM
>       s390x/cpumodel: improve defintion search without an IBC
> 
> Greg Kurz (1):
>       pc-bios/s390-ccw: use STRIP variable in Makefile
> 
> Halil Pasic (1):
>       s390x/css: catch section mismatch on load
> 
>  hw/s390x/css.c            | 19 +++++++++++++++++++
>  hw/s390x/virtio-ccw.c     |  6 +++++-
>  include/hw/s390x/ioinst.h |  1 +
>  pc-bios/s390-ccw/Makefile |  4 +++-
>  target/s390x/cpu_models.c | 10 ++++++++++
>  target/s390x/cpu_models.h | 10 +++++++---
>  target/s390x/ioinst.c     |  4 ++++
>  target/s390x/kvm.c        |  1 +
>  8 files changed, 50 insertions(+), 5 deletions(-)
> 

Re: [Qemu-devel] [PULL 0/5] s390x/kvm: various fixes

[Peter Maydell]

On 8 June 2017 at 15:42, Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 06/08/2017 03:18 PM, Christian Borntraeger wrote:
>> Peter,
>>
>> The following changes since commit 0748b3526e8cb78b9cd64208426bfc3d54a72b04:
>>
>>   Merge remote-tracking branch 'kwolf/tags/for-upstream' into staging (2017-05-30 14:15:15 +0100)
>>
>> are available in the git repository at:
>>
>>   ssh://git@github.com/borntraeger/qemu.git tags/s390x-20170608
>
> This address is certainly better
>
>   git://github.com/borntraeger/qemu.git tags/s390x-20170608
>

Applied, thanks.

-- PMM