Kernel: Simplify interrupt management

The IRQController object is RefCounted, and is shared between the
InterruptManagement class & IRQ handlers' classes.

IRQHandler, SharedIRQHandler & SpuriousInterruptHandler classes
use a responsible IRQ controller directly instead of calling
InterruptManagement for disable(), enable() or eoi().

Also, the initialization process of InterruptManagement is
simplified, so it doesn't rely on an ACPI parser to be initialized.

Kernel/Interrupts/IRQController.h

@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include <AK/RefCounted.h>
 #include <AK/String.h>
 #include <AK/Types.h>
 
@@ -36,7 +37,7 @@ enum class IRQControllerType {
     i82093AA = 2 /* Intel 82093AA I/O ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER (IOAPIC) */
 };
 
-class IRQController {
+class IRQController : public RefCounted<IRQController> {
 public:
     virtual ~IRQController() {}
 

Kernel/Interrupts/IRQHandler.cpp

@@ -34,6 +34,7 @@ namespace Kernel {
 
 IRQHandler::IRQHandler(u8 irq)
     : GenericInterruptHandler(irq)
+    , m_responsible_irq_controller(InterruptManagement::the().get_responsible_irq_controller(irq))
 {
 }
 
@@ -47,7 +48,8 @@ bool IRQHandler::eoi()
     dbg() << "EOI IRQ " << interrupt_number();
 #endif
     if (!m_shared_with_others) {
-        InterruptManagement::the().eoi(interrupt_number());
+        ASSERT(!m_responsible_irq_controller.is_null());
+        m_responsible_irq_controller->eoi(interrupt_number());
         return true;
     }
     return false;
@@ -59,7 +61,7 @@ void IRQHandler::enable_irq()
     dbg() << "Enable IRQ " << interrupt_number();
 #endif
     if (!m_shared_with_others)
-        InterruptManagement::the().enable(interrupt_number());
+        m_responsible_irq_controller->enable(interrupt_number());
     else
         m_enabled = true;
 }
@@ -70,7 +72,7 @@ void IRQHandler::disable_irq()
     dbg() << "Disable IRQ " << interrupt_number();
 #endif
     if (!m_shared_with_others)
-        InterruptManagement::the().disable(interrupt_number());
+        m_responsible_irq_controller->disable(interrupt_number());
     else
         m_enabled = false;
 }
@@ -79,6 +81,7 @@ void IRQHandler::change_irq_number(u8 irq)
 {
     InterruptDisabler disabler;
     change_interrupt_number(irq);
+    m_responsible_irq_controller = InterruptManagement::the().get_responsible_irq_controller(irq);
 }
 
 }

Kernel/Interrupts/IRQHandler.h

@@ -26,10 +26,12 @@
 
 #pragma once
 
+#include <AK/RefPtr.h>
 #include <AK/String.h>
 #include <AK/Types.h>
 #include <Kernel/Arch/i386/CPU.h>
 #include <Kernel/Interrupts/GenericInterruptHandler.h>
+#include <Kernel/Interrupts/IRQController.h>
 
 namespace Kernel {
 
@@ -53,11 +55,12 @@ public:
 
 protected:
     void change_irq_number(u8 irq);
-    explicit IRQHandler(u8 irq);
+    IRQHandler(u8 irq);
 
 private:
     bool m_shared_with_others { false };
     bool m_enabled { false };
+    RefPtr<IRQController> m_responsible_irq_controller;
 };
 
 }

Kernel/Interrupts/InterruptManagement.cpp

@@ -53,22 +53,7 @@ InterruptManagement& InterruptManagement::the()
 void InterruptManagement::initialize()
 {
     ASSERT(!InterruptManagement::initialized());
-    s_interrupt_management = new InterruptManagement(true);
+    s_interrupt_management = new InterruptManagement();
-}
-
-InterruptManagement::InterruptManagement(bool create_default_controller)
-{
-    if (create_default_controller)
-        m_interrupt_controllers[0] = make<PIC>();
-}
-
-void InterruptManagement::enable(u8 interrupt_vector)
-{
-    for (auto& irq_controller : InterruptManagement::the().m_interrupt_controllers) {
-        if (irq_controller->get_gsi_base() <= interrupt_vector)
-            if (!irq_controller->is_hard_disabled())
-                irq_controller->enable(interrupt_vector);
-    }
 }
 
 void InterruptManagement::enumerate_interrupt_handlers(Function<void(GenericInterruptHandler&)> callback)
