Kernel/Devices: Ensure appropriate locking on the Device map singleton

Devices might be removed and inserted at anytime, so let's ensure we
always do these kind of operations with a good known state of the
HashMap.

The VirtIO code was modified to create devices outside the IRQ handler,
so now it works with the new locking of the devices singleton, but a
better approach might be needed later on.

Kernel/Bus/VirtIO/Console.cpp

@@ -7,6 +7,7 @@
 
 #include <Kernel/Bus/VirtIO/Console.h>
 #include <Kernel/Sections.h>
+#include <Kernel/WorkQueue.h>
 
 namespace Kernel::VirtIO {
 
@@ -51,8 +52,11 @@ UNMAP_AFTER_INIT void Console::initialize()
 
             if (is_feature_accepted(VIRTIO_CONSOLE_F_MULTIPORT))
                 setup_multiport();
-            else
-                m_ports.append(make_ref_counted<VirtIO::ConsolePort>(0u, *this));
+            else {
+                auto port = make_ref_counted<VirtIO::ConsolePort>(0u, *this);
+                port->init_receive_buffer({});
+                m_ports.append(port);
+            }
         }
     }
 }
@@ -146,23 +150,26 @@ void Console::process_control_message(ControlMessage message)
 {
     switch (message.event) {
     case (u16)ControlEvent::DeviceAdd: {
-        u32 id = message.id;
-        if (id >= m_ports.size()) {
-            dbgln("Device provided an invalid port number {}. max_nr_ports: {}", id, m_ports.size());
-            return;
-        } else if (!m_ports.at(id).is_null()) {
-            dbgln("Device tried to add port {} which was already added!", id);
-            return;
-        }
+        g_io_work->queue([message, this]() -> void {
+            u32 id = message.id;
+            if (id >= m_ports.size()) {
+                dbgln("Device provided an invalid port number {}. max_nr_ports: {}", id, m_ports.size());
+                return;
+            } else if (!m_ports.at(id).is_null()) {
+                dbgln("Device tried to add port {} which was already added!", id);
+                return;
+            }
 
-        m_ports.at(id) = make_ref_counted<VirtIO::ConsolePort>(id, *this);
-        ControlMessage ready_event {
-            .id = static_cast<u32>(id),
-            .event = (u16)ControlEvent::PortReady,
-            .value = (u16)ControlMessage::Status::Success
-        };
+            m_ports.at(id) = make_ref_counted<VirtIO::ConsolePort>(id, *this);
+            ControlMessage ready_event {
+                .id = static_cast<u32>(id),
+                .event = (u16)ControlEvent::PortReady,
+                .value = (u16)ControlMessage::Status::Success
+            };
+
+            write_control_message(ready_event);
+        });
 
-        write_control_message(ready_event);
         break;
     }
     case (u16)ControlEvent::ConsolePort:

Kernel/Bus/VirtIO/ConsolePort.cpp

@@ -21,10 +21,9 @@ ConsolePort::ConsolePort(unsigned port, VirtIO::Console& console)
     m_transmit_buffer = make<Memory::RingBuffer>("VirtIO::ConsolePort Transmit", RINGBUFFER_SIZE);
     m_receive_queue = m_port == 0 ? 0 : m_port * 2 + 2;
     m_transmit_queue = m_port == 0 ? 1 : m_port * 2 + 3;
-    init_receive_buffer();
 }
 
-void ConsolePort::init_receive_buffer()
+void ConsolePort::init_receive_buffer(Badge<VirtIO::Console>)
 {
     auto& queue = m_console.get_queue(m_receive_queue);
     SpinlockLocker queue_lock(queue.lock());

Kernel/Bus/VirtIO/ConsolePort.h

@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <AK/Badge.h>
 #include <Kernel/Bus/VirtIO/Device.h>
 #include <Kernel/Devices/CharacterDevice.h>
 #include <Kernel/FileSystem/OpenFileDescription.h>
@@ -29,6 +30,8 @@ public:
     void set_open(Badge<VirtIO::Console>, bool state) { m_open = state; }
     bool is_open() const { return m_open; }
 
+    void init_receive_buffer(Badge<Console>);
+
 private:
     constexpr static size_t RINGBUFFER_SIZE = 2 * PAGE_SIZE;
 
@@ -40,8 +43,6 @@ private:
     virtual KResultOr<size_t> write(OpenFileDescription&, u64, const UserOrKernelBuffer&, size_t) override;
     virtual KResultOr<NonnullRefPtr<OpenFileDescription>> open(int options) override;
 
-    void init_receive_buffer();
-
     static unsigned next_device_id;
     u16 m_receive_queue {};
     u16 m_transmit_queue {};

Kernel/Devices/Device.cpp

@@ -7,28 +7,34 @@
 #include <AK/Singleton.h>
 #include <Kernel/Devices/Device.h>
 #include <Kernel/FileSystem/InodeMetadata.h>
+#include <Kernel/Locking/MutexProtected.h>
+#include <Kernel/Sections.h>
 
 namespace Kernel {
 
-static Singleton<HashMap<u32, Device*>> s_all_devices;
+static Singleton<MutexProtected<HashMap<u32, Device*>>> s_all_devices;
 
-HashMap<u32, Device*>& Device::all_devices()
+MutexProtected<HashMap<u32, Device*>>& Device::all_devices()
 {
     return *s_all_devices;
 }
 
 void Device::for_each(Function<void(Device&)> callback)
 {
-    for (auto& entry : all_devices())
-        callback(*entry.value);
+    all_devices().with_exclusive([&](auto& map) -> void {
+        for (auto& entry : map)
+            callback(*entry.value);
+    });
 }
 
 Device* Device::get_device(unsigned major, unsigned minor)
 {
-    auto it = all_devices().find(encoded_device(major, minor));
-    if (it == all_devices().end())
-        return nullptr;
-    return it->value;
+    return all_devices().with_exclusive([&](auto& map) -> Device* {
+        auto it = map.find(encoded_device(major, minor));
+        if (it == map.end())
+            return nullptr;
+        return it->value;
+    });
 }
 
 Device::Device(unsigned major, unsigned minor)
@@ -36,17 +42,23 @@ Device::Device(unsigned major, unsigned minor)
     , m_minor(minor)
 {
     u32 device_id = encoded_device(major, minor);
-    auto it = all_devices().find(device_id);
-    if (it != all_devices().end()) {
-        dbgln("Already registered {},{}: {}", major, minor, it->value->class_name());
-    }
-    VERIFY(!all_devices().contains(device_id));
-    all_devices().set(device_id, this);
+    all_devices().with_exclusive([&](auto& map) -> void {
+        auto it = map.find(device_id);
+        if (it != map.end()) {
+            dbgln("Already registered {},{}: {}", major, minor, it->value->class_name());
+        }
+        VERIFY(!map.contains(device_id));
+        map.set(device_id, this);
+    });
 }
 
 Device::~Device()
 {
-    all_devices().remove(encoded_device(m_major, m_minor));
+    u32 device_id = encoded_device(m_major, m_minor);
+    all_devices().with_exclusive([&](auto& map) -> void {
+        VERIFY(map.contains(device_id));
+        map.remove(encoded_device(m_major, m_minor));
+    });
 }
 
 String Device::absolute_path() const

Kernel/Devices/Device.h

@@ -19,6 +19,7 @@
 #include <AK/HashMap.h>
 #include <Kernel/Devices/AsyncDeviceRequest.h>
 #include <Kernel/FileSystem/File.h>
+#include <Kernel/FileSystem/SysFSComponent.h>
 #include <Kernel/Locking/Mutex.h>
 #include <Kernel/UnixTypes.h>
 
@@ -61,7 +62,7 @@ protected:
     void set_uid(UserID uid) { m_uid = uid; }
     void set_gid(GroupID gid) { m_gid = gid; }
 
-    static HashMap<u32, Device*>& all_devices();
+    static MutexProtected<HashMap<u32, Device*>>& all_devices();
 
 private:
     unsigned m_major { 0 };