* [Qemu-devel] [PATCH v2 0/2] ais fixups for 2.11 @ 2017-09-26 13:36 Christian Borntraeger 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available Christian Borntraeger 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines Christian Borntraeger 0 siblings, 2 replies; 12+ messages in thread From: Christian Borntraeger @ 2017-09-26 13:36 UTC (permalink / raw) To: Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, David Hildenbrand, Jason J . Herne, Christian Borntraeger v1->v2: - get rid of fs->ais_supported - update copyright Christian Borntraeger (2): s390x/ais: enable ais when migration is available s390x/ais: disable ais for compat machines hw/intc/s390_flic.c | 8 ++------ hw/intc/s390_flic_kvm.c | 18 +++++++++++++----- hw/s390x/css.c | 4 ++-- hw/s390x/s390-virtio-ccw.c | 20 +++++++++++++++++--- include/hw/s390x/s390-virtio-ccw.h | 3 +++ include/hw/s390x/s390_flic.h | 1 - target/s390x/kvm.c | 10 ++-------- 7 files changed, 39 insertions(+), 25 deletions(-) -- 2.9.4 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available 2017-09-26 13:36 [Qemu-devel] [PATCH v2 0/2] ais fixups for 2.11 Christian Borntraeger @ 2017-09-26 13:36 ` Christian Borntraeger 2017-09-26 13:43 ` David Hildenbrand 2017-09-27 5:45 ` Yi Min Zhao 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines Christian Borntraeger 1 sibling, 2 replies; 12+ messages in thread From: Christian Borntraeger @ 2017-09-26 13:36 UTC (permalink / raw) To: Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, David Hildenbrand, Jason J . Herne, Christian Borntraeger Instead of unconditionally enabling the KVM AIS capability in the kvm arch init function, do this in the flic realize function when we know if migration is available. This requires to initialize flic before the CPUs. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- hw/intc/s390_flic.c | 8 ++------ hw/intc/s390_flic_kvm.c | 16 +++++++++++----- hw/s390x/css.c | 4 ++-- hw/s390x/s390-virtio-ccw.c | 10 +++++++--- include/hw/s390x/s390_flic.h | 1 - target/s390x/kvm.c | 10 ++-------- 6 files changed, 24 insertions(+), 25 deletions(-) diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c index 6eaf178..9858f0e 100644 --- a/hw/intc/s390_flic.c +++ b/hw/intc/s390_flic.c @@ -1,7 +1,7 @@ /* * QEMU S390x floating interrupt controller (flic) * - * Copyright 2014 IBM Corp. + * Copyright IBM Corp. 2014, 2017 * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> * Cornelia Huck <cornelia.huck@de.ibm.com> * @@ -136,9 +136,7 @@ static void qemu_s390_flic_reset(DeviceState *dev) bool ais_needed(void *opaque) { - S390FLICState *s = opaque; - - return s->ais_supported; + return s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION); } static const VMStateDescription qemu_s390_flic_vmstate = { @@ -185,8 +183,6 @@ static void s390_flic_common_realize(DeviceState *dev, Error **errp) " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI); return; } - - fs->ais_supported = s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION); } static void s390_flic_class_init(ObjectClass *oc, void *data) diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c index 7ead17a..5c9d215 100644 --- a/hw/intc/s390_flic_kvm.c +++ b/hw/intc/s390_flic_kvm.c @@ -1,7 +1,7 @@ /* * QEMU S390x KVM floating interrupt controller (flic) * - * Copyright 2014 IBM Corp. + * Copyright IBM Corp. 2014, 2017 * Author(s): Jens Freimann <jfrei@linux.vnet.ibm.com> * Cornelia Huck <cornelia.huck@de.ibm.com> * @@ -164,7 +164,7 @@ static int kvm_s390_modify_ais_mode(S390FLICState *fs, uint8_t isc, .addr = (uint64_t)&req, }; - if (!fs->ais_supported) { + if (!s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { return -ENOSYS; } @@ -181,7 +181,7 @@ static int kvm_s390_inject_airq(S390FLICState *fs, uint8_t type, .attr = id, }; - if (!fs->ais_supported) { + if (!s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { return -ENOSYS; } @@ -459,7 +459,7 @@ static int kvm_flic_ais_post_load(void *opaque, int version_id) * migration from a host that has AIS to a host that has no AIS. * In that case the target system will reject the migration here. */ - if (!ais_needed(flic)) { + if (!s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { return -ENOSYS; } @@ -557,6 +557,12 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) test_attr.group = KVM_DEV_FLIC_CLEAR_IO_IRQ; flic_state->clear_io_supported = !ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr); + /* try enable the AIS facility */ + test_attr.group = KVM_DEV_FLIC_AISM_ALL; + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); + } + return; fail: error_propagate(errp, errp_local); @@ -578,7 +584,7 @@ static void kvm_s390_flic_reset(DeviceState *dev) flic_disable_wait_pfault(flic); - if (fs->ais_supported) { + if (s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { for (isc = 0; isc <= MAX_ISC; isc++) { rc = kvm_s390_modify_ais_mode(fs, isc, SIC_IRQ_MODE_ALL); if (rc) { diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 75d4f30..1d4bb73 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -1,7 +1,7 @@ /* * Channel subsystem base support. * - * Copyright 2012 IBM Corp. + * Copyright IBM Corp. 2012, 2017 * Author(s): Cornelia Huck <cornelia.huck@de.ibm.com> * * This work is licensed under the terms of the GNU GPL, version 2 or (at @@ -672,7 +672,7 @@ void css_adapter_interrupt(CssIoAdapterType type, uint8_t isc) } trace_css_adapter_interrupt(isc); - if (fs->ais_supported) { + if (s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { if (fsc->inject_airq(fs, type, isc, adapter->flags)) { error_report("Failed to inject airq with AIS supported"); exit(1); diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index fafbc6d..98c82c2 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -1,7 +1,7 @@ /* * virtio ccw machine * - * Copyright 2012 IBM Corp. + * Copyright IBM Corp. 2012, 2017 * Copyright (c) 2009 Alexander Graf <agraf@suse.de> * Author(s): Cornelia Huck <cornelia.huck@de.ibm.com> * @@ -279,11 +279,15 @@ static void ccw_init(MachineState *machine) s390_sclp_init(); s390_memory_init(machine->ram_size); + /* + * This might also enable some KVM features like AIS, so it must + * be called before the CPU model + */ + s390_flic_init(); + /* init CPUs (incl. CPU model) early so s390_has_feature() works */ s390_init_cpus(machine); - s390_flic_init(); - /* get a BUS */ css_bus = virtual_css_bus_init(); s390_init_ipl_dev(machine->kernel_filename, machine->kernel_cmdline, diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h index 7aab6ef..75fd83c 100644 --- a/include/hw/s390x/s390_flic.h +++ b/include/hw/s390x/s390_flic.h @@ -44,7 +44,6 @@ typedef struct S390FLICState { SysBusDevice parent_obj; /* to limit AdapterRoutes.num_routes for compat */ uint32_t adapter_routes_max_batch; - bool ais_supported; } S390FLICState; #define S390_FLIC_COMMON_CLASS(klass) \ diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index ebb75ca..9ee2ada 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -2,7 +2,7 @@ * QEMU S390x KVM implementation * * Copyright (c) 2009 Alexander Graf <agraf@suse.de> - * Copyright IBM Corp. 2012 + * Copyright IBM Corp. 2012, 2017 * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -311,13 +311,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s) } } - /* - * The migration interface for ais was introduced with kernel 4.13 - * but the capability itself had been active since 4.12. As migration - * support is considered necessary let's disable ais in the 2.10 - * machine. - */ - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ + /* The AIS enablement happens in the flic realize */ qemu_mutex_init(&qemu_sigp_mutex); -- 2.9.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available Christian Borntraeger @ 2017-09-26 13:43 ` David Hildenbrand 2017-09-26 14:06 ` Christian Borntraeger 2017-09-27 5:45 ` Yi Min Zhao 1 sibling, 1 reply; 12+ messages in thread From: David Hildenbrand @ 2017-09-26 13:43 UTC (permalink / raw) To: Christian Borntraeger, Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, Jason J . Herne > + /* try enable the AIS facility */ > + test_attr.group = KVM_DEV_FLIC_AISM_ALL; > + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { > + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); > + } > + > return; > fail: > error_propagate(errp, errp_local); > @@ -578,7 +584,7 @@ static void kvm_s390_flic_reset(DeviceState *dev) > > flic_disable_wait_pfault(flic); > > - if (fs->ais_supported) { > + if (s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { I don't remember if a reset is trigger on realization. Most probably not, but it could be. Would it hurt if the following code would not get called if the flic hasn't been used yet? (possible reset before cpu model has been initialized) - are kvm_s390_modify_ais_mode() calls required before ais can be used for the first time? If so, we could add a manual reset after the cpu model has been initialized. > for (isc = 0; isc <= MAX_ISC; isc++) { > rc = kvm_s390_modify_ais_mode(fs, isc, SIC_IRQ_MODE_ALL); > if (rc) { > diff --git a/hw/s390x/css.c b/hw/s390x/css.c > index 75d4f30..1d4bb73 100644 > --- a/hw/s390x/css.c > +++ b/hw/s390x/css.c > @@ -1,7 +1,7 @@ > /* > * Channel subsystem base support. > * > - * Copyright 2012 IBM Corp. > + * Copyright IBM Corp. 2012, 2017 > * Author(s): Cornelia Huck <cornelia.huck@de.ibm.com> > * > * This work is licensed under the terms of the GNU GPL, version 2 or (at > @@ -672,7 +672,7 @@ void css_adapter_interrupt(CssIoAdapterType type, uint8_t isc) > } > > trace_css_adapter_interrupt(isc); > - if (fs->ais_supported) { > + if (s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { > if (fsc->inject_airq(fs, type, isc, adapter->flags)) { > error_report("Failed to inject airq with AIS supported"); > exit(1); > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index fafbc6d..98c82c2 100644 > --- a/hw/s390x/s390-virtio-ccw.c Looks much better to me now. -- Thanks, David ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available 2017-09-26 13:43 ` David Hildenbrand @ 2017-09-26 14:06 ` Christian Borntraeger 2017-09-26 14:08 ` David Hildenbrand 2017-09-27 4:35 ` Yi Min Zhao 0 siblings, 2 replies; 12+ messages in thread From: Christian Borntraeger @ 2017-09-26 14:06 UTC (permalink / raw) To: David Hildenbrand, Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, Jason J . Herne On 09/26/2017 03:43 PM, David Hildenbrand wrote: > >> + /* try enable the AIS facility */ >> + test_attr.group = KVM_DEV_FLIC_AISM_ALL; >> + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { >> + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); >> + } >> + >> return; >> fail: >> error_propagate(errp, errp_local); >> @@ -578,7 +584,7 @@ static void kvm_s390_flic_reset(DeviceState *dev) >> >> flic_disable_wait_pfault(flic); >> >> - if (fs->ais_supported) { >> + if (s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { > > I don't remember if a reset is trigger on realization. Most probably > not, but it could be. > > Would it hurt if the following code would not get called if the flic > hasn't been used yet? (possible reset before cpu model has been > initialized) - are kvm_s390_modify_ais_mode() calls required before ais > can be used for the first time? Hmm, simm/nimm should default to zero in the kernel I guess. So I think it would not hurt. Yi Min, correct? Anyway it seems that reset is NOT called during realize, the first call is #0 0x00000000010e5178 kvm_s390_flic_reset (qemu-system-s390x) #1 0x000000000124bbc4 device_reset (qemu-system-s390x) #2 0x0000000001248cd0 qdev_reset_one (qemu-system-s390x) #3 0x0000000001249ea4 qdev_walk_children (qemu-system-s390x) #4 0x000000000124fd3a qbus_walk_children (qemu-system-s390x) #5 0x0000000001248e6c qbus_reset_all (qemu-system-s390x) #6 0x0000000001248eae qbus_reset_all_fn (qemu-system-s390x) #7 0x0000000001250a60 qemu_devices_reset (qemu-system-s390x) #8 0x0000000001143c80 s390_machine_reset (qemu-system-s390x) #9 0x00000000011c5b72 qemu_system_reset (qemu-system-s390x) #10 0x00000000011ceb8e main (qemu-system-s390x) #11 0x000003ff947a289a __libc_start_main (libc.so.6) #12 0x0000000001017646 _start (qemu-system-s390x) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available 2017-09-26 14:06 ` Christian Borntraeger @ 2017-09-26 14:08 ` David Hildenbrand 2017-09-27 4:35 ` Yi Min Zhao 1 sibling, 0 replies; 12+ messages in thread From: David Hildenbrand @ 2017-09-26 14:08 UTC (permalink / raw) To: Christian Borntraeger, Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, Jason J . Herne On 26.09.2017 16:06, Christian Borntraeger wrote: > > > On 09/26/2017 03:43 PM, David Hildenbrand wrote: >> >>> + /* try enable the AIS facility */ >>> + test_attr.group = KVM_DEV_FLIC_AISM_ALL; >>> + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { >>> + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); >>> + } >>> + >>> return; >>> fail: >>> error_propagate(errp, errp_local); >>> @@ -578,7 +584,7 @@ static void kvm_s390_flic_reset(DeviceState *dev) >>> >>> flic_disable_wait_pfault(flic); >>> >>> - if (fs->ais_supported) { >>> + if (s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { >> >> I don't remember if a reset is trigger on realization. Most probably >> not, but it could be. >> >> Would it hurt if the following code would not get called if the flic >> hasn't been used yet? (possible reset before cpu model has been >> initialized) - are kvm_s390_modify_ais_mode() calls required before ais >> can be used for the first time? > > Hmm, simm/nimm should default to zero in the kernel I guess. So I think it would > not hurt. Yi Min, correct? > > > Anyway it seems that reset is NOT called during realize, the first > call is > > #0 0x00000000010e5178 kvm_s390_flic_reset (qemu-system-s390x) > #1 0x000000000124bbc4 device_reset (qemu-system-s390x) > #2 0x0000000001248cd0 qdev_reset_one (qemu-system-s390x) > #3 0x0000000001249ea4 qdev_walk_children (qemu-system-s390x) > #4 0x000000000124fd3a qbus_walk_children (qemu-system-s390x) > #5 0x0000000001248e6c qbus_reset_all (qemu-system-s390x) > #6 0x0000000001248eae qbus_reset_all_fn (qemu-system-s390x) > #7 0x0000000001250a60 qemu_devices_reset (qemu-system-s390x) > #8 0x0000000001143c80 s390_machine_reset (qemu-system-s390x) > #9 0x00000000011c5b72 qemu_system_reset (qemu-system-s390x) > #10 0x00000000011ceb8e main (qemu-system-s390x) > #11 0x000003ff947a289a __libc_start_main (libc.so.6) > #12 0x0000000001017646 _start (qemu-system-s390x) > > Reviewed-by: David Hildenbrand <david@redhat.com> than -- Thanks, David ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available 2017-09-26 14:06 ` Christian Borntraeger 2017-09-26 14:08 ` David Hildenbrand @ 2017-09-27 4:35 ` Yi Min Zhao 1 sibling, 0 replies; 12+ messages in thread From: Yi Min Zhao @ 2017-09-27 4:35 UTC (permalink / raw) To: Christian Borntraeger, David Hildenbrand, Cornelia Huck Cc: Richard Henderson, Alexander Graf, Halil Pasic, qemu-devel, Jason J . Herne 在 2017/9/26 下午10:06, Christian Borntraeger 写道: > > On 09/26/2017 03:43 PM, David Hildenbrand wrote: >>> + /* try enable the AIS facility */ >>> + test_attr.group = KVM_DEV_FLIC_AISM_ALL; >>> + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { >>> + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); >>> + } >>> + >>> return; >>> fail: >>> error_propagate(errp, errp_local); >>> @@ -578,7 +584,7 @@ static void kvm_s390_flic_reset(DeviceState *dev) >>> >>> flic_disable_wait_pfault(flic); >>> >>> - if (fs->ais_supported) { >>> + if (s390_has_feat(S390_FEAT_ADAPTER_INT_SUPPRESSION)) { >> I don't remember if a reset is trigger on realization. Most probably >> not, but it could be. >> >> Would it hurt if the following code would not get called if the flic >> hasn't been used yet? (possible reset before cpu model has been >> initialized) - are kvm_s390_modify_ais_mode() calls required before ais >> can be used for the first time? > Hmm, simm/nimm should default to zero in the kernel I guess. So I think it would > not hurt. Yi Min, correct? Yes, the default mode is all interruption mode, IOW simm = 0 and nimm = 0. > > > Anyway it seems that reset is NOT called during realize, the first > call is > > #0 0x00000000010e5178 kvm_s390_flic_reset (qemu-system-s390x) > #1 0x000000000124bbc4 device_reset (qemu-system-s390x) > #2 0x0000000001248cd0 qdev_reset_one (qemu-system-s390x) > #3 0x0000000001249ea4 qdev_walk_children (qemu-system-s390x) > #4 0x000000000124fd3a qbus_walk_children (qemu-system-s390x) > #5 0x0000000001248e6c qbus_reset_all (qemu-system-s390x) > #6 0x0000000001248eae qbus_reset_all_fn (qemu-system-s390x) > #7 0x0000000001250a60 qemu_devices_reset (qemu-system-s390x) > #8 0x0000000001143c80 s390_machine_reset (qemu-system-s390x) > #9 0x00000000011c5b72 qemu_system_reset (qemu-system-s390x) > #10 0x00000000011ceb8e main (qemu-system-s390x) > #11 0x000003ff947a289a __libc_start_main (libc.so.6) > #12 0x0000000001017646 _start (qemu-system-s390x) > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available Christian Borntraeger 2017-09-26 13:43 ` David Hildenbrand @ 2017-09-27 5:45 ` Yi Min Zhao 2017-09-27 7:02 ` Christian Borntraeger 1 sibling, 1 reply; 12+ messages in thread From: Yi Min Zhao @ 2017-09-27 5:45 UTC (permalink / raw) To: Christian Borntraeger, Cornelia Huck Cc: Halil Pasic, David Hildenbrand, qemu-devel, Alexander Graf, Jason J . Herne, Richard Henderson 在 2017/9/26 下午9:36, Christian Borntraeger 写道: > @@ -557,6 +557,12 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) > test_attr.group = KVM_DEV_FLIC_CLEAR_IO_IRQ; > flic_state->clear_io_supported = !ioctl(flic_state->fd, > KVM_HAS_DEVICE_ATTR, test_attr); > + /* try enable the AIS facility */ > + test_attr.group = KVM_DEV_FLIC_AISM_ALL; > + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { > + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); Is there an indention error? Except this, the code LGTM. > + } > + > return; ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available 2017-09-27 5:45 ` Yi Min Zhao @ 2017-09-27 7:02 ` Christian Borntraeger 0 siblings, 0 replies; 12+ messages in thread From: Christian Borntraeger @ 2017-09-27 7:02 UTC (permalink / raw) To: Yi Min Zhao, Cornelia Huck Cc: Halil Pasic, David Hildenbrand, qemu-devel, Alexander Graf, Jason J . Herne, Richard Henderson On 09/27/2017 07:45 AM, Yi Min Zhao wrote: > > > 在 2017/9/26 下午9:36, Christian Borntraeger 写道: >> @@ -557,6 +557,12 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) >> test_attr.group = KVM_DEV_FLIC_CLEAR_IO_IRQ; >> flic_state->clear_io_supported = !ioctl(flic_state->fd, >> KVM_HAS_DEVICE_ATTR, test_attr); >> + /* try enable the AIS facility */ >> + test_attr.group = KVM_DEV_FLIC_AISM_ALL; >> + if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { >> + kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); yes, will fix. > Is there an indention error? > Except this, the code LGTM. >> + } >> + >> return; > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines 2017-09-26 13:36 [Qemu-devel] [PATCH v2 0/2] ais fixups for 2.11 Christian Borntraeger 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available Christian Borntraeger @ 2017-09-26 13:36 ` Christian Borntraeger 2017-09-26 13:51 ` David Hildenbrand 1 sibling, 1 reply; 12+ messages in thread From: Christian Borntraeger @ 2017-09-26 13:36 UTC (permalink / raw) To: Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, David Hildenbrand, Jason J . Herne, Christian Borntraeger, Dr. David Alan Gilbert With newer kernels that do support the ais feature (4.13) a qemu 2.11 will not only enable the ais feature for the 2.11 machine, but also for a <=2.10 compat machine. As this feature is not available in QEMU <=2.9 (and QEMU 2.10.1), this guest will fail to migrate back to an older qemu like 2.9 with: _snip_ error while loading state for instance 0x0 of device 's390-flic' _snip_ making the whole compat machine dis-functional. As a permanent fix, we need to fence the ais feature for machines <= 2.10 Due to ais being enabled on 2.10.0 (fixed in 2.10.1) this will prevent migration of ais-enabled guests from 2.10.0 with _snip_ qemu-system-s390x: Failed to load s390-flic/ais:tmp qemu-system-s390x: error while loading state for instance 0x0 of device 's390-flic' qemu-system-s390x: load of migration failed: Function not implemented _snip_ Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> Cc: Yi Min Zhao <zyimin@linux.vnet.ibm.com> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> --- hw/intc/s390_flic_kvm.c | 4 +++- hw/s390x/s390-virtio-ccw.c | 10 ++++++++++ include/hw/s390x/s390-virtio-ccw.h | 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c index 5c9d215..2e40968 100644 --- a/hw/intc/s390_flic_kvm.c +++ b/hw/intc/s390_flic_kvm.c @@ -22,6 +22,7 @@ #include "hw/s390x/s390_flic.h" #include "hw/s390x/adapter.h" #include "hw/s390x/css.h" +#include "hw/s390x/s390-virtio-ccw.h" #include "trace.h" #define FLIC_SAVE_INITIAL_SIZE getpagesize() @@ -559,7 +560,8 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp) KVM_HAS_DEVICE_ATTR, test_attr); /* try enable the AIS facility */ test_attr.group = KVM_DEV_FLIC_AISM_ALL; - if (!ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { + if (ais_allowed() && + !ioctl(flic_state->fd, KVM_HAS_DEVICE_ATTR, test_attr)) { kvm_vm_enable_cap(kvm_state, KVM_CAP_S390_AIS, 0); } diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 98c82c2..41b770a 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -435,6 +435,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) s390mc->cpu_model_allowed = true; s390mc->css_migration_enabled = true; s390mc->gs_allowed = true; + s390mc->ais_allowed = true; mc->init = ccw_init; mc->reset = s390_machine_reset; mc->hot_add_cpu = s390_hot_add_cpu; @@ -519,6 +520,11 @@ bool gs_allowed(void) return get_machine_class()->gs_allowed; } +bool ais_allowed(void) +{ + return get_machine_class()->ais_allowed; +} + static char *machine_get_loadparm(Object *obj, Error **errp) { S390CcwMachineState *ms = S390_CCW_MACHINE(obj); @@ -742,6 +748,10 @@ static void ccw_machine_2_10_instance_options(MachineState *machine) static void ccw_machine_2_10_class_options(MachineClass *mc) { + S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc); + + s390mc->ais_allowed = false; + ccw_machine_2_11_class_options(mc); SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_10); } diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h index a9a90c2..d25a98f 100644 --- a/include/hw/s390x/s390-virtio-ccw.h +++ b/include/hw/s390x/s390-virtio-ccw.h @@ -41,6 +41,7 @@ typedef struct S390CcwMachineClass { bool cpu_model_allowed; bool css_migration_enabled; bool gs_allowed; + bool ais_allowed; } S390CcwMachineClass; /* runtime-instrumentation allowed by the machine */ @@ -49,6 +50,8 @@ bool ri_allowed(void); bool cpu_model_allowed(void); /* guarded-storage allowed by the machine */ bool gs_allowed(void); +/* ais allowed by the machine */ +bool ais_allowed(void); /** * Returns true if (vmstate based) migration of the channel subsystem -- 2.9.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines Christian Borntraeger @ 2017-09-26 13:51 ` David Hildenbrand 2017-09-27 7:12 ` Christian Borntraeger 0 siblings, 1 reply; 12+ messages in thread From: David Hildenbrand @ 2017-09-26 13:51 UTC (permalink / raw) To: Christian Borntraeger, Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, Jason J . Herne, Dr. David Alan Gilbert On 26.09.2017 15:36, Christian Borntraeger wrote: > With newer kernels that do support the ais feature (4.13) a qemu 2.11 > will not only enable the ais feature for the 2.11 machine, but also > for a <=2.10 compat machine. As this feature is not available in > QEMU <=2.9 (and QEMU 2.10.1), this guest will fail to migrate > back to an older qemu like 2.9 with: > > _snip_ > error while loading state for instance 0x0 of device 's390-flic' > _snip_ > > making the whole compat machine dis-functional. As a permanent fix, we > need to fence the ais feature for machines <= 2.10 > > Due to ais being enabled on 2.10.0 (fixed in 2.10.1) this will prevent > migration of ais-enabled guests from 2.10.0 with > > _snip_ > qemu-system-s390x: Failed to load s390-flic/ais:tmp > qemu-system-s390x: error while loading state for instance 0x0 of device 's390-flic' > qemu-system-s390x: load of migration failed: Function not implemented > _snip_ > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > Cc: Yi Min Zhao <zyimin@linux.vnet.ibm.com> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > hw/intc/s390_flic_kvm.c | 4 +++- As discussed, I think we should use cpu_model_allowed() instead. -- Thanks, David ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines 2017-09-26 13:51 ` David Hildenbrand @ 2017-09-27 7:12 ` Christian Borntraeger 2017-09-27 8:03 ` Christian Borntraeger 0 siblings, 1 reply; 12+ messages in thread From: Christian Borntraeger @ 2017-09-27 7:12 UTC (permalink / raw) To: David Hildenbrand, Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, Jason J . Herne, Dr. David Alan Gilbert On 09/26/2017 03:51 PM, David Hildenbrand wrote: > On 26.09.2017 15:36, Christian Borntraeger wrote: >> With newer kernels that do support the ais feature (4.13) a qemu 2.11 >> will not only enable the ais feature for the 2.11 machine, but also >> for a <=2.10 compat machine. As this feature is not available in >> QEMU <=2.9 (and QEMU 2.10.1), this guest will fail to migrate >> back to an older qemu like 2.9 with: >> >> _snip_ >> error while loading state for instance 0x0 of device 's390-flic' >> _snip_ >> >> making the whole compat machine dis-functional. As a permanent fix, we >> need to fence the ais feature for machines <= 2.10 >> >> Due to ais being enabled on 2.10.0 (fixed in 2.10.1) this will prevent >> migration of ais-enabled guests from 2.10.0 with >> >> _snip_ >> qemu-system-s390x: Failed to load s390-flic/ais:tmp >> qemu-system-s390x: error while loading state for instance 0x0 of device 's390-flic' >> qemu-system-s390x: load of migration failed: Function not implemented >> _snip_ >> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> Cc: Yi Min Zhao <zyimin@linux.vnet.ibm.com> >> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> >> --- >> hw/intc/s390_flic_kvm.c | 4 +++- > > > As discussed, I think we should use cpu_model_allowed() instead. I think I still prefer the explicit check for ais-enabled to make managedsave (on the same system) continue to work if the user does not specify a cpu model at all (which will then fallback to the host model). We already fence other things (like guarded storage) and yes it will grow over time but the *_allowed things seem to be the smallest maintenance issue in this area. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines 2017-09-27 7:12 ` Christian Borntraeger @ 2017-09-27 8:03 ` Christian Borntraeger 0 siblings, 0 replies; 12+ messages in thread From: Christian Borntraeger @ 2017-09-27 8:03 UTC (permalink / raw) To: David Hildenbrand, Cornelia Huck Cc: Richard Henderson, Alexander Graf, Yi Min Zhao, Halil Pasic, qemu-devel, Jason J . Herne, Dr. David Alan Gilbert On 09/27/2017 09:12 AM, Christian Borntraeger wrote: > > > On 09/26/2017 03:51 PM, David Hildenbrand wrote: >> On 26.09.2017 15:36, Christian Borntraeger wrote: >>> With newer kernels that do support the ais feature (4.13) a qemu 2.11 >>> will not only enable the ais feature for the 2.11 machine, but also >>> for a <=2.10 compat machine. As this feature is not available in >>> QEMU <=2.9 (and QEMU 2.10.1), this guest will fail to migrate >>> back to an older qemu like 2.9 with: >>> >>> _snip_ >>> error while loading state for instance 0x0 of device 's390-flic' >>> _snip_ >>> >>> making the whole compat machine dis-functional. As a permanent fix, we >>> need to fence the ais feature for machines <= 2.10 >>> >>> Due to ais being enabled on 2.10.0 (fixed in 2.10.1) this will prevent >>> migration of ais-enabled guests from 2.10.0 with >>> >>> _snip_ >>> qemu-system-s390x: Failed to load s390-flic/ais:tmp >>> qemu-system-s390x: error while loading state for instance 0x0 of device 's390-flic' >>> qemu-system-s390x: load of migration failed: Function not implemented >>> _snip_ >>> >>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> Cc: Yi Min Zhao <zyimin@linux.vnet.ibm.com> >>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> >>> --- >>> hw/intc/s390_flic_kvm.c | 4 +++- >> >> >> As discussed, I think we should use cpu_model_allowed() instead. > I think I still prefer the explicit check for ais-enabled to make managedsave > (on the same system) continue to work if the user does not specify a cpu model > at all (which will then fallback to the host model). We already fence other > things (like guarded storage) and yes it will grow over time but the > *_allowed things seem to be the smallest maintenance issue in this area. Hmm, on the other hand this fails only when we migrate to an older qemu during managedsave. So yes, maybe this is just an "will not work anyway" ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-09-27 8:03 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-26 13:36 [Qemu-devel] [PATCH v2 0/2] ais fixups for 2.11 Christian Borntraeger 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 1/2] s390x/ais: enable ais when migration is available Christian Borntraeger 2017-09-26 13:43 ` David Hildenbrand 2017-09-26 14:06 ` Christian Borntraeger 2017-09-26 14:08 ` David Hildenbrand 2017-09-27 4:35 ` Yi Min Zhao 2017-09-27 5:45 ` Yi Min Zhao 2017-09-27 7:02 ` Christian Borntraeger 2017-09-26 13:36 ` [Qemu-devel] [PATCH v2 2/2] s390x/ais: disable ais for compat machines Christian Borntraeger 2017-09-26 13:51 ` David Hildenbrand 2017-09-27 7:12 ` Christian Borntraeger 2017-09-27 8:03 ` Christian Borntraeger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.