@@ -80,66 +65,49 @@ void InterruptManagement::enumerate_interrupt_handlers(Function<void(GenericInte
     }
 }
 
-void InterruptManagement::disable(u8 interrupt_vector)
-{
-    for (auto& irq_controller : InterruptManagement::the().m_interrupt_controllers) {
-        ASSERT(irq_controller != nullptr);
-        if (irq_controller->get_gsi_base() <= interrupt_vector)
-            if (!irq_controller->is_hard_disabled())
-                irq_controller->disable(interrupt_vector);
-    }
-}
-
-void InterruptManagement::eoi(u8 interrupt_vector)
-{
-    if (m_interrupt_controllers.size() == 1 && m_interrupt_controllers[0]->type() == IRQControllerType::i8259) {
-        m_interrupt_controllers[0]->eoi(interrupt_vector);
-        return;
-    }
-    for (auto& irq_controller : InterruptManagement::the().m_interrupt_controllers) {
-        ASSERT(irq_controller != nullptr);
-        if (irq_controller->get_gsi_base() <= interrupt_vector)
-            if (!irq_controller->is_hard_disabled())
-                irq_controller->eoi(interrupt_vector);
-    }
-}
-
 IRQController& InterruptManagement::get_interrupt_controller(int index)
 {
     ASSERT(index >= 0);
-    ASSERT(m_interrupt_controllers[index] != nullptr);
+    ASSERT(!m_interrupt_controllers[index].is_null());
     return *m_interrupt_controllers[index];
 }
 
-void InterruptManagement::switch_to_pic_mode()
+RefPtr<IRQController> InterruptManagement::get_responsible_irq_controller(u8 interrupt_vector)
 {
-    kprintf("Interrupts: PIC mode by default\n");
+    if (m_interrupt_controllers.size() == 1 && m_interrupt_controllers[0]->type() == IRQControllerType::i8259) {
-    SpuriousInterruptHandler::initialize(7);
+        return m_interrupt_controllers[0];
-    SpuriousInterruptHandler::initialize(15);
+    }
+    for (auto irq_controller : m_interrupt_controllers) {
+        if (irq_controller->get_gsi_base() <= interrupt_vector)
+            if (!irq_controller->is_hard_disabled())
+                return irq_controller;
+    }
+    ASSERT_NOT_REACHED();
 }
 
-void InterruptManagement::switch_to_ioapic_mode()
+PhysicalAddress InterruptManagement::search_for_madt()
 {
-    kprintf("Interrupts: Switch to IOAPIC mode failed, Reverting to PIC mode\n");
+    dbg() << "Early access to ACPI tables for interrupt setup";
+    auto rsdp = ACPI::StaticParsing::search_rsdp();
+    if (rsdp.is_null())
+        return {};
+    return ACPI::StaticParsing::search_table(rsdp, "APIC");
 }
 
-void AdvancedInterruptManagement::initialize(PhysicalAddress p_madt)
+InterruptManagement::InterruptManagement()
+    : m_madt(search_for_madt())
 {
-    ASSERT(!InterruptManagement::initialized());
+    if (m_madt.is_null()) {
-    s_interrupt_management = new AdvancedInterruptManagement(p_madt);
+        m_interrupt_controllers[0] = adopt(*new PIC());
-}
+        return;
+    }
 
-AdvancedInterruptManagement::AdvancedInterruptManagement(PhysicalAddress p_madt)
-    : InterruptManagement(false)
-    , m_madt(p_madt)
-{
     // FIXME: Check what is the actual data size then map accordingly
-    dbg() << "Interrupts: MADT @ P " << p_madt.as_ptr();
+    dbg() << "Interrupts: MADT @ P " << m_madt.as_ptr();
-    locate_isa_interrupt_overrides(p_madt);
+    locate_apic_data();
-    locate_ioapics(p_madt);
 }
 
