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

