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

[Matthew Rosato]

On 1/20/20 12:27 PM, Cornelia Huck wrote:
> On Mon, 20 Jan 2020 14:24:41 +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 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 <address@hidden>
> > Signed-off-by: Thomas Huth <address@hidden>
> > ---
> >   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.

> > +        kvm_vm_enable_cap(s, KVM_CAP_S390_AIS, 0);
> > +    }
> >   
> >       kvm_set_max_memslot_size(KVM_SLOT_MAX_BYTES);
> >       return 0;