| Message ID | 20200122101437.5069-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] target/s390x/kvm: Enable adapter interruption suppression again | expand |
On 22.01.20 11:14, Thomas Huth wrote: > The AIS feature has been disabled late in the v2.10 development cycle since > there were some issues with migration (see commit 3f2d07b3b01ea61126b - > "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted > to enable it again for newer machine types, but apparently we forgot to do > this so far. Let's do it now for the machines that support proper CPU models. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v5: Use cpu_model_allowed() as suggested by David. Seems to work as far > as I can test it without PCI cards, but ping-pong migration with > "-cpu host" from/to an older version of QEMU is now not working > anymore - but I think that's kind of expected since "-cpu host" > is not migration-safe anyway. Yes, exactly. > > target/s390x/kvm.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index 15260aeb9a..30112e529c 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -365,10 +365,13 @@ 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. > + * support is considered necessary, we only try to enable this for > + * newer machine types if KVM_CAP_S390_AIS_MIGRATION is available. > */ > - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ > + if (cpu_model_allowed() && kvm_kernel_irqchip_allowed() && > + kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) { > + kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); > + } > > kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES); > return 0; > Reviewed-by: David Hildenbrand <david@redhat.com>
On Wed, 22 Jan 2020 11:14:37 +0100 Thomas Huth <thuth@redhat.com> wrote: > The AIS feature has been disabled late in the v2.10 development cycle since > there were some issues with migration (see commit 3f2d07b3b01ea61126b - > "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted > to enable it again for newer machine types, but apparently we forgot to do > this so far. Let's do it now for the machines that support proper CPU models. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v5: Use cpu_model_allowed() as suggested by David. Seems to work as far > as I can test it without PCI cards, but ping-pong migration with > "-cpu host" from/to an older version of QEMU is now not working > anymore - but I think that's kind of expected since "-cpu host" > is not migration-safe anyway. Ok, so I'll wait for test results with pci cards before queuing this :) > > target/s390x/kvm.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index 15260aeb9a..30112e529c 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -365,10 +365,13 @@ 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. > + * support is considered necessary, we only try to enable this for > + * newer machine types if KVM_CAP_S390_AIS_MIGRATION is available. > */ > - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ > + if (cpu_model_allowed() && kvm_kernel_irqchip_allowed() && > + kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) { > + kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); > + } > > kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES); > return 0; Side note: as you do not add a new _allowed() function, you don't add the clarifying comment anymore -- any value in doing so as a separate patch? And maybe stating as well that new features of that type should rely on the cpu model?
On 22.01.20 11:29, Cornelia Huck wrote: > On Wed, 22 Jan 2020 11:14:37 +0100 > Thomas Huth <thuth@redhat.com> wrote: > >> The AIS feature has been disabled late in the v2.10 development cycle since >> there were some issues with migration (see commit 3f2d07b3b01ea61126b - >> "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted >> to enable it again for newer machine types, but apparently we forgot to do >> this so far. Let's do it now for the machines that support proper CPU models. >> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946 >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> v5: Use cpu_model_allowed() as suggested by David. Seems to work as far >> as I can test it without PCI cards, but ping-pong migration with >> "-cpu host" from/to an older version of QEMU is now not working >> anymore - but I think that's kind of expected since "-cpu host" >> is not migration-safe anyway. > > Ok, so I'll wait for test results with pci cards before queuing this :) > >> >> target/s390x/kvm.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index 15260aeb9a..30112e529c 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -365,10 +365,13 @@ 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. >> + * support is considered necessary, we only try to enable this for >> + * newer machine types if KVM_CAP_S390_AIS_MIGRATION is available. >> */ >> - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ >> + if (cpu_model_allowed() && kvm_kernel_irqchip_allowed() && >> + kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) { >> + kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); >> + } >> >> kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES); >> return 0; > > Side note: as you do not add a new _allowed() function, you don't add > the clarifying comment anymore -- any value in doing so as a separate > patch? And maybe stating as well that new features of that type should > rely on the cpu model? > +1, mentioning that cpu_model_allowed() is to be used for all such things (unlock new cpu features in a safe way for old compat machines) in the future.
On 22/01/2020 11.29, Cornelia Huck wrote: > On Wed, 22 Jan 2020 11:14:37 +0100 > Thomas Huth <thuth@redhat.com> wrote: > >> The AIS feature has been disabled late in the v2.10 development cycle since >> there were some issues with migration (see commit 3f2d07b3b01ea61126b - >> "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted >> to enable it again for newer machine types, but apparently we forgot to do >> this so far. Let's do it now for the machines that support proper CPU models. >> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946 >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> v5: Use cpu_model_allowed() as suggested by David. Seems to work as far >> as I can test it without PCI cards, but ping-pong migration with >> "-cpu host" from/to an older version of QEMU is now not working >> anymore - but I think that's kind of expected since "-cpu host" >> is not migration-safe anyway. > > Ok, so I'll wait for test results with pci cards before queuing this :) Ok, Matthew, could you please test one more time? >> target/s390x/kvm.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index 15260aeb9a..30112e529c 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -365,10 +365,13 @@ 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. >> + * support is considered necessary, we only try to enable this for >> + * newer machine types if KVM_CAP_S390_AIS_MIGRATION is available. >> */ >> - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ >> + if (cpu_model_allowed() && kvm_kernel_irqchip_allowed() && >> + kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) { >> + kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); >> + } >> >> kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES); >> return 0; > > Side note: as you do not add a new _allowed() function, you don't add > the clarifying comment anymore -- any value in doing so as a separate > patch? And maybe stating as well that new features of that type should > rely on the cpu model? Yes, I'm planning to send a patch once this one here got accepted. Thomas
On 1/22/20 5:33 AM, Thomas Huth wrote: > On 22/01/2020 11.29, Cornelia Huck wrote: >> On Wed, 22 Jan 2020 11:14:37 +0100 >> Thomas Huth <thuth@redhat.com> wrote: >> >>> The AIS feature has been disabled late in the v2.10 development cycle since >>> there were some issues with migration (see commit 3f2d07b3b01ea61126b - >>> "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted >>> to enable it again for newer machine types, but apparently we forgot to do >>> this so far. Let's do it now for the machines that support proper CPU models. >>> >>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946 >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> v5: Use cpu_model_allowed() as suggested by David. Seems to work as far >>> as I can test it without PCI cards, but ping-pong migration with >>> "-cpu host" from/to an older version of QEMU is now not working >>> anymore - but I think that's kind of expected since "-cpu host" >>> is not migration-safe anyway. >> >> Ok, so I'll wait for test results with pci cards before queuing this :) > > Ok, Matthew, could you please test one more time? > Ran all of the same tests as before, looks good! Tested-by: Matthew Rosato <mjrosato@linux.ibm.com>
On Wed, 22 Jan 2020 11:14:37 +0100 Thomas Huth <thuth@redhat.com> wrote: > The AIS feature has been disabled late in the v2.10 development cycle since > there were some issues with migration (see commit 3f2d07b3b01ea61126b - > "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted > to enable it again for newer machine types, but apparently we forgot to do > this so far. Let's do it now for the machines that support proper CPU models. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v5: Use cpu_model_allowed() as suggested by David. Seems to work as far > as I can test it without PCI cards, but ping-pong migration with > "-cpu host" from/to an older version of QEMU is now not working > anymore - but I think that's kind of expected since "-cpu host" > is not migration-safe anyway. > > target/s390x/kvm.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) Thanks, applied.
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 15260aeb9a..30112e529c 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -365,10 +365,13 @@ 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. + * support is considered necessary, we only try to enable this for + * newer machine types if KVM_CAP_S390_AIS_MIGRATION is available. */ - /* kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); */ + if (cpu_model_allowed() && kvm_kernel_irqchip_allowed() && + kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) { + kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0); + } kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES); return 0;
The AIS feature has been disabled late in the v2.10 development cycle since there were some issues with migration (see commit 3f2d07b3b01ea61126b - "s390x/ais: for 2.10 stable: disable ais facility"). We originally wanted to enable it again for newer machine types, but apparently we forgot to do this so far. Let's do it now for the machines that support proper CPU models. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946 Signed-off-by: Thomas Huth <thuth@redhat.com> --- v5: Use cpu_model_allowed() as suggested by David. Seems to work as far as I can test it without PCI cards, but ping-pong migration with "-cpu host" from/to an older version of QEMU is now not working anymore - but I think that's kind of expected since "-cpu host" is not migration-safe anyway. target/s390x/kvm.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)