From c82f5e9269b4b7f9a22e980935aae6565f9b47f2 Mon Sep 17 00:00:00 2001 From: Reto Buerki Date: Fri, 20 Mar 2015 14:25:07 +0100 Subject: [PATCH] hw_x86_64: Do not mask edge-triggered interrupts Do not mask edge-triggered interrupts to avoid losing them while masked, see Intel 82093AA I/O Advanced Programmable Interrupt Controller (IOAPIC) specification, section 3.4.2, "Interrupt Mask": "When this bit is 1, the interrupt signal is masked. Edge-sensitive interrupts signaled on a masked interrupt pin are ignored (i.e., not delivered or held pending)" Or to quote Linus Torvalds on the subject: "Now, edge-triggered interrupts are a _lot_ harder to mask, because the Intel APIC is an unbelievable piece of sh*t, and has the edge-detect logic _before_ the mask logic, so if a edge happens _while_ the device is masked, you'll never ever see the edge ever again (unmasking will not cause a new edge, so you simply lost the interrupt)." So when you "mask" an edge-triggered IRQ, you can't really mask it at all, because if you did that, you'd lose it forever if the IRQ comes in while you masked it. Instead, we're supposed to leave it active, and set a flag, and IF the IRQ comes in, we just remember it, and mask it at that point instead, and then on unmasking, we have to replay it by sending a self-IPI." [1] [1] - http://yarchive.net/comp/linux/edge_triggered_interrupts.html Ref #1448 --- .../base-hw/src/core/include/spec/x86/board.h | 1 + repos/base-hw/src/core/include/spec/x86/pic.h | 38 ++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/repos/base-hw/src/core/include/spec/x86/board.h b/repos/base-hw/src/core/include/spec/x86/board.h index dfb47706a..9fd54d8cd 100644 --- a/repos/base-hw/src/core/include/spec/x86/board.h +++ b/repos/base-hw/src/core/include/spec/x86/board.h @@ -27,6 +27,7 @@ namespace Genode VECTOR_REMAP_BASE = 48, TIMER_VECTOR_KERNEL = 32, TIMER_VECTOR_USER = 50, + ISA_IRQ_END = 15, }; }; } diff --git a/repos/base-hw/src/core/include/spec/x86/pic.h b/repos/base-hw/src/core/include/spec/x86/pic.h index be6613100..7880063f2 100644 --- a/repos/base-hw/src/core/include/spec/x86/pic.h +++ b/repos/base-hw/src/core/include/spec/x86/pic.h @@ -33,6 +33,8 @@ class Genode::Pic : public Mmio { private: + enum { REMAP_BASE = Board::VECTOR_REMAP_BASE }; + /* Registers */ struct EOI : Register<0x0b0, 32, true> { }; struct Svr : Register<0x0f0, 32> @@ -65,12 +67,14 @@ class Genode::Pic : public Mmio IOREDTBL = 0x10, }; - /* Create redirection table entry for given IRQ */ - uint64_t create_irt_entry(unsigned irq) + /** + * Create redirection table entry for given IRQ + */ + uint64_t _create_irt_entry(unsigned irq) { - uint32_t entry = Board::VECTOR_REMAP_BASE + irq; + uint32_t entry = REMAP_BASE + irq; - if (irq > 15) { + if (irq > Board::ISA_IRQ_END) { /* Use level-triggered, high-active mode for non-legacy * IRQs */ entry |= 1 << IRTE_BIT_POL | 1 << IRTE_BIT_TRG; @@ -78,6 +82,15 @@ class Genode::Pic : public Mmio return entry; } + /** + * Return whether 'irq' is an edge-triggered interrupt + */ + bool _edge_triggered(unsigned const irq) + { + return irq <= REMAP_BASE + Board::ISA_IRQ_END || + irq > REMAP_BASE + IRTE_COUNT; + } + public: Ioapic() : Mmio(Board::MMIO_IOAPIC_BASE) @@ -85,19 +98,24 @@ class Genode::Pic : public Mmio /* Remap all supported IRQs */ for (unsigned i = 0; i <= IRTE_COUNT; i++) { write(IOREDTBL + 2 * i); - write(create_irt_entry(i)); + write(_create_irt_entry(i)); } }; /* Set/unset mask bit of IRTE for given vector */ void toggle_mask(unsigned const vector, bool const set) { - if (vector < Board::VECTOR_REMAP_BASE || - vector > Board::VECTOR_REMAP_BASE + IRTE_COUNT) - return; + /* + * Only mask existing RTEs and do *not* mask edge-triggered + * interrupts to avoid losing them while masked, see Intel + * 82093AA I/O Advanced Programmable Interrupt Controller + * (IOAPIC) specification, section 3.4.2, "Interrupt Mask" + * flag and edge-triggered interrupts or: + * http://yarchive.net/comp/linux/edge_triggered_interrupts.html + */ + if (_edge_triggered(vector)) { return; } - write(IOREDTBL + (2 * (vector - - Board::VECTOR_REMAP_BASE))); + write(IOREDTBL + (2 * (vector - REMAP_BASE))); uint32_t val = read(); if (set) {