From 6f914ed0a4d5e5170f3d3a50573c55759e47061f Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 28 Feb 2020 22:33:41 +0200 Subject: [PATCH] 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 | 3 +- Kernel/Interrupts/IRQHandler.cpp | 9 +- Kernel/Interrupts/IRQHandler.h | 5 +- Kernel/Interrupts/InterruptManagement.cpp | 147 ++++++------------ Kernel/Interrupts/InterruptManagement.h | 27 +--- Kernel/Interrupts/SharedIRQHandler.cpp | 7 +- Kernel/Interrupts/SharedIRQHandler.h | 2 + .../Interrupts/SpuriousInterruptHandler.cpp | 7 +- Kernel/Interrupts/SpuriousInterruptHandler.h | 1 + 9 files changed, 80 insertions(+), 128 deletions(-) diff --git a/Kernel/Interrupts/IRQController.h b/Kernel/Interrupts/IRQController.h index fe32376fbf..f27c5bd0f9 100644 --- a/Kernel/Interrupts/IRQController.h +++ b/Kernel/Interrupts/IRQController.h @@ -26,6 +26,7 @@ #pragma once +#include #include #include @@ -36,7 +37,7 @@ enum class IRQControllerType { i82093AA = 2 /* Intel 82093AA I/O ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER (IOAPIC) */ }; -class IRQController { +class IRQController : public RefCounted { public: virtual ~IRQController() {} diff --git a/Kernel/Interrupts/IRQHandler.cpp b/Kernel/Interrupts/IRQHandler.cpp index 72d06a3161..05439b1ff5 100644 --- a/Kernel/Interrupts/IRQHandler.cpp +++ b/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); } } diff --git a/Kernel/Interrupts/IRQHandler.h b/Kernel/Interrupts/IRQHandler.h index fe6c9b0d0f..4505a4327e 100644 --- a/Kernel/Interrupts/IRQHandler.h +++ b/Kernel/Interrupts/IRQHandler.h @@ -26,10 +26,12 @@ #pragma once +#include #include #include #include #include +#include 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 m_responsible_irq_controller; }; } diff --git a/Kernel/Interrupts/InterruptManagement.cpp b/Kernel/Interrupts/InterruptManagement.cpp index 8c03565019..d96137cbd5 100644 --- a/Kernel/Interrupts/InterruptManagement.cpp +++ b/Kernel/Interrupts/InterruptManagement.cpp @@ -53,22 +53,7 @@ InterruptManagement& InterruptManagement::the() void InterruptManagement::initialize() { ASSERT(!InterruptManagement::initialized()); - s_interrupt_management = new InterruptManagement(true); -} - -InterruptManagement::InterruptManagement(bool create_default_controller) -{ - if (create_default_controller) - m_interrupt_controllers[0] = make(); -} - -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); - } + s_interrupt_management = new InterruptManagement(); } void InterruptManagement::enumerate_interrupt_handlers(Function callback) @@ -80,66 +65,49 @@ void InterruptManagement::enumerate_interrupt_handlers(Functionget_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 InterruptManagement::get_responsible_irq_controller(u8 interrupt_vector) { - kprintf("Interrupts: PIC mode by default\n"); - SpuriousInterruptHandler::initialize(7); - SpuriousInterruptHandler::initialize(15); + if (m_interrupt_controllers.size() == 1 && m_interrupt_controllers[0]->type() == IRQControllerType::i8259) { + return m_interrupt_controllers[0]; + } + 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()); - s_interrupt_management = new AdvancedInterruptManagement(p_madt); -} + if (m_madt.is_null()) { + 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(); - locate_isa_interrupt_overrides(p_madt); - locate_ioapics(p_madt); + dbg() << "Interrupts: MADT @ P " << m_madt.as_ptr(); + locate_apic_data(); } -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); - auto& madt = *(const ACPI_RAW::MADT*)region->vaddr().offset(p_madt.offset_in_page().get()).as_ptr(); + ASSERT(!m_madt.is_null()); + 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(); - index++; + m_interrupt_controllers[0] = adopt(*new PIC()); + 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) { - auto* ioapic_entry = (const ACPI_RAW::MADT_IOAPIC*)madt_entry; + if (madt_entry->type == (u8)ACPI::Structures::MADTEntryType::IOAPIC) { + 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[index] = make(*(ioapic_mmio_regs*)ioapic_entry->ioapic_address, ioapic_entry->gsi_base, m_isa_interrupt_overrides, m_pci_interrupt_overrides); - index++; + m_interrupt_controllers.resize(1 + irq_controller_count); + 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)); + irq_controller_count++; } - madt_entry = (ACPI_RAW::MADTEntryHeader*)(VirtualAddress((u32)madt_entry).offset(entry_length).get()); - entries_length -= entry_length; - 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; + if (madt_entry->type == (u8)ACPI::Structures::MADTEntryType::InterruptSourceOverride) { + auto* interrupt_override_entry = (const ACPI::Structures::MADTEntries::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) diff --git a/Kernel/Interrupts/InterruptManagement.h b/Kernel/Interrupts/InterruptManagement.h index 7f7697da40..50c695a18d 100644 --- a/Kernel/Interrupts/InterruptManagement.h +++ b/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 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); IRQController& get_interrupt_controller(int index); -protected: - explicit InterruptManagement(bool create_default_controller); - FixedArray> 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); - void locate_ioapics(PhysicalAddress madt); - void locate_isa_interrupt_overrides(PhysicalAddress madt); + InterruptManagement(); + PhysicalAddress search_for_madt(); + void locate_apic_data(); void locate_pci_interrupt_overrides(); + FixedArray> m_interrupt_controllers { 1 }; Vector> m_isa_interrupt_overrides; Vector> m_pci_interrupt_overrides; PhysicalAddress m_madt; diff --git a/Kernel/Interrupts/SharedIRQHandler.cpp b/Kernel/Interrupts/SharedIRQHandler.cpp index 9418738cfc..f770fee641 100644 --- a/Kernel/Interrupts/SharedIRQHandler.cpp +++ b/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()); } } diff --git a/Kernel/Interrupts/SharedIRQHandler.h b/Kernel/Interrupts/SharedIRQHandler.h index a33be4c4a1..56da1b5f3a 100644 --- a/Kernel/Interrupts/SharedIRQHandler.h +++ b/Kernel/Interrupts/SharedIRQHandler.h @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -57,5 +58,6 @@ private: explicit SharedIRQHandler(u8 interrupt_number); bool m_enabled; HashTable m_handlers; + RefPtr m_responsible_irq_controller; }; } diff --git a/Kernel/Interrupts/SpuriousInterruptHandler.cpp b/Kernel/Interrupts/SpuriousInterruptHandler.cpp index bc9ebe96a2..85d0d9beb5 100644 --- a/Kernel/Interrupts/SpuriousInterruptHandler.cpp +++ b/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()); } } diff --git a/Kernel/Interrupts/SpuriousInterruptHandler.h b/Kernel/Interrupts/SpuriousInterruptHandler.h index 5abb6988f4..0208b0f4f9 100644 --- a/Kernel/Interrupts/SpuriousInterruptHandler.h +++ b/Kernel/Interrupts/SpuriousInterruptHandler.h @@ -56,6 +56,7 @@ private: void disable_interrupt_vector(); explicit SpuriousInterruptHandler(u8 interrupt_number); bool m_enabled; + RefPtr m_responsible_irq_controller; OwnPtr m_real_handler; }; }