[Qemu-devel] [PATCH 0/3] apic: ExtINT fixes for QNX

[Qemu-devel] [PATCH 0/3] apic: ExtINT fixes for QNX

[Paolo Bonzini]

Richard Bilson reported some QNX emulation bugs.  I sent him offlist a
prototype patch, that he commented on.

The first patch here works around what is arguably a QNX bug, that is
triggered by the last two patches; but it is better to avoid spurious
HLT wakeups, so here it is.  The last two patches are what Richard
had tested, amended with a fix for Windows XP + KVM + QEMU apic.

Richard, can you confirm that these patches work?

Paolo

Paolo Bonzini (3):
  apic: avoid getting out of halted state on masked PIC interrupts
  apic: fix loss of IPI due to masked ExtINT
  apic: fix incorrect handling of ExtINT interrupts wrt processor
    priority

 hw/intc/apic.c    | 20 ++++++++++++--------
 target-i386/cpu.c |  8 ++++++--
 2 files changed, 18 insertions(+), 10 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 1/3] apic: avoid getting out of halted state on masked PIC interrupts

[Paolo Bonzini]

After the next patch, if a masked PIC interrupts causes CPU_INTERRUPT_POLL
to be set, the CPU will spuriously get out of halted state.  While this
is technically valid, we should avoid that.

Make CPU_INTERRUPT_POLL run apic_update_irq in the right thread and then
look at CPU_INTERRUPT_HARD.  If CPU_INTERRUPT_HARD does not get set,
do not report the CPU as having work.

Also move the handling of software-disabled APIC from apic_update_irq
to apic_irq_pending, and always trigger CPU_INTERRUPT_POLL.  This will
be important once we will add a case that resets CPU_INTERRUPT_HARD
from apic_update_irq.  We want to run it even if we go through
CPU_INTERRUPT_POLL, and even if the local APIC is software disabled.

Reported-by: Richard Bilson <rbilson@qnx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/apic.c    | 8 +++++---
 target-i386/cpu.c | 8 ++++++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 03ff9e9..0653409 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -349,6 +349,11 @@ static int apic_get_arb_pri(APICCommonState *s)
 static int apic_irq_pending(APICCommonState *s)
 {
     int irrv, ppr;
+
+    if (!(s->spurious_vec & APIC_SV_ENABLE)) {
+        return 0;
+    }
+
     irrv = get_highest_priority_int(s->irr);
     if (irrv < 0) {
         return 0;
@@ -366,9 +371,6 @@ static void apic_update_irq(APICCommonState *s)
 {
     CPUState *cpu;
 
-    if (!(s->spurious_vec & APIC_SV_ENABLE)) {
-        return;
-    }
     cpu = CPU(s->cpu);
     if (!qemu_cpu_is_self(cpu)) {
         cpu_interrupt(cpu, CPU_INTERRUPT_POLL);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index fa811a0..34885ba 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2875,8 +2875,12 @@ static bool x86_cpu_has_work(CPUState *cs)
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
-    return ((cs->interrupt_request & (CPU_INTERRUPT_HARD |
-                                      CPU_INTERRUPT_POLL)) &&
+    if (cs->interrupt_request & CPU_INTERRUPT_POLL) {
+        apic_poll_irq(cpu->apic_state);
+        cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL);
+    }
+
+    return ((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
             (env->eflags & IF_MASK)) ||
            (cs->interrupt_request & (CPU_INTERRUPT_NMI |
                                      CPU_INTERRUPT_INIT |
-- 
2.1.0

[Qemu-devel] [PATCH 2/3] apic: fix loss of IPI due to masked ExtINT

[Paolo Bonzini]

This patch fixes an obscure failure of the QNX kernel on QEMU x86 SMP.
In QNX, all hardware interrupts come via the PIC, and are delivered by
the cpu 0 LAPIC in ExtINT mode, while IPIs are delivered by the LAPIC
in fixed mode.

This bug happens as follows:
- cpu 0 masks a particular PIC interrupt
- IPI sent to cpu 0 (CPU_INTERRUPT_HARD is set)
- before the IPI is accepted, the masked interrupt line is asserted by the
device

Since the interrupt is masked, apic_deliver_pic_intr will clear
CPU_INTERRUPT_HARD. The IPI will still be set in the APIC irr, but since
CPU_INTERRUPT_HARD is not set the cpu will not notice. Depending on the
scenario this can cause a system hang, i.e. if cpu 0 is expected to unmask
the interrupt.

In order to fix this, do a full check of the APIC before an EXTINT
is acknowledged.  This can result in clearing CPU_INTERRUPT_HARD, but
can also result in delivering the lost IPI.

Reported-by: Richard Bilson <rbilson@qnx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/apic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 0653409..6ec5861 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -188,7 +188,7 @@ void apic_deliver_pic_intr(DeviceState *dev, int level)
             apic_reset_bit(s->irr, lvt & 0xff);
             /* fall through */
         case APIC_DM_EXTINT:
-            cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_HARD);
+            apic_update_irq(s);
             break;
         }
     }
@@ -376,6 +376,8 @@ static void apic_update_irq(APICCommonState *s)
         cpu_interrupt(cpu, CPU_INTERRUPT_POLL);
     } else if (apic_irq_pending(s) > 0) {
         cpu_interrupt(cpu, CPU_INTERRUPT_HARD);
+    } else if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
+        cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD);
     }
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH 3/3] apic: fix incorrect handling of ExtINT interrupts wrt processor priority

[Paolo Bonzini]

This fixes another failure with ExtINT, demonstrated by QNX.  The failure
mode is as follows:
- IPI sent to cpu 0 (bit set in APIC irr)
- IPI accepted by cpu 0 (bit cleared in irr, set in isr)
- IPI sent to cpu 0 (bit set in both irr and isr)
- PIC interrupt sent to cpu 0