-void AdvancedInterruptManagement::switch_to_pic_mode()
+void InterruptManagement::switch_to_pic_mode()
 {
     kprintf("Interrupts: Switch to Legacy PIC mode\n");
     SpuriousInterruptHandler::initialize(7);
@@ -155,7 +123,7 @@ void AdvancedInterruptManagement::switch_to_pic_mode()
     }
 }
 
-void AdvancedInterruptManagement::switch_to_ioapic_mode()
+void InterruptManagement::switch_to_ioapic_mode()
 {
     kprintf("Interrupts: Switch to IOAPIC mode\n");
     if (m_interrupt_controllers.size() == 1) {
@@ -175,52 +143,31 @@ void AdvancedInterruptManagement::switch_to_ioapic_mode()
     }
 }
 
-void AdvancedInterruptManagement::locate_ioapics(PhysicalAddress p_madt)
+void InterruptManagement::locate_apic_data()
 {
-    auto region = MM.allocate_kernel_region(p_madt.page_base(), (PAGE_SIZE * 2), "Initializing Interrupts", Region::Access::Read);
+    ASSERT(!m_madt.is_null());
-    auto& madt = *(const ACPI_RAW::MADT*)region->vaddr().offset(p_madt.offset_in_page().get()).as_ptr();
+    auto region = MM.allocate_kernel_region(m_madt.page_base(), (PAGE_SIZE * 2), "Initializing Interrupts", Region::Access::Read);
+    auto& madt = *(const ACPI::Structures::MADT*)region->vaddr().offset(m_madt.offset_in_page().get()).as_ptr();
 
-    int index = 0;
+    int irq_controller_count = 0;
     if (madt.flags & PCAT_COMPAT_FLAG) {
-        m_interrupt_controllers[0] = make<PIC>();
+        m_interrupt_controllers[0] = adopt(*new PIC());
-        index++;
+        irq_controller_count++;
     }
     size_t entry_index = 0;
-    size_t entries_length = madt.h.length - sizeof(ACPI_RAW::MADT);
+    size_t entries_length = madt.h.length - sizeof(ACPI::Structures::MADT);
     auto* madt_entry = madt.entries;
     while (entries_length > 0) {
         size_t entry_length = madt_entry->length;
-        if (madt_entry->type == (u8)ACPI_RAW::MADTEntryType::IOAPIC) {
+        if (madt_entry->type == (u8)ACPI::Structures::MADTEntryType::IOAPIC) {
-            auto* ioapic_entry = (const ACPI_RAW::MADT_IOAPIC*)madt_entry;
+            auto* ioapic_entry = (const ACPI::Structures::MADTEntries::IOAPIC*)madt_entry;
             dbg() << "IOAPIC found @ MADT entry " << entry_index << ", MMIO Registers @ Px" << String::format("%x", ioapic_entry->ioapic_address);
-            m_interrupt_controllers.resize(1 + index);
+            m_interrupt_controllers.resize(1 + irq_controller_count);
-            m_interrupt_controllers[index] = make<IOAPIC>(*(ioapic_mmio_regs*)ioapic_entry->ioapic_address, ioapic_entry->gsi_base, m_isa_interrupt_overrides, m_pci_interrupt_overrides);
+            m_interrupt_controllers[irq_controller_count] = adopt(*new IOAPIC(*(ioapic_mmio_regs*)ioapic_entry->ioapic_address, ioapic_entry->gsi_base, m_isa_interrupt_overrides, m_pci_interrupt_overrides));
-            index++;
+            irq_controller_count++;
         }
-        madt_entry = (ACPI_RAW::MADTEntryHeader*)(VirtualAddress((u32)madt_entry).offset(entry_length).get());
+        if (madt_entry->type == (u8)ACPI::Structures::MADTEntryType::InterruptSourceOverride) {
-        entries_length -= entry_length;
+            auto* interrupt_override_entry = (const ACPI::Structures::MADTEntries::InterruptSourceOverride*)madt_entry;
-        entry_index++;
-    }
-}
-void AdvancedInterruptManagement::locate_pci_interrupt_overrides()
-{
-    // FIXME: calling the MultiProcessorParser causes a pagefault.
-    ASSERT_NOT_REACHED();
-    m_pci_interrupt_overrides = MultiProcessorParser::the().get_pci_interrupt_redirections();
-}
-
-void AdvancedInterruptManagement::locate_isa_interrupt_overrides(PhysicalAddress p_madt)
-{
-    auto region = MM.allocate_kernel_region(p_madt.page_base(), (PAGE_SIZE * 2), "Initializing Interrupts", Region::Access::Read);
-    auto& madt = *(const ACPI_RAW::MADT*)region->vaddr().offset(p_madt.offset_in_page().get()).as_ptr();
-
-    size_t entry_index = 0;
-    size_t entries_length = madt.h.length - sizeof(ACPI_RAW::MADT);
-    auto* madt_entry = madt.entries;
-    while (entries_length > 0) {
-        size_t entry_length = madt_entry->length;
-        if (madt_entry->type == (u8)ACPI_RAW::MADTEntryType::InterruptSourceOverride) {
-            auto* interrupt_override_entry = (const ACPI_RAW::MADT_InterruptSourceOverride*)madt_entry;
             m_isa_interrupt_overrides.append(adopt(*new ISAInterruptOverrideMetadata(
                 interrupt_override_entry->bus,
                 interrupt_override_entry->source,
@@ -228,11 +175,17 @@ void AdvancedInterruptManagement::locate_isa_interrupt_overrides(PhysicalAddress
                 interrupt_override_entry->flags)));
             dbg() << "Interrupts: Overriding INT 0x" << String::format("%x", interrupt_override_entry->source) << " with GSI " << interrupt_override_entry->global_system_interrupt << ", for bus 0x" << String::format("%x", interrupt_override_entry->bus);
         }
-        madt_entry = (ACPI_RAW::MADTEntryHeader*)(VirtualAddress((u32)madt_entry).offset(entry_length).get());
+        madt_entry = (ACPI::Structures::MADTEntryHeader*)(VirtualAddress((u32)madt_entry).offset(entry_length).get());
         entries_length -= entry_length;
         entry_index++;
     }
 }
+void InterruptManagement::locate_pci_interrupt_overrides()
+{
+    // FIXME: calling the MultiProcessorParser causes a pagefault.
+    ASSERT_NOT_REACHED();
+    m_pci_interrupt_overrides = MultiProcessorParser::the().get_pci_interrupt_redirections();
+}
 
 ISAInterruptOverrideMetadata::ISAInterruptOverrideMetadata(u8 bus, u8 source, u32 global_system_interrupt, u16 flags)
     : m_bus(bus)

Kernel/Interrupts/InterruptManagement.h

@@ -40,6 +40,8 @@
 
 namespace Kernel {
 
+class ISAInterruptOverrideMetadata;
+
 class InterruptManagement {
 public:
     static InterruptManagement& the();
@@ -48,32 +50,17 @@ public:
 
     virtual void switch_to_pic_mode();
     virtual void switch_to_ioapic_mode();
+    RefPtr<IRQController> get_responsible_irq_controller(u8 interrupt_vector);
 
-    void enable(u8 interrupt_vector);
-    void disable(u8 interrupt_vector);
-    void eoi(u8 interrupt_vector);
     void enumerate_interrupt_handlers(Function<void(GenericInterruptHandler&)>);
     IRQController& get_interrupt_controller(int index);
 
-protected:
-    explicit InterruptManagement(bool create_default_controller);
-    FixedArray<OwnPtr<IRQController>> m_interrupt_controllers { 1 };
-};
-
-class ISAInterruptOverrideMetadata;
-class AdvancedInterruptManagement : public InterruptManagement {
-    friend class IOAPIC;
-
-public:
-    static void initialize(PhysicalAddress madt);
-    virtual void switch_to_ioapic_mode() override;
-    virtual void switch_to_pic_mode() override;
-
 private:
-    explicit AdvancedInterruptManagement(PhysicalAddress madt);
+    InterruptManagement();
-    void locate_ioapics(PhysicalAddress madt);
+    PhysicalAddress search_for_madt();
-    void locate_isa_interrupt_overrides(PhysicalAddress madt);
+    void locate_apic_data();
     void locate_pci_interrupt_overrides();
+    FixedArray<RefPtr<IRQController>> m_interrupt_controllers { 1 };
     Vector<RefPtr<ISAInterruptOverrideMetadata>> m_isa_interrupt_overrides;
     Vector<RefPtr<PCIInterruptOverrideMetadata>> m_pci_interrupt_overrides;
     PhysicalAddress m_madt;

Kernel/Interrupts/SharedIRQHandler.cpp

@@ -62,12 +62,13 @@ bool SharedIRQHandler::eoi()
 #ifdef INTERRUPT_DEBUG
     dbg() << "EOI IRQ " << interrupt_number();
 #endif
-    InterruptManagement::the().eoi(interrupt_number());
+    m_responsible_irq_controller->eoi(interrupt_number());
     return true;
 }
 
 SharedIRQHandler::SharedIRQHandler(u8 irq)
     : GenericInterruptHandler(irq)
+    , m_responsible_irq_controller(InterruptManagement::the().get_responsible_irq_controller(irq))
 {
 #ifdef INTERRUPT_DEBUG
     kprintf("Shared Interrupt Handler registered @ %d\n", m_interrupt_number);
@@ -114,7 +115,7 @@ void SharedIRQHandler::enable_interrupt_vector()
     if (m_enabled)
         return;
     m_enabled = true;
-    InterruptManagement::the().enable(interrupt_number());
+    m_responsible_irq_controller->enable(interrupt_number());
 }
 
 void SharedIRQHandler::disable_interrupt_vector()
@@ -122,7 +123,7 @@ void SharedIRQHandler::disable_interrupt_vector()
     if (!m_enabled)
         return;
     m_enabled = false;
-    InterruptManagement::the().disable(interrupt_number());
+    m_responsible_irq_controller->disable(interrupt_number());
 }
 
 }

Kernel/Interrupts/SharedIRQHandler.h

@@ -28,6 +28,7 @@
 
 #include <AK/HashTable.h>
 #include <AK/NonnullOwnPtr.h>
+#include <AK/RefPtr.h>
 #include <AK/Types.h>
 #include <Kernel/Arch/i386/CPU.h>
 #include <Kernel/Interrupts/GenericInterruptHandler.h>
@@ -57,5 +58,6 @@ private:
     explicit SharedIRQHandler(u8 interrupt_number);
     bool m_enabled;
     HashTable<GenericInterruptHandler*> m_handlers;
+    RefPtr<IRQController> m_responsible_irq_controller;
 };
 }

