Skip to contentSkip to conversation

De-assert PIC IRQs properly at APIC level

PATCH RESEND New Jan Kiszka 3 participants 5 messages Aug 18, 2008 – Aug 21, 2008

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH RESEND 1/1 De-assert PIC IRQs properly at APIC level

[in a separate thread as requested by Anthony]

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 <[email protected]>
---
 hw/apic.c |   23 ++++++++++++++++++++++-
 hw/pc.c   |    5 +----
 hw/pc.h   |    4 +---
 3 files changed, 24 insertions(+), 8 deletions(-)
hw/apic.c +22 -1
Index: 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;\
hw/pc.c +1 -4
Index: 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;
     }
 }
hw/pc.h +1 -3
Index: 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);
#1 lore raw

PATCH 1/2 De-assert ISA PIC IRQs properly

[ Taking latest isapc changes into account. ]

In case the PIC IRQ gets de-asserted on an isapc machine, we also have to reset CPU_INTERRUPT_HARD. This is what older qemu (before the routing-through-APIC changes) used to do as well.

Signed-off-by Jan Kiszka <[email protected]>
---
 hw/pc.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
hw/pc.c +6 -4
Index: b/hw/pc.c
===================================================================
@@ -118,17 +118,19 @@ static void pic_irq_request(void *opaque
 {
     CPUState *env = first_cpu;
 
-    if (!level)
-        return;
-
     if (env->apic_state) {
+        if (!level)
+            return;
         while (env) {
             if (apic_accept_pic_intr(env))
                 apic_local_deliver(env, APIC_LINT0);
             env = env->next_cpu;
         }
     } else {
-        cpu_interrupt(env, CPU_INTERRUPT_HARD);
+        if (level)
+            cpu_interrupt(env, CPU_INTERRUPT_HARD);
+        else
+            cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
     }
 }
Applied
#2 lore raw

Re: [Qemu-devel] [PATCH 1/2] De-assert ISA PIC IRQs properly

Quoted 1 line · Jan Kiszka

[ Taking latest isapc changes into account. ]

Applied, thanks.

Quoted 40 lines

In case the PIC IRQ gets de-asserted on an isapc machine, we also have to reset CPU_INTERRUPT_HARD. This is what older qemu (before the routing-through-APIC changes) used to do as well.

Signed-off-by Jan Kiszka <[email protected]>
---
 hw/pc.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Index: b/hw/pc.c
===================================================================

hw/pc.c +6 -4
@@ -118,17 +118,19 @@ static void pic_irq_request(void *opaque
 {
     CPUState *env = first_cpu;
 
-    if (!level)
-        return;
-
     if (env->apic_state) {
+        if (!level)
+            return;
         while (env) {
             if (apic_accept_pic_intr(env))
                 apic_local_deliver(env, APIC_LINT0);
             env = env->next_cpu;
         }
     } else {
-        cpu_interrupt(env, CPU_INTERRUPT_HARD);
+        if (level)
+            cpu_interrupt(env, CPU_INTERRUPT_HARD);
+        else
+            cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
     }
 }
Signature · 4 lines
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   [email protected]         | [email protected]
   `-    people.debian.org/~aurel32 | www.aurel32.net
#3 lore raw

PATCH 2/2 De-assert PIC IRQs properly at APIC level

In reply to Jan Kiszka

[ Taking latest isapc changes into account. ]

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. Fixes EFI-BIOS 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 <[email protected]>
---
 hw/apic.c |   23 ++++++++++++++++++++++-
 hw/pc.c   |    4 +---
 hw/pc.h   |    4 +---
 3 files changed, 24 insertions(+), 7 deletions(-)
hw/apic.c +22 -1
Index: 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;\
hw/pc.c +1 -3
Index: b/hw/pc.c
===================================================================
@@ -119,11 +119,9 @@ static void pic_irq_request(void *opaque
     CPUState *env = first_cpu;
 
     if (env->apic_state) {
-        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;
         }
     } else {
hw/pc.h +1 -3
Index: 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);
Applied
#4 lore raw
Quoted 1 line · Jan Kiszka

[ Taking latest isapc changes into account. ]

Applied, thanks.

Quoted 92 lines

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. Fixes EFI-BIOS 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 <[email protected]>
---
 hw/apic.c |   23 ++++++++++++++++++++++-
 hw/pc.c   |    4 +---
 hw/pc.h   |    4 +---
 3 files changed, 24 insertions(+), 7 deletions(-)

Index: b/hw/apic.c
===================================================================

hw/apic.c +22 -1
@@ -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
===================================================================

hw/pc.c +1 -3
@@ -119,11 +119,9 @@ static void pic_irq_request(void *opaque
     CPUState *env = first_cpu;
 
     if (env->apic_state) {
-        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;
         }
     } else {

Index: b/hw/pc.h
===================================================================

hw/pc.h +1 -3
@@ -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);
Signature · 4 lines
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   [email protected]         | [email protected]
   `-    people.debian.org/~aurel32 | www.aurel32.net