From 94b5024b1f466b4b8f03e7b2219f77e9a3a24d51 Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Thu, 26 Jan 2017 17:47:40 +0100 Subject: [PATCH 01/13] s390x/s390-virtio: get rid of DPRINTF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DPRINTF approach is likely to introduce bitrot, and the preferred way for debugging is tracing anyway. Fortunately, there are no users (left), so nuke it. Signed-off-by: Cornelia Huck Reviewed-by: Halil Pasic Reviewed-by: Philippe Mathieu-Daudé --- hw/s390x/s390-virtio.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c index 7a3a7fe5fd..9cfb09057e 100644 --- a/hw/s390x/s390-virtio.c +++ b/hw/s390x/s390-virtio.c @@ -44,16 +44,6 @@ #include "hw/s390x/ipl.h" #include "cpu.h" -//#define DEBUG_S390 - -#ifdef DEBUG_S390 -#define DPRINTF(fmt, ...) \ - do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) -#else -#define DPRINTF(fmt, ...) \ - do { } while (0) -#endif - #define MAX_BLK_DEVS 10 #define S390_TOD_CLOCK_VALUE_MISSING 0x00 From 409422cd83f36450a3c0bdd49163786cbff3a7ea Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Thu, 26 Jan 2017 20:23:30 +0100 Subject: [PATCH 02/13] s390x/kvm: detect some program check loops Sometimes (e.g. early boot) a guest is broken in such ways that it loops 100% delivering operation exceptions (illegal operation) but the pgm new PSW is not set properly. This will result in code being read from address zero, which usually contains another illegal op. Let's detect this case and put the guest in crashed state. Instead of only detecting this for address zero apply a heuristic that will work for any program check new psw so that it will also reach the crashed state if you provide some random elf file to the -kernel option. We do not want guest problem state to be able to trigger a guest panic, e.g. by faulting on an address that is the same as the program check new PSW, so we check for the problem state bit being off. With this we a: get rid of CPU consumption of such broken guests b: keep the program old PSW. This allows to find out the original illegal operation - making debugging such early boot issues much easier than with single stepping This relies on the kernel using a similar heuristic and passing such operation exceptions to user space. Signed-off-by: Christian Borntraeger Signed-off-by: Cornelia Huck --- target/s390x/kvm.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 25367807f4..5ec050cf89 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -1867,6 +1867,40 @@ static void unmanageable_intercept(S390CPU *cpu, const char *str, int pswoffset) qemu_system_guest_panicked(NULL); } +/* try to detect pgm check loops */ +static int handle_oper_loop(S390CPU *cpu, struct kvm_run *run) +{ + CPUState *cs = CPU(cpu); + PSW oldpsw, newpsw; + + cpu_synchronize_state(cs); + newpsw.mask = ldq_phys(cs->as, cpu->env.psa + + offsetof(LowCore, program_new_psw)); + newpsw.addr = ldq_phys(cs->as, cpu->env.psa + + offsetof(LowCore, program_new_psw) + 8); + oldpsw.mask = run->psw_mask; + oldpsw.addr = run->psw_addr; + /* + * Avoid endless loops of operation exceptions, if the pgm new + * PSW will cause a new operation exception. + * The heuristic checks if the pgm new psw is within 6 bytes before + * the faulting psw address (with same DAT, AS settings) and the + * new psw is not a wait psw and the fault was not triggered by + * problem state. In that case go into crashed state. + */ + + if (oldpsw.addr - newpsw.addr <= 6 && + !(newpsw.mask & PSW_MASK_WAIT) && + !(oldpsw.mask & PSW_MASK_PSTATE) && + (newpsw.mask & PSW_MASK_ASC) == (oldpsw.mask & PSW_MASK_ASC) && + (newpsw.mask & PSW_MASK_DAT) == (oldpsw.mask & PSW_MASK_DAT)) { + unmanageable_intercept(cpu, "operation exception loop", + offsetof(LowCore, program_new_psw)); + return EXCP_HALTED; + } + return 0; +} + static int handle_intercept(S390CPU *cpu) { CPUState *cs = CPU(cpu); @@ -1914,11 +1948,14 @@ static int handle_intercept(S390CPU *cpu) r = EXCP_HALTED; break; case ICPT_OPEREXC: - /* currently only instr 0x0000 after enabled via capability */ + /* check for break points */ r = handle_sw_breakpoint(cpu, run); if (r == -ENOENT) { - enter_pgmcheck(cpu, PGM_OPERATION); - r = 0; + /* Then check for potential pgm check loops */ + r = handle_oper_loop(cpu, run); + if (r == 0) { + enter_pgmcheck(cpu, PGM_OPERATION); + } } break; case ICPT_SOFT_INTERCEPT: From ba690c7171cb863643c21ab9f7e7116ad5df1c63 Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Wed, 25 Jan 2017 16:01:03 +0100 Subject: [PATCH 03/13] s390x/flic: fail migration on source already Current code puts a 'FLIC_FAILED' marker into the migration stream to indicate something went wrong while saving flic state and fails load if it encounters that marker. VMState's put routine recently gained the ability to return error codes (but did not wire it up yet). In order to be able to reap the benefits of returning an error and failing migration on the source already once this gets wired up in core, return an error in addition to storing 'FLIC_FAILED'. Suggested-by: Dr. David Alan Gilbert Signed-off-by: Cornelia Huck Reviewed-by: Jens Freimann Reviewed-by: Christian Borntraeger --- hw/intc/s390_flic_kvm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c index e86a84e49a..cc44bc4e1e 100644 --- a/hw/intc/s390_flic_kvm.c +++ b/hw/intc/s390_flic_kvm.c @@ -293,6 +293,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size, int len = FLIC_SAVE_INITIAL_SIZE; void *buf; int count; + int r = 0; flic_disable_wait_pfault((struct KVMS390FLICState *) opaque); @@ -303,7 +304,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size, * migration state */ error_report("flic: couldn't allocate memory"); qemu_put_be64(f, FLIC_FAILED); - return 0; + return -ENOMEM; } count = __get_all_irqs(flic, &buf, len); @@ -314,6 +315,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size, * target system to fail when attempting to load irqs from the * migration state */ qemu_put_be64(f, FLIC_FAILED); + r = count; } else { qemu_put_be64(f, count); qemu_put_buffer(f, (uint8_t *) buf, @@ -321,7 +323,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size, } g_free(buf); - return 0; + return r; } /** From 47e13dfd86e33397d55bc3c3d08ac0ec3088b138 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Tue, 14 Feb 2017 14:06:07 +0100 Subject: [PATCH 04/13] virtio-ccw: handle virtio 1 only devices As a preparation for wiring-up virtio-crypto, the first non-transitional virtio device on the ccw transport, let us introduce a mechanism for disabling revision 0. This is more or less equivalent with disabling legacy as revision 0 is legacy only, and legacy drivers use the revision 0 exclusively. Signed-off-by: Halil Pasic Signed-off-by: Cornelia Huck --- hw/s390x/virtio-ccw.c | 18 +++++++++++++++++- hw/s390x/virtio-ccw.h | 1 + 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 63c46373fb..32c414f23d 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -280,6 +280,15 @@ 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; + } + /* Look at the command. */ switch (ccw.cmd_code) { case CCW_CMD_SET_VQ: @@ -638,7 +647,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) * need to fetch it here. Nothing to do for now, though. */ if (dev->revision >= 0 || - revinfo.revision > virtio_ccw_rev_max(dev)) { + revinfo.revision > virtio_ccw_rev_max(dev) || + (dev->force_revision_1 && !revinfo.revision)) { ret = -ENOSYS; break; } @@ -669,6 +679,12 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp) if (!sch) { return; } + 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; + } sch->driver_data = dev; sch->ccw_cb = virtio_ccw_cb; diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 77d10f1671..6a04e94f66 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -94,6 +94,7 @@ struct VirtioCcwDevice { IndAddr *indicators2; IndAddr *summary_indicator; uint64_t ind_bit; + bool force_revision_1; }; /* The maximum virtio revision we support. */ From d2256070d207ec49a42bed6a99c2cf55a720fb43 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 3 Nov 2016 13:39:15 +0100 Subject: [PATCH 05/13] virtio-ccw: add virtio-crypto-ccw device Wire up virtio-crypto for the CCW based VIRTIO. Signed-off-by: Halil Pasic Signed-off-by: Cornelia Huck --- hw/s390x/virtio-ccw.c | 61 +++++++++++++++++++++++++++++++++++++++++++ hw/s390x/virtio-ccw.h | 12 +++++++++ 2 files changed, 73 insertions(+) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 32c414f23d..613d8c6615 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -894,6 +894,24 @@ static void virtio_ccw_rng_realize(VirtioCcwDevice *ccw_dev, Error **errp) NULL); } +static void virtio_ccw_crypto_realize(VirtioCcwDevice *ccw_dev, Error **errp) +{ + VirtIOCryptoCcw *dev = VIRTIO_CRYPTO_CCW(ccw_dev); + DeviceState *vdev = DEVICE(&dev->vdev); + Error *err = NULL; + + qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus)); + object_property_set_bool(OBJECT(vdev), true, "realized", &err); + if (err) { + error_propagate(errp, err); + return; + } + + object_property_set_link(OBJECT(vdev), + OBJECT(dev->vdev.conf.cryptodev), "cryptodev", + NULL); +} + /* DeviceState to VirtioCcwDevice. Note: used on datapath, * be careful and test performance if you change this. */ @@ -1534,6 +1552,48 @@ static const TypeInfo virtio_ccw_rng = { .class_init = virtio_ccw_rng_class_init, }; +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, + VIRTIO_CCW_MAX_REV), + DEFINE_PROP_END_OF_LIST(), +}; + +static void virtio_ccw_crypto_instance_init(Object *obj) +{ + VirtIOCryptoCcw *dev = VIRTIO_CRYPTO_CCW(obj); + VirtioCcwDevice *ccw_dev = VIRTIO_CCW_DEVICE(obj); + + ccw_dev->force_revision_1 = true; + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev), + TYPE_VIRTIO_CRYPTO); + + object_property_add_alias(obj, "cryptodev", OBJECT(&dev->vdev), + "cryptodev", &error_abort); +} + +static void virtio_ccw_crypto_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass); + + k->realize = virtio_ccw_crypto_realize; + k->exit = virtio_ccw_exit; + dc->reset = virtio_ccw_reset; + dc->props = virtio_ccw_crypto_properties; + set_bit(DEVICE_CATEGORY_MISC, dc->categories); +} + +static const TypeInfo virtio_ccw_crypto = { + .name = TYPE_VIRTIO_CRYPTO_CCW, + .parent = TYPE_VIRTIO_CCW_DEVICE, + .instance_size = sizeof(VirtIOCryptoCcw), + .instance_init = virtio_ccw_crypto_instance_init, + .class_init = virtio_ccw_crypto_class_init, +}; + static void virtio_ccw_busdev_realize(DeviceState *dev, Error **errp) { VirtioCcwDevice *_dev = (VirtioCcwDevice *)dev; @@ -1736,6 +1796,7 @@ static void virtio_ccw_register(void) #ifdef CONFIG_VHOST_VSOCK type_register_static(&vhost_vsock_ccw_info); #endif + type_register_static(&virtio_ccw_crypto); } type_init(virtio_ccw_register) diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 6a04e94f66..41d4010378 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -22,6 +22,7 @@ #endif #include "hw/virtio/virtio-balloon.h" #include "hw/virtio/virtio-rng.h" +#include "hw/virtio/virtio-crypto.h" #include "hw/virtio/virtio-bus.h" #ifdef CONFIG_VHOST_VSOCK #include "hw/virtio/vhost-vsock.h" @@ -183,6 +184,17 @@ typedef struct VirtIORNGCcw { VirtIORNG vdev; } VirtIORNGCcw; +/* virtio-crypto-ccw */ + +#define TYPE_VIRTIO_CRYPTO_CCW "virtio-crypto-ccw" +#define VIRTIO_CRYPTO_CCW(obj) \ + OBJECT_CHECK(VirtIOCryptoCcw, (obj), TYPE_VIRTIO_CRYPTO_CCW) + +typedef struct VirtIOCryptoCcw { + VirtioCcwDevice parent_obj; + VirtIOCrypto vdev; +} VirtIOCryptoCcw; + VirtIODevice *virtio_ccw_get_vdev(SubchDev *sch); #ifdef CONFIG_VIRTFS From 797b608638c5ac7492e789563bcbed9feff0423d Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Thu, 10 Dec 2015 12:55:06 +0100 Subject: [PATCH 06/13] virtio-ccw: Check the number of vqs in CCW_CMD_SET_IND We cannot support more than 64 virtqueues with the 64 bits provided by classic indicators. If a driver tries to setup classic indicators (which it is free to do even for virtio-1 devices) for a device with more than 64 virtqueues, we should reject the attempt so that the driver does not end up with an unusable device. This is in preparation for bumping the number of supported virtqueues on the ccw transport. Signed-off-by: Halil Pasic Reviewed-by: Cornelia Huck Reviewed-by: Christian Borntraeger Signed-off-by: Cornelia Huck --- hw/s390x/virtio-ccw.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 613d8c6615..771411ea3c 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -35,6 +35,8 @@ #include "trace.h" #include "hw/s390x/css-bridge.h" +#define NR_CLASSIC_INDICATOR_BITS 64 + static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size, VirtioCcwDevice *dev); @@ -509,6 +511,11 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) ret = -ENOSYS; break; } + if (virtio_get_num_queues(vdev) > NR_CLASSIC_INDICATOR_BITS) { + /* More queues than indicator bits --> trigger a reject */ + ret = -ENOSYS; + break; + } if (!ccw.cda) { ret = -EFAULT; } else { From e61cc6b5c6909fa69059036bb910ef1725dc7f90 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Fri, 9 Dec 2016 13:51:46 +0100 Subject: [PATCH 07/13] s390x: add property adapter_routes_max_batch To make virtio-ccw supports more that 64 virtqueues we will have to increase ADAPTER_ROUTES_MAX_GSI which is currently limiting the number if possible adapter routes. Of course increasing the number of supported routes can break backwards migration. Let us introduce a compatibility property adapter_routes_max_batch so client code can use the some old limit if in compatibility mode and retain the migration compatibility. Signed-off-by: Halil Pasic Signed-off-by: Cornelia Huck --- hw/intc/s390_flic.c | 28 ++++++++++++++++++++++++++++ include/hw/s390x/s390_flic.h | 2 ++ 2 files changed, 30 insertions(+) diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c index 6ab29efc65..bef4caf980 100644 --- a/hw/intc/s390_flic.c +++ b/hw/intc/s390_flic.c @@ -16,6 +16,8 @@ #include "migration/qemu-file.h" #include "hw/s390x/s390_flic.h" #include "trace.h" +#include "hw/qdev.h" +#include "qapi/error.h" S390FLICState *s390_get_flic(void) { @@ -85,6 +87,30 @@ static void qemu_s390_flic_class_init(ObjectClass *oc, void *data) fsc->clear_io_irq = qemu_s390_clear_io_flic; } +static Property s390_flic_common_properties[] = { + DEFINE_PROP_UINT32("adapter_routes_max_batch", S390FLICState, + adapter_routes_max_batch, ADAPTER_ROUTES_MAX_GSI), + DEFINE_PROP_END_OF_LIST(), +}; + +static void s390_flic_common_realize(DeviceState *dev, Error **errp) +{ + uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch; + + if (max_batch > ADAPTER_ROUTES_MAX_GSI) { + error_setg(errp, "flic adapter_routes_max_batch too big" + "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI); + } +} + +static void s390_flic_class_init(ObjectClass *oc, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(oc); + + dc->props = s390_flic_common_properties; + dc->realize = s390_flic_common_realize; +} + static const TypeInfo qemu_s390_flic_info = { .name = TYPE_QEMU_S390_FLIC, .parent = TYPE_S390_FLIC_COMMON, @@ -92,10 +118,12 @@ static const TypeInfo qemu_s390_flic_info = { .class_init = qemu_s390_flic_class_init, }; + static const TypeInfo s390_flic_common_info = { .name = TYPE_S390_FLIC_COMMON, .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(S390FLICState), + .class_init = s390_flic_class_init, .class_size = sizeof(S390FLICStateClass), }; diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h index 9094edadf5..9f0b05c71b 100644 --- a/include/hw/s390x/s390_flic.h +++ b/include/hw/s390x/s390_flic.h @@ -32,6 +32,8 @@ typedef struct AdapterRoutes { typedef struct S390FLICState { SysBusDevice parent_obj; + /* to limit AdapterRoutes.num_routes for compat */ + uint32_t adapter_routes_max_batch; } S390FLICState; From 0708afa704f4f06d823af5757b1a331cc5fce438 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Fri, 9 Dec 2016 19:58:10 +0100 Subject: [PATCH 08/13] virtio-ccw: check flic->adapter_routes_max_batch Currently VIRTIO_CCW_QUEUE_MAX is defined as ADAPTER_ROUTES_MAX_GSI. That is when checking queue max we implicitly check the constraint concerning the number of adapter routes. This won't be satisfactory any more (due to backward migration considerations) if ADAPTER_ROUTES_MAX_GSI changes (ADAPTER_ROUTES_MAX_GSI is going to change because we want to support up to VIRTIO_QUEUE_MAX queues per virtio-ccw device). Let us introduce a check on a recently introduce flic property which gives us the compatibility machine aware limit on adapter routes. Signed-off-by: Halil Pasic Signed-off-by: Cornelia Huck --- hw/s390x/virtio-ccw.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 771411ea3c..a2ea95947f 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -1319,6 +1319,7 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) CcwDevice *ccw_dev = CCW_DEVICE(d); SubchDev *sch = ccw_dev->sch; int n = virtio_get_num_queues(vdev); + S390FLICState *flic = s390_get_flic(); if (!virtio_has_feature(vdev->host_features, VIRTIO_F_VERSION_1)) { dev->max_rev = 0; @@ -1330,6 +1331,12 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) VIRTIO_CCW_QUEUE_MAX); return; } + if (virtio_get_num_queues(vdev) > flic->adapter_routes_max_batch) { + error_setg(errp, "The number of virtqueues %d " + "exceeds flic adapter route limit %d", n, + flic->adapter_routes_max_batch); + return; + } sch->id.cu_model = virtio_bus_get_vdev_id(&dev->bus); From 069097dad311ac9c6933d95ebee7c0b53b3378c4 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Fri, 9 Dec 2016 20:00:21 +0100 Subject: [PATCH 09/13] s390x: bump ADAPTER_ROUTES_MAX_GSI Let's increase ADAPTER_ROUTES_MAX_GSI to VIRTIO_QUEUE_MAX which is the largest demand foreseeable at the moment. Let us add a compatibility macro for the previous machines so client code can maintain backwards migration compatibility To not mess up migration compatibility for virtio-ccw VIRTIO_CCW_QUEUE_MAX is left at it's current value, and will be dropped when virtio-ccw is converted to use the capability of the flic introduced by this patch. Signed-off-by: Halil Pasic Signed-off-by: Cornelia Huck --- hw/s390x/s390-virtio-ccw.c | 7 ++++++- include/hw/s390x/s390_flic.h | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index e9a676797a..ea244bbf55 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -336,7 +336,12 @@ static const TypeInfo ccw_machine_info = { type_init(ccw_machine_register_##suffix) #define CCW_COMPAT_2_8 \ - HW_COMPAT_2_8 + HW_COMPAT_2_8 \ + {\ + .driver = TYPE_S390_FLIC_COMMON,\ + .property = "adapter_routes_max_batch",\ + .value = "64",\ + }, #define CCW_COMPAT_2_7 \ HW_COMPAT_2_7 diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h index 9f0b05c71b..7f8ec7541b 100644 --- a/include/hw/s390x/s390_flic.h +++ b/include/hw/s390x/s390_flic.h @@ -17,8 +17,14 @@ #include "hw/s390x/adapter.h" #include "hw/virtio/virtio.h" -#define ADAPTER_ROUTES_MAX_GSI 64 -#define VIRTIO_CCW_QUEUE_MAX ADAPTER_ROUTES_MAX_GSI +/* + * Reserve enough gsis to accommodate all virtio devices. + * If any other user of adapter routes needs more of these, + * we need to bump the value; but virtio looks like the + * maximum right now. + */ +#define ADAPTER_ROUTES_MAX_GSI VIRTIO_QUEUE_MAX +#define VIRTIO_CCW_QUEUE_MAX 64 typedef struct AdapterRoutes { AdapterInfo adapter; From b1914b824ade1706847428e64ef5637ffc0ae238 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Mon, 7 Dec 2015 16:45:17 +0100 Subject: [PATCH 10/13] virtio-ccw: support VIRTIO_QUEUE_MAX virtqueues The maximal number of virtqueues per device can be limited on a per transport basis. For virtio-ccw this limit is defined by VIRTIO_CCW_QUEUE_MAX, however the limitation used to come form the number of adapter routes supported by flic (via notifiers). Recently the limitation of the flic was adjusted so that it can accommodate VIRTIO_QUEUE_MAX queues, and is in the meanwhile checked for separately too. Let us remove the transport specific limitation of virtio-ccw by dropping VIRTIO_CCW_QUEUE_MAX and using VIRTIO_QUEUE_MAX instead. Signed-off-by: Halil Pasic Signed-off-by: Cornelia Huck --- hw/s390x/s390-virtio-ccw.c | 2 +- hw/s390x/virtio-ccw.c | 16 ++++++++-------- include/hw/s390x/s390_flic.h | 1 - 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index ea244bbf55..4f0d62b2d8 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -63,7 +63,7 @@ static int virtio_ccw_hcall_notify(const uint64_t *args) if (!sch || !css_subch_visible(sch)) { return -EINVAL; } - if (queue >= VIRTIO_CCW_QUEUE_MAX) { + if (queue >= VIRTIO_QUEUE_MAX) { return -EINVAL; } virtio_queue_notify(virtio_ccw_get_vdev(sch), queue); diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index a2ea95947f..00b3bde4e9 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -128,7 +128,7 @@ static int virtio_ccw_set_vqs(SubchDev *sch, VqInfoBlock *info, uint16_t num = info ? info->num : linfo->num; uint64_t desc = info ? info->desc : linfo->queue; - if (index >= VIRTIO_CCW_QUEUE_MAX) { + if (index >= VIRTIO_QUEUE_MAX) { return -EINVAL; } @@ -164,7 +164,7 @@ static int virtio_ccw_set_vqs(SubchDev *sch, VqInfoBlock *info, virtio_queue_set_vector(vdev, index, index); } /* tell notify handler in case of config change */ - vdev->config_vector = VIRTIO_CCW_QUEUE_MAX; + vdev->config_vector = VIRTIO_QUEUE_MAX; return 0; } @@ -565,7 +565,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) ccw.cda, MEMTXATTRS_UNSPECIFIED, NULL); - if (vq_config.index >= VIRTIO_CCW_QUEUE_MAX) { + if (vq_config.index >= VIRTIO_QUEUE_MAX) { ret = -EINVAL; break; } @@ -960,11 +960,11 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector) uint64_t indicators; /* queue indicators + secondary indicators */ - if (vector >= VIRTIO_CCW_QUEUE_MAX + 64) { + if (vector >= VIRTIO_QUEUE_MAX + 64) { return; } - if (vector < VIRTIO_CCW_QUEUE_MAX) { + if (vector < VIRTIO_QUEUE_MAX) { if (!dev->indicators) { return; } @@ -1325,10 +1325,10 @@ static void virtio_ccw_device_plugged(DeviceState *d, Error **errp) dev->max_rev = 0; } - if (virtio_get_num_queues(vdev) > VIRTIO_CCW_QUEUE_MAX) { + if (virtio_get_num_queues(vdev) > VIRTIO_QUEUE_MAX) { error_setg(errp, "The number of virtqueues %d " - "exceeds ccw limit %d", n, - VIRTIO_CCW_QUEUE_MAX); + "exceeds virtio limit %d", n, + VIRTIO_QUEUE_MAX); return; } if (virtio_get_num_queues(vdev) > flic->adapter_routes_max_batch) { diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h index 7f8ec7541b..f9e6890c90 100644 --- a/include/hw/s390x/s390_flic.h +++ b/include/hw/s390x/s390_flic.h @@ -24,7 +24,6 @@ * maximum right now. */ #define ADAPTER_ROUTES_MAX_GSI VIRTIO_QUEUE_MAX -#define VIRTIO_CCW_QUEUE_MAX 64 typedef struct AdapterRoutes { AdapterInfo adapter; From 5f706fdc164b20b48254eadf7bd413edace34499 Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Wed, 8 Feb 2017 09:40:00 +0100 Subject: [PATCH 11/13] s390x/arch_dump: use proper note name and note size In binutils/libbfd (bfd/elf.c) it is enforced that all s390 specific ELF notes like e.g. NT_S390_PREFIX or NT_S390_CTRS have "LINUX" specified as note name and that the namesz is 6. Otherwise the notes are ignored. QEMU currently uses "CORE" for these notes. Up to now this has not been a real problem because the dump analysis tool "crash" does handle that. But it will break all programs that use libbfd for processing ELF notes. So fix this and use "LINUX" for all s390 specific notes to comply with libbfd. Also set the correct namesz. Reported-by: Philipp Rudo Signed-off-by: Christian Borntraeger Signed-off-by: Cornelia Huck --- target/s390x/arch_dump.c | 43 ++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c index 4731869f6b..887cae947e 100644 --- a/target/s390x/arch_dump.c +++ b/target/s390x/arch_dump.c @@ -59,8 +59,7 @@ typedef struct S390xElfVregsHiStruct S390xElfVregsHi; typedef struct noteStruct { Elf64_Nhdr hdr; - char name[5]; - char pad3[3]; + char name[8]; union { S390xElfPrstatus prstatus; S390xElfFpregset fpregset; @@ -162,13 +161,19 @@ static void s390x_write_elf64_prefix(Note *note, S390CPU *cpu) } -static const struct NoteFuncDescStruct { +typedef struct NoteFuncDescStruct { int contents_size; void (*note_contents_func)(Note *note, S390CPU *cpu); -} note_func[] = { +} NoteFuncDesc; + +static const NoteFuncDesc note_core[] = { {sizeof(((Note *)0)->contents.prstatus), s390x_write_elf64_prstatus}, - {sizeof(((Note *)0)->contents.prefix), s390x_write_elf64_prefix}, {sizeof(((Note *)0)->contents.fpregset), s390x_write_elf64_fpregset}, + { 0, NULL} +}; + +static const NoteFuncDesc note_linux[] = { + {sizeof(((Note *)0)->contents.prefix), s390x_write_elf64_prefix}, {sizeof(((Note *)0)->contents.ctrs), s390x_write_elf64_ctrs}, {sizeof(((Note *)0)->contents.timer), s390x_write_elf64_timer}, {sizeof(((Note *)0)->contents.todcmp), s390x_write_elf64_todcmp}, @@ -178,22 +183,20 @@ static const struct NoteFuncDescStruct { { 0, NULL} }; -typedef struct NoteFuncDescStruct NoteFuncDesc; - - -static int s390x_write_all_elf64_notes(const char *note_name, +static int s390x_write_elf64_notes(const char *note_name, WriteCoreDumpFunction f, S390CPU *cpu, int id, - void *opaque) + void *opaque, + const NoteFuncDesc *funcs) { Note note; const NoteFuncDesc *nf; int note_size; int ret = -1; - for (nf = note_func; nf->note_contents_func; nf++) { + for (nf = funcs; nf->note_contents_func; nf++) { memset(¬e, 0, sizeof(note)); - note.hdr.n_namesz = cpu_to_be32(sizeof(note.name)); + 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)); (*nf->note_contents_func)(¬e, cpu); @@ -215,7 +218,13 @@ int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs, int cpuid, void *opaque) { S390CPU *cpu = S390_CPU(cs); - return s390x_write_all_elf64_notes("CORE", f, cpu, cpuid, opaque); + int r; + + r = s390x_write_elf64_notes("CORE", f, cpu, cpuid, opaque, note_core); + if (r) { + return r; + } + return s390x_write_elf64_notes("LINUX", f, cpu, cpuid, opaque, note_linux); } int cpu_get_dump_info(ArchDumpInfo *info, @@ -230,7 +239,7 @@ int cpu_get_dump_info(ArchDumpInfo *info, ssize_t cpu_get_note_size(int class, int machine, int nr_cpus) { - int name_size = 8; /* "CORE" or "QEMU" rounded */ + int name_size = 8; /* "LINUX" or "CORE" + pad */ size_t elf_note_size = 0; int note_head_size; const NoteFuncDesc *nf; @@ -240,7 +249,11 @@ ssize_t cpu_get_note_size(int class, int machine, int nr_cpus) note_head_size = sizeof(Elf64_Nhdr); - for (nf = note_func; nf->note_contents_func; nf++) { + for (nf = note_core; nf->note_contents_func; nf++) { + elf_note_size = elf_note_size + note_head_size + name_size + + nf->contents_size; + } + for (nf = note_linux; nf->note_contents_func; nf++) { elf_note_size = elf_note_size + note_head_size + name_size + nf->contents_size; } From f738f296eaaed719508207ba36b995ba73fe27db Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Wed, 8 Feb 2017 13:36:17 +0100 Subject: [PATCH 12/13] s390x/arch_dump: pass cpuid into notes sections we need to pass the cpuid into the pid field of the notes section, otherwise the notes for different CPUs all have 0: e.g. objdump -h shows: old: 5 .reg-s390-prefix/0 00000004 0000000000000000 0000000000000000 6 .reg-s390-prefix 00000004 0000000000000000 0000000000000000 21 .reg-s390-prefix/0 00000004 0000000000000000 0000000000000000 new: 5 .reg-s390-prefix/1 00000004 0000000000000000 0000000000000000 6 .reg-s390-prefix 00000004 0000000000000000 0000000000000000 21 .reg-s390-prefix/2 00000004 0000000000000000 0000000000000000 Reported-by: Philipp Rudo Signed-off-by: Christian Borntraeger Signed-off-by: Cornelia Huck --- target/s390x/arch_dump.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c index 887cae947e..105ae9a5d8 100644 --- a/target/s390x/arch_dump.c +++ b/target/s390x/arch_dump.c @@ -73,7 +73,7 @@ typedef struct noteStruct { } contents; } QEMU_PACKED Note; -static void s390x_write_elf64_prstatus(Note *note, S390CPU *cpu) +static void s390x_write_elf64_prstatus(Note *note, S390CPU *cpu, int id) { int i; S390xUserRegs *regs; @@ -87,9 +87,10 @@ static void s390x_write_elf64_prstatus(Note *note, S390CPU *cpu) regs->acrs[i] = cpu_to_be32(cpu->env.aregs[i]); regs->gprs[i] = cpu_to_be64(cpu->env.regs[i]); } + note->contents.prstatus.pid = id; } -static void s390x_write_elf64_fpregset(Note *note, S390CPU *cpu) +static void s390x_write_elf64_fpregset(Note *note, S390CPU *cpu, int id) { int i; CPUS390XState *cs = &cpu->env; @@ -101,7 +102,7 @@ static void s390x_write_elf64_fpregset(Note *note, S390CPU *cpu) } } -static void s390x_write_elf64_vregslo(Note *note, S390CPU *cpu) +static void s390x_write_elf64_vregslo(Note *note, S390CPU *cpu, int id) { int i; @@ -111,7 +112,7 @@ static void s390x_write_elf64_vregslo(Note *note, S390CPU *cpu) } } -static void s390x_write_elf64_vregshi(Note *note, S390CPU *cpu) +static void s390x_write_elf64_vregshi(Note *note, S390CPU *cpu, int id) { int i; S390xElfVregsHi *temp_vregshi; @@ -125,25 +126,25 @@ static void s390x_write_elf64_vregshi(Note *note, S390CPU *cpu) } } -static void s390x_write_elf64_timer(Note *note, S390CPU *cpu) +static void s390x_write_elf64_timer(Note *note, S390CPU *cpu, int id) { note->hdr.n_type = cpu_to_be32(NT_S390_TIMER); note->contents.timer = cpu_to_be64((uint64_t)(cpu->env.cputm)); } -static void s390x_write_elf64_todcmp(Note *note, S390CPU *cpu) +static void s390x_write_elf64_todcmp(Note *note, S390CPU *cpu, int id) { note->hdr.n_type = cpu_to_be32(NT_S390_TODCMP); note->contents.todcmp = cpu_to_be64((uint64_t)(cpu->env.ckc)); } -static void s390x_write_elf64_todpreg(Note *note, S390CPU *cpu) +static void s390x_write_elf64_todpreg(Note *note, S390CPU *cpu, int id) { note->hdr.n_type = cpu_to_be32(NT_S390_TODPREG); note->contents.todpreg = cpu_to_be32((uint32_t)(cpu->env.todpr)); } -static void s390x_write_elf64_ctrs(Note *note, S390CPU *cpu) +static void s390x_write_elf64_ctrs(Note *note, S390CPU *cpu, int id) { int i; @@ -154,7 +155,7 @@ static void s390x_write_elf64_ctrs(Note *note, S390CPU *cpu) } } -static void s390x_write_elf64_prefix(Note *note, S390CPU *cpu) +static void s390x_write_elf64_prefix(Note *note, S390CPU *cpu, int id) { note->hdr.n_type = cpu_to_be32(NT_S390_PREFIX); note->contents.prefix = cpu_to_be32((uint32_t)(cpu->env.psa)); @@ -163,7 +164,7 @@ static void s390x_write_elf64_prefix(Note *note, S390CPU *cpu) typedef struct NoteFuncDescStruct { int contents_size; - void (*note_contents_func)(Note *note, S390CPU *cpu); + void (*note_contents_func)(Note *note, S390CPU *cpu, int id); } NoteFuncDesc; static const NoteFuncDesc note_core[] = { @@ -199,7 +200,7 @@ static int s390x_write_elf64_notes(const char *note_name, 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)); - (*nf->note_contents_func)(¬e, cpu); + (*nf->note_contents_func)(¬e, cpu, id); note_size = sizeof(note) - sizeof(note.contents) + nf->contents_size; ret = f(¬e, note_size, opaque); From 9f94f84ce7df633142953806cc4c102765cabc0e Mon Sep 17 00:00:00 2001 From: Dong Jia Shi Date: Tue, 21 Feb 2017 10:09:04 +0100 Subject: [PATCH 13/13] s390x/css: handle format-0 TIC CCW correctly For TIC CCW, bit positions 8-32 of the format-1 CCW must contain zeros; otherwise, a program-check condition is generated. For format-0 TIC CCWs, bits 32-63 are ignored. To convert TIC from format-0 CCW to format-1 CCW correctly, let's clear bits 8-32 to guarantee compatibility. Reviewed-by: Pierre Morel Signed-off-by: Dong Jia Shi Signed-off-by: Cornelia Huck --- hw/s390x/css.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 0f2580d644..e32b2a4d42 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -368,13 +368,16 @@ static CCW1 copy_ccw_from_guest(hwaddr addr, bool fmt1) ret.cda = be32_to_cpu(tmp1.cda); } else { cpu_physical_memory_read(addr, &tmp0, sizeof(tmp0)); - ret.cmd_code = tmp0.cmd_code; - ret.flags = tmp0.flags; - ret.count = be16_to_cpu(tmp0.count); - ret.cda = be16_to_cpu(tmp0.cda1) | (tmp0.cda0 << 16); - if ((ret.cmd_code & 0x0f) == CCW_CMD_TIC) { - ret.cmd_code &= 0x0f; + if ((tmp0.cmd_code & 0x0f) == CCW_CMD_TIC) { + ret.cmd_code = CCW_CMD_TIC; + ret.flags = 0; + ret.count = 0; + } else { + ret.cmd_code = tmp0.cmd_code; + ret.flags = tmp0.flags; + ret.count = be16_to_cpu(tmp0.count); } + ret.cda = be16_to_cpu(tmp0.cda1) | (tmp0.cda0 << 16); } return ret; }