From a411a44fda3ba713e139331d4c29ea32f70c99d8 Mon Sep 17 00:00:00 2001 From: Liav A Date: Thu, 23 Sep 2021 10:50:45 +0300 Subject: [PATCH] Kernel/PCI: Cache interrupt line and interrupt pin of a device This allows us to remove the PCI::get_interrupt_line API function. As a result, this removes a bunch of not so great patterns that we used to cache PCI interrupt line in many IRQHandler derived classes instead of just using interrupt_number method of IRQHandler class. --- Kernel/Bus/PCI/API.cpp | 5 ----- Kernel/Bus/PCI/API.h | 1 - Kernel/Bus/PCI/Access.cpp | 4 +++- Kernel/Bus/PCI/Definitions.h | 13 ++++++++++++- Kernel/Bus/USB/UHCI/UHCIController.cpp | 10 +++++----- Kernel/Bus/USB/UHCI/UHCIController.h | 2 +- Kernel/Bus/VirtIO/Device.cpp | 2 +- Kernel/Net/E1000ENetworkAdapter.cpp | 5 ++--- Kernel/Net/E1000NetworkAdapter.cpp | 5 ++--- Kernel/Net/E1000NetworkAdapter.h | 1 - Kernel/Net/NE2000NetworkAdapter.cpp | 5 ++--- Kernel/Net/RTL8139NetworkAdapter.cpp | 5 ++--- Kernel/Net/RTL8168NetworkAdapter.cpp | 2 +- Kernel/Storage/AHCIController.cpp | 12 ++++++------ Kernel/Storage/AHCIController.h | 4 ++-- Kernel/Storage/IDEController.cpp | 5 +++-- Kernel/Storage/IDEController.h | 1 + 17 files changed, 43 insertions(+), 39 deletions(-) diff --git a/Kernel/Bus/PCI/API.cpp b/Kernel/Bus/PCI/API.cpp index 11e8ca2fbb..a1cfe92146 100644 --- a/Kernel/Bus/PCI/API.cpp +++ b/Kernel/Bus/PCI/API.cpp @@ -64,11 +64,6 @@ void disable_interrupt_line(Address address) write16(address, PCI_COMMAND, read16(address, PCI_COMMAND) | 1 << 10); } -u8 get_interrupt_line(Address address) -{ - return read8(address, PCI_INTERRUPT_LINE); -} - u32 get_BAR0(Address address) { return read32(address, PCI_BAR0); diff --git a/Kernel/Bus/PCI/API.h b/Kernel/Bus/PCI/API.h index 38798c7b23..d65d7ea8b9 100644 --- a/Kernel/Bus/PCI/API.h +++ b/Kernel/Bus/PCI/API.h @@ -22,7 +22,6 @@ bool is_io_space_enabled(Address); void enumerate(Function callback); void enable_interrupt_line(Address); void disable_interrupt_line(Address); -u8 get_interrupt_line(Address); void raw_access(Address, u32, size_t, u32); u32 get_BAR0(Address); u32 get_BAR1(Address); diff --git a/Kernel/Bus/PCI/Access.cpp b/Kernel/Bus/PCI/Access.cpp index a659028ba9..a37ae9e7f8 100644 --- a/Kernel/Bus/PCI/Access.cpp +++ b/Kernel/Bus/PCI/Access.cpp @@ -388,7 +388,9 @@ UNMAP_AFTER_INIT void Access::enumerate_functions(int type, u8 bus, u8 device, u RevisionID revision_id = read8_field(address, PCI_REVISION_ID); SubsystemID subsystem_id = read16_field(address, PCI_SUBSYSTEM_ID); SubsystemVendorID subsystem_vendor_id = read16_field(address, PCI_SUBSYSTEM_VENDOR_ID); - m_device_identifiers.append(DeviceIdentifier { address, id, revision_id, class_code, subclass_code, prog_if, subsystem_id, subsystem_vendor_id, get_capabilities(address) }); + InterruptLine interrupt_line = read8_field(address, PCI_INTERRUPT_LINE); + InterruptPin interrupt_pin = read8_field(address, PCI_INTERRUPT_PIN); + m_device_identifiers.append(DeviceIdentifier { address, id, revision_id, class_code, subclass_code, prog_if, subsystem_id, subsystem_vendor_id, interrupt_line, interrupt_pin, get_capabilities(address) }); } if (read_type == PCI_TYPE_BRIDGE && recursive && (!m_enumerated_buses.get(read8_field(address, PCI_SECONDARY_BUS)))) { diff --git a/Kernel/Bus/PCI/Definitions.h b/Kernel/Bus/PCI/Definitions.h index b2a9831d89..24bfa19a74 100644 --- a/Kernel/Bus/PCI/Definitions.h +++ b/Kernel/Bus/PCI/Definitions.h @@ -36,6 +36,7 @@ namespace Kernel { #define PCI_SUBSYSTEM_ID 0x2E // u16 #define PCI_CAPABILITIES_POINTER 0x34 // u8 #define PCI_INTERRUPT_LINE 0x3C // byte +#define PCI_INTERRUPT_PIN 0x3D // byte #define PCI_SECONDARY_BUS 0x19 // byte #define PCI_HEADER_TYPE_DEVICE 0 #define PCI_HEADER_TYPE_BRIDGE 1 @@ -187,11 +188,13 @@ TYPEDEF_DISTINCT_ORDERED_ID(u8, ProgrammingInterface); TYPEDEF_DISTINCT_ORDERED_ID(u8, RevisionID); TYPEDEF_DISTINCT_ORDERED_ID(u16, SubsystemID); TYPEDEF_DISTINCT_ORDERED_ID(u16, SubsystemVendorID); +TYPEDEF_DISTINCT_ORDERED_ID(u8, InterruptLine); +TYPEDEF_DISTINCT_ORDERED_ID(u8, InterruptPin); class Access; class DeviceIdentifier { public: - DeviceIdentifier(Address address, HardwareID hardware_id, RevisionID revision_id, ClassCode class_code, SubclassCode subclass_code, ProgrammingInterface prog_if, SubsystemID subsystem_id, SubsystemVendorID subsystem_vendor_id, Vector capabilities) + DeviceIdentifier(Address address, HardwareID hardware_id, RevisionID revision_id, ClassCode class_code, SubclassCode subclass_code, ProgrammingInterface prog_if, SubsystemID subsystem_id, SubsystemVendorID subsystem_vendor_id, InterruptLine interrupt_line, InterruptPin interrupt_pin, Vector capabilities) : m_address(address) , m_hardware_id(hardware_id) , m_revision_id(revision_id) @@ -200,6 +203,8 @@ public: , m_prog_if(prog_if) , m_subsystem_id(subsystem_id) , m_subsystem_vendor_id(subsystem_vendor_id) + , m_interrupt_line(interrupt_line) + , m_interrupt_pin(interrupt_pin) , m_capabilities(capabilities) { if constexpr (PCI_DEBUG) { @@ -219,6 +224,9 @@ public: SubsystemID subsystem_id() const { return m_subsystem_id; } SubsystemVendorID subsystem_vendor_id() const { return m_subsystem_vendor_id; } + InterruptLine interrupt_line() const { return m_interrupt_line; } + InterruptPin interrupt_pin() const { return m_interrupt_pin; } + void apply_subclass_code_change(Badge, SubclassCode new_subclass) { m_subclass_code = new_subclass; @@ -239,6 +247,9 @@ private: SubsystemID m_subsystem_id; SubsystemVendorID m_subsystem_vendor_id; + InterruptLine m_interrupt_line; + InterruptPin m_interrupt_pin; + Vector m_capabilities; }; diff --git a/Kernel/Bus/USB/UHCI/UHCIController.cpp b/Kernel/Bus/USB/UHCI/UHCIController.cpp index 6aec8877ff..b64f4791d0 100644 --- a/Kernel/Bus/USB/UHCI/UHCIController.cpp +++ b/Kernel/Bus/USB/UHCI/UHCIController.cpp @@ -65,7 +65,7 @@ static constexpr u16 UHCI_NUMBER_OF_FRAMES = 1024; KResultOr> UHCIController::try_to_initialize(PCI::DeviceIdentifier const& pci_device_identifier) { // NOTE: This assumes that address is pointing to a valid UHCI controller. - auto controller = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) UHCIController(pci_device_identifier.address()))); + auto controller = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) UHCIController(pci_device_identifier))); TRY(controller->initialize()); return controller; } @@ -74,7 +74,7 @@ KResult UHCIController::initialize() { dmesgln("UHCI: Controller found {} @ {}", PCI::get_hardware_id(pci_address()), pci_address()); dmesgln("UHCI: I/O base {}", m_io_base); - dmesgln("UHCI: Interrupt line: {}", PCI::get_interrupt_line(pci_address())); + dmesgln("UHCI: Interrupt line: {}", interrupt_number()); spawn_port_proc(); @@ -82,9 +82,9 @@ KResult UHCIController::initialize() return start(); } -UNMAP_AFTER_INIT UHCIController::UHCIController(PCI::Address address) - : PCI::Device(address) - , IRQHandler(PCI::get_interrupt_line(address)) +UNMAP_AFTER_INIT UHCIController::UHCIController(PCI::DeviceIdentifier const& pci_device_identifier) + : PCI::Device(pci_device_identifier.address()) + , IRQHandler(pci_device_identifier.interrupt_line().value()) , m_io_base(PCI::get_BAR4(pci_address()) & ~1) { } diff --git a/Kernel/Bus/USB/UHCI/UHCIController.h b/Kernel/Bus/USB/UHCI/UHCIController.h index b3ede96c3c..b018c3025b 100644 --- a/Kernel/Bus/USB/UHCI/UHCIController.h +++ b/Kernel/Bus/USB/UHCI/UHCIController.h @@ -53,7 +53,7 @@ public: KResult clear_port_feature(Badge, u8, HubFeatureSelector); private: - explicit UHCIController(PCI::Address); + explicit UHCIController(PCI::DeviceIdentifier const& pci_device_identifier); u16 read_usbcmd() { return m_io_base.offset(0).in(); } u16 read_usbsts() { return m_io_base.offset(0x2).in(); } diff --git a/Kernel/Bus/VirtIO/Device.cpp b/Kernel/Bus/VirtIO/Device.cpp index d9ad50b8a4..6e869168fa 100644 --- a/Kernel/Bus/VirtIO/Device.cpp +++ b/Kernel/Bus/VirtIO/Device.cpp @@ -152,7 +152,7 @@ UNMAP_AFTER_INIT void Device::initialize() UNMAP_AFTER_INIT VirtIO::Device::Device(PCI::DeviceIdentifier const& device_identifier) : PCI::Device(device_identifier.address()) - , IRQHandler(PCI::get_interrupt_line(device_identifier.address())) + , IRQHandler(device_identifier.interrupt_line().value()) , m_io_base(IOAddress(PCI::get_BAR0(pci_address()) & ~1)) , m_class_name(VirtIO::determine_device_class(device_identifier)) { diff --git a/Kernel/Net/E1000ENetworkAdapter.cpp b/Kernel/Net/E1000ENetworkAdapter.cpp index 1cc610fbca..e33a2d577c 100644 --- a/Kernel/Net/E1000ENetworkAdapter.cpp +++ b/Kernel/Net/E1000ENetworkAdapter.cpp @@ -186,7 +186,7 @@ UNMAP_AFTER_INIT RefPtr E1000ENetworkAdapter::try_to_initi return {}; if (!is_valid_device_id(pci_device_identifier.hardware_id().device_id)) return {}; - u8 irq = PCI::get_interrupt_line(pci_device_identifier.address()); + u8 irq = pci_device_identifier.interrupt_line().value(); auto adapter = adopt_ref_if_nonnull(new (nothrow) E1000ENetworkAdapter(pci_device_identifier.address(), irq)); if (!adapter) return {}; @@ -210,11 +210,10 @@ UNMAP_AFTER_INIT bool E1000ENetworkAdapter::initialize() m_mmio_region = region_or_error.release_value(); m_mmio_base = m_mmio_region->vaddr(); m_use_mmio = true; - m_interrupt_line = PCI::get_interrupt_line(pci_address()); dmesgln("E1000e: port base: {}", m_io_base); dmesgln("E1000e: MMIO base: {}", PhysicalAddress(PCI::get_BAR0(pci_address()) & 0xfffffffc)); dmesgln("E1000e: MMIO base size: {} bytes", mmio_base_size); - dmesgln("E1000e: Interrupt line: {}", m_interrupt_line); + dmesgln("E1000e: Interrupt line: {}", interrupt_number()); detect_eeprom(); dmesgln("E1000e: Has EEPROM? {}", m_has_eeprom); read_mac_address(); diff --git a/Kernel/Net/E1000NetworkAdapter.cpp b/Kernel/Net/E1000NetworkAdapter.cpp index d8f4ac617a..04679da459 100644 --- a/Kernel/Net/E1000NetworkAdapter.cpp +++ b/Kernel/Net/E1000NetworkAdapter.cpp @@ -164,7 +164,7 @@ UNMAP_AFTER_INIT RefPtr E1000NetworkAdapter::try_to_initial return {}; if (!is_valid_device_id(pci_device_identifier.hardware_id().device_id)) return {}; - u8 irq = PCI::get_interrupt_line(pci_device_identifier.address()); + u8 irq = pci_device_identifier.interrupt_line().value(); auto adapter = adopt_ref_if_nonnull(new (nothrow) E1000NetworkAdapter(pci_device_identifier.address(), irq)); if (!adapter) return {}; @@ -201,11 +201,10 @@ UNMAP_AFTER_INIT bool E1000NetworkAdapter::initialize() m_mmio_region = region_or_error.release_value(); m_mmio_base = m_mmio_region->vaddr(); m_use_mmio = true; - m_interrupt_line = PCI::get_interrupt_line(pci_address()); dmesgln("E1000: port base: {}", m_io_base); dmesgln("E1000: MMIO base: {}", PhysicalAddress(PCI::get_BAR0(pci_address()) & 0xfffffffc)); dmesgln("E1000: MMIO base size: {} bytes", mmio_base_size); - dmesgln("E1000: Interrupt line: {}", m_interrupt_line); + dmesgln("E1000: Interrupt line: {}", interrupt_number()); detect_eeprom(); dmesgln("E1000: Has EEPROM? {}", m_has_eeprom); read_mac_address(); diff --git a/Kernel/Net/E1000NetworkAdapter.h b/Kernel/Net/E1000NetworkAdapter.h index 5a1961ca57..49b9263701 100644 --- a/Kernel/Net/E1000NetworkAdapter.h +++ b/Kernel/Net/E1000NetworkAdapter.h @@ -91,7 +91,6 @@ protected: Array m_rx_buffers; Array m_tx_buffers; OwnPtr m_mmio_region; - u8 m_interrupt_line { 0 }; bool m_has_eeprom { false }; bool m_use_mmio { false }; EntropySource m_entropy_source; diff --git a/Kernel/Net/NE2000NetworkAdapter.cpp b/Kernel/Net/NE2000NetworkAdapter.cpp index 1949c71599..bdd03a691c 100644 --- a/Kernel/Net/NE2000NetworkAdapter.cpp +++ b/Kernel/Net/NE2000NetworkAdapter.cpp @@ -156,7 +156,7 @@ UNMAP_AFTER_INIT RefPtr NE2000NetworkAdapter::try_to_initi }; if (!ne2k_ids.span().contains_slow(pci_device_identifier.hardware_id())) return {}; - u8 irq = PCI::get_interrupt_line(pci_device_identifier.address()); + u8 irq = pci_device_identifier.interrupt_line().value(); return adopt_ref_if_nonnull(new (nothrow) NE2000NetworkAdapter(pci_device_identifier.address(), irq)); } @@ -169,9 +169,8 @@ UNMAP_AFTER_INIT NE2000NetworkAdapter::NE2000NetworkAdapter(PCI::Address address dmesgln("NE2000: Found @ {}", pci_address()); - m_interrupt_line = PCI::get_interrupt_line(pci_address()); dmesgln("NE2000: Port base: {}", m_io_base); - dmesgln("NE2000: Interrupt line: {}", m_interrupt_line); + dmesgln("NE2000: Interrupt line: {}", interrupt_number()); int ram_errors = ram_test(); dmesgln("NE2000: RAM test {}, got {} byte errors", (ram_errors == 0 ? "OK" : "KO"), ram_errors); diff --git a/Kernel/Net/RTL8139NetworkAdapter.cpp b/Kernel/Net/RTL8139NetworkAdapter.cpp index fbf0ab3b64..00ec3acdf5 100644 --- a/Kernel/Net/RTL8139NetworkAdapter.cpp +++ b/Kernel/Net/RTL8139NetworkAdapter.cpp @@ -117,7 +117,7 @@ UNMAP_AFTER_INIT RefPtr RTL8139NetworkAdapter::try_to_ini constexpr PCI::HardwareID rtl8139_id = { 0x10EC, 0x8139 }; if (pci_device_identifier.hardware_id() != rtl8139_id) return {}; - u8 irq = PCI::get_interrupt_line(pci_device_identifier.address()); + u8 irq = pci_device_identifier.interrupt_line().value(); return adopt_ref_if_nonnull(new (nothrow) RTL8139NetworkAdapter(pci_device_identifier.address(), irq)); } @@ -135,9 +135,8 @@ UNMAP_AFTER_INIT RTL8139NetworkAdapter::RTL8139NetworkAdapter(PCI::Address addre enable_bus_mastering(pci_address()); - m_interrupt_line = PCI::get_interrupt_line(pci_address()); dmesgln("RTL8139: I/O port base: {}", m_io_base); - dmesgln("RTL8139: Interrupt line: {}", m_interrupt_line); + dmesgln("RTL8139: Interrupt line: {}", interrupt_number()); // we add space to account for overhang from the last packet - the rtl8139 // can optionally guarantee that packets will be contiguous by diff --git a/Kernel/Net/RTL8168NetworkAdapter.cpp b/Kernel/Net/RTL8168NetworkAdapter.cpp index 3b2459b86d..8cb2ae5525 100644 --- a/Kernel/Net/RTL8168NetworkAdapter.cpp +++ b/Kernel/Net/RTL8168NetworkAdapter.cpp @@ -187,7 +187,7 @@ UNMAP_AFTER_INIT RefPtr RTL8168NetworkAdapter::try_to_ini return {}; if (pci_device_identifier.hardware_id().device_id != 0x8168) return {}; - u8 irq = PCI::get_interrupt_line(pci_device_identifier.address()); + u8 irq = pci_device_identifier.interrupt_line().value(); return adopt_ref_if_nonnull(new (nothrow) RTL8168NetworkAdapter(pci_device_identifier.address(), irq)); } diff --git a/Kernel/Storage/AHCIController.cpp b/Kernel/Storage/AHCIController.cpp index 09b9872ca7..4427a3aa16 100644 --- a/Kernel/Storage/AHCIController.cpp +++ b/Kernel/Storage/AHCIController.cpp @@ -17,7 +17,7 @@ namespace Kernel { NonnullRefPtr AHCIController::initialize(PCI::DeviceIdentifier const& pci_device_identifier) { - return adopt_ref(*new AHCIController(pci_device_identifier.address())); + return adopt_ref(*new AHCIController(pci_device_identifier)); } bool AHCIController::reset() @@ -79,13 +79,13 @@ volatile AHCI::HBA& AHCIController::hba() const return static_cast(*(volatile AHCI::HBA*)(m_hba_region->vaddr().as_ptr())); } -AHCIController::AHCIController(PCI::Address address) +AHCIController::AHCIController(PCI::DeviceIdentifier const& pci_device_identifier) : StorageController() - , PCI::Device(address) + , PCI::Device(pci_device_identifier.address()) , m_hba_region(default_hba_region()) , m_capabilities(capabilities()) { - initialize(); + initialize_hba(pci_device_identifier); } AHCI::HBADefinedCapabilities AHCIController::capabilities() const @@ -134,7 +134,7 @@ AHCIController::~AHCIController() { } -void AHCIController::initialize() +void AHCIController::initialize_hba(PCI::DeviceIdentifier const& pci_device_identifier) { if (!reset()) { dmesgln("{}: AHCI controller reset failed", pci_address()); @@ -150,7 +150,7 @@ void AHCIController::initialize() PCI::enable_interrupt_line(pci_address()); PCI::enable_bus_mastering(pci_address()); enable_global_interrupts(); - m_handlers.append(AHCIPortHandler::create(*this, PCI::get_interrupt_line(pci_address()), + m_handlers.append(AHCIPortHandler::create(*this, pci_device_identifier.interrupt_line().value(), AHCI::MaskedBitField((volatile u32&)(hba().control_regs.pi)))); } diff --git a/Kernel/Storage/AHCIController.h b/Kernel/Storage/AHCIController.h index de0d15d28e..3d8b3cdf46 100644 --- a/Kernel/Storage/AHCIController.h +++ b/Kernel/Storage/AHCIController.h @@ -41,8 +41,8 @@ private: void disable_global_interrupts() const; void enable_global_interrupts() const; - UNMAP_AFTER_INIT explicit AHCIController(PCI::Address address); - UNMAP_AFTER_INIT void initialize(); + UNMAP_AFTER_INIT explicit AHCIController(PCI::DeviceIdentifier const&); + UNMAP_AFTER_INIT void initialize_hba(PCI::DeviceIdentifier const&); AHCI::HBADefinedCapabilities capabilities() const; RefPtr device_by_port(u32 index) const; diff --git a/Kernel/Storage/IDEController.cpp b/Kernel/Storage/IDEController.cpp index f28e14461b..49c9188299 100644 --- a/Kernel/Storage/IDEController.cpp +++ b/Kernel/Storage/IDEController.cpp @@ -55,6 +55,7 @@ UNMAP_AFTER_INIT IDEController::IDEController(PCI::DeviceIdentifier const& devic : StorageController() , PCI::Device(device_identifier.address()) , m_prog_if(device_identifier.prog_if()) + , m_interrupt_line(device_identifier.interrupt_line()) { PCI::enable_io_space(device_identifier.address()); PCI::enable_memory_space(device_identifier.address()); @@ -114,7 +115,7 @@ UNMAP_AFTER_INIT void IDEController::initialize(bool force_pio) { auto bus_master_base = IOAddress(PCI::get_BAR4(pci_address()) & (~1)); dbgln("IDE controller @ {}: bus master base was set to {}", pci_address(), bus_master_base); - dbgln("IDE controller @ {}: interrupt line was set to {}", pci_address(), PCI::get_interrupt_line(pci_address())); + dbgln("IDE controller @ {}: interrupt line was set to {}", pci_address(), m_interrupt_line.value()); dbgln("IDE controller @ {}: {}", pci_address(), detect_controller_type(m_prog_if.value())); dbgln("IDE controller @ {}: primary channel DMA capable? {}", pci_address(), ((bus_master_base.offset(2).in() >> 5) & 0b11)); dbgln("IDE controller @ {}: secondary channel DMA capable? {}", pci_address(), ((bus_master_base.offset(2 + 8).in() >> 5) & 0b11)); @@ -131,7 +132,7 @@ UNMAP_AFTER_INIT void IDEController::initialize(bool force_pio) auto bar3 = PCI::get_BAR3(pci_address()); auto secondary_control_io = (bar3 == 0x1 || bar3 == 0) ? IOAddress(0x376) : IOAddress(bar3 & (~1)); - auto irq_line = PCI::get_interrupt_line(pci_address()); + auto irq_line = m_interrupt_line.value(); if (is_pci_native_mode_enabled()) { VERIFY(irq_line != 0); } diff --git a/Kernel/Storage/IDEController.h b/Kernel/Storage/IDEController.h index a73f5e5473..a4ca07201c 100644 --- a/Kernel/Storage/IDEController.h +++ b/Kernel/Storage/IDEController.h @@ -46,5 +46,6 @@ private: NonnullRefPtrVector m_channels; // FIXME: Find a better way to get the ProgrammingInterface PCI::ProgrammingInterface m_prog_if; + PCI::InterruptLine m_interrupt_line; }; }