From d92f62db4348c6ad0b32aab07ce516c4500720e3 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 8 May 2020 21:07:04 +0200 Subject: [PATCH] Kernel: Remove ref-counting from interrupt override metadata I don't see a reason for these to be reference-counted, and removing it simplifies a bunch of surrounding data structures. --- Kernel/ACPI/MultiProcessorParser.cpp | 44 +++++------------------ Kernel/ACPI/MultiProcessorParser.h | 23 ++---------- Kernel/Interrupts/IOAPIC.cpp | 18 +++++----- Kernel/Interrupts/IOAPIC.h | 22 ++++++++++-- Kernel/Interrupts/InterruptManagement.cpp | 34 ++---------------- Kernel/Interrupts/InterruptManagement.h | 42 ++++++++++++---------- 6 files changed, 66 insertions(+), 117 deletions(-) diff --git a/Kernel/ACPI/MultiProcessorParser.cpp b/Kernel/ACPI/MultiProcessorParser.cpp index 69b0d49837..7add45be12 100644 --- a/Kernel/ACPI/MultiProcessorParser.cpp +++ b/Kernel/ACPI/MultiProcessorParser.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -189,34 +190,34 @@ MultiProcessorParser& MultiProcessorParser::the() return *s_parser; } -Vector> MultiProcessorParser::get_pci_interrupt_redirections() +Vector MultiProcessorParser::get_pci_interrupt_redirections() { dbg() << "MultiProcessor: Get PCI IOAPIC redirections"; - Vector> overrides; + Vector overrides; Vector pci_bus_ids = get_pci_bus_ids(); for (auto entry : m_io_interrupt_redirection_entries) { auto entry_region = MM.allocate_kernel_region(PhysicalAddress(page_base_of((u32)entry)), PAGE_ROUND_UP(m_configuration_table_length), "MultiProcessor Parser Parsing Bus Entry", Region::Access::Read, false, true); auto* v_entry_ptr = (MultiProcessor::IOInterruptAssignmentEntry*)entry_region->vaddr().offset(offset_in_page((u32)entry)).as_ptr(); -#ifdef MULTIPROCESSOR_DEBUG +#ifdef MULTIPROCESSOR_DEBUG dbg() << "MultiProcessor: Parsing Entry P 0x" << String::format("%x", entry) << ", V " << v_entry_ptr; #endif for (auto id : pci_bus_ids) { if (id == v_entry_ptr->source_bus_id) { klog() << "Interrupts: Bus " << v_entry_ptr->source_bus_id << ", Polarity " << v_entry_ptr->polarity << ", Trigger Mode " << v_entry_ptr->trigger_mode << ", INT " << v_entry_ptr->source_bus_irq << ", IOAPIC " << v_entry_ptr->destination_ioapic_id << ", IOAPIC INTIN " << v_entry_ptr->destination_ioapic_intin_pin; - overrides.append(adopt(*new PCIInterruptOverrideMetadata( + overrides.empend( v_entry_ptr->source_bus_id, v_entry_ptr->polarity, v_entry_ptr->trigger_mode, v_entry_ptr->source_bus_irq, v_entry_ptr->destination_ioapic_id, - v_entry_ptr->destination_ioapic_intin_pin))); + v_entry_ptr->destination_ioapic_intin_pin); } } } - for (auto override_metadata : overrides) { - klog() << "Interrupts: Bus " << override_metadata->bus() << ", Polarity " << override_metadata->polarity() << ", PCI Device " << override_metadata->pci_device_number() << ", Trigger Mode " << override_metadata->trigger_mode() << ", INT " << override_metadata->pci_interrupt_pin() << ", IOAPIC " << override_metadata->ioapic_id() << ", IOAPIC INTIN " << override_metadata->ioapic_interrupt_pin(); + for (auto& override_metadata : overrides) { + klog() << "Interrupts: Bus " << override_metadata.bus() << ", Polarity " << override_metadata.polarity() << ", PCI Device " << override_metadata.pci_device_number() << ", Trigger Mode " << override_metadata.trigger_mode() << ", INT " << override_metadata.pci_interrupt_pin() << ", IOAPIC " << override_metadata.ioapic_id() << ", IOAPIC INTIN " << override_metadata.ioapic_interrupt_pin(); } return overrides; } @@ -231,32 +232,5 @@ PCIInterruptOverrideMetadata::PCIInterruptOverrideMetadata(u8 bus_id, u8 polarit , m_ioapic_interrupt_pin(ioapic_int_pin) { } -u8 PCIInterruptOverrideMetadata::bus() const -{ - return m_bus_id; -} -u8 PCIInterruptOverrideMetadata::polarity() const -{ - return m_polarity; -} -u8 PCIInterruptOverrideMetadata::trigger_mode() const -{ - return m_trigger_mode; -} -u8 PCIInterruptOverrideMetadata::pci_interrupt_pin() const -{ - return m_pci_interrupt_pin; -} -u8 PCIInterruptOverrideMetadata::pci_device_number() const -{ - return m_pci_device_number; -} -u32 PCIInterruptOverrideMetadata::ioapic_id() const -{ - return m_ioapic_id; -} -u16 PCIInterruptOverrideMetadata::ioapic_interrupt_pin() const -{ - return m_ioapic_interrupt_pin; -} + } diff --git a/Kernel/ACPI/MultiProcessorParser.h b/Kernel/ACPI/MultiProcessorParser.h index 1acbe610e4..cfaf60ac05 100644 --- a/Kernel/ACPI/MultiProcessorParser.h +++ b/Kernel/ACPI/MultiProcessorParser.h @@ -187,26 +187,7 @@ struct [[gnu::packed]] CompatibilityBusAddressSpaceModifierEntry } -class PCIInterruptOverrideMetadata : public RefCounted { -public: - PCIInterruptOverrideMetadata(u8 bus_id, u8 polarity, u8 trigger_mode, u8 source_irq, u32 ioapic_id, u16 ioapic_int_pin); - u8 bus() const; - u8 polarity() const; - u8 trigger_mode() const; - u8 pci_interrupt_pin() const; - u8 pci_device_number() const; - u32 ioapic_id() const; - u16 ioapic_interrupt_pin() const; - -private: - u8 m_bus_id; - u8 m_polarity; - u8 m_trigger_mode; - u8 m_pci_interrupt_pin; - u8 m_pci_device_number; - u32 m_ioapic_id; - u16 m_ioapic_interrupt_pin; -}; +class PCIInterruptOverrideMetadata; class MultiProcessorParser { public: @@ -214,7 +195,7 @@ public: static bool is_initialized(); static void initialize(); - Vector> get_pci_interrupt_redirections(); + Vector get_pci_interrupt_redirections(); protected: MultiProcessorParser(); diff --git a/Kernel/Interrupts/IOAPIC.cpp b/Kernel/Interrupts/IOAPIC.cpp index bd7f1eb4eb..8b03d29ae8 100644 --- a/Kernel/Interrupts/IOAPIC.cpp +++ b/Kernel/Interrupts/IOAPIC.cpp @@ -66,12 +66,11 @@ void IOAPIC::map_interrupt_redirection(u8 interrupt_vector) { InterruptDisabler disabler; for (auto redirection_override : InterruptManagement::the().isa_overrides()) { - ASSERT(!redirection_override.is_null()); - if (redirection_override->source() != interrupt_vector) + if (redirection_override.source() != interrupt_vector) continue; bool active_low; // See ACPI spec Version 6.2, page 205 to learn more about Interrupt Overriding Flags. - switch ((redirection_override->flags() & 0b11)) { + switch ((redirection_override.flags() & 0b11)) { case 0: active_low = false; break; @@ -87,7 +86,7 @@ void IOAPIC::map_interrupt_redirection(u8 interrupt_vector) bool trigger_level_mode; // See ACPI spec Version 6.2, page 205 to learn more about Interrupt Overriding Flags. - switch (((redirection_override->flags() >> 2) & 0b11)) { + switch (((redirection_override.flags() >> 2) & 0b11)) { case 0: trigger_level_mode = false; break; @@ -100,7 +99,7 @@ void IOAPIC::map_interrupt_redirection(u8 interrupt_vector) trigger_level_mode = true; break; } - configure_redirection_entry(redirection_override->gsi() - gsi_base(), InterruptManagement::acquire_mapped_interrupt_number(redirection_override->source()) + IRQ_VECTOR_BASE, DeliveryMode::Normal, false, active_low, trigger_level_mode, true, 0); + configure_redirection_entry(redirection_override.gsi() - gsi_base(), InterruptManagement::acquire_mapped_interrupt_number(redirection_override.source()) + IRQ_VECTOR_BASE, DeliveryMode::Normal, false, active_low, trigger_level_mode, true, 0); return; } isa_identity_map(interrupt_vector); @@ -135,12 +134,11 @@ void IOAPIC::map_isa_interrupts() { InterruptDisabler disabler; for (auto redirection_override : InterruptManagement::the().isa_overrides()) { - ASSERT(!redirection_override.is_null()); - if ((redirection_override->gsi() < gsi_base()) || (redirection_override->gsi() >= (gsi_base() + m_redirection_entries_count))) + if ((redirection_override.gsi() < gsi_base()) || (redirection_override.gsi() >= (gsi_base() + m_redirection_entries_count))) continue; bool active_low; // See ACPI spec Version 6.2, page 205 to learn more about Interrupt Overriding Flags. - switch ((redirection_override->flags() & 0b11)) { + switch ((redirection_override.flags() & 0b11)) { case 0: active_low = false; break; @@ -156,7 +154,7 @@ void IOAPIC::map_isa_interrupts() bool trigger_level_mode; // See ACPI spec Version 6.2, page 205 to learn more about Interrupt Overriding Flags. - switch (((redirection_override->flags() >> 2) & 0b11)) { + switch (((redirection_override.flags() >> 2) & 0b11)) { case 0: trigger_level_mode = false; break; @@ -169,7 +167,7 @@ void IOAPIC::map_isa_interrupts() trigger_level_mode = true; break; } - configure_redirection_entry(redirection_override->gsi() - gsi_base(), InterruptManagement::acquire_mapped_interrupt_number(redirection_override->source()) + IRQ_VECTOR_BASE, 0, false, active_low, trigger_level_mode, true, 0); + configure_redirection_entry(redirection_override.gsi() - gsi_base(), InterruptManagement::acquire_mapped_interrupt_number(redirection_override.source()) + IRQ_VECTOR_BASE, 0, false, active_low, trigger_level_mode, true, 0); } } diff --git a/Kernel/Interrupts/IOAPIC.h b/Kernel/Interrupts/IOAPIC.h index 713e28b9e0..7115126c46 100644 --- a/Kernel/Interrupts/IOAPIC.h +++ b/Kernel/Interrupts/IOAPIC.h @@ -36,8 +36,26 @@ struct [[gnu::packed]] ioapic_mmio_regs volatile u32 window; }; -class ISAInterruptOverrideMetadata; -class PCIInterruptOverrideMetadata; +class PCIInterruptOverrideMetadata { +public: + PCIInterruptOverrideMetadata(u8 bus_id, u8 polarity, u8 trigger_mode, u8 source_irq, u32 ioapic_id, u16 ioapic_int_pin); + u8 bus() const { return m_bus_id; } + u8 polarity() const { return m_polarity; } + u8 trigger_mode() const { return m_trigger_mode; } + u8 pci_interrupt_pin() const { return m_pci_interrupt_pin; } + u8 pci_device_number() const { return m_pci_device_number; } + u32 ioapic_id() const { return m_ioapic_id; } + u16 ioapic_interrupt_pin() const { return m_ioapic_interrupt_pin; } + +private: + const u8 m_bus_id; + const u8 m_polarity; + const u8 m_trigger_mode; + const u8 m_pci_interrupt_pin; + const u8 m_pci_device_number; + const u32 m_ioapic_id; + const u16 m_ioapic_interrupt_pin; +}; class IOAPIC final : public IRQController { public: diff --git a/Kernel/Interrupts/InterruptManagement.cpp b/Kernel/Interrupts/InterruptManagement.cpp index adf8a0dc44..80ac5288a0 100644 --- a/Kernel/Interrupts/InterruptManagement.cpp +++ b/Kernel/Interrupts/InterruptManagement.cpp @@ -83,11 +83,6 @@ IRQController& InterruptManagement::get_interrupt_controller(int index) return *m_interrupt_controllers[index]; } -Vector> InterruptManagement::isa_overrides() -{ - return m_isa_interrupt_overrides; -} - u8 InterruptManagement::acquire_mapped_interrupt_number(u8 original_irq) { if (!InterruptManagement::initialized()) { @@ -223,11 +218,11 @@ void InterruptManagement::locate_apic_data() } 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( + m_isa_interrupt_overrides.empend( interrupt_override_entry->bus, interrupt_override_entry->source, interrupt_override_entry->global_system_interrupt, - interrupt_override_entry->flags))); + 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::Structures::MADTEntryHeader*)(VirtualAddress((u32)madt_entry).offset(entry_length).get()); @@ -235,33 +230,10 @@ void InterruptManagement::locate_apic_data() entry_index++; } } + void InterruptManagement::locate_pci_interrupt_overrides() { 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) - , m_source(source) - , m_global_system_interrupt(global_system_interrupt) - , m_flags(flags) -{ -} - -u8 ISAInterruptOverrideMetadata::bus() const -{ - return m_bus; -} -u8 ISAInterruptOverrideMetadata::source() const -{ - return m_source; -} -u32 ISAInterruptOverrideMetadata::gsi() const -{ - return m_global_system_interrupt; -} -u16 ISAInterruptOverrideMetadata::flags() const -{ - return m_flags; -} } diff --git a/Kernel/Interrupts/InterruptManagement.h b/Kernel/Interrupts/InterruptManagement.h index 3e3d82fd12..71526e66e8 100644 --- a/Kernel/Interrupts/InterruptManagement.h +++ b/Kernel/Interrupts/InterruptManagement.h @@ -40,7 +40,27 @@ namespace Kernel { -class ISAInterruptOverrideMetadata; +class ISAInterruptOverrideMetadata { +public: + ISAInterruptOverrideMetadata(u8 bus, u8 source, u32 global_system_interrupt, u16 flags) + : m_bus(bus) + , m_source(source) + , m_global_system_interrupt(global_system_interrupt) + , m_flags(flags) + { + } + + u8 bus() const { return m_bus; } + u8 source() const { return m_source; } + u32 gsi() const { return m_global_system_interrupt; } + u16 flags() const { return m_flags; } + +private: + const u8 m_bus; + const u8 m_source; + const u32 m_global_system_interrupt; + const u16 m_flags; +}; class InterruptManagement { public: @@ -56,7 +76,7 @@ public: bool smp_enabled() const { return m_smp_enabled; } RefPtr get_responsible_irq_controller(u8 interrupt_vector); - Vector> isa_overrides(); + const Vector& isa_overrides() const { return m_isa_interrupt_overrides; } u8 get_mapped_interrupt_vector(u8 original_irq); u8 get_irq_vector(u8 mapped_interrupt_vector); @@ -71,23 +91,9 @@ private: void locate_pci_interrupt_overrides(); bool m_smp_enabled { false }; FixedArray> m_interrupt_controllers { 1 }; - Vector> m_isa_interrupt_overrides; - Vector> m_pci_interrupt_overrides; + Vector m_isa_interrupt_overrides; + Vector m_pci_interrupt_overrides; PhysicalAddress m_madt; }; -class ISAInterruptOverrideMetadata : public RefCounted { -public: - ISAInterruptOverrideMetadata(u8 bus, u8 source, u32 global_system_interrupt, u16 flags); - u8 bus() const; - u8 source() const; - u32 gsi() const; - u16 flags() const; - -private: - u8 m_bus; - u8 m_source; - u32 m_global_system_interrupt; - u16 m_flags; -}; }