The PIC interrupt causes CPU_INTERRUPT_HARD to be set, but
apic_irq_pending observes that the highest pending APIC interrupt priority
(the IPI) is the same as the processor priority (since the IPI is still
being handled), so apic_get_interrupt returns a spurious interrupt rather
than the pending PIC interrupt. The result is an endless sequence of
spurious interrupts, since nothing will clear CPU_INTERRUPT_HARD.

Instead, ExtINT interrupts should have ignored the processor priority.
Calling apic_check_pic early in apic_get_interrupt ensures that
apic_deliver_pic_intr is called instead of delivering the spurious
interrupt.  apic_deliver_pic_intr then clears CPU_INTERRUPT_HARD if needed.

Reported-by: Richard Bilson <rbilson@qnx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/intc/apic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 6ec5861..0f97b47 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -571,7 +571,10 @@ int apic_get_interrupt(DeviceState *dev)
     apic_sync_vapic(s, SYNC_FROM_VAPIC);
     intno = apic_irq_pending(s);
 
-    if (intno == 0) {
+    /* if there is an interrupt from the 8259, let the caller handle
+     * that first since ExtINT interrupts ignore the priority.
+     */
+    if (intno == 0 || apic_check_pic(s)) {
         apic_sync_vapic(s, SYNC_TO_VAPIC);
         return -1;
     } else if (intno < 0) {
@@ -582,9 +585,6 @@ int apic_get_interrupt(DeviceState *dev)
     apic_set_bit(s->isr, intno);
     apic_sync_vapic(s, SYNC_TO_VAPIC);
 
-    /* re-inject if there is still a pending PIC interrupt */
-    apic_check_pic(s);
-
     apic_update_irq(s);
 
     return intno;
-- 
2.1.0

Re: [Qemu-devel] [PATCH 0/3] apic: ExtINT fixes for QNX

[Richard Bilson]

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

I've been testing these against our entire kernel regression suite. While they definitely fix the original problems, there is one test that now hangs. I'm going to have to get to the bottom of that failure before I can sign off.

________________________________________________________________________
From: Paolo Bonzini <pbonzini@redhat.com<mailto:pbonzini@redhat.com>>
Date: Tue Nov 11 2014 09:23:38 GMT-0500 (EST)
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>>
Cc: Richard Bilson <rbilson@qnx.com<mailto:rbilson@qnx.com>>
Subject: [PATCH 0/3] apic: ExtINT fixes for QNX

Richard Bilson reported some QNX emulation bugs. I sent him offlist a prototype patch, that he commented on.

The first patch here works around what is arguably a QNX bug, that is
triggered by the last two patches; but it is better to avoid spurious
HLT wakeups, so here it is. The last two patches are what Richard
had tested, amended with a fix for Windows XP + KVM + QEMU apic.

Richard, can you confirm that these patches work?

Paolo

Paolo Bonzini (3):
apic: avoid getting out of halted state on masked PIC interrupts
apic: fix loss of IPI due to masked ExtINT
apic: fix incorrect handling of ExtINT interrupts wrt processor
priority

hw/intc/apic.c | 20 ++++++++++++--------
target-i386/cpu.c | 8 ++++++--
2 files changed, 18 insertions(+), 10 deletions(-)

--
2.1.0



[-- Attachment #2: Type: text/html, Size: 2606 bytes --]

Re: [Qemu-devel] [PATCH 0/3] apic: ExtINT fixes for QNX

[Richard Bilson]

[-- Attachment #1: Type: text/plain, Size: 1996 bytes --]

The one failure turns out to be an intermittent failure that has nothing to do with the patch and probably nothing to do with qemu at all. So I heartily approve of these patches.

Thanks very much, Paolo, for taking the time to implement these fixes.

From: Richard Bilson <rbilson@qnx.com<mailto:rbilson@qnx.com>>
Date: Fri Nov 14 2014 12:53:49 GMT-0500 (EST)
To: Paolo Bonzini <pbonzini@redhat.com<mailto:pbonzini@redhat.com>>, qemu-devel@nongnu.org <qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>>
Subject: Re: [PATCH 0/3] apic: ExtINT fixes for QNX

I've been testing these against our entire kernel regression suite. While they definitely fix the original problems, there is one test that now hangs. I'm going to have to get to the bottom of that failure before I can sign off.

___________________​____________________​____________________​_____________
From: Paolo Bonzini <pbonzini@redhat.com<mailto:pbonzini@redhat.com>>
Date: Tue Nov 11 2014 09:23:38 GMT-0500 (EST)
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>>
Cc: Richard Bilson <rbilson@qnx.com<mailto:rbilson@qnx.com>>
Subject: [PATCH 0/3] apic: ExtINT fixes for QNX

Richard Bilson reported some QNX emulation bugs. I sent him offlist a prototype patch, that he commented on.

The first patch here works around what is arguably a QNX bug, that is
triggered by the last two patches; but it is better to avoid spurious
HLT wakeups, so here it is. The last two patches are what Richard
had tested, amended with a fix for Windows XP + KVM + QEMU apic.

Richard, can you confirm that these patches work?

Paolo

Paolo Bonzini (3):
apic: avoid getting out of halted state on masked PIC interrupts
apic: fix loss of IPI due to masked ExtINT
apic: fix incorrect handling of ExtINT interrupts wrt processor
priority

hw/intc/apic.c | 20 ++++++++++++--------
target-i386/cpu.c | 8 ++++++--
2 files changed, 18 insertions(+), 10 deletions(-)

--
2.1.0



[-- Attachment #2: Type: text/html, Size: 3833 bytes --]