x86: sends the EOI later in the IRQ path
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 25 Oct 2013 21:18:52 +0000 (14:18 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 16 Jan 2014 19:16:43 +0000 (11:16 -0800)
I've gone back and forth on this.  Check out 1b50da7e for reference.

Right now, we're taking more spurious IRQs, probably due to sending EOI
early.  It's not necessary for ipi_is_pending() to send early either,
and I don't see other OSs doing it.

kern/arch/x86/apic.c
kern/arch/x86/trap.c

index e71d74a..e8633e1 100644 (file)
@@ -127,12 +127,12 @@ bool lapic_get_irr_bit(uint8_t vector)
  * ExtInts.  To prevent abuse, we'll use it just for IPIs for now (which only
  * come via the APIC).
  *
- * Note that if you call this from an interrupt handler for 'vector' before you
- * EOI, the ISR will show your bit as set.  It is the EOI that clears the bit
- * from the ISR. */
+ * We only check the IRR, due to how we send EOIs.  Since we don't send til
+ * after handlers return, the ISR will show pending for the current IRQ.  It is
+ * the EOI that clears the bit from the ISR. */
 bool ipi_is_pending(uint8_t vector)
 {
-       return lapic_get_isr_bit(vector) || lapic_get_isr_bit(vector);
+       return lapic_get_isr_bit(vector);
 }
 
 /*
index 0761789..6ba6975 100644 (file)
@@ -473,17 +473,13 @@ void irq_handler(struct hw_trapframe *hw_tf)
        /* Coupled with cpu_halt() and smp_idle() */
        abort_halt(hw_tf);
        //if (core_id())
+       if (hw_tf->tf_trapno != LAPIC_TIMER_DEFAULT_VECTOR)     /* timer irq */
+       if (hw_tf->tf_trapno != 255) /* kmsg */
+       if (hw_tf->tf_trapno != 36)     /* serial */
                printd("Incoming IRQ, ISR: %d on core %d\n", hw_tf->tf_trapno,
                       core_id());
        if (check_spurious_irq(hw_tf->tf_trapno))
                goto out_no_eoi;
-       /* Send the EOI.  This means the PIC/LAPIC can send us the same IRQ vector,
-        * and we'll handle it as soon as we reenable IRQs.  This does *not* mean
-        * the hardware device that triggered the IRQ had its IRQ reset.  This does
-        * mean we shouldn't enable irqs in a handler that isn't reentrant. */
-       assert(hw_tf->tf_trapno >= 32);
-       send_eoi(hw_tf->tf_trapno);
-
        extern handler_wrapper_t (RO handler_wrappers)[NUM_HANDLER_WRAPPERS];
        // determine the interrupt handler table to use.  for now, pick the global
        handler_t *handler_tbl = interrupt_handlers;
@@ -494,6 +490,8 @@ void irq_handler(struct hw_trapframe *hw_tf)
        if ((I_SMP_CALL0 <= hw_tf->tf_trapno) &&
            (hw_tf->tf_trapno <= I_SMP_CALL_LAST))
                down_checklist(handler_wrappers[hw_tf->tf_trapno & 0x0f].cpu_list);
+       /* Keep in sync with ipi_is_pending */
+       send_eoi(hw_tf->tf_trapno);
        /* Fall-through */
 out_no_eoi:
        dec_irq_depth(pcpui);