From 60e68042cf70f271308dc6b4b22b609d054af929 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Nov 2014 13:14:05 +0100 Subject: [PATCH 1/3] apic: avoid getting out of halted state on masked PIC interrupts 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 Tested-by: Richard Bilson Signed-off-by: Paolo Bonzini --- hw/intc/apic.c | 8 +++++--- target-i386/cpu.c | 10 ++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/hw/intc/apic.c b/hw/intc/apic.c index 03ff9e94f2..0653409ed0 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 3f13dfe5f5..e9df33e5c3 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2912,8 +2912,14 @@ 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 !defined(CONFIG_USER_ONLY) + if (cs->interrupt_request & CPU_INTERRUPT_POLL) { + apic_poll_irq(cpu->apic_state); + cpu_reset_interrupt(cs, CPU_INTERRUPT_POLL); + } +#endif + + return ((cs->interrupt_request & CPU_INTERRUPT_HARD) && (env->eflags & IF_MASK)) || (cs->interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_INIT | From 8092cb71322ca488deeb7c750ff8022ffcc2f9a6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Nov 2014 13:14:14 +0100 Subject: [PATCH 2/3] apic: fix loss of IPI due to masked ExtINT 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 Tested-by: Richard Bilson Signed-off-by: Paolo Bonzini --- 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 0653409ed0..6ec58615b4 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); } } From 5224c88dd3f771702d450780a25f155e0fc8bb2b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Nov 2014 13:14:18 +0100 Subject: [PATCH 3/3] apic: fix incorrect handling of ExtINT interrupts wrt processor priority 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 Tested-by: Richard Bilson Signed-off-by: Paolo Bonzini --- 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 6ec58615b4..0f97b47925 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;