From 0110253e690f37ff0add0c8d75f47747041d75fa Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Mon, 25 Jan 2021 14:53:32 +0100 Subject: [PATCH 1/8] s390x/cpu_model: disallow unpack for --only-migratable 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 Reviewed-by: David Hildenbrand Reviewed-by: Halil Pasic Message-Id: <20210125135332.181324-1-borntraeger@de.ibm.com> Signed-off-by: Cornelia Huck --- 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 35179f9dc7..dd474c5e9a 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)) { From ea1b90b4fcb1230b2c85f3fd4ee09a84ddca7a6f Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 5 Feb 2021 10:39:21 +0100 Subject: [PATCH 2/8] target/s390x/arch_dump: Fix warning for the name field in the PT_NOTE section 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 Reviewed-by: Christian Borntraeger Message-Id: <20210205093921.848260-1-thuth@redhat.com> Signed-off-by: Cornelia Huck --- 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 50fa0ae4b6..cc1330876b 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(¬e, 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)(¬e, cpu, id); note_size = sizeof(note) - sizeof(note.contents) + nf->contents_size; From 24056cbfd577fd219d55c03f69df66e6351456e7 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 18 Feb 2021 04:40:59 +0100 Subject: [PATCH 3/8] hw/s390x: fix build for virtio-9p-ccw 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 Fixes: 2c44220d05 ("meson: convert hw/arch*") Reported-by: Jakob Naucke Cc: qemu-stable@nongnu.org Reviewed-by: Thomas Huth Message-Id: <20210218034059.1096078-1-pasic@linux.ibm.com> Signed-off-by: Cornelia Huck --- 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 2a7818d94b..91495b5631 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) From 403af209db8c030ed1e000640cd3cd80c6882883 Mon Sep 17 00:00:00 2001 From: Matthew Rosato Date: Thu, 18 Feb 2021 15:53:29 -0500 Subject: [PATCH 4/8] s390x/pci: restore missing Query PCI Function CLP data 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 Message-Id: <1613681609-9349-1-git-send-email-mjrosato@linux.ibm.com> Signed-off-by: Cornelia Huck --- 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 654fac6c0a..4b8326afa4 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 + From 151fcdfd628a396845b275831e920ddbc10a9f18 Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Tue, 16 Feb 2021 12:18:30 +0100 Subject: [PATCH 5/8] virtio-ccw: commands on revision-less devices 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 Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Acked-by: Halil Pasic 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 4582e94ae7..06c0605681 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. */ From a54b8ac340c20531daa89929c5ce7fed89fa401d Mon Sep 17 00:00:00 2001 From: Pierre Morel Date: Fri, 19 Feb 2021 14:39:33 +0100 Subject: [PATCH 6/8] css: SCHIB measurement block origin must be aligned 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 Reviewed-by: Thomas Huth Message-Id: <1613741973-3711-2-git-send-email-pmorel@linux.ibm.com> Signed-off-by: Cornelia Huck --- target/s390x/ioinst.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c index a412926d27..1ee11522e1 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; } From d6cd66311f527ee29c1d7b0988059cda00ad92fa Mon Sep 17 00:00:00 2001 From: Eric Farman Date: Wed, 3 Mar 2021 17:07:39 +0100 Subject: [PATCH 7/8] vfio-ccw: Do not read region ret_code after write 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 Signed-off-by: Eric Farman Message-Id: <20210303160739.2179378-1-farman@linux.ibm.com> Signed-off-by: Cornelia Huck --- 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 bc78a0ad76..b2df708e4b 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: From 39d5d1404ed695f4a1cd2b117a6cf2d92dd8e8b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 3 Mar 2021 19:22:02 +0100 Subject: [PATCH 8/8] target/s390x/kvm: Simplify debug code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We already have the 'run' variable holding 'cs->kvm_run' value. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Message-Id: <20210303182219.1631042-3-philmd@redhat.com> Signed-off-by: Cornelia Huck --- 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 7a892d663d..73f816a722 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: