From d1e88a5141502c5b63c694f089213932d2d5192e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20Offenh=C3=A4user?= Date: Tue, 14 Mar 2023 14:11:31 +0100 Subject: [PATCH] Kernel: Propagate errors in StorageController reset() and shutdown() These used to signal an error with a boolean return type. We now return a sensible errno instead. --- Kernel/Storage/ATA/AHCI/Controller.cpp | 10 ++--- Kernel/Storage/ATA/AHCI/Controller.h | 4 +- Kernel/Storage/ATA/GenericIDE/Controller.cpp | 8 ++-- Kernel/Storage/ATA/GenericIDE/Controller.h | 4 +- Kernel/Storage/NVMe/NVMeController.cpp | 41 ++++++++++---------- Kernel/Storage/NVMe/NVMeController.h | 8 ++-- Kernel/Storage/Ramdisk/Controller.cpp | 8 ++-- Kernel/Storage/Ramdisk/Controller.h | 4 +- Kernel/Storage/StorageController.h | 4 +- 9 files changed, 45 insertions(+), 46 deletions(-) diff --git a/Kernel/Storage/ATA/AHCI/Controller.cpp b/Kernel/Storage/ATA/AHCI/Controller.cpp index 3bbaa6f17a7..525c84bc0c2 100644 --- a/Kernel/Storage/ATA/AHCI/Controller.cpp +++ b/Kernel/Storage/ATA/AHCI/Controller.cpp @@ -25,7 +25,7 @@ UNMAP_AFTER_INIT NonnullRefPtr AHCIController::initialize(PCI::D return controller; } -bool AHCIController::reset() +ErrorOr AHCIController::reset() { dmesgln_pci(*this, "{}: AHCI controller reset", device_identifier().address()); { @@ -40,7 +40,7 @@ bool AHCIController::reset() // Note: The HBA is locked or hung if we waited more than 1 second! while (true) { if (retry > 1000) - return false; + return Error::from_errno(ETIMEDOUT); if (!(hba().control_regs.ghc & 1)) break; microseconds_delay(1000); @@ -67,12 +67,12 @@ bool AHCIController::reset() m_ports[index] = port; port->reset(); } - return true; + return {}; } -bool AHCIController::shutdown() +ErrorOr AHCIController::shutdown() { - TODO(); + return Error::from_errno(ENOTIMPL); } size_t AHCIController::devices_count() const diff --git a/Kernel/Storage/ATA/AHCI/Controller.h b/Kernel/Storage/ATA/AHCI/Controller.h index 9fccadcdd12..0be7064369a 100644 --- a/Kernel/Storage/ATA/AHCI/Controller.h +++ b/Kernel/Storage/ATA/AHCI/Controller.h @@ -30,8 +30,8 @@ public: virtual StringView device_name() const override { return "AHCI"sv; } virtual LockRefPtr device(u32 index) const override; - virtual bool reset() override; - virtual bool shutdown() override; + virtual ErrorOr reset() override; + virtual ErrorOr shutdown() override; virtual size_t devices_count() const override; virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override; virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override; diff --git a/Kernel/Storage/ATA/GenericIDE/Controller.cpp b/Kernel/Storage/ATA/GenericIDE/Controller.cpp index b4458d8bb6c..bca33dcb4cf 100644 --- a/Kernel/Storage/ATA/GenericIDE/Controller.cpp +++ b/Kernel/Storage/ATA/GenericIDE/Controller.cpp @@ -20,14 +20,14 @@ UNMAP_AFTER_INIT NonnullLockRefPtr IDEController::initialize() return adopt_lock_ref(*new IDEController()); } -bool IDEController::reset() +ErrorOr IDEController::reset() { - TODO(); + return Error::from_errno(ENOTIMPL); } -bool IDEController::shutdown() +ErrorOr IDEController::shutdown() { - TODO(); + return Error::from_errno(ENOTIMPL); } size_t IDEController::devices_count() const diff --git a/Kernel/Storage/ATA/GenericIDE/Controller.h b/Kernel/Storage/ATA/GenericIDE/Controller.h index 75f8907018f..e47d7662b53 100644 --- a/Kernel/Storage/ATA/GenericIDE/Controller.h +++ b/Kernel/Storage/ATA/GenericIDE/Controller.h @@ -22,8 +22,8 @@ public: virtual ~IDEController() override; virtual LockRefPtr device(u32 index) const override final; - virtual bool reset() override final; - virtual bool shutdown() override final; + virtual ErrorOr reset() override final; + virtual ErrorOr shutdown() override final; virtual size_t devices_count() const override final; virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override final; virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override final; diff --git a/Kernel/Storage/NVMe/NVMeController.cpp b/Kernel/Storage/NVMe/NVMeController.cpp index 6d848e0b36d..24d5b4657d7 100644 --- a/Kernel/Storage/NVMe/NVMeController.cpp +++ b/Kernel/Storage/NVMe/NVMeController.cpp @@ -87,13 +87,13 @@ bool NVMeController::wait_for_ready(bool expected_ready_bit_value) return true; } -bool NVMeController::reset_controller() +ErrorOr NVMeController::reset_controller() { if ((m_controller_regs->cc & (1 << CC_EN_BIT)) != 0) { // If the EN bit is already set, we need to wait // until the RDY bit is 1, otherwise the behavior is undefined if (!wait_for_ready(true)) - return false; + return Error::from_errno(ETIMEDOUT); } auto cc = m_controller_regs->cc; @@ -106,18 +106,18 @@ bool NVMeController::reset_controller() // Wait until the RDY bit is cleared if (!wait_for_ready(false)) - return false; + return Error::from_errno(ETIMEDOUT); - return true; + return {}; } -bool NVMeController::start_controller() +ErrorOr NVMeController::start_controller() { if (!(m_controller_regs->cc & (1 << CC_EN_BIT))) { // If the EN bit is not already set, we need to wait // until the RDY bit is 0, otherwise the behavior is undefined if (!wait_for_ready(false)) - return false; + return Error::from_errno(ETIMEDOUT); } auto cc = m_controller_regs->cc; @@ -132,9 +132,9 @@ bool NVMeController::start_controller() // Wait until the RDY bit is set if (!wait_for_ready(true)) - return false; + return Error::from_errno(ETIMEDOUT); - return true; + return {}; } UNMAP_AFTER_INIT u32 NVMeController::get_admin_q_dept() @@ -232,19 +232,16 @@ size_t NVMeController::devices_count() const return m_device_count; } -bool NVMeController::reset() +ErrorOr NVMeController::reset() { - if (!reset_controller()) - return false; - if (!start_controller()) - return false; - return true; + TRY(reset_controller()); + TRY(start_controller()); + return {}; } -bool NVMeController::shutdown() +ErrorOr NVMeController::shutdown() { - TODO(); - return false; + return Error::from_errno(ENOTIMPL); } void NVMeController::complete_current_request([[maybe_unused]] AsyncDeviceRequest::RequestResult result) @@ -261,9 +258,10 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::create_admin_queue(Optional i Vector> sq_dma_pages; auto cq_size = round_up_to_power_of_two(CQ_SIZE(qdepth), 4096); auto sq_size = round_up_to_power_of_two(SQ_SIZE(qdepth), 4096); - if (!reset_controller()) { + auto maybe_error = reset_controller(); + if (maybe_error.is_error()) { dmesgln_pci(*this, "Failed to reset the NVMe controller"); - return EFAULT; + return maybe_error; } { auto buffer = TRY(MM.allocate_dma_buffer_pages(cq_size, "Admin CQ queue"sv, Memory::Region::Access::ReadWrite, cq_dma_pages)); @@ -283,9 +281,10 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::create_admin_queue(Optional i m_controller_regs->acq = reinterpret_cast(AK::convert_between_host_and_little_endian(cq_dma_pages.first()->paddr().as_ptr())); m_controller_regs->asq = reinterpret_cast(AK::convert_between_host_and_little_endian(sq_dma_pages.first()->paddr().as_ptr())); - if (!start_controller()) { + maybe_error = start_controller(); + if (maybe_error.is_error()) { dmesgln_pci(*this, "Failed to restart the NVMe controller"); - return EFAULT; + return maybe_error; } set_admin_queue_ready_flag(); m_admin_queue = TRY(NVMeQueue::try_create(0, irq, qdepth, move(cq_dma_region), cq_dma_pages, move(sq_dma_region), sq_dma_pages, move(doorbell_regs))); diff --git a/Kernel/Storage/NVMe/NVMeController.h b/Kernel/Storage/NVMe/NVMeController.h index b2e27f5cb95..78787c46deb 100644 --- a/Kernel/Storage/NVMe/NVMeController.h +++ b/Kernel/Storage/NVMe/NVMeController.h @@ -32,13 +32,13 @@ public: virtual StringView device_name() const override { return "NVMeController"sv; } protected: - bool reset() override; - bool shutdown() override; + ErrorOr reset() override; + ErrorOr shutdown() override; void complete_current_request(AsyncDeviceRequest::RequestResult result) override; public: - bool reset_controller(); - bool start_controller(); + ErrorOr reset_controller(); + ErrorOr start_controller(); u32 get_admin_q_dept(); u16 submit_admin_command(NVMeSubmission& sub, bool sync = false) diff --git a/Kernel/Storage/Ramdisk/Controller.cpp b/Kernel/Storage/Ramdisk/Controller.cpp index d414b0ca28f..db6b96785ff 100644 --- a/Kernel/Storage/Ramdisk/Controller.cpp +++ b/Kernel/Storage/Ramdisk/Controller.cpp @@ -16,14 +16,14 @@ ErrorOr> RamdiskController::try_initialize() return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) RamdiskController())); } -bool RamdiskController::reset() +ErrorOr RamdiskController::reset() { - TODO(); + return Error::from_errno(ENOTIMPL); } -bool RamdiskController::shutdown() +ErrorOr RamdiskController::shutdown() { - TODO(); + return Error::from_errno(ENOTIMPL); } size_t RamdiskController::devices_count() const diff --git a/Kernel/Storage/Ramdisk/Controller.h b/Kernel/Storage/Ramdisk/Controller.h index 7f345990bf8..74d35c5f5b9 100644 --- a/Kernel/Storage/Ramdisk/Controller.h +++ b/Kernel/Storage/Ramdisk/Controller.h @@ -23,8 +23,8 @@ public: virtual ~RamdiskController() override; virtual LockRefPtr device(u32 index) const override; - virtual bool reset() override; - virtual bool shutdown() override; + virtual ErrorOr reset() override; + virtual ErrorOr shutdown() override; virtual size_t devices_count() const override; virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override; diff --git a/Kernel/Storage/StorageController.h b/Kernel/Storage/StorageController.h index bcd4465517b..70b7426cab1 100644 --- a/Kernel/Storage/StorageController.h +++ b/Kernel/Storage/StorageController.h @@ -33,8 +33,8 @@ public: u32 hardware_relative_controller_id() const { return m_hardware_relative_controller_id; } protected: - virtual bool reset() = 0; - virtual bool shutdown() = 0; + virtual ErrorOr reset() = 0; + virtual ErrorOr shutdown() = 0; virtual void complete_current_request(AsyncDeviceRequest::RequestResult) = 0;