Sorry, I have an additional comment about the commit log title.

I would suggest you to describe the problem you solved rather than the action 
you made. It would be more straightforward for others.

If you refine the log title like this, you have my R-b.

Reviewed-by: Feng Tian <[email protected]>

Thanks
Feng

-----Original Message-----
From: Tian, Feng 
Sent: Wednesday, June 1, 2016 4:53 PM
To: Wu, Hao A <[email protected]>; [email protected]
Cc: Tian, Feng <[email protected]>
Subject: RE: [PATCH] MdeModulePkg NvmExpressDxe: Add check before creating an 
aync IO queue

Reviewed-by: Feng Tian <[email protected]>

Thanks
Feng

-----Original Message-----
From: Wu, Hao A 
Sent: Wednesday, June 1, 2016 4:48 PM
To: [email protected]
Cc: Wu, Hao A <[email protected]>; Tian, Feng <[email protected]>
Subject: [PATCH] MdeModulePkg NvmExpressDxe: Add check before creating an aync 
IO queue

The Maximum Queue Entries Supported (MQES) field in the CAP (Controller
Capabilities) register for a NVMe controller restrict the maximum individual 
queue size that the controller supports.

The origin code does not check this value and always uses a hardcode value when 
creating I/O submission/completion queues for asynchronous transmission. The 
hardcode value might be larger than the MQES field, this will lead to an 
'Invalid Queue Size' error when creating I/O submission/completion queues.

The patch will add checks to make sure proper queue size is passed when 
creating I/O submission/completion queues.

Cc: Feng Tian <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <[email protected]>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 26 ++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
index 4f83a92..a173504 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
@@ -683,6 +683,7 @@ NvmeCreateIoCompletionQueue (
   EFI_STATUS                               Status;
   NVME_ADMIN_CRIOCQ                        CrIoCq;
   UINT32                                   Index;
+  UINT16                                   QueueSize;
 
   Status = EFI_SUCCESS;
 
@@ -701,8 +702,18 @@ NvmeCreateIoCompletionQueue (
     CommandPacket.CommandTimeout = NVME_GENERIC_TIMEOUT;
     CommandPacket.QueueType      = NVME_ADMIN_QUEUE;
 
+    if (Index == 1) {
+      QueueSize = NVME_CCQ_SIZE;
+    } else {
+      if (Private->Cap.Mqes > NVME_ASYNC_CCQ_SIZE) {
+        QueueSize = NVME_ASYNC_CCQ_SIZE;
+      } else {
+        QueueSize = Private->Cap.Mqes;
+      }
+    }
+
     CrIoCq.Qid   = Index;
-    CrIoCq.Qsize = (Index == 1) ? NVME_CCQ_SIZE : NVME_ASYNC_CCQ_SIZE;
+    CrIoCq.Qsize = QueueSize;
     CrIoCq.Pc    = 1;
     CopyMem (&CommandPacket.NvmeCmd->Cdw10, &CrIoCq, sizeof 
(NVME_ADMIN_CRIOCQ));
     CommandPacket.NvmeCmd->Flags = CDW10_VALID | CDW11_VALID; @@ -741,6 +752,7 
@@ NvmeCreateIoSubmissionQueue (
   EFI_STATUS                               Status;
   NVME_ADMIN_CRIOSQ                        CrIoSq;
   UINT32                                   Index;
+  UINT16                                   QueueSize;
 
   Status = EFI_SUCCESS;
 
@@ -759,8 +771,18 @@ NvmeCreateIoSubmissionQueue (
     CommandPacket.CommandTimeout = NVME_GENERIC_TIMEOUT;
     CommandPacket.QueueType      = NVME_ADMIN_QUEUE;
 
+    if (Index == 1) {
+      QueueSize = NVME_CSQ_SIZE;
+    } else {
+      if (Private->Cap.Mqes > NVME_ASYNC_CSQ_SIZE) {
+        QueueSize = NVME_ASYNC_CSQ_SIZE;
+      } else {
+        QueueSize = Private->Cap.Mqes;
+      }
+    }
+
     CrIoSq.Qid   = Index;
-    CrIoSq.Qsize = (Index == 1) ? NVME_CSQ_SIZE : NVME_ASYNC_CSQ_SIZE;
+    CrIoSq.Qsize = QueueSize;
     CrIoSq.Pc    = 1;
     CrIoSq.Cqid  = Index;
     CrIoSq.Qprio = 0;
--
1.9.5.msysgit.0

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to