From 4744ccbff0a165b05c2c4b6978cb1aa78aca80f7 Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 22 Apr 2022 18:52:20 +0300 Subject: [PATCH] Kernel/Storage: Add LUN address to each StorageDevice LUN address is essentially how people used to address SCSI devices back in the day we had these devices more in use. However, SCSI was taken as an abstraction layer for many Unix and Unix-like systems, so it still common to see LUN addresses in use. In Serenity, we don't really provide such abstraction layer, and therefore until now, we didn't use LUNs too. However (again), this changes, as we want to let users to address their devices under SysFS easily. LUNs make sense in that regard, because they can be easily adapted to different interfaces besides SCSI. For example, for legacy ATA hard drive being connected to the first IDE controller which was enumerated on the PCI bus, and then to the primary channel as slave device, the LUN address would be 0:0:1. To make this happen, we add unique ID number to each StorageController, which increments by 1 for each new instance of StorageController. Then, we adapt the ATA and NVMe devices to use these numbers and generate LUN in the construction time. --- Kernel/CMakeLists.txt | 1 + Kernel/Storage/ATA/ATADevice.cpp | 7 ++++++- Kernel/Storage/NVMe/NVMeController.cpp | 6 +++--- Kernel/Storage/NVMe/NVMeController.h | 2 +- Kernel/Storage/NVMe/NVMeNameSpace.cpp | 9 +++++---- Kernel/Storage/NVMe/NVMeNameSpace.h | 6 ++++-- Kernel/Storage/Ramdisk/Device.cpp | 4 ++-- Kernel/Storage/StorageController.cpp | 17 +++++++++++++++++ Kernel/Storage/StorageController.h | 7 +++++++ Kernel/Storage/StorageDevice.cpp | 3 ++- Kernel/Storage/StorageDevice.h | 19 ++++++++++++++++++- Kernel/Storage/StorageManagement.cpp | 8 ++++++++ Kernel/Storage/StorageManagement.h | 2 ++ 13 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 Kernel/Storage/StorageController.cpp diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt index ca829adf64a..f0a0cef579e 100644 --- a/Kernel/CMakeLists.txt +++ b/Kernel/CMakeLists.txt @@ -116,6 +116,7 @@ set(KERNEL_SOURCES Storage/NVMe/NVMeQueue.cpp Storage/Ramdisk/Controller.cpp Storage/Ramdisk/Device.cpp + Storage/StorageController.cpp Storage/StorageDevice.cpp Storage/StorageManagement.cpp DoubleBuffer.cpp diff --git a/Kernel/Storage/ATA/ATADevice.cpp b/Kernel/Storage/ATA/ATADevice.cpp index b7c14f1d74b..6acbaf7f089 100644 --- a/Kernel/Storage/ATA/ATADevice.cpp +++ b/Kernel/Storage/ATA/ATADevice.cpp @@ -13,8 +13,13 @@ namespace Kernel { +static StorageDevice::LUNAddress convert_ata_address_to_lun_address(ATAController const& controller, ATADevice::Address ata_address) +{ + return StorageDevice::LUNAddress { controller.controller_id(), ata_address.port, ata_address.subport }; +} + ATADevice::ATADevice(ATAController const& controller, ATADevice::Address ata_address, MinorNumber minor_number, u16 capabilities, u16 logical_sector_size, u64 max_addressable_block, NonnullOwnPtr early_storage_name) - : StorageDevice(StorageManagement::storage_type_major_number(), minor_number, logical_sector_size, max_addressable_block, move(early_storage_name)) + : StorageDevice(convert_ata_address_to_lun_address(controller, ata_address), StorageManagement::storage_type_major_number(), minor_number, logical_sector_size, max_addressable_block, move(early_storage_name)) , m_controller(controller) , m_ata_address(ata_address) , m_capabilities(capabilities) diff --git a/Kernel/Storage/NVMe/NVMeController.cpp b/Kernel/Storage/NVMe/NVMeController.cpp index 5744dee0f1c..e60731bb44b 100644 --- a/Kernel/Storage/NVMe/NVMeController.cpp +++ b/Kernel/Storage/NVMe/NVMeController.cpp @@ -19,13 +19,13 @@ #include namespace Kernel { -Atomic NVMeController::controller_id {}; +Atomic NVMeController::s_controller_id {}; UNMAP_AFTER_INIT ErrorOr> NVMeController::try_initialize(Kernel::PCI::DeviceIdentifier const& device_identifier, bool is_queue_polled) { auto controller = TRY(adopt_nonnull_ref_or_enomem(new NVMeController(device_identifier))); TRY(controller->initialize(is_queue_polled)); - NVMeController::controller_id++; + NVMeController::s_controller_id++; return controller; } @@ -207,7 +207,7 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::identify_and_init_namespaces() dbgln_if(NVME_DEBUG, "NVMe: Block count is {} and Block size is {}", block_counts, block_size); - m_namespaces.append(TRY(NVMeNameSpace::try_create(m_queues, controller_id.load(), nsid, block_counts, block_size))); + m_namespaces.append(TRY(NVMeNameSpace::try_create(*this, m_queues, s_controller_id.load(), nsid, block_counts, block_size))); m_device_count++; dbgln_if(NVME_DEBUG, "NVMe: Initialized namespace with NSID: {}", nsid); } diff --git a/Kernel/Storage/NVMe/NVMeController.h b/Kernel/Storage/NVMe/NVMeController.h index 6f8615c0bb1..198da902b11 100644 --- a/Kernel/Storage/NVMe/NVMeController.h +++ b/Kernel/Storage/NVMe/NVMeController.h @@ -77,6 +77,6 @@ private: AK::Time m_ready_timeout; u32 m_bar { 0 }; u8 m_dbl_stride { 0 }; - static Atomic controller_id; + static Atomic s_controller_id; }; } diff --git a/Kernel/Storage/NVMe/NVMeNameSpace.cpp b/Kernel/Storage/NVMe/NVMeNameSpace.cpp index ec6e0cf95d4..86f312784f1 100644 --- a/Kernel/Storage/NVMe/NVMeNameSpace.cpp +++ b/Kernel/Storage/NVMe/NVMeNameSpace.cpp @@ -7,21 +7,22 @@ #include "NVMeNameSpace.h" #include #include +#include #include namespace Kernel { -UNMAP_AFTER_INIT ErrorOr> NVMeNameSpace::try_create(NonnullRefPtrVector queues, u8 controller_id, u16 nsid, size_t storage_size, size_t lba_size) +UNMAP_AFTER_INIT ErrorOr> NVMeNameSpace::try_create(NVMeController const& controller, NonnullRefPtrVector queues, u8 controller_id, u16 nsid, size_t storage_size, size_t lba_size) { auto minor_number = StorageManagement::generate_storage_minor_number(); auto major_number = StorageManagement::storage_type_major_number(); auto device_name_kstring = TRY(KString::formatted("nvme{:d}n{:d}", controller_id, nsid)); - auto device = TRY(DeviceManagement::try_create_device(move(queues), storage_size, lba_size, major_number.value(), minor_number.value(), nsid, move(device_name_kstring))); + auto device = TRY(DeviceManagement::try_create_device(StorageDevice::LUNAddress { controller.controller_id(), nsid, 0 }, move(queues), storage_size, lba_size, major_number.value(), minor_number.value(), nsid, move(device_name_kstring))); return device; } -UNMAP_AFTER_INIT NVMeNameSpace::NVMeNameSpace(NonnullRefPtrVector queues, size_t max_addresable_block, size_t lba_size, size_t major_number, size_t minor_number, u16 nsid, NonnullOwnPtr dev_name) - : StorageDevice(major_number, minor_number, lba_size, max_addresable_block, move(dev_name)) +UNMAP_AFTER_INIT NVMeNameSpace::NVMeNameSpace(LUNAddress logical_unit_number_address, NonnullRefPtrVector queues, size_t max_addresable_block, size_t lba_size, size_t major_number, size_t minor_number, u16 nsid, NonnullOwnPtr dev_name) + : StorageDevice(logical_unit_number_address, major_number, minor_number, lba_size, max_addresable_block, move(dev_name)) , m_nsid(nsid) , m_queues(move(queues)) { diff --git a/Kernel/Storage/NVMe/NVMeNameSpace.h b/Kernel/Storage/NVMe/NVMeNameSpace.h index e14a2b79df8..6a1a1b10aa6 100644 --- a/Kernel/Storage/NVMe/NVMeNameSpace.h +++ b/Kernel/Storage/NVMe/NVMeNameSpace.h @@ -19,17 +19,19 @@ #include namespace Kernel { + +class NVMeController; class NVMeNameSpace : public StorageDevice { friend class DeviceManagement; public: - static ErrorOr> try_create(NonnullRefPtrVector queues, u8 controller_id, u16 nsid, size_t storage_size, size_t lba_size); + static ErrorOr> try_create(NVMeController const&, NonnullRefPtrVector queues, u8 controller_id, u16 nsid, size_t storage_size, size_t lba_size); CommandSet command_set() const override { return CommandSet::NVMe; }; void start_request(AsyncBlockDeviceRequest& request) override; private: - NVMeNameSpace(NonnullRefPtrVector queues, size_t storage_size, size_t lba_size, size_t major_number, size_t minor_number, u16 nsid, NonnullOwnPtr early_device_name); + NVMeNameSpace(LUNAddress, NonnullRefPtrVector queues, size_t storage_size, size_t lba_size, size_t major_number, size_t minor_number, u16 nsid, NonnullOwnPtr early_device_name); virtual InterfaceType interface_type() const override { return InterfaceType::NVMe; } u16 m_nsid; diff --git a/Kernel/Storage/Ramdisk/Device.cpp b/Kernel/Storage/Ramdisk/Device.cpp index 5d4689a3592..ad173c41204 100644 --- a/Kernel/Storage/Ramdisk/Device.cpp +++ b/Kernel/Storage/Ramdisk/Device.cpp @@ -25,8 +25,8 @@ NonnullRefPtr RamdiskDevice::create(RamdiskController const& cont return device_or_error.release_value(); } -RamdiskDevice::RamdiskDevice(RamdiskController const&, NonnullOwnPtr&& region, int major, int minor, NonnullOwnPtr device_name) - : StorageDevice(major, minor, 512, region->size() / 512, move(device_name)) +RamdiskDevice::RamdiskDevice(RamdiskController const& controller, NonnullOwnPtr&& region, int major, int minor, NonnullOwnPtr device_name) + : StorageDevice(LUNAddress { controller.controller_id(), 0, 0 }, major, minor, 512, region->size() / 512, move(device_name)) , m_region(move(region)) { dmesgln("Ramdisk: Device #{} @ {}, Capacity={}", minor, m_region->vaddr(), max_addressable_block() * 512); diff --git a/Kernel/Storage/StorageController.cpp b/Kernel/Storage/StorageController.cpp new file mode 100644 index 00000000000..739c1ec1ae4 --- /dev/null +++ b/Kernel/Storage/StorageController.cpp @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2022, Liav A. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +namespace Kernel { + +StorageController::StorageController() + : m_controller_id(StorageManagement::generate_controller_id()) +{ +} + +} diff --git a/Kernel/Storage/StorageController.h b/Kernel/Storage/StorageController.h index e99674abdf6..769441adda9 100644 --- a/Kernel/Storage/StorageController.h +++ b/Kernel/Storage/StorageController.h @@ -29,10 +29,17 @@ public: virtual RefPtr device(u32 index) const = 0; virtual size_t devices_count() const = 0; + u32 controller_id() const { return m_controller_id; } + protected: virtual bool reset() = 0; virtual bool shutdown() = 0; virtual void complete_current_request(AsyncDeviceRequest::RequestResult) = 0; + + StorageController(); + +private: + u32 const m_controller_id { 0 }; }; } diff --git a/Kernel/Storage/StorageDevice.cpp b/Kernel/Storage/StorageDevice.cpp index 6c8cc784cbb..8ca455ea81a 100644 --- a/Kernel/Storage/StorageDevice.cpp +++ b/Kernel/Storage/StorageDevice.cpp @@ -14,9 +14,10 @@ namespace Kernel { -StorageDevice::StorageDevice(MajorNumber major, MinorNumber minor, size_t sector_size, u64 max_addressable_block, NonnullOwnPtr device_name) +StorageDevice::StorageDevice(LUNAddress logical_unit_number_address, MajorNumber major, MinorNumber minor, size_t sector_size, u64 max_addressable_block, NonnullOwnPtr device_name) : BlockDevice(major, minor, sector_size) , m_early_storage_device_name(move(device_name)) + , m_logical_unit_number_address(logical_unit_number_address) , m_max_addressable_block(max_addressable_block) , m_blocks_per_page(PAGE_SIZE / block_size()) { diff --git a/Kernel/Storage/StorageDevice.h b/Kernel/Storage/StorageDevice.h index a7747b1a119..eb06214a804 100644 --- a/Kernel/Storage/StorageDevice.h +++ b/Kernel/Storage/StorageDevice.h @@ -45,6 +45,20 @@ public: NVMe, }; + // Note: The most reliable way to address this device from userspace interfaces, + // such as SysFS, is to have one way to enumerate everything in the eyes of userspace. + // Therefore, SCSI LUN (logical unit number) addressing seem to be the most generic way to do this. + // For example, on a legacy ATA instance, one might connect an harddrive to the second IDE controller, + // to the Primary channel as a slave device, which translates to LUN 1:0:1. + // On NVMe, for example, connecting a second PCIe NVMe storage device as a sole NVMe namespace translates + // to LUN 1:0:0. + // TODO: LUNs are also useful also when specifying the boot drive on boot. Consider doing that. + struct LUNAddress { + u32 controller_id; + u32 target_id; + u32 disk_id; + }; + public: virtual u64 max_addressable_block() const { return m_max_addressable_block; } @@ -62,6 +76,8 @@ public: void add_partition(NonnullRefPtr disk_partition) { MUST(m_partitions.try_append(disk_partition)); } + LUNAddress const& logical_unit_number_address() const { return m_logical_unit_number_address; } + virtual CommandSet command_set() const = 0; StringView interface_type_to_string_view() const; @@ -71,7 +87,7 @@ public: virtual ErrorOr ioctl(OpenFileDescription&, unsigned request, Userspace arg) final; protected: - StorageDevice(MajorNumber, MinorNumber, size_t, u64, NonnullOwnPtr); + StorageDevice(LUNAddress, MajorNumber, MinorNumber, size_t, u64, NonnullOwnPtr); // ^DiskDevice virtual StringView class_name() const override; @@ -82,6 +98,7 @@ private: // FIXME: Remove this method after figuring out another scheme for naming. NonnullOwnPtr m_early_storage_device_name; + LUNAddress const m_logical_unit_number_address; u64 m_max_addressable_block { 0 }; size_t m_blocks_per_page { 0 }; }; diff --git a/Kernel/Storage/StorageManagement.cpp b/Kernel/Storage/StorageManagement.cpp index 2da6379b865..9f173ae8643 100644 --- a/Kernel/Storage/StorageManagement.cpp +++ b/Kernel/Storage/StorageManagement.cpp @@ -30,6 +30,7 @@ namespace Kernel { static Singleton s_the; static Atomic s_device_minor_number; +static Atomic s_controller_id; static constexpr StringView partition_uuid_prefix = "PARTUUID:"sv; @@ -244,6 +245,13 @@ MinorNumber StorageManagement::generate_storage_minor_number() return minor_number; } +u32 StorageManagement::generate_controller_id() +{ + auto controller_id = s_controller_id.load(); + s_controller_id++; + return controller_id; +} + NonnullRefPtr StorageManagement::root_filesystem() const { auto boot_device_description = boot_block_device(); diff --git a/Kernel/Storage/StorageManagement.h b/Kernel/Storage/StorageManagement.h index 475a53540d8..e4a85b59e30 100644 --- a/Kernel/Storage/StorageManagement.h +++ b/Kernel/Storage/StorageManagement.h @@ -31,6 +31,8 @@ public: static MajorNumber storage_type_major_number(); static MinorNumber generate_storage_minor_number(); + static u32 generate_controller_id(); + void remove_device(StorageDevice&); private: