From 567b3a481076083a6a5ad0e443bbaca71e396e44 Mon Sep 17 00:00:00 2001 From: Pankaj Raghav Date: Sun, 16 Jan 2022 17:47:42 +0530 Subject: [PATCH] Kernel: Add individual struct definitions for NVMeSubmission Only a generic struct definition was present for NVMeSubmission. To improve type safety and clarity, added an union of NVMeSubmission structs that are applicable to the command being submitted. --- Kernel/Storage/NVMe/NVMeController.cpp | 32 +++++------ Kernel/Storage/NVMe/NVMeController.h | 2 +- Kernel/Storage/NVMe/NVMeDefinitions.h | 75 ++++++++++++++++++++++---- Kernel/Storage/NVMe/NVMeQueue.cpp | 20 ++++--- Kernel/Storage/NVMe/NVMeQueue.h | 4 +- 5 files changed, 94 insertions(+), 39 deletions(-) diff --git a/Kernel/Storage/NVMe/NVMeController.cpp b/Kernel/Storage/NVMe/NVMeController.cpp index ad72ac2c9f..4c3a28499e 100644 --- a/Kernel/Storage/NVMe/NVMeController.cpp +++ b/Kernel/Storage/NVMe/NVMeController.cpp @@ -42,6 +42,7 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::initialize() PCI::enable_bus_mastering(m_pci_device_id.address()); m_bar = PCI::get_BAR0(m_pci_device_id.address()) & BAR_ADDR_MASK; static_assert(sizeof(ControllerRegister) == REG_SQ0TDBL_START); + static_assert(sizeof(NVMeSubmission) == (1 << SQ_WIDTH)); // Map only until doorbell register for the controller // Queues will individually map the doorbell register respectively @@ -163,8 +164,8 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::identify_and_init_namespaces() NVMeSubmission sub {}; u16 status = 0; sub.op = OP_ADMIN_IDENTIFY; - sub.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(prp_dma_buffer->paddr().as_ptr())); - sub.cdw10 = NVMe_CNS_ID_ACTIVE_NS & 0xff; + sub.identify.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(prp_dma_buffer->paddr().as_ptr())); + sub.identify.cns = NVMe_CNS_ID_ACTIVE_NS & 0xff; status = submit_admin_command(sub, true); if (status) { dmesgln("Failed to identify active namespace command"); @@ -185,9 +186,9 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::identify_and_init_namespaces() if (nsid == 0) break; sub.op = OP_ADMIN_IDENTIFY; - sub.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(prp_dma_buffer->paddr().as_ptr())); - sub.cdw10 = NVMe_CNS_ID_NS & 0xff; - sub.nsid = nsid; + sub.identify.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(prp_dma_buffer->paddr().as_ptr())); + sub.identify.cns = NVMe_CNS_ID_NS & 0xff; + sub.identify.nsid = nsid; status = submit_admin_command(sub, true); if (status) { dmesgln("Failed identify namespace with nsid {}", nsid); @@ -296,7 +297,6 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::create_admin_queue(u8 irq) UNMAP_AFTER_INIT ErrorOr NVMeController::create_io_queue(u8 irq, u8 qid) { - NVMeSubmission sub {}; OwnPtr cq_dma_region; NonnullRefPtrVector cq_dma_pages; OwnPtr sq_dma_region; @@ -304,8 +304,6 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::create_io_queue(u8 irq, u8 qid) auto cq_size = round_up_to_power_of_two(CQ_SIZE(IO_QUEUE_SIZE), 4096); auto sq_size = round_up_to_power_of_two(SQ_SIZE(IO_QUEUE_SIZE), 4096); - static_assert(sizeof(NVMeSubmission) == (1 << SQ_WIDTH)); - { auto buffer = TRY(MM.allocate_dma_buffer_pages(cq_size, "IO CQ queue", Memory::Region::Access::ReadWrite, cq_dma_pages)); cq_dma_region = move(buffer); @@ -321,25 +319,29 @@ UNMAP_AFTER_INIT ErrorOr NVMeController::create_io_queue(u8 irq, u8 qid) } { + NVMeSubmission sub {}; sub.op = OP_ADMIN_CREATE_COMPLETION_QUEUE; - sub.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(cq_dma_pages.first().paddr().as_ptr())); + sub.create_cq.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(cq_dma_pages.first().paddr().as_ptr())); + sub.create_cq.cqid = qid; // The queue size is 0 based - sub.cdw10 = AK::convert_between_host_and_little_endian(((IO_QUEUE_SIZE - 1) << 16 | qid)); + sub.create_cq.qsize = AK::convert_between_host_and_little_endian(IO_QUEUE_SIZE - 1); auto flags = QUEUE_IRQ_ENABLED | QUEUE_PHY_CONTIGUOUS; // TODO: Eventually move to MSI. // For now using pin based interrupts. Clear the first 16 bits // to use pin-based interrupts. - sub.cdw11 = AK::convert_between_host_and_little_endian(flags & 0xFFFF); + sub.create_cq.cq_flags = AK::convert_between_host_and_little_endian(flags & 0xFFFF); submit_admin_command(sub, true); } { + NVMeSubmission sub {}; sub.op = OP_ADMIN_CREATE_SUBMISSION_QUEUE; - sub.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(sq_dma_pages.first().paddr().as_ptr())); + sub.create_sq.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(sq_dma_pages.first().paddr().as_ptr())); + sub.create_sq.sqid = qid; // The queue size is 0 based - sub.cdw10 = AK::convert_between_host_and_little_endian(((IO_QUEUE_SIZE - 1) << 16 | qid)); + sub.create_sq.qsize = AK::convert_between_host_and_little_endian(IO_QUEUE_SIZE - 1); auto flags = QUEUE_IRQ_ENABLED | QUEUE_PHY_CONTIGUOUS; - // The qid used below points to the completion queue qid - sub.cdw11 = AK::convert_between_host_and_little_endian(qid << 16 | flags); + sub.create_sq.cqid = qid; + sub.create_sq.sq_flags = AK::convert_between_host_and_little_endian(flags); submit_admin_command(sub, true); } diff --git a/Kernel/Storage/NVMe/NVMeController.h b/Kernel/Storage/NVMe/NVMeController.h index 48b104ec9b..bc85786c10 100644 --- a/Kernel/Storage/NVMe/NVMeController.h +++ b/Kernel/Storage/NVMe/NVMeController.h @@ -42,7 +42,7 @@ public: bool start_controller(); u32 get_admin_q_dept(); - u16 submit_admin_command(struct NVMeSubmission& sub, bool sync = false) + u16 submit_admin_command(NVMeSubmission& sub, bool sync = false) { // First queue is always the admin queue if (sync) { diff --git a/Kernel/Storage/NVMe/NVMeDefinitions.h b/Kernel/Storage/NVMe/NVMeDefinitions.h index 9d7fb356ee..5ba4911e46 100644 --- a/Kernel/Storage/NVMe/NVMeDefinitions.h +++ b/Kernel/Storage/NVMe/NVMeDefinitions.h @@ -9,9 +9,6 @@ #include #include -struct NVMeCompletion; -struct NVMeSubmission; - struct ControllerRegister { u64 cap; u32 vs; @@ -129,7 +126,7 @@ enum IOCommandOpcode { static constexpr u8 QUEUE_PHY_CONTIGUOUS = (1 << 0); static constexpr u8 QUEUE_IRQ_ENABLED = (1 << 1); -struct NVMeCompletion { +struct [[gnu::packed]] NVMeCompletion { LittleEndian cmd_spec; LittleEndian res; @@ -140,18 +137,15 @@ struct NVMeCompletion { LittleEndian status; /* did the command fail, and if so, why? */ }; -struct DataPtr { +struct [[gnu::packed]] DataPtr { LittleEndian prp1; LittleEndian prp2; }; -struct NVMeSubmission { - LittleEndian op; - LittleEndian flags; - LittleEndian cmdid; +struct [[gnu::packed]] NVMeGenericCmd { LittleEndian nsid; LittleEndian rsvd; - LittleEndian meta_ptr; + LittleEndian metadata; struct DataPtr data_ptr; LittleEndian cdw10; LittleEndian cdw11; @@ -160,3 +154,64 @@ struct NVMeSubmission { LittleEndian cdw14; LittleEndian cdw15; }; + +struct [[gnu::packed]] NVMeRWCmd { + LittleEndian nsid; + LittleEndian rsvd; + LittleEndian metadata; + struct DataPtr data_ptr; + LittleEndian slba; + LittleEndian length; + LittleEndian control; + LittleEndian dsmgmt; + LittleEndian reftag; + LittleEndian apptag; + LittleEndian appmask; +}; + +struct [[gnu::packed]] NVMeIdentifyCmd { + LittleEndian nsid; + LittleEndian rsvd1[2]; + struct DataPtr data_ptr; + u8 cns; + u8 rsvd2; + LittleEndian ctrlid; + u8 rsvd3[3]; + u8 csi; + u64 rsvd4[2]; +}; + +struct [[gnu::packed]] NVMeCreateCQCmd { + u32 rsvd1[5]; + LittleEndian prp1; + u64 rsvd2; + LittleEndian cqid; + LittleEndian qsize; + LittleEndian cq_flags; + LittleEndian irq_vector; + u64 rsvd12[2]; +}; + +struct [[gnu::packed]] NVMeCreateSQCmd { + u32 rsvd1[5]; + LittleEndian prp1; + u64 rsvd2; + LittleEndian sqid; + LittleEndian qsize; + LittleEndian sq_flags; + LittleEndian cqid; + u64 rsvd12[2]; +}; + +struct [[gnu::packed]] NVMeSubmission { + u8 op; + u8 flags; + LittleEndian cmdid; + union [[gnu::packed]] { + NVMeGenericCmd generic; + NVMeIdentifyCmd identify; + NVMeRWCmd rw; + NVMeCreateCQCmd create_cq; + NVMeCreateSQCmd create_sq; + }; +}; diff --git a/Kernel/Storage/NVMe/NVMeQueue.cpp b/Kernel/Storage/NVMe/NVMeQueue.cpp index 3f1058d104..8d5c2a4ffd 100644 --- a/Kernel/Storage/NVMe/NVMeQueue.cpp +++ b/Kernel/Storage/NVMe/NVMeQueue.cpp @@ -93,7 +93,7 @@ bool NVMeQueue::handle_irq(const RegisterState&) return nr_of_processed_cqes ? true : false; } -void NVMeQueue::submit_sqe(struct NVMeSubmission& sub) +void NVMeQueue::submit_sqe(NVMeSubmission& sub) { SpinlockLocker lock(m_sq_lock); // For now let's use sq tail as a unique command id. @@ -144,12 +144,11 @@ void NVMeQueue::read(AsyncBlockDeviceRequest& request, u16 nsid, u64 index, u32 m_current_request = request; sub.op = OP_NVME_READ; - sub.nsid = nsid; - sub.cdw10 = AK::convert_between_host_and_little_endian(index & 0xFFFFFFFF); - sub.cdw11 = AK::convert_between_host_and_little_endian(index >> 32); + sub.rw.nsid = nsid; + sub.rw.slba = AK::convert_between_host_and_little_endian(index); // No. of lbas is 0 based - sub.cdw12 = AK::convert_between_host_and_little_endian((count - 1) & 0xFFFF); - sub.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(m_rw_dma_page->paddr().as_ptr())); + sub.rw.length = AK::convert_between_host_and_little_endian((count - 1) & 0xFFFF); + sub.rw.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(m_rw_dma_page->paddr().as_ptr())); full_memory_barrier(); submit_sqe(sub); @@ -166,12 +165,11 @@ void NVMeQueue::write(AsyncBlockDeviceRequest& request, u16 nsid, u64 index, u32 return; } sub.op = OP_NVME_WRITE; - sub.nsid = nsid; - sub.cdw10 = AK::convert_between_host_and_little_endian(index & 0xFFFFFFFF); - sub.cdw11 = AK::convert_between_host_and_little_endian(index >> 32); + sub.rw.nsid = nsid; + sub.rw.slba = AK::convert_between_host_and_little_endian(index); // No. of lbas is 0 based - sub.cdw12 = AK::convert_between_host_and_little_endian((count - 1) & 0xFFFF); - sub.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(m_rw_dma_page->paddr().as_ptr())); + sub.rw.length = AK::convert_between_host_and_little_endian((count - 1) & 0xFFFF); + sub.rw.data_ptr.prp1 = reinterpret_cast(AK::convert_between_host_and_little_endian(m_rw_dma_page->paddr().as_ptr())); full_memory_barrier(); submit_sqe(sub); diff --git a/Kernel/Storage/NVMe/NVMeQueue.h b/Kernel/Storage/NVMe/NVMeQueue.h index b95ea6cf5d..86476eca3a 100644 --- a/Kernel/Storage/NVMe/NVMeQueue.h +++ b/Kernel/Storage/NVMe/NVMeQueue.h @@ -34,8 +34,8 @@ public: explicit NVMeQueue(u16 qid, u8 irq, u32 q_depth, OwnPtr cq_dma_region, NonnullRefPtrVector cq_dma_page, OwnPtr sq_dma_region, NonnullRefPtrVector sq_dma_page, Memory::TypedMapping db_regs); bool is_admin_queue() { return m_admin_queue; }; bool handle_irq(const RegisterState&) override; - void submit_sqe(struct NVMeSubmission&); - u16 submit_sync_sqe(struct NVMeSubmission&); + void submit_sqe(NVMeSubmission&); + u16 submit_sync_sqe(NVMeSubmission&); void read(AsyncBlockDeviceRequest& request, u16 nsid, u64 index, u32 count); void write(AsyncBlockDeviceRequest& request, u16 nsid, u64 index, u32 count); void enable_interrupts() { enable_irq(); };