We now have a different strategy for EOI depending on trigger mode:
For edge triggered, the eoi comes before the handler so we don't
miss interrupts. For level triggered, the eoi comes after the handler
since a high interrupt line doesn't keep triggering until it has
been eoi'd so makes sense to handle those first.
To facilitate this, we store a helper struct irqinfo that
contains the vector and the trigger mode per GSI, so we can look up
in O(1) from memory instead of asking hardware for the trigger mode
and vector every time we need it.
TESTED: On SMP this stops failure in xhci from hanging rumpnet.
When a second usb driver is executed, the second one detects a problem
and disables the uhub0 port, which breaks the first usb driver.
But this does not block rumpnet from serving packets, sharing irq 11.
The usb stack can be recovered by killing both usb drivers and
restarting one. Before this change, the behaviour was that irq 11
became stuck and unusable for any device.
Parallel compiling speed seems improved with this commit on SMP.
UP+apic still works as per usual.
TODO: We still need to work out a strategy to have interrupts enabled
during the handler, so that nested interrupts that occur via code that
is executed inside the irq handler to make the device raise a new
interrupt, are triggered. Currently these do not work properly.
I believe this is the remaining bug with interrupts.
---
i386/i386/apic.h | 5 +++++
i386/i386at/interrupt.S | 49 +++++++++++++++++++++++++++--------------
i386/i386at/ioapic.c | 15 +++++++++++--
x86_64/interrupt.S | 46 +++++++++++++++++++++++++-------------
4 files changed, 82 insertions(+), 33 deletions(-)
diff --git a/i386/i386/apic.h b/i386/i386/apic.h
index 92fb900a..a91fc9ac 100644
--- a/i386/i386/apic.h
+++ b/i386/i386/apic.h
@@ -235,6 +235,11 @@ typedef struct ApicInfo {
struct IrqOverrideData irq_override_list[MAX_IRQ_OVERRIDE];
} ApicInfo;
+struct irqinfo {
+ uint8_t trigger;
+ uint8_t vector;
+};
+
int apic_data_init(void);
void apic_add_cpu(uint16_t apic_id);
void apic_lapic_init(ApicLocalUnit* lapic_ptr);
diff --git a/i386/i386at/interrupt.S b/i386/i386at/interrupt.S
index 77424b43..4662d024 100644
--- a/i386/i386at/interrupt.S
+++ b/i386/i386at/interrupt.S
@@ -61,6 +61,35 @@ ENTRY(interrupt)
je _call_local_ast
#endif
+ movb %cs:EXT(irqinfo)(%ecx),%al /* look up
irq_info[irq].trigger */
+ testb $1,%al /* was this a level triggered
interrupt? */
+ jz _eoi /* no, edge: eoi before handling */
+
+_call_handler:
+
+ movl S_IPL,%eax
+ movl %eax,4(%esp) /* previous ipl as 2nd arg */
+
+ movl S_RET,%eax
+ movl %eax,8(%esp) /* return address as 3rd arg */
+
+ movl S_REGS,%eax
+ movl %eax,12(%esp) /* address of interrupted registers as
4th arg */
+
+ movl S_IRQ,%eax /* copy irq number */
+
+ shll $2,%eax /* irq * 4 */
+ movl EXT(iunit)(%eax),%edx /* get device unit number */
+ movl %edx,(%esp) /* unit number as 1st arg */
+
+ call *EXT(ivect)(%eax) /* call interrupt handler */
+
+ movl S_IRQ,%ecx /* restore irq number */
+ movb %cs:EXT(irqinfo)(%ecx),%al /* look up
irq_info[irq].trigger */
+ testb $1,%al /* was this a level triggered
interrupt? */
+ jz _completed /* no, edge: we are done */
+
+_eoi:
#ifndef APIC
movl $1,%eax
shll %cl,%eax /* get corresponding IRQ mask */
@@ -102,22 +131,10 @@ ENTRY(interrupt)
call EXT(ioapic_irq_eoi) /* ioapic irq specific EOI */
#endif
- movl S_IPL,%eax
- movl %eax,4(%esp) /* previous ipl as 2nd arg */
-
- movl S_RET,%eax
- movl %eax,8(%esp) /* return address as 3rd arg */
-
- movl S_REGS,%eax
- movl %eax,12(%esp) /* address of interrupted registers as
4th arg */
-
- movl S_IRQ,%eax /* copy irq number */
-
- shll $2,%eax /* irq * 4 */
- movl EXT(iunit)(%eax),%edx /* get device unit number */
- movl %edx,(%esp) /* unit number as 1st arg */
-
- call *EXT(ivect)(%eax) /* call interrupt handler */
+ movl S_IRQ,%ecx /* restore irq number */
+ movb %cs:EXT(irqinfo)(%ecx),%al /* look up
irq_info[irq].trigger */
+ testb $1,%al /* was this a level triggered
interrupt? */
+ jz _call_handler /* no, edge: call handler */
_completed:
movl S_IPL,%eax /* restore previous ipl */
diff --git a/i386/i386at/ioapic.c b/i386/i386at/ioapic.c
index a6c0fd6a..5dc7321c 100644
--- a/i386/i386at/ioapic.c
+++ b/i386/i386at/ioapic.c
@@ -128,6 +128,9 @@ interrupt_handler_fn ivect[NINTR] = {
/* 63 */ intnull,
};
+struct irqinfo irqinfo[NINTR];
+struct irqinfo *irq_info = &irqinfo[0];
+
void
picdisable(void)
{
@@ -329,8 +332,7 @@ ioapic_irq_eoi(int pin)
//}
} else {
volatile ApicIoUnit *ioapic = apic_get_ioapic(apic)->ioapic;
- ioapic_read_entry(apic, pin, &entry.both);
- ioapic->eoi.r = entry.both.vector;
+ ioapic->eoi.r = irq_info[pin].vector;
}
simple_unlock_irq(s, &ioapic_lock);
@@ -420,6 +422,9 @@ ioapic_configure(void)
entry.both.vector = IOAPIC_INT_BASE + gsi;
ioapic_write_entry(apic, pin, entry.both);
+ irq_info[pin].vector = entry.both.vector;
+ irq_info[pin].trigger = entry.both.trigger;
+
/* Set initial state to masked */
mask_irq(pin);
@@ -454,6 +459,9 @@ ioapic_configure(void)
entry.both.vector = IOAPIC_INT_BASE + gsi;
ioapic_write_entry(apic, pin, entry.both);
+ irq_info[pin].vector = entry.both.vector;
+ irq_info[pin].trigger = entry.both.trigger;
+
/* Set initial state to masked */
mask_irq(pin);
}
@@ -478,6 +486,9 @@ ioapic_configure(void)
entry.both.vector = IOAPIC_INT_BASE + gsi;
ioapic_write_entry(apic, pin, entry.both);
+ irq_info[pin + ngsis].vector = entry.both.vector;
+ irq_info[pin + ngsis].trigger = entry.both.trigger;
+
/* Set initial state to masked */
mask_irq(pin + ngsis);
}
diff --git a/x86_64/interrupt.S b/x86_64/interrupt.S
index 6fb77727..533b0df5 100644
--- a/x86_64/interrupt.S
+++ b/x86_64/interrupt.S
@@ -61,6 +61,33 @@ ENTRY(interrupt)
je _call_local_ast
#endif
+ movb %cs:EXT(irqinfo)(%ecx),%al /* look up
irq_info[irq].trigger */
+ testb $1,%al /* was this a level triggered
interrupt? */
+ jz _eoi /* no, edge: eoi before handling */
+
+_call_handler:
+ ;
+ movq S_IPL,S_ARG1 /* previous ipl as 2nd arg */
+
+ ;
+ movq S_RET,S_ARG2 /* return address as 3th arg */
+
+ ;
+ movq S_REGS,S_ARG3 /* address of interrupted registers as
4th arg */
+
+ movl S_IRQ,%eax /* copy irq number */
+ shll $2,%eax /* irq * 4 */
+ movl EXT(iunit)(%rax),%edi /* get device unit number as 1st arg */
+
+ shll $1,%eax /* irq * 8 */
+ call *EXT(ivect)(%rax) /* call interrupt handler */
+
+ movl S_IRQ,%ecx /* restore irq number */
+ movb %cs:EXT(irqinfo)(%ecx),%al /* look up
irq_info[irq].trigger */
+ testb $1,%al /* was this a level triggered
interrupt? */
+ jz _completed /* no, edge: we are done */
+
+_eoi:
#ifndef APIC
movl $1,%eax
shll %cl,%eax /* get corresponding IRQ mask */
@@ -102,21 +129,10 @@ ENTRY(interrupt)
call EXT(ioapic_irq_eoi) /* ioapic irq specific EOI */
#endif
- ;
- movq S_IPL,S_ARG1 /* previous ipl as 2nd arg */
-
- ;
- movq S_RET,S_ARG2 /* return address as 3th arg */
-
- ;
- movq S_REGS,S_ARG3 /* address of interrupted registers as
4th arg */
-
- movl S_IRQ,%eax /* copy irq number */
- shll $2,%eax /* irq * 4 */
- movl EXT(iunit)(%rax),%edi /* get device unit number as 1st arg */
-
- shll $1,%eax /* irq * 8 */
- call *EXT(ivect)(%rax) /* call interrupt handler */
+ movl S_IRQ,%ecx /* restore irq number */
+ movb %cs:EXT(irqinfo)(%ecx),%al /* look up
irq_info[irq].trigger */
+ testb $1,%al /* was this a level triggered
interrupt? */
+ jz _call_handler /* no, edge: call handler after eoi */
_completed:
movl S_IPL,%edi /* restore previous ipl */
--
2.45.2
