Browse Source

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.
Julian Offenhäuser 2 years ago
parent
commit
d1e88a5141

+ 5 - 5
Kernel/Storage/ATA/AHCI/Controller.cpp

@@ -25,7 +25,7 @@ UNMAP_AFTER_INIT NonnullRefPtr<AHCIController> AHCIController::initialize(PCI::D
     return controller;
     return controller;
 }
 }
 
 
-bool AHCIController::reset()
+ErrorOr<void> AHCIController::reset()
 {
 {
     dmesgln_pci(*this, "{}: AHCI controller reset", device_identifier().address());
     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!
         // Note: The HBA is locked or hung if we waited more than 1 second!
         while (true) {
         while (true) {
             if (retry > 1000)
             if (retry > 1000)
-                return false;
+                return Error::from_errno(ETIMEDOUT);
             if (!(hba().control_regs.ghc & 1))
             if (!(hba().control_regs.ghc & 1))
                 break;
                 break;
             microseconds_delay(1000);
             microseconds_delay(1000);
@@ -67,12 +67,12 @@ bool AHCIController::reset()
         m_ports[index] = port;
         m_ports[index] = port;
         port->reset();
         port->reset();
     }
     }
-    return true;
+    return {};
 }
 }
 
 
-bool AHCIController::shutdown()
+ErrorOr<void> AHCIController::shutdown()
 {
 {
-    TODO();
+    return Error::from_errno(ENOTIMPL);
 }
 }
 
 
 size_t AHCIController::devices_count() const
 size_t AHCIController::devices_count() const

+ 2 - 2
Kernel/Storage/ATA/AHCI/Controller.h

@@ -30,8 +30,8 @@ public:
     virtual StringView device_name() const override { return "AHCI"sv; }
     virtual StringView device_name() const override { return "AHCI"sv; }
 
 
     virtual LockRefPtr<StorageDevice> device(u32 index) const override;
     virtual LockRefPtr<StorageDevice> device(u32 index) const override;
-    virtual bool reset() override;
-    virtual bool shutdown() override;
+    virtual ErrorOr<void> reset() override;
+    virtual ErrorOr<void> shutdown() override;
     virtual size_t devices_count() const override;
     virtual size_t devices_count() const override;
     virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override;
     virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;

+ 4 - 4
Kernel/Storage/ATA/GenericIDE/Controller.cpp

@@ -20,14 +20,14 @@ UNMAP_AFTER_INIT NonnullLockRefPtr<IDEController> IDEController::initialize()
     return adopt_lock_ref(*new IDEController());
     return adopt_lock_ref(*new IDEController());
 }
 }
 
 
-bool IDEController::reset()
+ErrorOr<void> IDEController::reset()
 {
 {
-    TODO();
+    return Error::from_errno(ENOTIMPL);
 }
 }
 
 
-bool IDEController::shutdown()
+ErrorOr<void> IDEController::shutdown()
 {
 {
-    TODO();
+    return Error::from_errno(ENOTIMPL);
 }
 }
 
 
 size_t IDEController::devices_count() const
 size_t IDEController::devices_count() const

+ 2 - 2
Kernel/Storage/ATA/GenericIDE/Controller.h

@@ -22,8 +22,8 @@ public:
     virtual ~IDEController() override;
     virtual ~IDEController() override;
 
 
     virtual LockRefPtr<StorageDevice> device(u32 index) const override final;
     virtual LockRefPtr<StorageDevice> device(u32 index) const override final;
-    virtual bool reset() override final;
-    virtual bool shutdown() override final;
+    virtual ErrorOr<void> reset() override final;
+    virtual ErrorOr<void> shutdown() override final;
     virtual size_t devices_count() const override final;
     virtual size_t devices_count() const override final;
     virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override final;
     virtual void start_request(ATADevice const&, AsyncBlockDeviceRequest&) override final;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override final;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override final;

+ 20 - 21
Kernel/Storage/NVMe/NVMeController.cpp

@@ -87,13 +87,13 @@ bool NVMeController::wait_for_ready(bool expected_ready_bit_value)
     return true;
     return true;
 }
 }
 
 
-bool NVMeController::reset_controller()
+ErrorOr<void> NVMeController::reset_controller()
 {
 {
     if ((m_controller_regs->cc & (1 << CC_EN_BIT)) != 0) {
     if ((m_controller_regs->cc & (1 << CC_EN_BIT)) != 0) {
         // If the EN bit is already set, we need to wait
         // If the EN bit is already set, we need to wait
         // until the RDY bit is 1, otherwise the behavior is undefined
         // until the RDY bit is 1, otherwise the behavior is undefined
         if (!wait_for_ready(true))
         if (!wait_for_ready(true))
-            return false;
+            return Error::from_errno(ETIMEDOUT);
     }
     }
 
 
     auto cc = m_controller_regs->cc;
     auto cc = m_controller_regs->cc;
@@ -106,18 +106,18 @@ bool NVMeController::reset_controller()
 
 
     // Wait until the RDY bit is cleared
     // Wait until the RDY bit is cleared
     if (!wait_for_ready(false))
     if (!wait_for_ready(false))
-        return false;
+        return Error::from_errno(ETIMEDOUT);
 
 
-    return true;
+    return {};
 }
 }
 
 
-bool NVMeController::start_controller()
+ErrorOr<void> NVMeController::start_controller()
 {
 {
     if (!(m_controller_regs->cc & (1 << CC_EN_BIT))) {
     if (!(m_controller_regs->cc & (1 << CC_EN_BIT))) {
         // If the EN bit is not already set, we need to wait
         // If the EN bit is not already set, we need to wait
         // until the RDY bit is 0, otherwise the behavior is undefined
         // until the RDY bit is 0, otherwise the behavior is undefined
         if (!wait_for_ready(false))
         if (!wait_for_ready(false))
-            return false;
+            return Error::from_errno(ETIMEDOUT);
     }
     }
 
 
     auto cc = m_controller_regs->cc;
     auto cc = m_controller_regs->cc;
@@ -132,9 +132,9 @@ bool NVMeController::start_controller()
 
 
     // Wait until the RDY bit is set
     // Wait until the RDY bit is set
     if (!wait_for_ready(true))
     if (!wait_for_ready(true))
-        return false;
+        return Error::from_errno(ETIMEDOUT);
 
 
-    return true;
+    return {};
 }
 }
 
 
 UNMAP_AFTER_INIT u32 NVMeController::get_admin_q_dept()
 UNMAP_AFTER_INIT u32 NVMeController::get_admin_q_dept()
@@ -232,19 +232,16 @@ size_t NVMeController::devices_count() const
     return m_device_count;
     return m_device_count;
 }
 }
 
 
-bool NVMeController::reset()
+ErrorOr<void> 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<void> NVMeController::shutdown()
 {
 {
-    TODO();
-    return false;
+    return Error::from_errno(ENOTIMPL);
 }
 }
 
 
 void NVMeController::complete_current_request([[maybe_unused]] AsyncDeviceRequest::RequestResult result)
 void NVMeController::complete_current_request([[maybe_unused]] AsyncDeviceRequest::RequestResult result)
