[PULL 0/8] s390x update

[PULL 0/8] s390x update

[Cornelia Huck]

The following changes since commit c40ae5a3ee387b13116948cbfe7824f03311db7e:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-03-03 16:55:15 +0000)

are available in the Git repository at:

  https://gitlab.com/cohuck/qemu.git tags/s390x-20210305

for you to fetch changes up to 39d5d1404ed695f4a1cd2b117a6cf2d92dd8e8b9:

  target/s390x/kvm: Simplify debug code (2021-03-04 14:19:08 +0100)

----------------------------------------------------------------
some accumulated s390x fixes

----------------------------------------------------------------

Christian Borntraeger (1):
  s390x/cpu_model: disallow unpack for --only-migratable

Cornelia Huck (1):
  virtio-ccw: commands on revision-less devices

Eric Farman (1):
  vfio-ccw: Do not read region ret_code after write

Halil Pasic (1):
  hw/s390x: fix build for virtio-9p-ccw

Matthew Rosato (1):
  s390x/pci: restore missing Query PCI Function CLP data

Philippe Mathieu-Daudé (1):
  target/s390x/kvm: Simplify debug code

Pierre Morel (1):
  css: SCHIB measurement block origin must be aligned

Thomas Huth (1):
  target/s390x/arch_dump: Fix warning for the name field in the PT_NOTE
    section

 hw/s390x/meson.build      |  4 +++-
 hw/s390x/s390-pci-inst.c  |  5 +++++
 hw/s390x/virtio-ccw.c     | 21 ++++++++++++++-------
 hw/vfio/ccw.c             | 12 ++++++------
 target/s390x/arch_dump.c  |  4 +++-
 target/s390x/cpu_models.c | 10 ++++++++++
 target/s390x/ioinst.c     |  6 ++++++
 target/s390x/kvm.c        |  3 +--
 8 files changed, 48 insertions(+), 17 deletions(-)

-- 
2.26.2

[PULL 1/8] s390x/cpu_model: disallow unpack for --only-migratable

[Cornelia Huck]

From: Christian Borntraeger <borntraeger@de.ibm.com>

Secure execution (aka protected virtualization) guests cannot be
migrated at the moment. If the unpack facility is provided in the cpu
model, a guest may choose to transition to secure mode, making the
guest unmigratable at that point in time. If the machine was explicitly
started with --only-migratable, we would get a failure only when the
guest actually tries to transition; instead, explicitly disallow the
unpack facility if --only-migratable was specified to avoid late
surprises.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
Message-Id: <20210125135332.181324-1-borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/cpu_models.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 35179f9dc7ba..dd474c5e9ad1 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -26,6 +26,7 @@
 #include "qapi/qmp/qdict.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
 #include "hw/pci/pci.h"
 #endif
 #include "qapi/qapi-commands-machine-target.h"
@@ -878,6 +879,15 @@ static void check_compatibility(const S390CPUModel *max_model,
         return;
     }
 
