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 <address@hidden> 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 <address@hidden>
>> ---
>>  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