Kernel: Wait for NVMe controller to change enabled state

We need to wait up to CAP.TO units of 500ms when changing CC.EN to
enable or disable the controller.

3 files changed, 61 insertions, 15 deletions

diff --git a/Kernel/Storage/NVMe/NVMeController.cpp b/Kernel/Storage/NVMe/NVMeController.cpp
index 91bdbaf8c6..f52df95f02 100644
--- a/Kernel/Storage/NVMe/NVMeController.cpp
+++ b/Kernel/Storage/NVMe/NVMeController.cpp
@@ -47,11 +47,14 @@ ErrorOr<void> NVMeController::initialize()
     // Queues will individually map the doorbell register respectively
     m_controller_regs = TRY(Memory::map_typed_writable<volatile ControllerRegister>(PhysicalAddress(m_bar)));
 
+    auto caps = m_controller_regs->cap;
+    m_ready_timeout = Time::from_milliseconds(CAP_TO(caps) * 500); // CAP.TO is in 500ms units
+
     calculate_doorbell_stride();
     TRY(create_admin_queue(irq));
     VERIFY(m_admin_queue_ready == true);
 
-    VERIFY(IO_QUEUE_SIZE < MQES(m_controller_regs->cap));
+    VERIFY(IO_QUEUE_SIZE < MQES(caps));
     dbgln_if(NVME_DEBUG, "NVMe: IO queue depth is: {}", IO_QUEUE_SIZE);
 
     // Create an IO queue per core
@@ -63,14 +66,37 @@ ErrorOr<void> NVMeController::initialize()
     return {};
 }
 
+bool NVMeController::wait_for_ready(bool expected_ready_bit_value)
+{
+    static constexpr size_t one_ms_io_delay = 1000;
+    auto wait_iterations = max(1, m_ready_timeout.to_milliseconds());
+
+    u32 expected_rdy = expected_ready_bit_value ? 1 : 0;
+    while (((m_controller_regs->csts >> CSTS_RDY_BIT) & 0x1) != expected_rdy) {
+        IO::delay(one_ms_io_delay);
+
+        if (--wait_iterations == 0) {
+            if (((m_controller_regs->csts >> CSTS_RDY_BIT) & 0x1) != expected_rdy) {
+                dbgln_if(NVME_DEBUG, "NVMEController: CSTS.RDY still not set to {} after {} ms", expected_rdy, m_ready_timeout.to_milliseconds());
+                return false;
+            }
+            break;
+        }
+    }
+    return true;
+}
+
 bool NVMeController::reset_controller()
 {
-    volatile u32 cc, csts;
-    csts = m_controller_regs->csts;
-    if ((csts & (1 << CSTS_RDY_BIT)) != 0x1)
-        return false;
+    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;
+    }
+
+    auto cc = m_controller_regs->cc;
 
-    cc = m_controller_regs->cc;
     cc = cc & ~(1 << CC_EN_BIT);
 
     m_controller_regs->cc = cc;
@@ -78,8 +104,8 @@ bool NVMeController::reset_controller()
     IO::delay(10);
     full_memory_barrier();
 
-    csts = m_controller_regs->csts;
-    if ((csts & (1 << CSTS_RDY_BIT)) != 0x0)
+    // Wait until the RDY bit is cleared
+    if (!wait_for_ready(false))
         return false;
 
     return true;
@@ -87,12 +113,14 @@ bool NVMeController::reset_controller()
 
 bool NVMeController::start_controller()
 {
-    volatile u32 cc, csts;
-    csts = m_controller_regs->csts;
-    if ((csts & (1 << CSTS_RDY_BIT)) != 0x0)
-        return false;
+    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;
+    }
 
-    cc = m_controller_regs->cc;
+    auto cc = m_controller_regs->cc;
 
     cc = cc | (1 << CC_EN_BIT);
     cc = cc | (CQ_WIDTH << CC_IOCQES_BIT);
@@ -102,8 +130,9 @@ bool NVMeController::start_controller()
 
     IO::delay(10);
     full_memory_barrier();
-    csts = m_controller_regs->csts;
-    if ((csts & (1 << CSTS_RDY_BIT)) != 0x1)
+
+    // Wait until the RDY bit is set
+    if (!wait_for_ready(true))
         return false;
 
     return true;
diff --git a/Kernel/Storage/NVMe/NVMeController.h b/Kernel/Storage/NVMe/NVMeController.h
index 3ac08eb7ca..48b104ec9b 100644
--- a/Kernel/Storage/NVMe/NVMeController.h
+++ b/Kernel/Storage/NVMe/NVMeController.h
@@ -10,6 +10,7 @@
 #include <AK/NonnullRefPtrVector.h>
 #include <AK/OwnPtr.h>
 #include <AK/RefPtr.h>
+#include <AK/Time.h>
 #include <AK/Tuple.h>
 #include <AK/Types.h>
 #include <Kernel/Bus/PCI/Device.h>
@@ -63,6 +64,7 @@ private:
     {
         m_dbl_stride = (m_controller_regs->cap >> CAP_DBL_SHIFT) & CAP_DBL_MASK;
     }
+    bool wait_for_ready(bool);
 
 private:
     PCI::DeviceIdentifier m_pci_device_id;
@@ -72,6 +74,7 @@ private:
     Memory::TypedMapping<volatile ControllerRegister> m_controller_regs;
     bool m_admin_queue_ready { false };
     size_t m_device_count {};
+    AK::Time m_ready_timeout;
     u32 m_bar;
     u8 m_dbl_stride;
     static Atomic<u8> controller_id;
diff --git a/Kernel/Storage/NVMe/NVMeDefinitions.h b/Kernel/Storage/NVMe/NVMeDefinitions.h
index 5a5aece11c..9d7fb356ee 100644
--- a/Kernel/Storage/NVMe/NVMeDefinitions.h
+++ b/Kernel/Storage/NVMe/NVMeDefinitions.h
@@ -46,17 +46,31 @@ static constexpr u8 DBL_REG_SIZE = 8;
 // CAP
 static constexpr u8 CAP_DBL_SHIFT = 32;
 static constexpr u8 CAP_DBL_MASK = 0xf;
+static constexpr u8 CAP_TO_SHIFT = 24;
+static constexpr u64 CAP_TO_MASK = 0xff << CAP_TO_SHIFT;
 static constexpr u16 MQES(u64 cap)
 {
     return (cap & 0xffff) + 1;
 }
 
+static constexpr u32 CAP_TO(u64 cap)
+{
+    return (cap & CAP_TO_MASK) >> CAP_TO_SHIFT;
+}
+
 // CC – Controller Configuration
 static constexpr u8 CC_EN_BIT = 0x0;
 static constexpr u8 CSTS_RDY_BIT = 0x0;
+static constexpr u8 CSTS_SHST_SHIFT = 2;
+static constexpr u32 CSTS_SHST_MASK = 0x3 << CSTS_SHST_SHIFT;
 static constexpr u8 CC_IOSQES_BIT = 16;
 static constexpr u8 CC_IOCQES_BIT = 20;
 
+static constexpr u32 CSTS_SHST(u32 x)
+{
+    return (x & CSTS_SHST_MASK) >> CSTS_SHST_SHIFT;
+}
+
 static constexpr u16 CC_AQA_MASK = (0xfff);
 static constexpr u16 ACQ_SIZE(u32 x)
 {