Kernel/Devices: Remove the DeviceManagement singleton

This change has many improvements: - We don't use `LockRefPtr` to hold instances of many base devices as with the DeviceManagement class. Instead, we have a saner pattern of holding them in a `NonnullRefPtr<T> const`, in a small-text footprint class definition in the `Device.cpp` file. - The awkwardness of using `::the()` each time we need to get references to mostly-static objects (like the Event queue) in runtime is now gone in the migration to using the `Device` class. - Acquiring a device feel more obvious because we use now the Device class for this method. The method name is improved as well.
SerenityOS · Oct 4, 2024 · 4f14a2c · 4f14a2c
1 parent 66a4095
commit 4f14a2c
Show file tree

Hide file tree

Showing 97 changed files with 326 additions and 389 deletions.
diff --git a/Documentation/Kernel/DevelopmentGuidelines.md b/Documentation/Kernel/DevelopmentGuidelines.md
@@ -203,7 +203,7 @@ We allocate them in `Kernel/API/MajorNumberAllocation.h`, based on this set of r
 Currently, we have many devices that are either inserted at boot time but also devices that could be inserted
 afterwards.
 
-To make it easier writing device drivers, when constructing an object from a `Device`-derived class, the usual pattern is to use `DeviceManagement` `try_create_device` method.
+To make it easier writing device drivers, when constructing an object from a `Device`-derived class, the usual pattern is to use `Device` `try_create_device` method.
 For example, constructing a `VirtIOGPU3DDevice` is done this way:
 ```c++
 ErrorOr<NonnullLockRefPtr<VirtIOGPU3DDevice>> VirtIOGPU3DDevice::try_create(VirtIOGraphicsAdapter& adapter)
@@ -215,11 +215,11 @@ ErrorOr<NonnullLockRefPtr<VirtIOGPU3DDevice>> VirtIOGPU3DDevice::try_create(Virt
  Memory::Region::Access::ReadWrite,
  AllocationStrategy::AllocateNow));
  auto kernel_context_id = TRY(adapter.create_context());
- return TRY(DeviceManagement::try_create_device<VirtIOGPU3DDevice>(adapter, move(region_result), kernel_context_id));
+ return TRY(Device::try_create_device<VirtIOGPU3DDevice>(adapter, move(region_result), kernel_context_id));
 }
 ```
 
-The reason for using `DeviceManagement` `try_create_device` method is because that method
+The reason for using `Device` `try_create_device` method is because that method
 calls the virtual `Device` `after_inserting()` method which does crucial initialization steps
 to register the device and expose it by the usual userspace interfaces.
 

diff --git a/Kernel/Arch/aarch64/RPi/MiniUART.cpp b/Kernel/Arch/aarch64/RPi/MiniUART.cpp
@@ -58,7 +58,7 @@ constexpr FlatPtr AUX_ENABLES = 0x21'5000;
 
 UNMAP_AFTER_INIT ErrorOr<NonnullLockRefPtr<MiniUART>> MiniUART::create()
 {
- return DeviceManagement::try_create_device<MiniUART>();
+ return Device::try_create_device<MiniUART>();
 }
 
 // FIXME: Consider not hardcoding the minor number and allocate it dynamically.

