Skip to content

Commit

Permalink
Kernel/PCI: Cache interrupt line and interrupt pin of a device
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
supercomputer7 authored and awesomekling committed Sep 29, 2021
1 parent 057f5a1 commit a411a44
Show file tree
Hide file tree
Showing 17 changed files with 43 additions and 39 deletions.
5 changes: 0 additions & 5 deletions Kernel/Bus/PCI/API.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion Kernel/Bus/PCI/API.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ bool is_io_space_enabled(Address);
void enumerate(Function<void(Address, DeviceIdentifier const&)> 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);
Expand Down
4 changes: 3 additions & 1 deletion Kernel/Bus/PCI/Access.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)))) {
Expand Down
13 changes: 12 additions & 1 deletion Kernel/Bus/PCI/Definitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<Capability> 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<Capability> capabilities)
: m_address(address)
, m_hardware_id(hardware_id)
, m_revision_id(revision_id)
Expand All @@ -200,6 +203,8 @@ class DeviceIdentifier {
, 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) {
Expand All @@ -219,6 +224,9 @@ class DeviceIdentifier {
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<Access>, SubclassCode new_subclass)
{
m_subclass_code = new_subclass;
Expand All @@ -239,6 +247,9 @@ class DeviceIdentifier {
SubsystemID m_subsystem_id;
SubsystemVendorID m_subsystem_vendor_id;

InterruptLine m_interrupt_line;
InterruptPin m_interrupt_pin;

Vector<Capability> m_capabilities;
};

Expand Down
10 changes: 5 additions & 5 deletions Kernel/Bus/USB/UHCI/UHCIController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static constexpr u16 UHCI_NUMBER_OF_FRAMES = 1024;
KResultOr<NonnullRefPtr<UHCIController>> 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;
}
Expand All @@ -74,17 +74,17 @@ 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();

TRY(reset());
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)
{
}
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Bus/USB/UHCI/UHCIController.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class UHCIController final
KResult clear_port_feature(Badge<UHCIRootHub>, 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>(); }
u16 read_usbsts() { return m_io_base.offset(0x2).in<u16>(); }
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Bus/VirtIO/Device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
5 changes: 2 additions & 3 deletions Kernel/Net/E1000ENetworkAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ UNMAP_AFTER_INIT RefPtr<E1000ENetworkAdapter> 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 {};
Expand All @@ -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();
Expand Down
5 changes: 2 additions & 3 deletions Kernel/Net/E1000NetworkAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ UNMAP_AFTER_INIT RefPtr<E1000NetworkAdapter> 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 {};
Expand Down Expand Up @@ -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();
Expand Down
1 change: 0 additions & 1 deletion Kernel/Net/E1000NetworkAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ class E1000NetworkAdapter : public NetworkAdapter
Array<void*, number_of_rx_descriptors> m_rx_buffers;
Array<void*, number_of_tx_descriptors> m_tx_buffers;
OwnPtr<Memory::Region> m_mmio_region;
u8 m_interrupt_line { 0 };
bool m_has_eeprom { false };
bool m_use_mmio { false };
EntropySource m_entropy_source;
Expand Down
5 changes: 2 additions & 3 deletions Kernel/Net/NE2000NetworkAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ UNMAP_AFTER_INIT RefPtr<NE2000NetworkAdapter> 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));
}

Expand All @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions Kernel/Net/RTL8139NetworkAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ UNMAP_AFTER_INIT RefPtr<RTL8139NetworkAdapter> 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));
}

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Net/RTL8168NetworkAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ UNMAP_AFTER_INIT RefPtr<RTL8168NetworkAdapter> 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));
}

Expand Down
12 changes: 6 additions & 6 deletions Kernel/Storage/AHCIController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Kernel {

NonnullRefPtr<AHCIController> 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()
Expand Down Expand Up @@ -79,13 +79,13 @@ volatile AHCI::HBA& AHCIController::hba() const
return static_cast<volatile AHCI::HBA&>(*(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
Expand Down Expand Up @@ -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());
Expand All @@ -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))));
}

Expand Down
4 changes: 2 additions & 2 deletions Kernel/Storage/AHCIController.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ class AHCIController final : public StorageController
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<StorageDevice> device_by_port(u32 index) const;
Expand Down
5 changes: 3 additions & 2 deletions Kernel/Storage/IDEController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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<u8>() >> 5) & 0b11));
dbgln("IDE controller @ {}: secondary channel DMA capable? {}", pci_address(), ((bus_master_base.offset(2 + 8).in<u8>() >> 5) & 0b11));
Expand All @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions Kernel/Storage/IDEController.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,6 @@ class IDEController final : public StorageController
NonnullRefPtrVector<IDEChannel> m_channels;
// FIXME: Find a better way to get the ProgrammingInterface
PCI::ProgrammingInterface m_prog_if;
PCI::InterruptLine m_interrupt_line;
};
}

0 comments on commit a411a44

Please sign in to comment.