+#ifndef CONFIG_USER_ONLY
+    if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) {
+        error_setg(errp, "The unpack facility is not compatible with "
+                   "the --only-migratable option. You must remove either "
+                   "the 'unpack' facility or the --only-migratable option");
+        return;
+    }
+#endif
+
     /* detect the missing features to properly report them */
     bitmap_andnot(missing, model->features, max_model->features, S390_FEAT_MAX);
     if (bitmap_empty(missing, S390_FEAT_MAX)) {
-- 
2.26.2

[PULL 2/8] target/s390x/arch_dump: Fix warning for the name field in the PT_NOTE section

[Cornelia Huck]

From: Thomas Huth <thuth@redhat.com>

There is a compiler warning with GCC 9.3 when compiling with
the -fsanitize=thread compiler flag:

 In function 'strncpy',
    inlined from 's390x_write_elf64_notes' at ../target/s390x/arch_dump.c:219:9:
 /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
  '__builtin_strncpy' specified bound 8 equals destination size
  [-Werror=stringop-truncation]

Since the name should always be NUL-terminated, let's use g_strlcpy() to
silence this warning. And while we're at it, also add an assert() to make
sure that the provided names always fit the size field (which is fine for
the current callers, the function is called once with "CORE" and once with
"LINUX" as a name).

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Message-Id: <20210205093921.848260-1-thuth@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/arch_dump.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
index 50fa0ae4b679..cc1330876bef 100644
--- a/target/s390x/arch_dump.c
+++ b/target/s390x/arch_dump.c
@@ -212,11 +212,13 @@ static int s390x_write_elf64_notes(const char *note_name,
     int note_size;
     int ret = -1;
 
+    assert(strlen(note_name) < sizeof(note.name));
+
     for (nf = funcs; nf->note_contents_func; nf++) {
         memset(&note, 0, sizeof(note));
         note.hdr.n_namesz = cpu_to_be32(strlen(note_name) + 1);
         note.hdr.n_descsz = cpu_to_be32(nf->contents_size);
-        strncpy(note.name, note_name, sizeof(note.name));
+        g_strlcpy(note.name, note_name, sizeof(note.name));
         (*nf->note_contents_func)(&note, cpu, id);
 
         note_size = sizeof(note) - sizeof(note.contents) + nf->contents_size;
-- 
2.26.2

[PULL 3/8] hw/s390x: fix build for virtio-9p-ccw

[Cornelia Huck]

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

Commit  2c44220d05 ("meson: convert hw/arch*"), which migrated the old
Makefile.objs to meson.build  accidentally excluded virtio-ccw-9p.c and
thus the virtio-9p-ccw device from the build (and potentially also
included the file virtio-ccw-blk.c twice in the source set). And since
CONFIG_VIRTFS can't be used the way it was used here (see commit
2c9dce0196 ("meson: do not use CONFIG_VIRTFS")), the preconditions have
to be written differently.

Let's fix this!

Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
Fixes: 2c44220d05 ("meson: convert hw/arch*")
Reported-by: Jakob Naucke <jakob.naucke@ibm.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20210218034059.1096078-1-pasic@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 2a7818d94b94..91495b563146 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -40,7 +40,9 @@ virtio_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-ccw-net.c'))
 virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-ccw-rng.c'))
 virtio_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio-ccw-scsi.c'))
 virtio_ss.add(when: 'CONFIG_VIRTIO_SERIAL', if_true: files('virtio-ccw-serial.c'))
-virtio_ss.add(when: ['CONFIG_VIRTIO_9P', 'CONFIG_VIRTFS'], if_true: files('virtio-ccw-blk.c'))
+if have_virtfs
+  virtio_ss.add(when: 'CONFIG_VIRTIO_9P', if_true: files('virtio-ccw-9p.c'))
+endif
 virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-ccw.c'))
 virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs-ccw.c'))
 s390x_ss.add_all(when: 'CONFIG_VIRTIO_CCW', if_true: virtio_ss)
-- 
2.26.2

[PULL 4/8] s390x/pci: restore missing Query PCI Function CLP data

[Cornelia Huck]

From: Matthew Rosato <mjrosato@linux.ibm.com>

Some CLP response data was accidentally dropped when fixing endianness
issues with the Query PCI Function CLP response.  All of these values are
sent as 0s to the guest for emulated devices, so the impact is only
observed on passthrough devices.

Fixes: a4e2fff1b104 ("s390x/pci: fix endianness issues")
Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
Message-Id: <1613681609-9349-1-git-send-email-mjrosato@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/s390-pci-inst.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 654fac6c0a5b..4b8326afa4f2 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -284,10 +284,15 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
         stq_p(&resquery->sdma, pbdev->zpci_fn.sdma);
         stq_p(&resquery->edma, pbdev->zpci_fn.edma);
         stw_p(&resquery->pchid, pbdev->zpci_fn.pchid);
+        stw_p(&resquery->vfn, pbdev->zpci_fn.vfn);
         resquery->flags = pbdev->zpci_fn.flags;
         resquery->pfgid = pbdev->zpci_fn.pfgid;
+        resquery->pft = pbdev->zpci_fn.pft;
+        resquery->fmbl = pbdev->zpci_fn.fmbl;
         stl_p(&resquery->fid, pbdev->zpci_fn.fid);
         stl_p(&resquery->uid, pbdev->zpci_fn.uid);
+        memcpy(resquery->pfip, pbdev->zpci_fn.pfip, CLP_PFIP_NR_SEGMENTS);
+        memcpy(resquery->util_str, pbdev->zpci_fn.util_str, CLP_UTIL_STR_LEN);
 
         for (i = 0; i < PCI_BAR_COUNT; i++) {
             uint32_t data = pci_get_long(pbdev->pdev->config +
-- 
2.26.2

[PULL 5/8] virtio-ccw: commands on revision-less devices

[Cornelia Huck]

The virtio standard specifies that any non-transitional device must
reject commands prior to revision setting (which we do). Devices
that are transitional need to assume revision 0 (legacy) if the
driver sends a non-revision-setting command first in order to
support legacy drivers. We neglected to do the latter.

Fortunately, nearly everything worked as intended anyway; the only
problem was not properly rejecting revision setting after some other
command had been issued. Easy to fix by setting revision to 0 if
we see a non-revision command on a legacy-capable revision-less
device.

Found by code inspection, not observed in the wild.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Halil Pasic <pasic@linux.ibm.com>
Message-Id: <20210216111830.1087847-1-cohuck@redhat.com>
---
 hw/s390x/virtio-ccw.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 4582e94ae7dc..06c06056814b 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -327,13 +327,20 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
                                    ccw.cmd_code);
     check_len = !((ccw.flags & CCW_FLAG_SLI) && !(ccw.flags & CCW_FLAG_DC));
 
-    if (dev->force_revision_1 && dev->revision < 0 &&
-        ccw.cmd_code != CCW_CMD_SET_VIRTIO_REV) {
-        /*
-         * virtio-1 drivers must start with negotiating to a revision >= 1,
-         * so post a command reject for all other commands
-         */
-        return -ENOSYS;
+    if (dev->revision < 0 && ccw.cmd_code != CCW_CMD_SET_VIRTIO_REV) {
+        if (dev->force_revision_1) {
+            /*
+             * virtio-1 drivers must start with negotiating to a revision >= 1,
+             * so post a command reject for all other commands
+             */
+            return -ENOSYS;
+        } else {
+            /*
+             * If the driver issues any command that is not SET_VIRTIO_REV,
+             * we'll have to operate the device in legacy mode.
+             */
+            dev->revision = 0;
+        }
     }
 
     /* Look at the command. */
-- 
2.26.2

[PULL 6/8] css: SCHIB measurement block origin must be aligned

[Cornelia Huck]

From: Pierre Morel <pmorel@linux.ibm.com>

The Measurement Block Origin inside the SCHIB is used when
Measurement Block format 1 is in used and must be aligned
on 64 bytes otherwise an operand exception is recognized
when issuing the Modify Sub CHannel (MSCH) instruction.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1613741973-3711-2-git-send-email-pmorel@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/ioinst.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index a412926d2780..1ee11522e150 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -121,6 +121,12 @@ static int ioinst_schib_valid(SCHIB *schib)
     if (be32_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_XMWME) {
         return 0;
     }
+    /* for MB format 1 bits 26-31 of word 11 must be 0 */
+    /* MBA uses words 10 and 11, it means align on 2**6 */
+    if ((be16_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
+        (be64_to_cpu(schib->mba) & 0x03fUL)) {
+        return 0;
+    }
     return 1;
 }
 
-- 
2.26.2

[PULL 7/8] vfio-ccw: Do not read region ret_code after write

[Cornelia Huck]

From: Eric Farman <farman@linux.ibm.com>

A pwrite() call returns the number of bytes written (or -1 on error),
and vfio-ccw compares this number with the size of the region to
determine if an error had occurred or not.

If they are not equal, this is a failure and the errno is used to
determine exactly how things failed. An errno of zero is possible
(though unlikely) in this situation and would be translated to a
successful operation.

If they ARE equal, the ret_code field is read from the region to
determine how to proceed. While the kernel sets the ret_code field
as necessary, the region and thus this field is not "written back"
to the user. So the value can only be what it was initialized to,
which is zero.

So, let's convert an unexpected length with errno of zero to a
return code of -EFAULT, and explicitly set an expected length to
a return code of zero. This will be a little safer and clearer.

Suggested-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
Message-Id: <20210303160739.2179378-1-farman@linux.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/vfio/ccw.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index bc78a0ad7638..b2df708e4b01 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -104,9 +104,9 @@ again:
             goto again;
         }
         error_report("vfio-ccw: write I/O region failed with errno=%d", errno);
-        ret = -errno;
+        ret = errno ? -errno : -EFAULT;
     } else {
-        ret = region->ret_code;
+        ret = 0;
     }
     switch (ret) {
     case 0:
@@ -192,9 +192,9 @@ again:
             goto again;
         }
         error_report("vfio-ccw: write cmd region failed with errno=%d", errno);
-        ret = -errno;
+        ret = errno ? -errno : -EFAULT;
     } else {
-        ret = region->ret_code;
+        ret = 0;
     }
     switch (ret) {
     case 0:
@@ -232,9 +232,9 @@ again:
             goto again;
         }
         error_report("vfio-ccw: write cmd region failed with errno=%d", errno);
-        ret = -errno;
+        ret = errno ? -errno : -EFAULT;
     } else {
-        ret = region->ret_code;
+        ret = 0;
     }
     switch (ret) {
     case 0:
-- 
2.26.2

[PULL 8/8] target/s390x/kvm: Simplify debug code

[Cornelia Huck]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

We already have the 'run' variable holding 'cs->kvm_run' value.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20210303182219.1631042-3-philmd@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/kvm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 7a892d663df6..73f816a72227 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1785,8 +1785,7 @@ static int handle_intercept(S390CPU *cpu)
     int icpt_code = run->s390_sieic.icptcode;
     int r = 0;
 
-    DPRINTF("intercept: 0x%x (at 0x%lx)\n", icpt_code,
-            (long)cs->kvm_run->psw_addr);
+    DPRINTF("intercept: 0x%x (at 0x%lx)\n", icpt_code, (long)run->psw_addr);
     switch (icpt_code) {
         case ICPT_INSTRUCTION:
         case ICPT_PV_INSTR:
-- 
2.26.2

Re: [PULL 0/8] s390x update

[Peter Maydell]

On Fri, 5 Mar 2021 at 15:55, Cornelia Huck <cohuck@redhat.com> wrote:
>
> The following changes since commit c40ae5a3ee387b13116948cbfe7824f03311db7e:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-03-03 16:55:15 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/cohuck/qemu.git tags/s390x-20210305
>
> for you to fetch changes up to 39d5d1404ed695f4a1cd2b117a6cf2d92dd8e8b9:
>
>   target/s390x/kvm: Simplify debug code (2021-03-04 14:19:08 +0100)
>
> ----------------------------------------------------------------
> some accumulated s390x fixes
>
> ----------------------------------------------------------------
>



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM