[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Matthew Rosato
Subject: Re: [PATCH v3] target/s390x/kvm: Enable adapter interruption suppression again
Date: Tue, 21 Jan 2020 11:05:18 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0

On 1/21/20 10:22 AM, Thomas Huth wrote:
On 21/01/2020 15.46, Cornelia Huck wrote:
On Tue, 21 Jan 2020 09:33:02 -0500
Matthew Rosato <address@hidden> wrote:

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

Adding 'kvm_kernel_irqchip_required() &&' seems to do the trick;

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:


Libvirt detection works with this patch.

Alternatively, if I run qemu with kernel_irqchip=off and ais=true, I get:
qemu-system-s390x: Some features requested in the CPU model are not available in the configuration: ais

Which was the same result as Connie's proposal.

It reads a bit odd to me at first, but looking at the code quick I think this is the right answer - kvm_kernel_irqchip_allowed() will only return false when kernel_irqchip has been forced off as above, whereas kernel_irqchip_required will also return false in the case where no setting was specified (this is what tripped libvirt up).

Looks good to me, thanks Thomas.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]