[PATCH gnumach] Fix interrupt handling

[Damien Zammit]

Logic for interrupts:

- interrupt.S raises spl (thus IF cleared)
- interrupt.S EOI
- interrupt.S calls the handler
   - for pure in-kernel handlers, they do whatever they want with IF
     cleared.
   - when a userland handler is registers, queue_intr masks the irq.
- interrupt.S lowers spl with splx_cli, thus IF still cleared
- iret, that clears the IF

- later on, userland acks the IRQ, that unmasks the irq

The key to this change is that all interrupts, including IPIs, are
treated the same way. Eg. the spl level is now raised before an IPI and
restored after. Also, EOI is not needed inside irq_acknowledge.

With this change and the experimental change not to dispatch threads
direct to idle processors in the scheduler, I no longer observe kernel
faults, but an occasional hang does occur.

---
 device/intr.c           | 16 ++++++++++++++--
 i386/i386at/interrupt.S | 28 +++++++++++++---------------
 x86_64/interrupt.S      | 28 +++++++++++++---------------
 3 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/device/intr.c b/device/intr.c
index 15029440..97cfa377 100644
--- a/device/intr.c
+++ b/device/intr.c
@@ -50,6 +50,20 @@ search_intr (struct irqdev *dev, ipc_port_t dst_port)
   return NULL;
 }
 
