diff options
author | Liav A <liavalb@gmail.com> | 2021-08-21 06:55:25 +0300 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-08-23 01:07:45 +0200 |
commit | 7b9c3439ec1d4d0c2122897fdfeb72fe52814d7a (patch) | |
tree | 96c1bf1a4ef695ba2f5d4308001cd372daf4dc84 | |
parent | d071ce352c3fd9a6f21dc9c6e17626269ec15bc4 (diff) | |
download | serenity-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.h | 1 | ||||
-rw-r--r-- | Kernel/Bus/PCI/Device.cpp | 32 | ||||
-rw-r--r-- | Kernel/Bus/PCI/Device.h | 26 | ||||
-rw-r--r-- | Kernel/Bus/USB/UHCI/UHCIController.cpp | 3 | ||||
-rw-r--r-- | Kernel/Bus/USB/UHCI/UHCIController.h | 7 | ||||
-rw-r--r-- | Kernel/Bus/VirtIO/VirtIO.cpp | 3 | ||||
-rw-r--r-- | Kernel/Bus/VirtIO/VirtIO.h | 7 | ||||
-rw-r--r-- | Kernel/CMakeLists.txt | 1 | ||||
-rw-r--r-- | Kernel/Devices/PCISerialDevice.h | 2 | ||||
-rw-r--r-- | Kernel/Net/E1000ENetworkAdapter.h | 2 | ||||
-rw-r--r-- | Kernel/Net/E1000NetworkAdapter.cpp | 3 | ||||
-rw-r--r-- | Kernel/Net/E1000NetworkAdapter.h | 5 | ||||
-rw-r--r-- | Kernel/Net/NE2000NetworkAdapter.cpp | 3 | ||||
-rw-r--r-- | Kernel/Net/NE2000NetworkAdapter.h | 6 | ||||
-rw-r--r-- | Kernel/Net/RTL8139NetworkAdapter.cpp | 3 | ||||
-rw-r--r-- | Kernel/Net/RTL8139NetworkAdapter.h | 6 | ||||
-rw-r--r-- | Kernel/Net/RTL8168NetworkAdapter.cpp | 3 | ||||
-rw-r--r-- | Kernel/Net/RTL8168NetworkAdapter.h | 6 |
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); |