@@ -261,9 +258,10 @@ UNMAP_AFTER_INIT ErrorOr<void> NVMeController::create_admin_queue(Optional<u8> i
     Vector<NonnullRefPtr<Memory::PhysicalPage>> sq_dma_pages;
     Vector<NonnullRefPtr<Memory::PhysicalPage>> sq_dma_pages;
     auto cq_size = round_up_to_power_of_two(CQ_SIZE(qdepth), 4096);
     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);
     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");
         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));
         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<void> NVMeController::create_admin_queue(Optional<u8> i
     m_controller_regs->acq = reinterpret_cast<u64>(AK::convert_between_host_and_little_endian(cq_dma_pages.first()->paddr().as_ptr()));
     m_controller_regs->acq = reinterpret_cast<u64>(AK::convert_between_host_and_little_endian(cq_dma_pages.first()->paddr().as_ptr()));
     m_controller_regs->asq = reinterpret_cast<u64>(AK::convert_between_host_and_little_endian(sq_dma_pages.first()->paddr().as_ptr()));
     m_controller_regs->asq = reinterpret_cast<u64>(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");
         dmesgln_pci(*this, "Failed to restart the NVMe controller");
-        return EFAULT;
+        return maybe_error;
     }
     }
     set_admin_queue_ready_flag();
     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)));
     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)));

+ 4 - 4
Kernel/Storage/NVMe/NVMeController.h

@@ -32,13 +32,13 @@ public:
     virtual StringView device_name() const override { return "NVMeController"sv; }
     virtual StringView device_name() const override { return "NVMeController"sv; }
 
 
 protected:
 protected:
-    bool reset() override;
-    bool shutdown() override;
+    ErrorOr<void> reset() override;
+    ErrorOr<void> shutdown() override;
     void complete_current_request(AsyncDeviceRequest::RequestResult result) override;
     void complete_current_request(AsyncDeviceRequest::RequestResult result) override;
 
 
 public:
 public:
-    bool reset_controller();
-    bool start_controller();
+    ErrorOr<void> reset_controller();
+    ErrorOr<void> start_controller();
     u32 get_admin_q_dept();
     u32 get_admin_q_dept();
 
 
     u16 submit_admin_command(NVMeSubmission& sub, bool sync = false)
     u16 submit_admin_command(NVMeSubmission& sub, bool sync = false)

+ 4 - 4
Kernel/Storage/Ramdisk/Controller.cpp

@@ -16,14 +16,14 @@ ErrorOr<NonnullRefPtr<RamdiskController>> RamdiskController::try_initialize()
     return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) RamdiskController()));
     return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) RamdiskController()));
 }
 }
 
 
-bool RamdiskController::reset()
+ErrorOr<void> RamdiskController::reset()
 {
 {
-    TODO();
+    return Error::from_errno(ENOTIMPL);
 }
 }
 
 
-bool RamdiskController::shutdown()
+ErrorOr<void> RamdiskController::shutdown()
 {
 {
-    TODO();
+    return Error::from_errno(ENOTIMPL);
 }
 }
 
 
 size_t RamdiskController::devices_count() const
 size_t RamdiskController::devices_count() const

+ 2 - 2
Kernel/Storage/Ramdisk/Controller.h

@@ -23,8 +23,8 @@ public:
     virtual ~RamdiskController() override;
     virtual ~RamdiskController() override;
 
 
     virtual LockRefPtr<StorageDevice> device(u32 index) const override;
     virtual LockRefPtr<StorageDevice> device(u32 index) const override;
-    virtual bool reset() override;
-    virtual bool shutdown() override;
+    virtual ErrorOr<void> reset() override;
+    virtual ErrorOr<void> shutdown() override;
     virtual size_t devices_count() const override;
     virtual size_t devices_count() const override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) override;
 
 

+ 2 - 2
Kernel/Storage/StorageController.h

@@ -33,8 +33,8 @@ public:
     u32 hardware_relative_controller_id() const { return m_hardware_relative_controller_id; }
     u32 hardware_relative_controller_id() const { return m_hardware_relative_controller_id; }
 
 
 protected:
 protected:
-    virtual bool reset() = 0;
-    virtual bool shutdown() = 0;
+    virtual ErrorOr<void> reset() = 0;
+    virtual ErrorOr<void> shutdown() = 0;
 
 
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) = 0;
     virtual void complete_current_request(AsyncDeviceRequest::RequestResult) = 0;