From fb7b4caa574e8d4ad72657525590c5076d97a65a Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 27 Aug 2021 08:08:43 +0300 Subject: [PATCH] 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 | 1 + Kernel/Storage/AHCIPort.cpp | 22 ++++++ Kernel/Storage/AHCIPort.h | 5 +- Kernel/Storage/Partition/DiskPartition.cpp | 13 ++-- Kernel/Storage/Partition/DiskPartition.h | 3 +- Kernel/Storage/SATADiskDevice.cpp | 2 +- Kernel/Storage/SATADiskDevice.h | 2 +- Kernel/Storage/StorageDevice.h | 7 +- Kernel/Storage/StorageManagement.cpp | 82 +++++++++++----------- Kernel/Storage/StorageManagement.h | 18 ++--- Kernel/init.cpp | 2 +- 11 files changed, 96 insertions(+), 61 deletions(-) diff --git a/Kernel/Devices/BlockDevice.h b/Kernel/Devices/BlockDevice.h index 23cfe42700..afc1a82929 100644 --- a/Kernel/Devices/BlockDevice.h +++ b/Kernel/Devices/BlockDevice.h @@ -6,6 +6,7 @@ #pragma once +#include #include namespace Kernel { diff --git a/Kernel/Storage/AHCIPort.cpp b/Kernel/Storage/AHCIPort.cpp index 9840a78da4..e9267fbe5f 100644 --- a/Kernel/Storage/AHCIPort.cpp +++ b/Kernel/Storage/AHCIPort.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include 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()); diff --git a/Kernel/Storage/AHCIPort.h b/Kernel/Storage/AHCIPort.h index a0f9146773..2609711662 100644 --- a/Kernel/Storage/AHCIPort.h +++ b/Kernel/Storage/AHCIPort.h @@ -8,6 +8,8 @@ #include #include +#include +#include #include #include #include @@ -30,7 +32,8 @@ class AsyncBlockDeviceRequest; class AHCIPortHandler; class SATADiskDevice; -class AHCIPort : public RefCounted { +class AHCIPort : public RefCounted + , public Weakable { friend class AHCIPortHandler; friend class SATADiskDevice; diff --git a/Kernel/Storage/Partition/DiskPartition.cpp b/Kernel/Storage/Partition/DiskPartition.cpp index 367a3b3a4f..17d474f011 100644 --- a/Kernel/Storage/Partition/DiskPartition.cpp +++ b/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(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(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 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 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 diff --git a/Kernel/Storage/Partition/DiskPartition.h b/Kernel/Storage/Partition/DiskPartition.h index 0352ee2883..46f99a1fcd 100644 --- a/Kernel/Storage/Partition/DiskPartition.h +++ b/Kernel/Storage/Partition/DiskPartition.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include @@ -32,7 +33,7 @@ private: DiskPartition(BlockDevice&, unsigned, DiskPartitionMetadata); - NonnullRefPtr m_device; + WeakPtr m_device; DiskPartitionMetadata m_metadata; }; diff --git a/Kernel/Storage/SATADiskDevice.cpp b/Kernel/Storage/SATADiskDevice.cpp index ff9ba79597..87effd650e 100644 --- a/Kernel/Storage/SATADiskDevice.cpp +++ b/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 diff --git a/Kernel/Storage/SATADiskDevice.h b/Kernel/Storage/SATADiskDevice.h index da1755e05f..8c1138b121 100644 --- a/Kernel/Storage/SATADiskDevice.h +++ b/Kernel/Storage/SATADiskDevice.h @@ -37,7 +37,7 @@ private: // ^DiskDevice virtual StringView class_name() const override; - NonnullRefPtr m_port; + WeakPtr m_port; }; } diff --git a/Kernel/Storage/StorageDevice.h b/Kernel/Storage/StorageDevice.h index ce1f989018..6f54efeff8 100644 --- a/Kernel/Storage/StorageDevice.h +++ b/Kernel/Storage/StorageDevice.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -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 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> m_list_node; NonnullRefPtr m_storage_controller; NonnullRefPtrVector m_partitions; u64 m_max_addressable_block; diff --git a/Kernel/Storage/StorageManagement.cpp b/Kernel/Storage/StorageManagement.cpp index 8e2f8b9a22..6777a74fc6 100644 --- a/Kernel/Storage/StorageManagement.cpp +++ b/Kernel/Storage/StorageManagement.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -21,21 +22,16 @@ namespace Kernel { -static StorageManagement* s_the; -static size_t s_device_minor_number; +static Singleton s_the; +static Atomic s_device_minor_number; -UNMAP_AFTER_INIT StorageManagement::StorageManagement(String boot_argument, bool force_pio) - : m_boot_argument(boot_argument) - , m_controllers(enumerate_controllers(force_pio)) - , m_storage_devices(enumerate_storage_devices()) - , m_disk_partitions(enumerate_disk_partitions()) +UNMAP_AFTER_INIT StorageManagement::StorageManagement() { - s_device_minor_number = 0; - if (!boot_argument_contains_partition_uuid()) { - determine_boot_device(); - return; - } - determine_boot_device_with_partition_uuid(); +} + +void StorageManagement::remove_device(StorageDevice& device) +{ + m_storage_devices.remove(device); } 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 StorageManagement::enumerate_controllers(bool force_pio) const +UNMAP_AFTER_INIT void StorageManagement::enumerate_controllers(bool force_pio) { - NonnullRefPtrVector 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()); - return controllers; + m_controllers.append(RamdiskController::initialize()); } -UNMAP_AFTER_INIT NonnullRefPtrVector StorageManagement::enumerate_storage_devices() const +UNMAP_AFTER_INIT void StorageManagement::enumerate_storage_devices() { VERIFY(!m_controllers.is_empty()); - NonnullRefPtrVector 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 StorageManagement::try_to_initialize_partition_table(const StorageDevice& device) const @@ -99,7 +92,7 @@ UNMAP_AFTER_INIT OwnPtr StorageManagement::try_to_initialize_par return {}; } -UNMAP_AFTER_INIT NonnullRefPtrVector StorageManagement::enumerate_disk_partitions() const +UNMAP_AFTER_INIT void StorageManagement::enumerate_disk_partitions() const { VERIFY(!m_storage_devices.is_empty()); NonnullRefPtrVector partitions; @@ -119,7 +112,6 @@ UNMAP_AFTER_INIT NonnullRefPtrVector 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& partition : m_disk_partitions) { - if (partition.metadata().unique_guid().is_zero()) - continue; - if (partition.metadata().unique_guid() == partition_uuid) { - m_boot_block_device = partition; - break; + for (auto& storage_device : m_storage_devices) { + for (auto& partition : storage_device.partitions()) { + if (partition.metadata().unique_guid().is_zero()) + continue; + if (partition.metadata().unique_guid() == partition_uuid) { + m_boot_block_device = partition; + break; + } } } } RefPtr 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 StorageManagement::root_filesystem() const @@ -191,15 +186,18 @@ NonnullRefPtr 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()); - s_the = new StorageManagement(root_device, force_pio); + VERIFY(s_device_minor_number == 0); + 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() diff --git a/Kernel/Storage/StorageManagement.h b/Kernel/Storage/StorageManagement.h index e3f0af53da..70c30be50c 100644 --- a/Kernel/Storage/StorageManagement.h +++ b/Kernel/Storage/StorageManagement.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -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 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 enumerate_controllers(bool force_pio) const; - NonnullRefPtrVector enumerate_storage_devices() const; - NonnullRefPtrVector enumerate_disk_partitions() const; + void enumerate_controllers(bool force_pio); + void enumerate_storage_devices(); + void enumerate_disk_partitions() const; void determine_boot_device(); void determine_boot_device_with_partition_uuid(); @@ -46,10 +49,9 @@ private: RefPtr boot_block_device() const; String m_boot_argument; - RefPtr m_boot_block_device { nullptr }; + WeakPtr m_boot_block_device; NonnullRefPtrVector m_controllers; - NonnullRefPtrVector m_storage_devices; - NonnullRefPtrVector m_disk_partitions; + IntrusiveList, &StorageDevice::m_list_node> m_storage_devices; }; } diff --git a/Kernel/init.cpp b/Kernel/init.cpp index a08b8dd6c8..7024175fb3 100644 --- a/Kernel/init.cpp +++ b/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"); }