Skip to content

Commit 96e1391

Browse files
supercomputer7timschumi
authored andcommitted
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.
1 parent b1f470f commit 96e1391

File tree

97 files changed

+327
-389
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

97 files changed

+327
-389
lines changed

Documentation/Kernel/DevelopmentGuidelines.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ We allocate them in `Kernel/API/MajorNumberAllocation.h`, based on this set of r
204204
Currently, we have many devices that are either inserted at boot time but also devices that could be inserted
205205
afterwards.
206206

207-
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.
207+
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.
208208
For example, constructing a `VirtIOGPU3DDevice` is done this way:
209209

210210
```c++
@@ -217,11 +217,11 @@ ErrorOr<NonnullLockRefPtr<VirtIOGPU3DDevice>> VirtIOGPU3DDevice::try_create(Virt
217217
Memory::Region::Access::ReadWrite,
218218
AllocationStrategy::AllocateNow));
219219
auto kernel_context_id = TRY(adapter.create_context());
220-
return TRY(DeviceManagement::try_create_device<VirtIOGPU3DDevice>(adapter, move(region_result), kernel_context_id));
220+
return TRY(Device::try_create_device<VirtIOGPU3DDevice>(adapter, move(region_result), kernel_context_id));
221221
}
222222
```
223223
224-
The reason for using `DeviceManagement` `try_create_device` method is because that method
224+
The reason for using `Device` `try_create_device` method is because that method
225225
calls the virtual `Device` `after_inserting()` method which does crucial initialization steps
226226
to register the device and expose it by the usual userspace interfaces.
227227

Kernel/Arch/aarch64/RPi/MiniUART.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ constexpr FlatPtr AUX_ENABLES = 0x21'5000;
5858

5959
UNMAP_AFTER_INIT ErrorOr<NonnullLockRefPtr<MiniUART>> MiniUART::create()
6060
{
61-
return DeviceManagement::try_create_device<MiniUART>();
61+
return Device::try_create_device<MiniUART>();
6262
}
6363

6464
// FIXME: Consider not hardcoding the minor number and allocate it dynamically.

Kernel/Arch/aarch64/RPi/MiniUART.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#pragma once
88

99
#include <Kernel/Devices/CharacterDevice.h>
10-
#include <Kernel/Devices/DeviceManagement.h>
10+
#include <Kernel/Devices/Device.h>
1111
#include <Kernel/Locking/Spinlock.h>
1212

1313
namespace Kernel::RPi {
@@ -17,7 +17,7 @@ struct MiniUARTRegisters;
1717
// Makes the secondary "mini UART" (UART1) available to the userspace.
1818
// See bcm2711-peripherals.pdf chapter "2.2. Mini UART".
1919
class MiniUART final : public CharacterDevice {
20-
friend class Kernel::DeviceManagement;
20+
friend class Kernel::Device;
2121

2222
public:
2323
static ErrorOr<NonnullLockRefPtr<MiniUART>> create();

Kernel/Arch/init.cpp

+3-5
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#include <Kernel/Bus/VirtIO/Device.h>
2121
#include <Kernel/Bus/VirtIO/Transport/PCIe/Detect.h>
2222
#include <Kernel/Devices/Audio/Management.h>
23-
#include <Kernel/Devices/DeviceManagement.h>
23+
#include <Kernel/Devices/Device.h>
2424
#include <Kernel/Devices/FUSEDevice.h>
2525
#include <Kernel/Devices/GPU/Console/BootDummyConsole.h>
2626
#include <Kernel/Devices/GPU/Console/BootFramebufferConsole.h>
@@ -278,11 +278,9 @@ extern "C" [[noreturn]] UNMAP_AFTER_INIT NO_SANITIZE_COVERAGE void init([[maybe_
278278
// Initialize TimeManagement before using randomness!
279279
TimeManagement::initialize(0);
280280

281-
DeviceManagement::initialize();
282281
SysFSComponentRegistry::initialize();
283-
DeviceManagement::the().attach_null_device(*NullDevice::must_initialize());
284-
DeviceManagement::the().attach_console_device(*ConsoleDevice::must_create());
285-
DeviceManagement::the().attach_device_control_device(*DeviceControlDevice::must_create());
282+
283+
Device::initialize_base_devices();
286284

287285
__stack_chk_guard = get_fast_random<uintptr_t>();
288286

Kernel/Arch/x86_64/Hypervisor/BochsDisplayConnector.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include <Kernel/Arch/x86_64/IO.h>
1010
#include <Kernel/Bus/PCI/Access.h>
1111
#include <Kernel/Debug.h>
12-
#include <Kernel/Devices/DeviceManagement.h>
12+
#include <Kernel/Devices/Device.h>
1313
#include <Kernel/Devices/GPU/Bochs/Definitions.h>
1414
#include <Kernel/Devices/GPU/Console/ContiguousFramebufferConsole.h>
1515
#include <Kernel/Devices/GPU/Management.h>
@@ -47,7 +47,7 @@ LockRefPtr<BochsDisplayConnector> BochsDisplayConnector::try_create_for_vga_isa_
4747
// Since this is probably hardcoded at other OSes in their guest drivers,
4848
// we can assume this is going to stay the same framebuffer physical address for
4949
// this device and will not be changed in the future.
50-
auto device_or_error = DeviceManagement::try_create_device<BochsDisplayConnector>(PhysicalAddress(0xE0000000), video_ram_64k_chunks_count * (64 * KiB));
50+
auto device_or_error = Device::try_create_device<BochsDisplayConnector>(PhysicalAddress(0xE0000000), video_ram_64k_chunks_count * (64 * KiB));
5151
VERIFY(!device_or_error.is_error());
5252
auto connector = device_or_error.release_value();
5353
MUST(connector->create_attached_framebuffer_console());
@@ -57,7 +57,7 @@ LockRefPtr<BochsDisplayConnector> BochsDisplayConnector::try_create_for_vga_isa_
5757

5858
NonnullLockRefPtr<BochsDisplayConnector> BochsDisplayConnector::must_create(PhysicalAddress framebuffer_address, size_t framebuffer_resource_size, bool virtual_box_hardware)
5959
{
60-
auto device_or_error = DeviceManagement::try_create_device<BochsDisplayConnector>(framebuffer_address, framebuffer_resource_size);
60+
auto device_or_error = Device::try_create_device<BochsDisplayConnector>(framebuffer_address, framebuffer_resource_size);
6161
VERIFY(!device_or_error.is_error());
6262
auto connector = device_or_error.release_value();
6363
MUST(connector->create_attached_framebuffer_console());

Kernel/Arch/x86_64/Hypervisor/BochsDisplayConnector.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ namespace Kernel {
1919
class BochsDisplayConnector
2020
: public DisplayConnector {
2121
friend class BochsGraphicsAdapter;
22-
friend class DeviceManagement;
22+
friend class Device;
2323
friend class GraphicsManagement;
2424

2525
public:

Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
#include <Kernel/Arch/x86_64/Hypervisor/VMWareBackdoor.h>
88
#include <Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.h>
9-
#include <Kernel/Devices/DeviceManagement.h>
9+
#include <Kernel/Devices/Device.h>
1010
#include <Kernel/Sections.h>
1111

1212
namespace Kernel {

Kernel/Arch/x86_64/ISABus/HID/VMWareMouseDevice.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace Kernel {
1616

1717
class VMWareMouseDevice final : public PS2MouseDevice {
1818
public:
19-
friend class DeviceManagement;
19+
friend class Device;
2020
static ErrorOr<NonnullOwnPtr<VMWareMouseDevice>> try_to_initialize(SerialIOController const&, SerialIOController::PortIndex, MouseDevice const&);
2121
virtual ~VMWareMouseDevice() override;
2222

Kernel/Arch/x86_64/ISABus/SerialDevice.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* SPDX-License-Identifier: BSD-2-Clause
55
*/
66

7-
#include <Kernel/Devices/DeviceManagement.h>
7+
#include <Kernel/Devices/Device.h>
88
#include <Kernel/Devices/SerialDevice.h>
99
#include <Kernel/Library/IOWindow.h>
1010
#include <Kernel/Sections.h>
@@ -24,22 +24,22 @@ UNMAP_AFTER_INIT NonnullLockRefPtr<SerialDevice> SerialDevice::must_create(size_
2424
switch (com_number) {
2525
case 0: {
2626
auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM1_ADDR), 16).release_value_but_fixme_should_propagate_errors();
27-
serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 0).release_value();
27+
serial_device = Device::try_create_device<SerialDevice>(move(io_window), 0).release_value();
2828
break;
2929
}
3030
case 1: {
3131
auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM2_ADDR), 16).release_value_but_fixme_should_propagate_errors();
32-
serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 1).release_value();
32+
serial_device = Device::try_create_device<SerialDevice>(move(io_window), 1).release_value();
3333
break;
3434
}
3535
case 2: {
3636
auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM3_ADDR), 16).release_value_but_fixme_should_propagate_errors();
37-
serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 2).release_value();
37+
serial_device = Device::try_create_device<SerialDevice>(move(io_window), 2).release_value();
3838
break;
3939
}
4040
case 3: {
4141
auto io_window = IOWindow::create_for_io_space(IOAddress(SERIAL_COM4_ADDR), 16).release_value_but_fixme_should_propagate_errors();
42-
serial_device = DeviceManagement::try_create_device<SerialDevice>(move(io_window), 3).release_value();
42+
serial_device = Device::try_create_device<SerialDevice>(move(io_window), 3).release_value();
4343
break;
4444
}
4545
default:

Kernel/Bus/USB/Drivers/HID/MouseDriver.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
#include <Kernel/Bus/USB/USBClasses.h>
1111
#include <Kernel/Bus/USB/USBEndpoint.h>
1212
#include <Kernel/Bus/USB/USBRequest.h>
13-
#include <Kernel/Devices/DeviceManagement.h>
13+
#include <Kernel/Devices/Device.h>
1414
#include <Kernel/Devices/HID/Management.h>
1515

1616
namespace Kernel::USB {

Kernel/CMakeLists.txt

-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ set(KERNEL_SOURCES
6565
Devices/BlockDevice.cpp
6666
Devices/CharacterDevice.cpp
6767
Devices/Device.cpp
68-
Devices/DeviceManagement.cpp
6968
Devices/FUSEDevice.cpp
7069
Devices/PCISerialDevice.cpp
7170
Devices/SerialDevice.cpp

Kernel/Devices/Audio/AC97/AC97.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include <AK/Format.h>
88
#include <Kernel/Arch/Delay.h>
99
#include <Kernel/Devices/Audio/AC97/AC97.h>
10-
#include <Kernel/Devices/DeviceManagement.h>
10+
#include <Kernel/Devices/Device.h>
1111
#include <Kernel/Interrupts/InterruptDisabler.h>
1212
#include <Kernel/Memory/AnonymousVMObject.h>
1313

Kernel/Devices/Audio/Channel.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include <Kernel/API/Ioctl.h>
88
#include <Kernel/API/MajorNumberAllocation.h>
99
#include <Kernel/Devices/Audio/Management.h>
10-
#include <Kernel/Devices/DeviceManagement.h>
10+
#include <Kernel/Devices/Device.h>
1111
#include <Kernel/Devices/Generic/RandomDevice.h>
1212
#include <Kernel/Sections.h>
1313
#include <Kernel/Security/Random.h>
@@ -16,7 +16,7 @@ namespace Kernel {
1616

1717
UNMAP_AFTER_INIT ErrorOr<NonnullRefPtr<AudioChannel>> AudioChannel::create(AudioController const& controller, size_t channel_index)
1818
{
19-
auto channel = TRY(DeviceManagement::try_create_device<AudioChannel>(controller, channel_index));
19+
auto channel = TRY(Device::try_create_device<AudioChannel>(controller, channel_index));
2020

2121
// FIXME: Ideally, we would want the audio controller to run a channel at a device's initial sample
2222
// rate instead of hardcoding 44.1 KHz here. However, most audio is provided at 44.1 KHz and as

Kernel/Devices/Audio/Channel.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace Kernel {
1313
class AudioController;
1414
class AudioChannel final
1515
: public CharacterDevice {
16-
friend class DeviceManagement;
16+
friend class Device;
1717

1818
public:
1919
static ErrorOr<NonnullRefPtr<AudioChannel>> create(AudioController const&, size_t channel_index);

Kernel/Devices/BaseDevices.h

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright (c) 2024, Liav A. <[email protected]>
3+
*
4+
* SPDX-License-Identifier: BSD-2-Clause
5+
*/
6+
7+
#pragma once
8+
9+
#include <AK/NonnullRefPtr.h>
10+
#include <AK/Types.h>
11+
#include <Kernel/Devices/Generic/ConsoleDevice.h>
12+
#include <Kernel/Devices/Generic/DeviceControlDevice.h>
13+
#include <Kernel/Devices/Generic/NullDevice.h>
14+
15+
namespace Kernel {
16+
17+
struct BaseDevices {
18+
NonnullRefPtr<NullDevice> const null_device;
19+
NonnullRefPtr<ConsoleDevice> const console_device;
20+
NonnullRefPtr<DeviceControlDevice> const device_control_device;
21+
};
22+
23+
}

Kernel/Devices/Device.cpp

+105-6
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
*/
66

77
#include <AK/Singleton.h>
8+
#include <Kernel/Devices/BaseDevices.h>
9+
#include <Kernel/Devices/BlockDevice.h>
10+
#include <Kernel/Devices/CharacterDevice.h>
811
#include <Kernel/Devices/Device.h>
9-
#include <Kernel/Devices/DeviceManagement.h>
1012
#include <Kernel/FileSystem/InodeMetadata.h>
1113
#include <Kernel/FileSystem/SysFS/Component.h>
1214
#include <Kernel/FileSystem/SysFS/Subsystems/DeviceIdentifiers/BlockDevicesDirectory.h>
@@ -15,21 +17,70 @@
1517

1618
namespace Kernel {
1719

18-
Device::Device(MajorNumber major, MinorNumber minor)
19-
: m_major(major)
20-
, m_minor(minor)
20+
struct AllDevicesDetails {
21+
SpinlockProtected<HashMap<u64, BlockDevice*>, LockRank::None> block_devices {};
22+
SpinlockProtected<HashMap<u64, CharacterDevice*>, LockRank::None> char_devices {};
23+
SpinlockProtected<CircularQueue<DeviceEvent, 100>, LockRank::None> event_queue {};
24+
// NOTE: There's no locking on this pointer because we expect to initialize it once
25+
// and never touch it again.
26+
OwnPtr<BaseDevices> base_devices;
27+
};
28+
29+
static Singleton<AllDevicesDetails> s_all_details;
30+
31+
SpinlockProtected<CircularQueue<DeviceEvent, 100>, LockRank::None>& Device::event_queue()
2132
{
33+
return s_all_details->event_queue;
34+
}
35+
36+
BaseDevices* Device::base_devices()
37+
{
38+
return s_all_details->base_devices.ptr();
39+
}
40+
41+
UNMAP_AFTER_INIT void Device::initialize_base_devices()
42+
{
43+
auto base_devices = MUST(adopt_nonnull_own_or_enomem(new (nothrow) BaseDevices(*NullDevice::must_initialize(), *ConsoleDevice::must_create(), *DeviceControlDevice::must_create())));
44+
s_all_details->base_devices = move(base_devices);
45+
}
46+
47+
RefPtr<Device> Device::acquire_by_type_and_major_minor_numbers(DeviceNodeType type, MajorNumber major, MinorNumber minor)
48+
{
49+
VERIFY(type == DeviceNodeType::Block || type == DeviceNodeType::Character);
50+
51+
auto find_device_in_map = [major, minor](auto& map) -> RefPtr<Device> {
52+
auto it = map.find(encoded_device(major.value(), minor.value()));
53+
if (it == map.end())
54+
return nullptr;
55+
return *it->value;
56+
};
57+
58+
if (type == DeviceNodeType::Block) {
59+
return s_all_details->block_devices.with([&](auto& map) -> RefPtr<Device> {
60+
return find_device_in_map(map);
61+
});
62+
}
63+
64+
return s_all_details->char_devices.with([&](auto& map) -> RefPtr<Device> {
65+
return find_device_in_map(map);
66+
});
2267
}
2368

2469
void Device::before_will_be_destroyed_remove_from_device_management()
2570
{
26-
DeviceManagement::the().before_device_removal({}, *this);
71+
before_device_removal({}, *this);
2772
m_state = State::BeingRemoved;
2873
}
2974

3075
void Device::after_inserting_add_to_device_management()
3176
{
32-
DeviceManagement::the().after_inserting_device({}, *this);
77+
after_inserting_device({}, *this);
78+
}
79+
80+
Device::Device(MajorNumber major, MinorNumber minor)
81+
: m_major(major)
82+
, m_minor(minor)
83+
{
3384
}
3485

3586
ErrorOr<void> Device::after_inserting()
@@ -80,4 +131,52 @@ void Device::process_next_queued_request(Badge<AsyncDeviceRequest>, AsyncDeviceR
80131
evaluate_block_conditions();
81132
}
82133

134+
void Device::after_inserting_device(Badge<Device>, Device& device)
135+
{
136+
if (device.is_block_device()) {
137+
s_all_details->block_devices.with([&](auto& map) -> void {
138+
add_device_to_map<BlockDevice>(map, device);
139+
});
140+
} else {
141+
VERIFY(device.is_character_device());
142+
s_all_details->char_devices.with([&](auto& map) -> void {
143+
add_device_to_map<CharacterDevice>(map, device);
144+
});
145+
}
146+
147+
s_all_details->event_queue.with([&](auto& queue) {
148+
DeviceEvent event { DeviceEvent::State::Inserted, device.is_block_device(), device.major().value(), device.minor().value() };
149+
queue.enqueue(event);
150+
});
151+
152+
if (s_all_details->base_devices)
153+
s_all_details->base_devices->device_control_device->evaluate_block_conditions();
154+
}
155+
156+
void Device::before_device_removal(Badge<Device>, Device& device)
157+
{
158+
u64 device_id = encoded_device(device.major(), device.minor());
159+
160+
if (device.is_block_device()) {
161+
s_all_details->block_devices.with([&](auto& map) -> void {
162+
VERIFY(map.contains(device_id));
163+
map.remove(encoded_device(device.major(), device.minor()));
164+
});
165+
} else {
166+
VERIFY(device.is_character_device());
167+
s_all_details->char_devices.with([&](auto& map) -> void {
168+
VERIFY(map.contains(device_id));
169+
map.remove(encoded_device(device.major(), device.minor()));
170+
});
171+
}
172+
173+
s_all_details->event_queue.with([&](auto& queue) {
174+
DeviceEvent event { DeviceEvent::State::Removed, device.is_block_device(), device.major().value(), device.minor().value() };
175+
queue.enqueue(event);
176+
});
177+
178+
if (s_all_details->base_devices)
179+
s_all_details->base_devices->device_control_device->evaluate_block_conditions();
180+
}
181+
83182
}

0 commit comments

Comments
 (0)