diff --git a/Kernel/Arch/aarch64/RPi/MiniUART.h b/Kernel/Arch/aarch64/RPi/MiniUART.h
@@ -7,7 +7,7 @@
 #pragma once
 
 #include <Kernel/Devices/CharacterDevice.h>
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Locking/Spinlock.h>
 
 namespace Kernel::RPi {
@@ -17,7 +17,7 @@ struct MiniUARTRegisters;
 // Makes the secondary "mini UART" (UART1) available to the userspace.
 // See bcm2711-peripherals.pdf chapter "2.2. Mini UART".
 class MiniUART final : public CharacterDevice {
- friend class Kernel::DeviceManagement;
+ friend class Kernel::Device;
 
 public:
  static ErrorOr<NonnullLockRefPtr<MiniUART>> create();

diff --git a/Kernel/Arch/init.cpp b/Kernel/Arch/init.cpp
@@ -20,7 +20,7 @@
 #include <Kernel/Bus/VirtIO/Device.h>
 #include <Kernel/Bus/VirtIO/Transport/PCIe/Detect.h>
 #include <Kernel/Devices/Audio/Management.h>
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Devices/FUSEDevice.h>
 #include <Kernel/Devices/GPU/Console/BootDummyConsole.h>
 #include <Kernel/Devices/GPU/Console/BootFramebufferConsole.h>
@@ -278,11 +278,9 @@ extern "C" [[noreturn]] UNMAP_AFTER_INIT NO_SANITIZE_COVERAGE void init([[maybe_
  // Initialize TimeManagement before using randomness!
  TimeManagement::initialize(0);
 
- DeviceManagement::initialize();
  SysFSComponentRegistry::initialize();
- DeviceManagement::the().attach_null_device(*NullDevice::must_initialize());
- DeviceManagement::the().attach_console_device(*ConsoleDevice::must_create());
- DeviceManagement::the().attach_device_control_device(*DeviceControlDevice::must_create());
+
+ Device::initialize_base_devices();
 
  __stack_chk_guard = get_fast_random<uintptr_t>();
 

diff --git a/Kernel/Arch/x86_64/Hypervisor/BochsDisplayConnector.cpp b/Kernel/Arch/x86_64/Hypervisor/BochsDisplayConnector.cpp
@@ -9,7 +9,7 @@
 #include <Kernel/Arch/x86_64/IO.h>
 #include <Kernel/Bus/PCI/Access.h>
 #include <Kernel/Debug.h>
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Devices/GPU/Bochs/Definitions.h>
 #include <Kernel/Devices/GPU/Console/ContiguousFramebufferConsole.h>
 #include <Kernel/Devices/GPU/Management.h>
@@ -47,7 +47,7 @@ LockRefPtr<BochsDisplayConnector> BochsDisplayConnector::try_create_for_vga_isa_
  // Since this is probably hardcoded at other OSes in their guest drivers,
  // we can assume this is going to stay the same framebuffer physical address for
  // this device and will not be changed in the future.
- auto device_or_error = DeviceManagement::try_create_device<BochsDisplayConnector>(PhysicalAddress(0xE0000000), video_ram_64k_chunks_count * (64 * KiB));
+ auto device_or_error = Device::try_create_device<BochsDisplayConnector>(PhysicalAddress(0xE0000000), video_ram_64k_chunks_count * (64 * KiB));
  VERIFY(!device_or_error.is_error());
  auto connector = device_or_error.release_value();
  MUST(connector->create_attached_framebuffer_console());
@@ -57,7 +57,7 @@ LockRefPtr<BochsDisplayConnector> BochsDisplayConnector::try_create_for_vga_isa_
 
 NonnullLockRefPtr<BochsDisplayConnector> BochsDisplayConnector::must_create(PhysicalAddress framebuffer_address, size_t framebuffer_resource_size, bool virtual_box_hardware)
 {
- auto device_or_error = DeviceManagement::try_create_device<BochsDisplayConnector>(framebuffer_address, framebuffer_resource_size);
+ auto device_or_error = Device::try_create_device<BochsDisplayConnector>(framebuffer_address, framebuffer_resource_size);
  VERIFY(!device_or_error.is_error());
  auto connector = device_or_error.release_value();
  MUST(connector->create_attached_framebuffer_console());

diff --git a/Kernel/Arch/x86_64/Hypervisor/BochsDisplayConnector.h b/Kernel/Arch/x86_64/Hypervisor/BochsDisplayConnector.h
@@ -19,7 +19,7 @@ namespace Kernel {
 class BochsDisplayConnector
  : public DisplayConnector {
  friend class BochsGraphicsAdapter;
- friend class DeviceManagement;
+ friend class Device;
  friend class GraphicsManagement;
 
 public:

diff --git a/Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.cpp b/Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.cpp
@@ -6,7 +6,7 @@
 
 #include <Kernel/Arch/x86_64/Hypervisor/VMWareBackdoor.h>
 #include <Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.h>
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Sections.h>
 
 namespace Kernel {

diff --git a/Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.h b/Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.h
@@ -16,7 +16,7 @@ namespace Kernel {
 
 class VMWareMouseDevice final : public PS2MouseDevice {
 public:
- friend class DeviceManagement;
+ friend class Device;
  static ErrorOr<NonnullOwnPtr<VMWareMouseDevice>> try_to_initialize(SerialIOController const&, SerialIOController::PortIndex, MouseDevice const&);
  virtual ~VMWareMouseDevice() override;
 

diff --git a/Kernel/Arch/x86_64/ISABus/SerialDevice.cpp b/Kernel/Arch/x86_64/ISABus/SerialDevice.cpp
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Devices/SerialDevice.h>
 #include <Kernel/Library/IOWindow.h>
 #include <Kernel/Sections.h>
@@ -24,22 +24,22 @@ UNMAP_AFTER_INIT NonnullLockRefPtr<SerialDevice> SerialDevice::must_create(size_
  switch (com_number) {
  case 0: {
  auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM1_ADDR), 16).release_value_but_fixme_should_propagate_errors();
- serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 0).release_value();
+ serial_device = Device::try_create_device<SerialDevice>(move(io_window), 0).release_value();
  break;
  }
  case 1: {
  auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM2_ADDR), 16).release_value_but_fixme_should_propagate_errors();
- serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 1).release_value();
+ serial_device = Device::try_create_device<SerialDevice>(move(io_window), 1).release_value();
  break;
  }
  case 2: {
  auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM3_ADDR), 16).release_value_but_fixme_should_propagate_errors();
- serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 2).release_value();
+ serial_device = Device::try_create_device<SerialDevice>(move(io_window), 2).release_value();
  break;
  }
  case 3: {
  auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM4_ADDR), 16).release_value_but_fixme_should_propagate_errors();
- serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 3).release_value();
+ serial_device = Device::try_create_device<SerialDevice>(move(io_window), 3).release_value();
  break;
  }
  default:

diff --git a/Kernel/Bus/USB/Drivers/HID/MouseDriver.cpp b/Kernel/Bus/USB/Drivers/HID/MouseDriver.cpp
@@ -10,7 +10,7 @@
 #include <Kernel/Bus/USB/USBClasses.h>
 #include <Kernel/Bus/USB/USBEndpoint.h>
 #include <Kernel/Bus/USB/USBRequest.h>
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Devices/HID/Management.h>
 
 namespace Kernel::USB {

diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt
@@ -65,7 +65,6 @@ set(KERNEL_SOURCES
  Devices/BlockDevice.cpp
  Devices/CharacterDevice.cpp
  Devices/Device.cpp
- Devices/DeviceManagement.cpp
  Devices/FUSEDevice.cpp
  Devices/PCISerialDevice.cpp
  Devices/SerialDevice.cpp

diff --git a/Kernel/Devices/Audio/AC97/AC97.cpp b/Kernel/Devices/Audio/AC97/AC97.cpp
@@ -7,7 +7,7 @@
 #include <AK/Format.h>
 #include <Kernel/Arch/Delay.h>
 #include <Kernel/Devices/Audio/AC97/AC97.h>
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Interrupts/InterruptDisabler.h>
 #include <Kernel/Memory/AnonymousVMObject.h>
 

diff --git a/Kernel/Devices/Audio/Channel.cpp b/Kernel/Devices/Audio/Channel.cpp
@@ -7,7 +7,7 @@
 #include <Kernel/API/Ioctl.h>
 #include <Kernel/API/MajorNumberAllocation.h>
 #include <Kernel/Devices/Audio/Management.h>
-#include <Kernel/Devices/DeviceManagement.h>
+#include <Kernel/Devices/Device.h>
 #include <Kernel/Devices/Generic/RandomDevice.h>
 #include <Kernel/Sections.h>
 #include <Kernel/Security/Random.h>
@@ -16,7 +16,7 @@ namespace Kernel {
 
 UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AudioChannel>> AudioChannel::create(AudioController const& controller, size_t channel_index)
 {
- auto channel = TRY(DeviceManagement::try_create_device<AudioChannel>(controller, channel_index));
+ auto channel = TRY(Device::try_create_device<AudioChannel>(controller, channel_index));
 
  // FIXME: Ideally, we would want the audio controller to run a channel at a device's initial sample
  // rate instead of hardcoding 44.1 KHz here. However, most audio is provided at 44.1 KHz and as

diff --git a/Kernel/Devices/Audio/Channel.h b/Kernel/Devices/Audio/Channel.h
@@ -13,7 +13,7 @@ namespace Kernel {
 class AudioController;
 class AudioChannel final
  : public CharacterDevice {
- friend class DeviceManagement;
+ friend class Device;
 
 public:
  static ErrorOr<NonnullRefPtr<AudioChannel>> create(AudioController const&, size_t channel_index);

diff --git a/Kernel/Devices/BaseDevices.h b/Kernel/Devices/BaseDevices.h
@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2024, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/NonnullRefPtr.h>
+#include <AK/Types.h>
+#include <Kernel/Devices/Generic/ConsoleDevice.h>
+#include <Kernel/Devices/Generic/DeviceControlDevice.h>
+#include <Kernel/Devices/Generic/NullDevice.h>
+
+namespace Kernel {
+
+struct BaseDevices {
+ NonnullRefPtr<NullDevice> const null_device;
+ NonnullRefPtr<ConsoleDevice> const console_device;
+ NonnullRefPtr<DeviceControlDevice> const device_control_device;
+};
+
+}
diff --git a/Kernel/Devices/Device.cpp b/Kernel/Devices/Device.cpp
@@ -5,8 +5,10 @@
  */
 
 #include <AK/Singleton.h>
+#include <Kernel/Devices/BaseDevices.h>
+#include <Kernel/Devices/BlockDevice.h>
+#include <Kernel/Devices/CharacterDevice.h>
 #include <Kernel/Devices/Device.h>
-#include <Kernel/Devices/DeviceManagement.h>
 #include <Kernel/FileSystem/InodeMetadata.h>
 #include <Kernel/FileSystem/SysFS/Component.h>
 #include <Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/BlockDevicesDirectory.h>
@@ -15,21 +17,70 @@
 
 namespace Kernel {
 
-Device::Device(MajorNumber major, MinorNumber minor)
- : m_major(major)
- , m_minor(minor)
+struct AllDevicesDetails {
+ SpinlockProtected<HashMap<u64, BlockDevice*>, LockRank::None> block_devices {};
+ SpinlockProtected<HashMap<u64, CharacterDevice*>, LockRank::None> char_devices {};
+ SpinlockProtected<CircularQueue<DeviceEvent, 100>, LockRank::None> event_queue {};
+ // NOTE: There's no locking on this pointer because we expect to initialize it once
+ // and never touch it again.
+ OwnPtr<BaseDevices> base_devices;
+};
+
+static Singleton<AllDevicesDetails> s_all_details;
+
+SpinlockProtected<CircularQueue<DeviceEvent, 100>, LockRank::None>& Device::event_queue()
 {
+ return s_all_details->event_queue;
+}
+
+BaseDevices* Device::base_devices()
+{
+ return s_all_details->base_devices.ptr();
+}
+
+UNMAP_AFTER_INIT void Device::initialize_base_devices()
+{
+ auto base_devices = MUST(adopt_nonnull_own_or_enomem(new (nothrow) BaseDevices(*NullDevice::must_initialize(), *ConsoleDevice::must_create(), *DeviceControlDevice::must_create())));
+ s_all_details->base_devices = move(base_devices);
+}
+
+RefPtr<Device> Device::acquire_by_type_and_major_minor_numbers(DeviceNodeType type, MajorNumber major, MinorNumber minor)
+{
+ VERIFY(type == DeviceNodeType::Block || type == DeviceNodeType::Character);
+
+ auto find_device_in_map = [major, minor](auto& map) -> RefPtr<Device> {
+ auto it = map.find(encoded_device(major.value(), minor.value()));
+ if (it == map.end())
+ return nullptr;
+ return *it->value;
+ };
+
+ if (type == DeviceNodeType::Block) {
+ return s_all_details->block_devices.with([&](auto& map) -> RefPtr<Device> {
+ return find_device_in_map(map);
+ });
+ }
+
+ return s_all_details->char_devices.with([&](auto& map) -> RefPtr<Device> {
+ return find_device_in_map(map);
+ });
 }
 
 void Device::before_will_be_destroyed_remove_from_device_management()
 {
- DeviceManagement::the().before_device_removal({}, *this);
+ before_device_removal({}, *this);
  m_state = State::BeingRemoved;
 }
 
 void Device::after_inserting_add_to_device_management()
 {
- DeviceManagement::the().after_inserting_device({}, *this);
+ after_inserting_device({}, *this);
+}
+
+Device::Device(MajorNumber major, MinorNumber minor)
+ : m_major(major)
+ , m_minor(minor)
+{
 }
 
 ErrorOr<void> Device::after_inserting()
@@ -80,4 +131,52 @@ void Device::process_next_queued_request(Badge<AsyncDeviceRequest>, AsyncDeviceR
  evaluate_block_conditions();
 }
 
+void Device::after_inserting_device(Badge<Device>, Device& device)
+{
+ if (device.is_block_device()) {
+ s_all_details->block_devices.with([&](auto& map) -> void {
+ add_device_to_map<BlockDevice>(map, device);
+ });
+ } else {
+ VERIFY(device.is_character_device());
+ s_all_details->char_devices.with([&](auto& map) -> void {
+ add_device_to_map<CharacterDevice>(map, device);
+ });
+ }
+
+ s_all_details->event_queue.with([&](auto& queue) {
+ DeviceEvent event { DeviceEvent::State::Inserted, device.is_block_device(), device.major().value(), device.minor().value() };
+ queue.enqueue(event);
+ });
+
+ if (s_all_details->base_devices)
+ s_all_details->base_devices->device_control_device->evaluate_block_conditions();
+}
+
+void Device::before_device_removal(Badge<Device>, Device& device)
+{
+ u64 device_id = encoded_device(device.major(), device.minor());
+
+ if (device.is_block_device()) {
+ s_all_details->block_devices.with([&](auto& map) -> void {
+ VERIFY(map.contains(device_id));
+ map.remove(encoded_device(device.major(), device.minor()));
+ });
+ } else {
+ VERIFY(device.is_character_device());
+ s_all_details->char_devices.with([&](auto& map) -> void {
+ VERIFY(map.contains(device_id));
+ map.remove(encoded_device(device.major(), device.minor()));
+ });
+ }
+
+ s_all_details->event_queue.with([&](auto& queue) {
+ DeviceEvent event { DeviceEvent::State::Removed, device.is_block_device(), device.major().value(), device.minor().value() };
+ queue.enqueue(event);
+ });
+
+ if (s_all_details->base_devices)
+ s_all_details->base_devices->device_control_device->evaluate_block_conditions();
+}
+
 }