[Qemu-devel] Re: Breakage with local APIC routing

[Jan Kiszka]

Jan Kiszka wrote:
> Johannes Schindelin wrote:
>> Hi,
>>
>> due to the change in revision 3371 (well, at that time, CVS was used, 
>> which was no better than Subversion) installation of win64 is broken in 
>> QEmu.  The commit message reads like this:
>>
>>      Don't route PIC interrupts through the local APIC if the local 
>>      APIC config says so. By Ari Kivity.
>>
>> A bit of research showed that the patch was actually originally from Qing 
>> He, but he told me privately that the part that actually broke win64 (the 
>> removal of the call to cpu_reset_interrupt(), as opposed to moving that 
>> call into the "else" condition) was not part of his patch.
>>
>> Unfortunately, a lot has been done to the APIC handling in the meantime, 
>> so it is not a simple matter of a revert.
>>
>> Being a complete idiot when it comes to APICs, I have no clue how to fix 
>> the issue.
>>
>> However, I am quite willing to test whatever patch is thrown at me.
>>
>> Can somebody help?
> 
> I recalled some earlier post on this which claimed to fix the issue and
> found it in the archive:
> 
> http://permalink.gmane.org/gmane.comp.emulators.qemu/25415
> 
> However, no one replied on this, and I'm right now not sure if the fix
> is OK for all cases. But it is a starting point for new discussion about
> what is actually borken here.

This might not be the last word, but my feeling that it's closer to the
actual issue than the older fix. Please give it a try. Comments welcome.

--------

Ensure that PIC-delivered IRQs are properly de-asserted in case the APIC
is in EXTINT or FIXED mode (with level-triggering selected) on LINT0.
Should fix Win64 boot issues. This patch also cleans up a bit the
interface between PIC and APIC, making apic_local_deliver private again.

Signed-off-by: Jan Kiszka <address@hidden>
---
 hw/apic.c |   23 ++++++++++++++++++++++-
 hw/pc.c   |    5 +----
 hw/pc.h   |    4 +---
 3 files changed, 24 insertions(+), 8 deletions(-)

Index: b/hw/apic.c
===================================================================
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -166,7 +166,7 @@ static inline void reset_bit(uint32_t *t
     tab[i] &= ~mask;
 }
 
-void apic_local_deliver(CPUState *env, int vector)
+static void apic_local_deliver(CPUState *env, int vector)
 {
     APICState *s = env->apic_state;
     uint32_t lvt = s->lvt[vector];
@@ -197,6 +197,27 @@ void apic_local_deliver(CPUState *env, i
     }
 }
 
+void apic_deliver_pic_intr(CPUState *env, int level)
+{
+    if (level)
+        apic_local_deliver(env, APIC_LVT_LINT0);
+    else {
+        APICState *s = env->apic_state;
+        uint32_t lvt = s->lvt[APIC_LVT_LINT0];
+
+        switch ((lvt >> 8) & 7) {
+        case APIC_DM_FIXED:
+            if (!(lvt & APIC_LVT_LEVEL_TRIGGER))
+                break;
+            reset_bit(s->irr, lvt & 0xff);
+            /* fall through */
+        case APIC_DM_EXTINT:
+            cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
+            break;
+        }
+    }
+}
+
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
     int __i, __j, __mask;\
Index: b/hw/pc.c
===================================================================
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -118,12 +118,9 @@ static void pic_irq_request(void *opaque
 {
     CPUState *env = first_cpu;
 
-    if (!level)
-        return;
-
     while (env) {
         if (apic_accept_pic_intr(env))
-            apic_local_deliver(env, APIC_LINT0);
+            apic_deliver_pic_intr(env, level);
         env = env->next_cpu;
     }
 }
Index: b/hw/pc.h
===================================================================
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -40,11 +40,9 @@ void irq_info(void);
 /* APIC */
 typedef struct IOAPICState IOAPICState;
 
-#define APIC_LINT0     3
-
 int apic_init(CPUState *env);
 int apic_accept_pic_intr(CPUState *env);
-void apic_local_deliver(CPUState *env, int vector);
+void apic_deliver_pic_intr(CPUState *env, int level);
 int apic_get_interrupt(CPUState *env);
 IOAPICState *ioapic_init(void);
 void ioapic_set_irq(void *opaque, int vector, int level);