Kernel/Storage: Implement basic AHCI hotplug support

This is really a basic support for AHCI hotplug events, so we know how
to add a node representing the device in /sys/dev/block and removing it
according to the event type (insertion/removal).

This change doesn't take into account what happens if the device was
mounted or a read/write operation is being handled.

For this to work correctly, StorageManagement now uses the Singleton
container, as it might be accessed simultaneously from many CPUs
for hotplug events. DiskPartition holds a WeakPtr instead of a RefPtr,
to allow removal of a StorageDevice object from the heap.
StorageDevices are now stored and being referenced to via an
IntrusiveList to make it easier to remove them on hotplug event.

In future changes, all of the stated above might change, but for now,
this commit represents the least amount of changes to make everything
to work correctly.

Kernel/Devices/BlockDevice.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/Weakable.h>
 #include <Kernel/Devices/Device.h>
 
 namespace Kernel {

Kernel/Storage/AHCIPort.cpp

@@ -15,6 +15,7 @@
 #include <Kernel/Storage/AHCIPort.h>
 #include <Kernel/Storage/ATA.h>
 #include <Kernel/Storage/SATADiskDevice.h>
+#include <Kernel/Storage/StorageManagement.h>
 #include <Kernel/WorkQueue.h>
 
 namespace Kernel {
@@ -70,6 +71,22 @@ void AHCIPort::handle_interrupt()
     if (m_interrupt_status.raw_value() == 0) {
         return;
     }
+    if (m_interrupt_status.is_set(AHCI::PortInterruptFlag::PRC) && m_interrupt_status.is_set(AHCI::PortInterruptFlag::PC)) {
+        clear_sata_error_register();
+        if ((m_port_registers.ssts & 0xf) != 3) {
+            m_connected_device->prepare_for_unplug();
+            StorageManagement::the().remove_device(*m_connected_device);
+            g_io_work->queue([this]() {
+                m_connected_device->before_removing();
+                m_connected_device.clear();
+            });
+        } else {
+            g_io_work->queue([this]() {
+                reset();
+            });
+        }
+        return;
+    }
     if (m_interrupt_status.is_set(AHCI::PortInterruptFlag::PRC)) {
         clear_sata_error_register();
         m_wait_connect_for_completion = true;
@@ -102,6 +119,11 @@ void AHCIPort::handle_interrupt()
                 MutexLocker locker(m_lock);
                 VERIFY(m_current_request);
                 VERIFY(m_current_scatter_list);
+                if (!m_connected_device) {
+                    dbgln_if(AHCI_DEBUG, "AHCI Port {}: Request success", representative_port_index());
+                    complete_current_request(AsyncDeviceRequest::Failure);
+                    return;
+                }
                 if (m_current_request->request_type() == AsyncBlockDeviceRequest::Read) {
                     if (auto result = m_current_request->write_to_buffer(m_current_request->buffer(), m_current_scatter_list->dma_region().as_ptr(), m_connected_device->block_size() * m_current_request->block_count()); result.is_error()) {
                         dbgln_if(AHCI_DEBUG, "AHCI Port {}: Request failure, memory fault occurred when reading in data.", representative_port_index());

Kernel/Storage/AHCIPort.h

@@ -8,6 +8,8 @@
 
 #include <AK/OwnPtr.h>
 #include <AK/RefPtr.h>
+#include <AK/WeakPtr.h>
+#include <AK/Weakable.h>
 #include <Kernel/Devices/Device.h>
 #include <Kernel/IO.h>
 #include <Kernel/Interrupts/IRQHandler.h>
@@ -30,7 +32,8 @@ class AsyncBlockDeviceRequest;
 
 class AHCIPortHandler;
 class SATADiskDevice;
-class AHCIPort : public RefCounted<AHCIPort> {
+class AHCIPort : public RefCounted<AHCIPort>
+    , public Weakable<AHCIPort> {
     friend class AHCIPortHandler;
     friend class SATADiskDevice;
 

Kernel/Storage/Partition/DiskPartition.cpp

@@ -33,7 +33,10 @@ const DiskPartitionMetadata& DiskPartition::metadata() const
 
 void DiskPartition::start_request(AsyncBlockDeviceRequest& request)
 {
-    auto sub_request_or_error = m_device->try_make_request<AsyncBlockDeviceRequest>(request.request_type(),
+    auto device = m_device.strong_ref();
+    if (!device)
+        request.complete(AsyncBlockDeviceRequest::RequestResult::Failure);
+    auto sub_request_or_error = device->try_make_request<AsyncBlockDeviceRequest>(request.request_type(),
         request.block_index() + m_metadata.start_block(), request.block_count(), request.buffer(), request.buffer_size());
     if (sub_request_or_error.is_error())
         TODO();
@@ -44,28 +47,28 @@ KResultOr<size_t> DiskPartition::read(OpenFileDescription& fd, u64 offset, UserO
 {
     unsigned adjust = m_metadata.start_block() * block_size();
     dbgln_if(OFFD_DEBUG, "DiskPartition::read offset={}, adjust={}, len={}", fd.offset(), adjust, len);
-    return m_device->read(fd, offset + adjust, outbuf, len);
+    return m_device.strong_ref()->read(fd, offset + adjust, outbuf, len);
 }
 
 bool DiskPartition::can_read(const OpenFileDescription& fd, size_t offset) const
 {
     unsigned adjust = m_metadata.start_block() * block_size();
     dbgln_if(OFFD_DEBUG, "DiskPartition::can_read offset={}, adjust={}", offset, adjust);
-    return m_device->can_read(fd, offset + adjust);
+    return m_device.strong_ref()->can_read(fd, offset + adjust);
 }
 
 KResultOr<size_t> DiskPartition::write(OpenFileDescription& fd, u64 offset, const UserOrKernelBuffer& inbuf, size_t len)
 {
     unsigned adjust = m_metadata.start_block() * block_size();
     dbgln_if(OFFD_DEBUG, "DiskPartition::write offset={}, adjust={}, len={}", offset, adjust, len);
-    return m_device->write(fd, offset + adjust, inbuf, len);
+    return m_device.strong_ref()->write(fd, offset + adjust, inbuf, len);
 }
 
 bool DiskPartition::can_write(const OpenFileDescription& fd, size_t offset) const
 {
     unsigned adjust = m_metadata.start_block() * block_size();
     dbgln_if(OFFD_DEBUG, "DiskPartition::can_write offset={}, adjust={}", offset, adjust);
-    return m_device->can_write(fd, offset + adjust);
+    return m_device.strong_ref()->can_write(fd, offset + adjust);
 }
 
 StringView DiskPartition::class_name() const

Kernel/Storage/Partition/DiskPartition.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <AK/RefPtr.h>
+#include <AK/WeakPtr.h>
 #include <Kernel/Devices/BlockDevice.h>
 #include <Kernel/Storage/Partition/DiskPartitionMetadata.h>
 
@@ -32,7 +33,7 @@ private:
 
     DiskPartition(BlockDevice&, unsigned, DiskPartitionMetadata);
 
-    NonnullRefPtr<BlockDevice> m_device;
+    WeakPtr<BlockDevice> m_device;
     DiskPartitionMetadata m_metadata;
 };
 

Kernel/Storage/SATADiskDevice.cpp

@@ -34,7 +34,7 @@ StringView SATADiskDevice::class_name() const
 
 void SATADiskDevice::start_request(AsyncBlockDeviceRequest& request)
 {
-    m_port->start_request(request);
+    m_port.strong_ref()->start_request(request);
 }
 
 String SATADiskDevice::storage_name() const

Kernel/Storage/SATADiskDevice.h

@@ -37,7 +37,7 @@ private:
 
     // ^DiskDevice
     virtual StringView class_name() const override;
-    NonnullRefPtr<AHCIPort> m_port;
+    WeakPtr<AHCIPort> m_port;
 };
 
 }

Kernel/Storage/StorageDevice.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/IntrusiveList.h>
 #include <Kernel/Devices/BlockDevice.h>
 #include <Kernel/Interrupts/IRQHandler.h>
 #include <Kernel/Locking/Mutex.h>
@@ -16,7 +17,6 @@ namespace Kernel {
 
 class StorageDevice : public BlockDevice {
     friend class StorageManagement;
-    AK_MAKE_ETERNAL
 
 public:
     virtual u64 max_addressable_block() const { return m_max_addressable_block; }
@@ -32,6 +32,10 @@ public:
     // FIXME: This is being used only during early boot, find a better way to find devices...
     virtual String storage_name() const = 0;
 
+    virtual void prepare_for_unplug() { m_partitions.clear(); }
+
+    NonnullRefPtrVector<DiskPartition> partitions() const { return m_partitions; }
+
 protected:
     StorageDevice(const StorageController&, size_t, u64);
     StorageDevice(const StorageController&, int, int, size_t, u64);
@@ -39,6 +43,7 @@ protected:
     virtual StringView class_name() const override;
 
 private:
+    mutable IntrusiveListNode<StorageDevice, RefPtr<StorageDevice>> m_list_node;
     NonnullRefPtr<StorageController> m_storage_controller;
     NonnullRefPtrVector<DiskPartition> m_partitions;
     u64 m_max_addressable_block;

Kernel/Storage/StorageManagement.cpp

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <AK/Singleton.h>
 #include <AK/UUID.h>
 #include <Kernel/Bus/PCI/API.h>
 #include <Kernel/Bus/PCI/Access.h>
@@ -21,21 +22,16 @@
 
 namespace Kernel {
 
-static StorageManagement* s_the;
+static Singleton<StorageManagement> s_the;
-static size_t s_device_minor_number;
+static Atomic<size_t> s_device_minor_number;
 
-UNMAP_AFTER_INIT StorageManagement::StorageManagement(String boot_argument, bool force_pio)
+UNMAP_AFTER_INIT StorageManagement::StorageManagement()
-    : m_boot_argument(boot_argument)
-    , m_controllers(enumerate_controllers(force_pio))
-    , m_storage_devices(enumerate_storage_devices())
-    , m_disk_partitions(enumerate_disk_partitions())
 {
-    s_device_minor_number = 0;
+}
-    if (!boot_argument_contains_partition_uuid()) {
+
-        determine_boot_device();
+void StorageManagement::remove_device(StorageDevice& device)
-        return;
+{
-    }
+    m_storage_devices.remove(device);
-    determine_boot_device_with_partition_uuid();
 }
 
 bool StorageManagement::boot_argument_contains_partition_uuid()
@@ -43,40 +39,37 @@ bool StorageManagement::boot_argument_contains_partition_uuid()
     return m_boot_argument.starts_with("PARTUUID=");
 }
 
-UNMAP_AFTER_INIT NonnullRefPtrVector<StorageController> StorageManagement::enumerate_controllers(bool force_pio) const
+UNMAP_AFTER_INIT void StorageManagement::enumerate_controllers(bool force_pio)
 {
-    NonnullRefPtrVector<StorageController> controllers;
+    VERIFY(m_controllers.is_empty());
     if (!kernel_command_line().disable_physical_storage()) {
         if (kernel_command_line().is_ide_enabled()) {
             PCI::enumerate([&](const PCI::Address& address, PCI::ID) {
                 if (PCI::get_class(address) == PCI_MASS_STORAGE_CLASS_ID && PCI::get_subclass(address) == PCI_IDE_CTRL_SUBCLASS_ID) {
-                    controllers.append(IDEController::initialize(address, force_pio));
+                    m_controllers.append(IDEController::initialize(address, force_pio));
                 }
             });
         }
         PCI::enumerate([&](const PCI::Address& address, PCI::ID) {
             if (PCI::get_class(address) == PCI_MASS_STORAGE_CLASS_ID && PCI::get_subclass(address) == PCI_SATA_CTRL_SUBCLASS_ID && PCI::get_programming_interface(address) == PCI_AHCI_IF_PROGIF) {
-                controllers.append(AHCIController::initialize(address));
+                m_controllers.append(AHCIController::initialize(address));
             }
         });
     }
-    controllers.append(RamdiskController::initialize());
+    m_controllers.append(RamdiskController::initialize());
-    return controllers;
 }
 
-UNMAP_AFTER_INIT NonnullRefPtrVector<StorageDevice> StorageManagement::enumerate_storage_devices() const
+UNMAP_AFTER_INIT void StorageManagement::enumerate_storage_devices()
 {
     VERIFY(!m_controllers.is_empty());
-    NonnullRefPtrVector<StorageDevice> devices;
     for (auto& controller : m_controllers) {
         for (size_t device_index = 0; device_index < controller.devices_count(); device_index++) {
             auto device = controller.device(device_index);
             if (device.is_null())
                 continue;
-            devices.append(device.release_nonnull());
+            m_storage_devices.append(device.release_nonnull());
         }
     }
-    return devices;
 }
 
 UNMAP_AFTER_INIT OwnPtr<PartitionTable> StorageManagement::try_to_initialize_partition_table(const StorageDevice& device) const
@@ -99,7 +92,7 @@ UNMAP_AFTER_INIT OwnPtr<PartitionTable> StorageManagement::try_to_initialize_par
     return {};
 }
 
-UNMAP_AFTER_INIT NonnullRefPtrVector<DiskPartition> StorageManagement::enumerate_disk_partitions() const
+UNMAP_AFTER_INIT void StorageManagement::enumerate_disk_partitions() const
 {
     VERIFY(!m_storage_devices.is_empty());
     NonnullRefPtrVector<DiskPartition> partitions;
@@ -119,7 +112,6 @@ UNMAP_AFTER_INIT NonnullRefPtrVector<DiskPartition> StorageManagement::enumerate
         }
         device_index++;
     }
-    return partitions;
 }
 
 UNMAP_AFTER_INIT void StorageManagement::determine_boot_device()
@@ -141,7 +133,7 @@ UNMAP_AFTER_INIT void StorageManagement::determine_boot_device()
 
 UNMAP_AFTER_INIT void StorageManagement::determine_boot_device_with_partition_uuid()
 {
-    VERIFY(!m_disk_partitions.is_empty());
+    VERIFY(!m_storage_devices.is_empty());
     VERIFY(m_boot_argument.starts_with("PARTUUID="));
 
     auto partition_uuid = UUID(m_boot_argument.substring_view(strlen("PARTUUID=")));
@@ -149,20 +141,21 @@ UNMAP_AFTER_INIT void StorageManagement::determine_boot_device_with_partition_uu
     if (partition_uuid.to_string().length() != 36) {
         PANIC("StorageManagement: Specified partition UUID is not valid");
     }
-
+    for (auto& storage_device : m_storage_devices) {
-    for (auto& partition : m_disk_partitions) {
+        for (auto& partition : storage_device.partitions()) {
-        if (partition.metadata().unique_guid().is_zero())
+            if (partition.metadata().unique_guid().is_zero())
-            continue;
+                continue;
-        if (partition.metadata().unique_guid() == partition_uuid) {
+            if (partition.metadata().unique_guid() == partition_uuid) {
-            m_boot_block_device = partition;
+                m_boot_block_device = partition;
-            break;
+                break;
+            }
         }
     }
 }
 
 RefPtr<BlockDevice> StorageManagement::boot_block_device() const
 {
-    return m_boot_block_device;
+    return m_boot_block_device.strong_ref();
 }
 
 int StorageManagement::major_number()
@@ -171,7 +164,9 @@ int StorageManagement::major_number()
 }
 int StorageManagement::minor_number()
 {
-    return s_device_minor_number++;
+    auto minor_number = s_device_minor_number.load();
+    s_device_minor_number++;
+    return minor_number;
 }
 
 NonnullRefPtr<FileSystem> StorageManagement::root_filesystem() const
@@ -191,15 +186,18 @@ NonnullRefPtr<FileSystem> StorageManagement::root_filesystem() const
     return file_system;
 }
 
-bool StorageManagement::initialized()
-{
-    return (s_the != nullptr);
-}
-
 UNMAP_AFTER_INIT void StorageManagement::initialize(String root_device, bool force_pio)
 {
-    VERIFY(!StorageManagement::initialized());
+    VERIFY(s_device_minor_number == 0);
-    s_the = new StorageManagement(root_device, force_pio);
+    m_boot_argument = root_device;
+    enumerate_controllers(force_pio);
+    enumerate_storage_devices();
+    enumerate_disk_partitions();
+    if (!boot_argument_contains_partition_uuid()) {
+        determine_boot_device();
+        return;
+    }
+    determine_boot_device_with_partition_uuid();
 }
 
 StorageManagement& StorageManagement::the()

Kernel/Storage/StorageManagement.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/IntrusiveList.h>
 #include <AK/NonnullRefPtr.h>
 #include <AK/NonnullRefPtrVector.h>
 #include <AK/Types.h>
@@ -21,9 +22,9 @@ class StorageManagement {
     AK_MAKE_ETERNAL;
 
 public:
-    StorageManagement(String boot_argument, bool force_pio);
+    StorageManagement();
     static bool initialized();
-    static void initialize(String boot_argument, bool force_pio);
+    void initialize(String boot_argument, bool force_pio);
     static StorageManagement& the();
 
     NonnullRefPtr<FileSystem> root_filesystem() const;
@@ -31,12 +32,14 @@ public:
     static int major_number();
     static int minor_number();
 
+    void remove_device(StorageDevice&);
+
 private:
     bool boot_argument_contains_partition_uuid();
 
-    NonnullRefPtrVector<StorageController> enumerate_controllers(bool force_pio) const;
+    void enumerate_controllers(bool force_pio);
-    NonnullRefPtrVector<StorageDevice> enumerate_storage_devices() const;
+    void enumerate_storage_devices();
-    NonnullRefPtrVector<DiskPartition> enumerate_disk_partitions() const;
+    void enumerate_disk_partitions() const;
 
     void determine_boot_device();
     void determine_boot_device_with_partition_uuid();
@@ -46,10 +49,9 @@ private:
     RefPtr<BlockDevice> boot_block_device() const;
 
     String m_boot_argument;
-    RefPtr<BlockDevice> m_boot_block_device { nullptr };
+    WeakPtr<BlockDevice> m_boot_block_device;
     NonnullRefPtrVector<StorageController> m_controllers;
-    NonnullRefPtrVector<StorageDevice> m_storage_devices;
+    IntrusiveList<StorageDevice, RefPtr<StorageDevice>, &StorageDevice::m_list_node> m_storage_devices;
-    NonnullRefPtrVector<DiskPartition> m_disk_partitions;
 };
 
 }

Kernel/init.cpp

@@ -319,7 +319,7 @@ void init_stage2(void*)
     PTYMultiplexer::initialize();
     SB16::detect();
 
-    StorageManagement::initialize(kernel_command_line().root_device(), kernel_command_line().is_force_pio());
+    StorageManagement::the().initialize(kernel_command_line().root_device(), kernel_command_line().is_force_pio());
     if (VirtualFileSystem::the().mount_root(StorageManagement::the().root_filesystem()).is_error()) {
         PANIC("VirtualFileSystem::mount_root failed");
     }