+
+/*
+ * Interrupt handling logic:
+ *
+ * interrupt.S raises spl (thus IF cleared)
+ * interrupt.S EOI
+ * interrupt.S calls the handler
+ *   - for pure in-kernel handlers, they do whatever they want with IF cleared.
+ *   - when a userland handler is registered, queue_intr masks the irq.
+ * interrupt.S lowers spl with splx_cli, thus IF still cleared
+ * iret, that also clears the IF
+ *
+ * later on, (irq_acknowledge), userland acks the IRQ, that unmasks the irq
+ */
 kern_return_t
 irq_acknowledge (ipc_port_t receive_port)
 {
@@ -76,8 +90,6 @@ irq_acknowledge (ipc_port_t receive_port)
   if (ret)
     return ret;
 
-  (*(irqtab.irqdev_ack)) (&irqtab, e->id);
-
   __enable_irq (irqtab.irq[e->id]);
 
   return D_SUCCESS;
diff --git a/i386/i386at/interrupt.S b/i386/i386at/interrupt.S
index 8ae6b97c..77424b43 100644
--- a/i386/i386at/interrupt.S
+++ b/i386/i386at/interrupt.S
@@ -45,14 +45,6 @@ ENTRY(interrupt)
        ret                             /* if so, just return */
 1:
 #endif
-#if NCPUS > 1
-       cmpl    $CALL_PMAP_UPDATE,%eax  /* was this a SMP pmap_update request? 
*/
-       je      _call_single
-
-       cmpl    $CALL_AST_CHECK,%eax    /* was this a SMP remote -> local ast 
request? */
-       je      _call_local_ast
-#endif
-
        subl    $24,%esp                /* Two local variables + 4 parameters */
        movl    %eax,S_IRQ              /* save irq number */
 
@@ -61,6 +53,14 @@ ENTRY(interrupt)
 
        movl    S_IRQ,%ecx              /* restore irq number */
 
+#if NCPUS > 1
+       cmpl    $CALL_PMAP_UPDATE,%ecx  /* was this a SMP pmap_update request? 
*/
+       je      _call_single
+
+       cmpl    $CALL_AST_CHECK,%ecx    /* was this a SMP remote -> local ast 
request? */
+       je      _call_local_ast
+#endif
+
 #ifndef APIC
        movl    $1,%eax
        shll    %cl,%eax                /* get corresponding IRQ mask */
@@ -98,11 +98,8 @@ ENTRY(interrupt)
        outb    %al,$(PIC_MASTER_OCW)   /* unmask master */
 2:
 #else
-       cmpl    $16,%ecx                /* was this a low ISA intr? */
-       jge     _no_eoi                 /* no, must be PCI (let irq_ack handle 
EOI) */
        movl    %ecx,(%esp)             /* load irq number as 1st arg */
        call    EXT(ioapic_irq_eoi)     /* ioapic irq specific EOI */
-_no_eoi:
 #endif
 
        movl    S_IPL,%eax
@@ -122,6 +119,7 @@ _no_eoi:
 
        call    *EXT(ivect)(%eax)       /* call interrupt handler */
 
+_completed:
        movl    S_IPL,%eax              /* restore previous ipl */
        movl    %eax,(%esp)
        call    splx_cli                /* restore previous ipl */
@@ -131,14 +129,14 @@ _no_eoi:
 
 #if NCPUS > 1
 _call_single:
-       cli                             /* no nested interrupts */
        call    EXT(lapic_eoi)          /* lapic EOI before the handler to 
allow extra update */
        call    EXT(pmap_update_interrupt)
-       ret
+       jmp     _completed
 
 _call_local_ast:
-       call    EXT(ast_check)          /* AST check on this cpu */
        call    EXT(lapic_eoi)          /* lapic EOI */
-       ret
+       call    EXT(ast_check)          /* AST check on this cpu */
+       jmp     _completed
+
 #endif
 END(interrupt)
diff --git a/x86_64/interrupt.S b/x86_64/interrupt.S
index fcd5a032..6fb77727 100644
--- a/x86_64/interrupt.S
+++ b/x86_64/interrupt.S
@@ -45,14 +45,6 @@ ENTRY(interrupt)
        ret                             /* if so, just return */
 1:
 #endif
-#if NCPUS > 1
-       cmpl    $CALL_PMAP_UPDATE,%eax  /* was this a SMP pmap_update request? 
*/
-       je      _call_single
-
-       cmpl    $CALL_AST_CHECK,%eax    /* was this a SMP remote -> local ast 
request? */
-       je      _call_local_ast
-#endif
-
        subq    $16,%rsp                /* Two local variables */
        movl    %eax,S_IRQ              /* save irq number */
 
@@ -61,6 +53,14 @@ ENTRY(interrupt)
 
        movl    S_IRQ,%ecx              /* restore irq number */
 
+#if NCPUS > 1
+       cmpl    $CALL_PMAP_UPDATE,%ecx  /* was this a SMP pmap_update request? 
*/
+       je      _call_single
+
+       cmpl    $CALL_AST_CHECK,%ecx    /* was this a SMP remote -> local ast 
request? */
+       je      _call_local_ast
+#endif
+
 #ifndef APIC
        movl    $1,%eax
        shll    %cl,%eax                /* get corresponding IRQ mask */
@@ -98,11 +98,8 @@ ENTRY(interrupt)
        outb    %al,$(PIC_MASTER_OCW)   /* unmask master */
 2:
 #else
-       cmpl    $16,%ecx                /* was this a low ISA intr? */
-       jge      _no_eoi                /* no, must be PCI (let irq_ack handle 
EOI) */
        movl    %ecx,%edi               /* load irq number as 1st arg */
        call    EXT(ioapic_irq_eoi)     /* ioapic irq specific EOI */
-_no_eoi:
 #endif
 
        ;
@@ -121,6 +118,7 @@ _no_eoi:
        shll    $1,%eax                 /* irq * 8 */
        call    *EXT(ivect)(%rax)       /* call interrupt handler */
 
+_completed:
        movl    S_IPL,%edi              /* restore previous ipl */
        call    splx_cli                /* restore previous ipl */
 
@@ -129,14 +127,14 @@ _no_eoi:
 
 #if NCPUS > 1
 _call_single:
-       cli                             /* no nested interrupts */
        call    EXT(lapic_eoi)          /* lapic EOI before the handler to 
allow extra update */
        call    EXT(pmap_update_interrupt)
-       ret
+       jmp     _completed
 
 _call_local_ast:
-       call    EXT(ast_check)          /* AST check on this cpu */
        call    EXT(lapic_eoi)          /* lapic EOI */
-       ret
+       call    EXT(ast_check)          /* AST check on this cpu */
+       jmp     _completed
+
 #endif
 END(interrupt)
-- 
2.40.1