Kernel/Interrupts/SpuriousInterruptHandler.cpp

@@ -45,12 +45,13 @@ bool SpuriousInterruptHandler::eoi()
 {
     // FIXME: Actually check if IRQ7 or IRQ15 are spurious, and if not, call EOI with the correct interrupt number.
     if (interrupt_number() == 15)
-        InterruptManagement::the().eoi(7);
+        m_responsible_irq_controller->eoi(7);
     return false;
 }
 
 SpuriousInterruptHandler::SpuriousInterruptHandler(u8 irq)
     : GenericInterruptHandler(irq)
+    , m_responsible_irq_controller(InterruptManagement::the().get_responsible_irq_controller(irq))
 {
 }
 
@@ -69,7 +70,7 @@ void SpuriousInterruptHandler::enable_interrupt_vector()
     if (m_enabled)
         return;
     m_enabled = true;
-    InterruptManagement::the().enable(interrupt_number());
+    m_responsible_irq_controller->enable(interrupt_number());
 }
 
 void SpuriousInterruptHandler::disable_interrupt_vector()
@@ -77,7 +78,7 @@ void SpuriousInterruptHandler::disable_interrupt_vector()
     if (!m_enabled)
         return;
     m_enabled = false;
-    InterruptManagement::the().disable(interrupt_number());
+    m_responsible_irq_controller->disable(interrupt_number());
 }
 
 }

Kernel/Interrupts/SpuriousInterruptHandler.h

@@ -56,6 +56,7 @@ private:
     void disable_interrupt_vector();
     explicit SpuriousInterruptHandler(u8 interrupt_number);
     bool m_enabled;
+    RefPtr<IRQController> m_responsible_irq_controller;
     OwnPtr<GenericInterruptHandler> m_real_handler;
 };
 }