On 21/01/2020 15.46, Cornelia Huck wrote:
> On Tue, 21 Jan 2020 09:33:02 -0500
> Matthew Rosato <mjros...@linux.ibm.com> wrote:
>
>> On 1/20/20 12:27 PM, Cornelia Huck wrote:
>>> On Mon, 20 Jan 2020 14:24:41 +0100
>>> Thomas Huth <th...@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 for the new s390-ccw-virtio-5.0 machine now.
>>>>
>>>> While at it, also add a more verbose comment why we need the *_allowed()
>>>> wrappers in s390-virtio-ccw.c.
>>>>
>>>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1756946
>>>> Reviewed-by: David Hildenbrand <da...@redhat.com>
>>>> Signed-off-by: Thomas Huth <th...@redhat.com>
>>>> ---
>>>> v3: Moved "s390mc->kvm_ais_allowed = false" to the end of the function
>>>>
>>>> hw/s390x/s390-virtio-ccw.c | 20 +++++++++++++++++---
>>>> include/hw/s390x/s390-virtio-ccw.h | 3 +++
>>>> target/s390x/kvm.c | 9 ++++++---
>>>> 3 files changed, 26 insertions(+), 6 deletions(-)
>>>
>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>> index 15260aeb9a..cf4fb4f2d9 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 (kvm_ais_allowed() &&
>>>> + kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) {
>>>
>>> Hnm, we actually need a kernel irqchip with the kvm flic to get ais to
>>> work; else we'll fail with
>>>
>>> qemu-system-s390x: Failed to inject airq with AIS supported
>>>
>>> in the kernel_irqchip=off case, as we won't have an I/O adapter
>>> registered.
>>>
>>> Adding 'kvm_kernel_irqchip_required() &&' seems to do the trick;
>>> comments?
>>>
>>
>> In spirit, I agree with this idea. But, a quick test shows that putting
>> this check here results in ais=off for the 'none' machine case (libvirt
>> capabilities detection). I think we have to only look at
>> kvm_kernel_irqchip_required() when working with a real machine.
>
> Sigh, I think you're right again. We need to check for the 'none'
> machine here; but I can't think of a non-ugly way to do so...
I think it might work when using kvm_kernel_irqchip_allowed() instead of
kvm_kernel_irqchip_required() ... Matthew, could you please give it a
try with this patch on top of mine:
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -368,7 +368,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
* support is considered necessary, we only try to enable this for
* newer machine types if KVM_CAP_S390_AIS_MIGRATION is available.
*/
- if (kvm_ais_allowed() &&
+ if (kvm_ais_allowed() && kvm_kernel_irqchip_allowed() &&
kvm_check_extension(s, KVM_CAP_S390_AIS_MIGRATION)) {
kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);
}
Thomas
