[PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[Thomas Huth]

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(-)

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;
-- 
2.18.1

Re: [PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[David Hildenbrand]

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>

-- 
Thanks,

David / dhildenb

Re: [PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[Cornelia Huck]

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?

Re: [PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[David Hildenbrand]

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.

-- 
Thanks,

David / dhildenb

Re: [PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[Thomas Huth]

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

Re: [PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[Matthew Rosato]

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>

Re: [PATCH v5] target/s390x/kvm: Enable adapter interruption suppression again

[Cornelia Huck]

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.