summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLiav A <liavalb@gmail.com>2021-08-21 06:55:25 +0300
committerAndreas Kling <kling@serenityos.org>2021-08-23 01:07:45 +0200
commit7b9c3439ec1d4d0c2122897fdfeb72fe52814d7a (patch)
tree96c1bf1a4ef695ba2f5d4308001cd372daf4dc84
parentd071ce352c3fd9a6f21dc9c6e17626269ec15bc4 (diff)
downloadserenity-7b9c3439ec1d4d0c2122897fdfeb72fe52814d7a.zip
Kernel/PCI: Delete PCI::Device in its current form
I created this class a long time ago just to be able to quickly make a PCI device to also represent an interrupt handler (because PCI devices have this capability for most devices). Then after a while I introduced the PCI::DeviceController, which is really almost the same thing (a PCI device class that has Address member in it), but is not tied to interrupts so it can have no interrupts, or spawn interrupt handlers however it wants to seems fit. However I decided it's time to say goodbye for this class for a couple of reasons: 1. It made a whole bunch of weird patterns where you had a PCI::Device and a PCI::DeviceController being used in the topic of implementation, where originally, they meant to be used mutually exclusively (you can't and really don't want to use both). 2. We can really make all the classes that inherit from PCI::Device to inherit from IRQHandler at this point. Later on, when we have MSI interrupts support, we can go further and untie things even more. 3. It makes it possible to simplify the VirtIO implementation to a great extent. While this commit almost doesn't change it, future changes can untangle some complexity in the VirtIO code. For UHCIController, E1000NetworkAdapter, NE2000NetworkAdapter, RTL8139NetworkAdapter, RTL8168NetworkAdapter, E1000ENetworkAdapter we are simply making them to inherit the IRQHandler. This makes some sense, because the first 3 devices will never support anything besides IRQs. For the last 2, they might have MSI support, so when we start to utilize those, we might need to untie these classes from IRQHandler and spawn IRQHandler(s) or MSIHandler(s) as needed. The VirtIODevice class is also a case where we currently need to use both PCI::DeviceController and IRQHandler classes as parents, but it could also be untied from the latter.
-rw-r--r--Kernel/Bus/PCI/Definitions.h1
-rw-r--r--Kernel/Bus/PCI/Device.cpp32
-rw-r--r--Kernel/Bus/PCI/Device.h26
-rw-r--r--Kernel/Bus/USB/UHCI/UHCIController.cpp3
-rw-r--r--Kernel/Bus/USB/UHCI/UHCIController.h7
-rw-r--r--Kernel/Bus/VirtIO/VirtIO.cpp3
-rw-r--r--Kernel/Bus/VirtIO/VirtIO.h7
-rw-r--r--Kernel/CMakeLists.txt1
-rw-r--r--Kernel/Devices/PCISerialDevice.h2
-rw-r--r--Kernel/Net/E1000ENetworkAdapter.h2
-rw-r--r--Kernel/Net/E1000NetworkAdapter.cpp3
-rw-r--r--Kernel/Net/E1000NetworkAdapter.h5
-rw-r--r--Kernel/Net/NE2000NetworkAdapter.cpp3
-rw-r--r--Kernel/Net/NE2000NetworkAdapter.h6
-rw-r--r--Kernel/Net/RTL8139NetworkAdapter.cpp3
-rw-r--r--Kernel/Net/RTL8139NetworkAdapter.h6
-rw-r--r--Kernel/Net/RTL8168NetworkAdapter.cpp3
-rw-r--r--Kernel/Net/RTL8168NetworkAdapter.h6
18 files changed, 37 insertions, 82 deletions
diff --git a/Kernel/Bus/PCI/Definitions.h b/Kernel/Bus/PCI/Definitions.h
index ce12e56687..f3dd65507d 100644
--- a/Kernel/Bus/PCI/Definitions.h
+++ b/Kernel/Bus/PCI/Definitions.h
@@ -256,7 +256,6 @@ class WindowedMMIOAccess;
class IOAccess;
class MMIOSegment;
class DeviceController;
-class Device;
}
diff --git a/Kernel/Bus/PCI/Device.cpp b/Kernel/Bus/PCI/Device.cpp
deleted file mode 100644
index ecb30d9c96..0000000000
--- a/Kernel/Bus/PCI/Device.cpp
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * Copyright (c) 2020, Liav A. <liavalb@hotmail.co.il>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <Kernel/Bus/PCI/Device.h>
-
-namespace Kernel {
-namespace PCI {
-
-Device::Device(Address address)
- : IRQHandler(get_interrupt_line(address))
- , m_pci_address(address)
-{
- // FIXME: Register PCI device somewhere...
-}
-
-Device::Device(Address address, u8 interrupt_vector)
- : IRQHandler(interrupt_vector)
- , m_pci_address(address)
-{
- // FIXME: Register PCI device somewhere...
-}
-
-Device::~Device()
-{
- // FIXME: Unregister the device
-}
-
-}
-}
diff --git a/Kernel/Bus/PCI/Device.h b/Kernel/Bus/PCI/Device.h
deleted file mode 100644
index a2fc714501..0000000000
--- a/Kernel/Bus/PCI/Device.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * Copyright (c) 2020, Liav A. <liavalb@hotmail.co.il>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <AK/Types.h>
-#include <Kernel/Bus/PCI/Definitions.h>
-#include <Kernel/Interrupts/IRQHandler.h>
-
-namespace Kernel {
-class PCI::Device : public IRQHandler {
-public:
- Address pci_address() const { return m_pci_address; };
-
-protected:
- Device(Address pci_address);
- Device(Address pci_address, u8 interrupt_vector);
- ~Device();
-
-private:
- Address m_pci_address;
-};
-}
diff --git a/Kernel/Bus/USB/UHCI/UHCIController.cpp b/Kernel/Bus/USB/UHCI/UHCIController.cpp
index f396770434..5244e1b56f 100644
--- a/Kernel/Bus/USB/UHCI/UHCIController.cpp
+++ b/Kernel/Bus/USB/UHCI/UHCIController.cpp
@@ -92,7 +92,8 @@ KResult UHCIController::initialize()
}
UNMAP_AFTER_INIT UHCIController::UHCIController(PCI::Address address)
- : PCI::Device(address)
+ : PCI::DeviceController(address)
+ , IRQHandler(PCI::get_interrupt_line(address))
, 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 6e9e3559ce..41c66319ae 100644
--- a/Kernel/Bus/USB/UHCI/UHCIController.h
+++ b/Kernel/Bus/USB/UHCI/UHCIController.h
@@ -10,12 +10,13 @@
#include <AK/Platform.h>
#include <AK/NonnullOwnPtr.h>
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/Bus/USB/UHCI/UHCIDescriptorPool.h>
#include <Kernel/Bus/USB/UHCI/UHCIDescriptorTypes.h>
#include <Kernel/Bus/USB/UHCI/UHCIRootHub.h>
#include <Kernel/Bus/USB/USBController.h>
#include <Kernel/IO.h>
+#include <Kernel/Interrupts/IRQHandler.h>
#include <Kernel/Memory/AnonymousVMObject.h>
#include <Kernel/Process.h>
#include <Kernel/Time/TimeManagement.h>
@@ -24,7 +25,8 @@ namespace Kernel::USB {
class UHCIController final
: public USBController
- , public PCI::Device {
+ , public PCI::DeviceController
+ , public IRQHandler {
static constexpr u8 MAXIMUM_NUMBER_OF_TDS = 128; // Upper pool limit. This consumes the second page we have allocated
static constexpr u8 MAXIMUM_NUMBER_OF_QHS = 64;
@@ -109,5 +111,4 @@ private:
// Bitfield containing whether a given port should signal a change in suspend or not.
u8 m_port_suspend_change_statuses { 0 };
};
-
}
diff --git a/Kernel/Bus/VirtIO/VirtIO.cpp b/Kernel/Bus/VirtIO/VirtIO.cpp
index 5743b308b5..86f60dc601 100644
--- a/Kernel/Bus/VirtIO/VirtIO.cpp
+++ b/Kernel/Bus/VirtIO/VirtIO.cpp
@@ -44,7 +44,8 @@ UNMAP_AFTER_INIT void VirtIO::detect()
}
UNMAP_AFTER_INIT VirtIODevice::VirtIODevice(PCI::Address address, String class_name)
- : PCI::Device(address, PCI::get_interrupt_line(address))
+ : PCI::DeviceController(address)
+ , IRQHandler(PCI::get_interrupt_line(address))
, m_class_name(move(class_name))
, m_io_base(IOAddress(PCI::get_BAR0(pci_address()) & ~1))
{
diff --git a/Kernel/Bus/VirtIO/VirtIO.h b/Kernel/Bus/VirtIO/VirtIO.h
index acc02dadd3..e79b6dde81 100644
--- a/Kernel/Bus/VirtIO/VirtIO.h
+++ b/Kernel/Bus/VirtIO/VirtIO.h
@@ -8,7 +8,7 @@
#include <AK/NonnullOwnPtrVector.h>
#include <Kernel/Bus/PCI/Access.h>
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/Bus/VirtIO/VirtIOQueue.h>
#include <Kernel/IO.h>
#include <Kernel/Interrupts/IRQHandler.h>
@@ -84,7 +84,9 @@ public:
static void detect();
};
-class VirtIODevice : public PCI::Device {
+class VirtIODevice
+ : public PCI::DeviceController
+ , public IRQHandler {
public:
virtual ~VirtIODevice() override;
@@ -240,5 +242,4 @@ private:
bool m_did_setup_queues { false };
u32 m_notify_multiplier { 0 };
};
-
}
diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt
index 3c80c972ae..ef29a0c76b 100644
--- a/Kernel/CMakeLists.txt
+++ b/Kernel/CMakeLists.txt
@@ -25,7 +25,6 @@ set(KERNEL_SOURCES
Arch/PC/BIOS.cpp
Arch/x86/SmapDisabler.h
Bus/PCI/Access.cpp
- Bus/PCI/Device.cpp
Bus/PCI/DeviceController.cpp
Bus/PCI/IOAccess.cpp
Bus/PCI/MMIOAccess.cpp
diff --git a/Kernel/Devices/PCISerialDevice.h b/Kernel/Devices/PCISerialDevice.h
index 34beb5942b..88dbc74b3f 100644
--- a/Kernel/Devices/PCISerialDevice.h
+++ b/Kernel/Devices/PCISerialDevice.h
@@ -6,7 +6,7 @@
#pragma once
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/Bus/PCI/IDs.h>
#include <Kernel/Devices/CharacterDevice.h>
#include <Kernel/Devices/SerialDevice.h>
diff --git a/Kernel/Net/E1000ENetworkAdapter.h b/Kernel/Net/E1000ENetworkAdapter.h
index 7c0b017ba0..d00dc0b300 100644
--- a/Kernel/Net/E1000ENetworkAdapter.h
+++ b/Kernel/Net/E1000ENetworkAdapter.h
@@ -9,7 +9,7 @@
#include <AK/NonnullOwnPtrVector.h>
#include <AK/OwnPtr.h>
#include <Kernel/Bus/PCI/Access.h>
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/IO.h>
#include <Kernel/Interrupts/IRQHandler.h>
#include <Kernel/Net/E1000NetworkAdapter.h>
diff --git a/Kernel/Net/E1000NetworkAdapter.cpp b/Kernel/Net/E1000NetworkAdapter.cpp
index 32289fc7ed..597835f1fe 100644
--- a/Kernel/Net/E1000NetworkAdapter.cpp
+++ b/Kernel/Net/E1000NetworkAdapter.cpp
@@ -220,7 +220,8 @@ UNMAP_AFTER_INIT bool E1000NetworkAdapter::initialize()
}
UNMAP_AFTER_INIT E1000NetworkAdapter::E1000NetworkAdapter(PCI::Address address, u8 irq)
- : PCI::Device(address, irq)
+ : PCI::DeviceController(address)
+ , IRQHandler(irq)
, m_rx_descriptors_region(MM.allocate_contiguous_kernel_region(Memory::page_round_up(sizeof(e1000_rx_desc) * number_of_rx_descriptors + 16), "E1000 RX Descriptors", Memory::Region::Access::ReadWrite))
, m_tx_descriptors_region(MM.allocate_contiguous_kernel_region(Memory::page_round_up(sizeof(e1000_tx_desc) * number_of_tx_descriptors + 16), "E1000 TX Descriptors", Memory::Region::Access::ReadWrite))
{
diff --git a/Kernel/Net/E1000NetworkAdapter.h b/Kernel/Net/E1000NetworkAdapter.h
index dad0f45ebf..2037e2a293 100644
--- a/Kernel/Net/E1000NetworkAdapter.h
+++ b/Kernel/Net/E1000NetworkAdapter.h
@@ -8,7 +8,7 @@
#include <AK/OwnPtr.h>
#include <Kernel/Bus/PCI/Access.h>
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/IO.h>
#include <Kernel/Interrupts/IRQHandler.h>
#include <Kernel/Net/NetworkAdapter.h>
@@ -17,7 +17,8 @@
namespace Kernel {
class E1000NetworkAdapter : public NetworkAdapter
- , public PCI::Device {
+ , public PCI::DeviceController
+ , public IRQHandler {
public:
static RefPtr<E1000NetworkAdapter> try_to_initialize(PCI::Address);
diff --git a/Kernel/Net/NE2000NetworkAdapter.cpp b/Kernel/Net/NE2000NetworkAdapter.cpp
index 7346a88f19..c942103fb5 100644
--- a/Kernel/Net/NE2000NetworkAdapter.cpp
+++ b/Kernel/Net/NE2000NetworkAdapter.cpp
@@ -161,7 +161,8 @@ UNMAP_AFTER_INIT RefPtr<NE2000NetworkAdapter> NE2000NetworkAdapter::try_to_initi
}
UNMAP_AFTER_INIT NE2000NetworkAdapter::NE2000NetworkAdapter(PCI::Address address, u8 irq)
- : PCI::Device(address, irq)
+ : PCI::DeviceController(address)
+ , IRQHandler(irq)
, m_io_base(PCI::get_BAR0(pci_address()) & ~3)
{
set_interface_name(address);
diff --git a/Kernel/Net/NE2000NetworkAdapter.h b/Kernel/Net/NE2000NetworkAdapter.h
index 01f7f53b09..955e29b738 100644
--- a/Kernel/Net/NE2000NetworkAdapter.h
+++ b/Kernel/Net/NE2000NetworkAdapter.h
@@ -8,15 +8,17 @@
#include <AK/OwnPtr.h>
#include <Kernel/Bus/PCI/Access.h>
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/IO.h>
+#include <Kernel/Interrupts/IRQHandler.h>
#include <Kernel/Net/NetworkAdapter.h>
#include <Kernel/Random.h>
namespace Kernel {
class NE2000NetworkAdapter final : public NetworkAdapter
- , public PCI::Device {
+ , public PCI::DeviceController
+ , public IRQHandler {
public:
static RefPtr<NE2000NetworkAdapter> try_to_initialize(PCI::Address);
diff --git a/Kernel/Net/RTL8139NetworkAdapter.cpp b/Kernel/Net/RTL8139NetworkAdapter.cpp
index 59cd92701c..22596f5928 100644
--- a/Kernel/Net/RTL8139NetworkAdapter.cpp
+++ b/Kernel/Net/RTL8139NetworkAdapter.cpp
@@ -122,7 +122,8 @@ UNMAP_AFTER_INIT RefPtr<RTL8139NetworkAdapter> RTL8139NetworkAdapter::try_to_ini
}
UNMAP_AFTER_INIT RTL8139NetworkAdapter::RTL8139NetworkAdapter(PCI::Address address, u8 irq)
- : PCI::Device(address, irq)
+ : PCI::DeviceController(address)
+ , IRQHandler(irq)
, m_io_base(PCI::get_BAR0(pci_address()) & ~1)
, m_rx_buffer(MM.allocate_contiguous_kernel_region(Memory::page_round_up(RX_BUFFER_SIZE + PACKET_SIZE_MAX), "RTL8139 RX", Memory::Region::Access::ReadWrite))
, m_packet_buffer(MM.allocate_contiguous_kernel_region(Memory::page_round_up(PACKET_SIZE_MAX), "RTL8139 Packet buffer", Memory::Region::Access::ReadWrite))
diff --git a/Kernel/Net/RTL8139NetworkAdapter.h b/Kernel/Net/RTL8139NetworkAdapter.h
index 988402af84..f04c76dfe5 100644
--- a/Kernel/Net/RTL8139NetworkAdapter.h
+++ b/Kernel/Net/RTL8139NetworkAdapter.h
@@ -8,8 +8,9 @@
#include <AK/OwnPtr.h>
#include <Kernel/Bus/PCI/Access.h>
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/IO.h>
+#include <Kernel/Interrupts/IRQHandler.h>
#include <Kernel/Net/NetworkAdapter.h>
#include <Kernel/Random.h>
@@ -18,7 +19,8 @@ namespace Kernel {
#define RTL8139_TX_BUFFER_COUNT 4
class RTL8139NetworkAdapter final : public NetworkAdapter
- , public PCI::Device {
+ , public PCI::DeviceController
+ , public IRQHandler {
public:
static RefPtr<RTL8139NetworkAdapter> try_to_initialize(PCI::Address);
diff --git a/Kernel/Net/RTL8168NetworkAdapter.cpp b/Kernel/Net/RTL8168NetworkAdapter.cpp
index 52c225efac..be6edc0538 100644
--- a/Kernel/Net/RTL8168NetworkAdapter.cpp
+++ b/Kernel/Net/RTL8168NetworkAdapter.cpp
@@ -192,7 +192,8 @@ UNMAP_AFTER_INIT RefPtr<RTL8168NetworkAdapter> RTL8168NetworkAdapter::try_to_ini
}
UNMAP_AFTER_INIT RTL8168NetworkAdapter::RTL8168NetworkAdapter(PCI::Address address, u8 irq)
- : PCI::Device(address, irq)
+ : PCI::DeviceController(address)
+ , IRQHandler(irq)
, m_io_base(PCI::get_BAR0(pci_address()) & ~1)
, m_rx_descriptors_region(MM.allocate_contiguous_kernel_region(Memory::page_round_up(sizeof(TXDescriptor) * (number_of_rx_descriptors + 1)), "RTL8168 RX", Memory::Region::Access::ReadWrite))
, m_tx_descriptors_region(MM.allocate_contiguous_kernel_region(Memory::page_round_up(sizeof(RXDescriptor) * (number_of_tx_descriptors + 1)), "RTL8168 TX", Memory::Region::Access::ReadWrite))
diff --git a/Kernel/Net/RTL8168NetworkAdapter.h b/Kernel/Net/RTL8168NetworkAdapter.h
index cc4d8497dd..01cb285a2c 100644
--- a/Kernel/Net/RTL8168NetworkAdapter.h
+++ b/Kernel/Net/RTL8168NetworkAdapter.h
@@ -9,8 +9,9 @@
#include <AK/NonnullOwnPtrVector.h>
#include <AK/OwnPtr.h>
#include <Kernel/Bus/PCI/Access.h>
-#include <Kernel/Bus/PCI/Device.h>
+#include <Kernel/Bus/PCI/DeviceController.h>
#include <Kernel/IO.h>
+#include <Kernel/Interrupts/IRQHandler.h>
#include <Kernel/Net/NetworkAdapter.h>
#include <Kernel/Random.h>
@@ -18,7 +19,8 @@ namespace Kernel {
// RTL8618 / RTL8111 Driver based on https://people.freebsd.org/~wpaul/RealTek/RTL8111B_8168B_Registers_DataSheet_1.0.pdf
class RTL8168NetworkAdapter final : public NetworkAdapter
- , public PCI::Device {
+ , public PCI::DeviceController
+ , public IRQHandler {
public:
static RefPtr<RTL8168NetworkAdapter> try_to_initialize(PCI::Address);