From a65b0cbe4a487b225bfb23182ed65f292263b516 Mon Sep 17 00:00:00 2001
From: Pankaj Raghav <p.raghav@samsung.com>
Date: Mon, 27 Mar 2023 15:18:08 +0200
Subject: [PATCH] Kernel/NVMeQueue: Use waitqueue in submit_sync_sqe

The current way we handle sync commands is very ugly and depends on lot
of preconditions. Now that we have an end_io handler for a request, we
can use WaitQueue to do sync commands more elegantly.

This does depend on block layer sending one request at a time but this
change is a step forward towards better IO handling.
---
 Kernel/Storage/NVMe/NVMeQueue.cpp | 22 ++++++----------------
 Kernel/Storage/NVMe/NVMeQueue.h   |  1 +
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/Kernel/Storage/NVMe/NVMeQueue.cpp b/Kernel/Storage/NVMe/NVMeQueue.cpp
index a7bf981825..c1b40a3869 100644
--- a/Kernel/Storage/NVMe/NVMeQueue.cpp
+++ b/Kernel/Storage/NVMe/NVMeQueue.cpp
@@ -105,7 +105,7 @@ void NVMeQueue::submit_sqe(NVMeSubmission& sub)
 u16 NVMeQueue::submit_sync_sqe(NVMeSubmission& sub)
 {
     // For now let's use sq tail as a unique command id.
-    u16 cqe_cid;
+    u16 cmd_status;
     u16 cid = get_request_cid();
     sub.cmdid = cid;
 
@@ -114,24 +114,14 @@ u16 NVMeQueue::submit_sync_sqe(NVMeSubmission& sub)
 
         if (m_requests.contains(sub.cmdid) && m_requests.get(sub.cmdid).release_value().used)
             VERIFY_NOT_REACHED();
-        m_requests.set(sub.cmdid, { nullptr, true, nullptr });
+        m_requests.set(sub.cmdid, { nullptr, true, [this, &cmd_status](u16 status) mutable { cmd_status = status; m_sync_wait_queue.wake_all(); } });
     }
-
     submit_sqe(sub);
-    do {
-        int index;
-        {
-            SpinlockLocker lock(m_cq_lock);
-            index = m_cq_head - 1;
-            if (index < 0)
-                index = m_qdepth - 1;
-        }
-        cqe_cid = m_cqe_array[index].command_id;
-        microseconds_delay(1);
-    } while (cid != cqe_cid);
 
-    auto status = CQ_STATUS_FIELD(m_cqe_array[m_cq_head].status);
-    return status;
+    // FIXME: Only sync submissions (usually used for admin commands) use a WaitQueue based IO. Eventually we need to
+    //  move this logic into the block layer instead of sprinkling them in the driver code.
+    m_sync_wait_queue.wait_forever("NVMe sync submit"sv);
+    return cmd_status;
 }
 
 void NVMeQueue::read(AsyncBlockDeviceRequest& request, u16 nsid, u64 index, u32 count)
diff --git a/Kernel/Storage/NVMe/NVMeQueue.h b/Kernel/Storage/NVMe/NVMeQueue.h
index e1d5b39029..2dd9cb73fc 100644
--- a/Kernel/Storage/NVMe/NVMeQueue.h
+++ b/Kernel/Storage/NVMe/NVMeQueue.h
@@ -95,6 +95,7 @@ private:
     OwnPtr<Memory::Region> m_sq_dma_region;
     Vector<NonnullRefPtr<Memory::PhysicalPage>> m_sq_dma_page;
     Span<NVMeCompletion> m_cqe_array;
+    WaitQueue m_sync_wait_queue;
     Memory::TypedMapping<DoorbellRegister volatile> m_db_regs;
     NonnullRefPtr<Memory::PhysicalPage const> m_rw_dma_page;
 };