MdeModulePkg: Add alternative queue sizes in nvme driver to support s…

…pecific devices

Add a new PCD that toggles between the old driver behavior and new behavior that uses a different hardcoded queue size to support specific devices.  Default behavior should not change.

MdeModulePkg/NvmExpressDxe: Improve NVMe controller init robustness

It has been observed that some faulty NVMe devices may return
invalid values. The code in NvmExpressDxe recognizes the controller
is not responding correctly and issues an ASSERT() often in DEBUG
builds or a reset in RELEASE builds.

The following changes are made to NvmeControllerInit() to prevent a
faulty NVMe device from halting the overall boot:

1. Check the Vendor ID and Device ID to verify they are read properly
   and if not, return EFI_DEVICE_ERROR
2. Replace the ASSERT() when Memory Page Size Minimum (Cap.Mpsmin)
   with an error message and return EFI_DEVICE_ERROR

Signed-off-by: Michael Kubacki <[email protected]>

MdeModulePkg/NvmExpressDxe: Correct function parameter modifer

Updates the `Cap` parameter for `ReadNvmeControllerCapabilities()`
to be `OUT` since the buffer pointed to is written within the
function.

Signed-off-by: Michael Kubacki <[email protected]>

MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c

@@ -578,6 +578,9 @@ ProcessAsyncTaskList (
   EFI_BLOCK_IO2_TOKEN           *Token;
   BOOLEAN                       HasNewItem;
   EFI_STATUS                    Status;
+  // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+  UINT16  QueueSize = PcdGetBool (PcdSupportAlternativeQueueSize) ?
+                      NVME_ALTERNATIVE_MAX_QUEUE_SIZE : NVME_ASYNC_CCQ_SIZE;
 
   Private    = (NVME_CONTROLLER_PRIVATE_DATA *)Context;
   QueueId    = 2;
@@ -720,7 +723,8 @@ ProcessAsyncTaskList (
     }
 
     Private->CqHdbl[QueueId].Cqh++;
-    if (Private->CqHdbl[QueueId].Cqh > MIN (NVME_ASYNC_CCQ_SIZE, Private->Cap.Mqes)) {
+    // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+    if (Private->CqHdbl[QueueId].Cqh > MIN (QueueSize, Private->Cap.Mqes)) {
       Private->CqHdbl[QueueId].Cqh = 0;
       Private->Pt[QueueId]        ^= 1;
     }
@@ -953,6 +957,9 @@ NvmExpressDriverBindingStart (
   EFI_PHYSICAL_ADDRESS                MappedAddr;
   UINTN                               Bytes;
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL  *Passthru;
+  // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+  UINTN  QueuePageCount = PcdGetBool (PcdSupportAlternativeQueueSize) ?
+                          NVME_ALTERNATIVE_TOTAL_QUEUE_BUFFER_IN_PAGES : 6;
 
   DEBUG ((DEBUG_INFO, "NvmExpressDriverBindingStart: start\n"));
 
@@ -1024,7 +1031,13 @@ NvmExpressDriverBindingStart (
       DEBUG ((DEBUG_WARN, "NvmExpressDriverBindingStart: failed to enable 64-bit DMA (%r)\n", Status));
     }
 
+    // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+
+    //
+    // Depending on PCD disablement, either support the default or alternative
+    // queue sizes.
     //
+    // Default:
     // 6 x 4kB aligned buffers will be carved out of this buffer.
     // 1st 4kB boundary is the start of the admin submission queue.
     // 2nd 4kB boundary is the start of the admin completion queue.
@@ -1035,19 +1048,31 @@ NvmExpressDriverBindingStart (
     //
     // Allocate 6 pages of memory, then map it for bus master read and write.
     //
+    // Alternative:
+    // 15 x 4kB aligned buffers will be carved out of this buffer.
+    // 1st 4kB boundary is the start of the admin submission queue.
+    // 5th 4kB boundary is the start of the admin completion queue.
+    // 6th 4kB boundary is the start of I/O submission queue #1.
+    // 10th 4kB boundary is the start of I/O completion queue #1.
+    // 11th 4kB boundary is the start of I/O submission queue #2.
+    // 15th 4kB boundary is the start of I/O completion queue #2.
+    //
+    // Allocate 15 pages of memory, then map it for bus master read and write.
+    //
     Status = PciIo->AllocateBuffer (
                       PciIo,
                       AllocateAnyPages,
                       EfiBootServicesData,
-                      6,
+                      QueuePageCount,
                       (VOID **)&Private->Buffer,
                       0
                       );
     if (EFI_ERROR (Status)) {
       goto Exit;
     }
 
-    Bytes  = EFI_PAGES_TO_SIZE (6);
+    // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+    Bytes  = EFI_PAGES_TO_SIZE (QueuePageCount);
     Status = PciIo->Map (
                       PciIo,
                       EfiPciIoOperationBusMasterCommonBuffer,
@@ -1057,7 +1082,8 @@ NvmExpressDriverBindingStart (
                       &Private->Mapping
                       );
 
-    if (EFI_ERROR (Status) || (Bytes != EFI_PAGES_TO_SIZE (6))) {
+    // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+    if (EFI_ERROR (Status) || (Bytes != EFI_PAGES_TO_SIZE (QueuePageCount))) {
       goto Exit;
     }
 
@@ -1167,7 +1193,8 @@ NvmExpressDriverBindingStart (
   }
 
   if ((Private != NULL) && (Private->Buffer != NULL)) {
-    PciIo->FreeBuffer (PciIo, 6, Private->Buffer);
+    // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+    PciIo->FreeBuffer (PciIo, QueuePageCount, Private->Buffer);
   }
 
   if ((Private != NULL) && (Private->ControllerData != NULL)) {
@@ -1243,6 +1270,9 @@ NvmExpressDriverBindingStop (
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL  *PassThru;
   BOOLEAN                             IsEmpty;
   EFI_TPL                             OldTpl;
+  // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+  UINT16  QueuePageCount = PcdGetBool (PcdSupportAlternativeQueueSize) ?
+                           NVME_ALTERNATIVE_TOTAL_QUEUE_BUFFER_IN_PAGES : 6;
 
   if (NumberOfChildren == 0) {
     Status = gBS->OpenProtocol (
@@ -1289,7 +1319,8 @@ NvmExpressDriverBindingStop (
       }
 
       if (Private->Buffer != NULL) {
-        Private->PciIo->FreeBuffer (Private->PciIo, 6, Private->Buffer);
+        // MU_CHANGE - Support alternative hardware queue sizes in NVME driver
+        Private->PciIo->FreeBuffer (Private->PciIo, QueuePageCount, Private->Buffer);
       }
 
       FreePool (Private->ControllerData);

MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h

@@ -110,6 +110,21 @@ extern EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL  gNvmExpressDriverSupportedEfiV
 #define NVME_ALL_NAMESPACES  0xFFFFFFFF
 // MU_CHANGE End - Add Media Sanitize
 
+// MU_CHANGE [BEGIN] - Support alternative hardware queue sizes in NVME driver
+
+//
+// NVMe DXE to accommodate hardware which requires queue size 255.
+// Driver supports queue size up to 255 (4 page SQ buffer).
+// DXE driver creates queue size MIN(Cap.Mqes, NVME_MAX_QUEUE_SIZE) for all queues.
+// Driver allocates queue buffer to support 255 max queue size.
+// Each submission queue buffer is allocated as 64B * 256 = 4 * 4kB = 4 pages.
+// Each completion queue buffer is allocated as 16B * 256 = 4kB = 1 page.
+//
+#define NVME_ALTERNATIVE_MAX_QUEUE_SIZE               255
+#define NVME_ALTERNATIVE_TOTAL_QUEUE_BUFFER_IN_PAGES  NVME_MAX_QUEUES * 5
+
+// MU_CHANGE [END]
+
 #define NVME_CONTROLLER_ID  0
 
 //

MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf

@@ -79,6 +79,11 @@
   gMediaSanitizeProtocolGuid                  ## PRODUCES # MU_CHANGE - Add Media Sanitize
   gEfiResetNotificationProtocolGuid           ## CONSUMES
 
+[Pcd]
+  ## MU_CHANGE [BEGIN] - Support alternative hardware queue sizes in NVME driver
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSupportAlternativeQueueSize ## CONSUMES
+  ## MU_CHANGE [END]
+
 # [Event]
 # EVENT_TYPE_RELATIVE_TIMER ## SOMETIMES_CONSUMES
 #

MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c

@@ -30,8 +30,10 @@ UINTN  mNvmeControllerNumber = 0;
 **/
 EFI_STATUS
 ReadNvmeControllerCapabilities (
-  IN NVME_CONTROLLER_PRIVATE_DATA  *Private,
-  IN NVME_CAP                      *Cap
+  // MU_CHANGE [BEGIN] - Correct Cap parameter modifier
+  IN  NVME_CONTROLLER_PRIVATE_DATA  *Private,
+  OUT NVME_CAP                      *Cap
+  // MU_CHANGE [END] - Correct Cap parameter modifier
   )
 {
   EFI_PCI_IO_PROTOCOL  *PciIo;
@@ -606,16 +608,19 @@ NvmeCreateIoCompletionQueue (
     CommandPacket.CommandTimeout = NVME_GENERIC_TIMEOUT;
     CommandPacket.QueueType      = NVME_ADMIN_QUEUE;
 
-    if (Index == 1) {
+    // MU_CHANGE [BEGIN] - Support alternative hardware queue sizes in NVME driver
+    if (PcdGetBool (PcdSupportAlternativeQueueSize)) {
+      QueueSize = MIN (NVME_ALTERNATIVE_MAX_QUEUE_SIZE, Private->Cap.Mqes);
+    } else if (Index == 1) {
       QueueSize = NVME_CCQ_SIZE;
+    } else if (Private->Cap.Mqes > NVME_ASYNC_CCQ_SIZE) {
+      QueueSize = NVME_ASYNC_CCQ_SIZE;
     } else {
-      if (Private->Cap.Mqes > NVME_ASYNC_CCQ_SIZE) {
-        QueueSize = NVME_ASYNC_CCQ_SIZE;
-      } else {
-        QueueSize = Private->Cap.Mqes;
-      }
+      QueueSize = Private->Cap.Mqes;
     }
 
+    // MU_CHANGE [END]
+
     CrIoCq.Qid   = Index;
     CrIoCq.Qsize = QueueSize;
     CrIoCq.Pc    = 1;
@@ -678,16 +683,19 @@ NvmeCreateIoSubmissionQueue (
     CommandPacket.CommandTimeout = NVME_GENERIC_TIMEOUT;
     CommandPacket.QueueType      = NVME_ADMIN_QUEUE;
 
-    if (Index == 1) {
-      QueueSize = NVME_CSQ_SIZE;
+    // MU_CHANGE [BEGIN] - Support alternative hardware queue sizes in NVME driver
+    if (PcdGetBool (PcdSupportAlternativeQueueSize)) {
+      QueueSize = MIN (NVME_ALTERNATIVE_MAX_QUEUE_SIZE, Private->Cap.Mqes);
+    } else if (Index == 1) {
+      QueueSize = NVME_CCQ_SIZE;
+    } else if (Private->Cap.Mqes > NVME_ASYNC_CCQ_SIZE) {
+      QueueSize = NVME_ASYNC_CCQ_SIZE;
     } else {
-      if (Private->Cap.Mqes > NVME_ASYNC_CSQ_SIZE) {
-        QueueSize = NVME_ASYNC_CSQ_SIZE;
-      } else {
-        QueueSize = Private->Cap.Mqes;
-      }
+      QueueSize = Private->Cap.Mqes;
     }
 
+    // MU_CHANGE [END]
+
     CrIoSq.Qid   = Index;
     CrIoSq.Qsize = QueueSize;
     CrIoSq.Pc    = 1;
@@ -732,13 +740,36 @@ NvmeControllerInit (
   NVME_AQA             Aqa;
   NVME_ASQ             Asq;
   NVME_ACQ             Acq;
+  UINT16               VidDid[2]; // MU_CHANGE - Improve NVMe controller init robustness
   UINT8                Sn[21];
   UINT8                Mn[41];
 
+  // MU_CHANGE [BEGIN] - Improve NVMe controller init robustness
+  PciIo = Private->PciIo;
+
+  //
+  // Verify the controller is still accessible
+  //
+  Status = PciIo->Pci.Read (
+                        PciIo,
+                        EfiPciIoWidthUint16,
+                        PCI_VENDOR_ID_OFFSET,
+                        ARRAY_SIZE (VidDid),
+                        VidDid
+                        );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return EFI_DEVICE_ERROR;
+  }
+
+  if ((VidDid[0] == 0xFFFF) || (VidDid[1] == 0xFFFF)) {
+    return EFI_DEVICE_ERROR;
+  }
+
   //
   // Enable this controller.
   //
-  PciIo  = Private->PciIo;
+  // MU_CHANGE [END] - Improve NVMe controller init robustness
   Status = PciIo->Attributes (
                     PciIo,
                     EfiPciIoAttributeOperationSupported,
@@ -777,7 +808,17 @@ NvmeControllerInit (
   //
   // Currently the driver only supports 4k page size.
   //
-  ASSERT ((Private->Cap.Mpsmin + 12) <= EFI_PAGE_SHIFT);
+
+  // MU_CHANGE [BEGIN] - Improve NVMe controller init robustness
+
+  // Currently, this means Cap.Mpsmin must be zero for an EFI_PAGE_SHIFT size of 12.
+  // ASSERT ((Private->Cap.Mpsmin + 12) <= EFI_PAGE_SHIFT);
+  if ((Private->Cap.Mpsmin + 12) > EFI_PAGE_SHIFT) {
+    DEBUG ((DEBUG_ERROR, "NvmeControllerInit: Mpsmin is larger than expected (0x%02x).\n", Private->Cap.Mpsmin));
+    return EFI_DEVICE_ERROR;
+  }
+
+  // MU_CHANGE [END] - Improve NVMe controller init robustness
 
   Private->Cid[0]        = 0;
   Private->Cid[1]        = 0;
@@ -802,10 +843,12 @@ NvmeControllerInit (
   //
   // set number of entries admin submission & completion queues.
   //
-  Aqa.Asqs  = NVME_ASQ_SIZE;
+  // MU_CHANGE [BEGIN] - Support alternative hardware queue sizes in NVME driver
+  Aqa.Asqs  = PcdGetBool (PcdSupportAlternativeQueueSize) ? MIN (NVME_ALTERNATIVE_MAX_QUEUE_SIZE, Private->Cap.Mqes) : NVME_ASQ_SIZE;
   Aqa.Rsvd1 = 0;
-  Aqa.Acqs  = NVME_ACQ_SIZE;
+  Aqa.Acqs  = PcdGetBool (PcdSupportAlternativeQueueSize) ? MIN (NVME_ALTERNATIVE_MAX_QUEUE_SIZE, Private->Cap.Mqes) : NVME_ACQ_SIZE;
   Aqa.Rsvd2 = 0;
+  // MU_CHANGE [END]
 
   //
   // Address of admin submission queue.
@@ -815,24 +858,47 @@ NvmeControllerInit (
   //
   // Address of admin completion queue.
   //
-  Acq = (UINT64)(UINTN)(Private->BufferPciAddr + EFI_PAGE_SIZE) & ~0xFFF;
+  // MU_CHANGE [BEGIN] - Support alternative hardware queue sizes in NVME driver
+  if (PcdGetBool (PcdSupportAlternativeQueueSize)) {
+    Acq = (UINT64)(UINTN)(Private->BufferPciAddr + 4 * EFI_PAGE_SIZE) & ~0xFFF;
+  } else {
+    Acq = (UINT64)(UINTN)(Private->BufferPciAddr + EFI_PAGE_SIZE) & ~0xFFF;
+  }
 
   //
   // Address of I/O submission & completion queue.
   //
-  ZeroMem (Private->Buffer, EFI_PAGES_TO_SIZE (6));
-  Private->SqBuffer[0]        = (NVME_SQ *)(UINTN)(Private->Buffer);
-  Private->SqBufferPciAddr[0] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr);
-  Private->CqBuffer[0]        = (NVME_CQ *)(UINTN)(Private->Buffer + 1 * EFI_PAGE_SIZE);
-  Private->CqBufferPciAddr[0] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 1 * EFI_PAGE_SIZE);
-  Private->SqBuffer[1]        = (NVME_SQ *)(UINTN)(Private->Buffer + 2 * EFI_PAGE_SIZE);
-  Private->SqBufferPciAddr[1] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr + 2 * EFI_PAGE_SIZE);
-  Private->CqBuffer[1]        = (NVME_CQ *)(UINTN)(Private->Buffer + 3 * EFI_PAGE_SIZE);
-  Private->CqBufferPciAddr[1] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 3 * EFI_PAGE_SIZE);
-  Private->SqBuffer[2]        = (NVME_SQ *)(UINTN)(Private->Buffer + 4 * EFI_PAGE_SIZE);
-  Private->SqBufferPciAddr[2] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr + 4 * EFI_PAGE_SIZE);
-  Private->CqBuffer[2]        = (NVME_CQ *)(UINTN)(Private->Buffer + 5 * EFI_PAGE_SIZE);
-  Private->CqBufferPciAddr[2] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 5 * EFI_PAGE_SIZE);
+  if (PcdGetBool (PcdSupportAlternativeQueueSize)) {
+    ZeroMem (Private->Buffer, EFI_PAGES_TO_SIZE (NVME_ALTERNATIVE_TOTAL_QUEUE_BUFFER_IN_PAGES));
+    Private->SqBuffer[0]        = (NVME_SQ *)(UINTN)(Private->Buffer);
+    Private->SqBufferPciAddr[0] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr);
+    Private->CqBuffer[0]        = (NVME_CQ *)(UINTN)(Private->Buffer + 4 * EFI_PAGE_SIZE);
+    Private->CqBufferPciAddr[0] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 4 * EFI_PAGE_SIZE);
+    Private->SqBuffer[1]        = (NVME_SQ *)(UINTN)(Private->Buffer + 5 * EFI_PAGE_SIZE);
+    Private->SqBufferPciAddr[1] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr + 5 * EFI_PAGE_SIZE);
+    Private->CqBuffer[1]        = (NVME_CQ *)(UINTN)(Private->Buffer + 9 * EFI_PAGE_SIZE);
+    Private->CqBufferPciAddr[1] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 9 * EFI_PAGE_SIZE);
+    Private->SqBuffer[2]        = (NVME_SQ *)(UINTN)(Private->Buffer + 10 * EFI_PAGE_SIZE);
+    Private->SqBufferPciAddr[2] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr + 10 * EFI_PAGE_SIZE);
+    Private->CqBuffer[2]        = (NVME_CQ *)(UINTN)(Private->Buffer + 14 * EFI_PAGE_SIZE);
+    Private->CqBufferPciAddr[2] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 14 * EFI_PAGE_SIZE);
+  } else {
+    ZeroMem (Private->Buffer, EFI_PAGES_TO_SIZE (6));
+    Private->SqBuffer[0]        = (NVME_SQ *)(UINTN)(Private->Buffer);
+    Private->SqBufferPciAddr[0] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr);
+    Private->CqBuffer[0]        = (NVME_CQ *)(UINTN)(Private->Buffer + 1 * EFI_PAGE_SIZE);
+    Private->CqBufferPciAddr[0] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 1 * EFI_PAGE_SIZE);
+    Private->SqBuffer[1]        = (NVME_SQ *)(UINTN)(Private->Buffer + 2 * EFI_PAGE_SIZE);
+    Private->SqBufferPciAddr[1] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr + 2 * EFI_PAGE_SIZE);
+    Private->CqBuffer[1]        = (NVME_CQ *)(UINTN)(Private->Buffer + 3 * EFI_PAGE_SIZE);
+    Private->CqBufferPciAddr[1] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 3 * EFI_PAGE_SIZE);
+    Private->SqBuffer[2]        = (NVME_SQ *)(UINTN)(Private->Buffer + 4 * EFI_PAGE_SIZE);
+    Private->SqBufferPciAddr[2] = (NVME_SQ *)(UINTN)(Private->BufferPciAddr + 4 * EFI_PAGE_SIZE);
+    Private->CqBuffer[2]        = (NVME_CQ *)(UINTN)(Private->Buffer + 5 * EFI_PAGE_SIZE);
+    Private->CqBufferPciAddr[2] = (NVME_CQ *)(UINTN)(Private->BufferPciAddr + 5 * EFI_PAGE_SIZE);
+  }
+
+  // MU_CHANGE [END]
 
   DEBUG ((DEBUG_INFO, "Private->Buffer = [%016X]\n", (UINT64)(UINTN)Private->Buffer));
   DEBUG ((DEBUG_INFO, "Admin     Submission Queue size (Aqa.Asqs) = [%08X]\n", Aqa.Asqs));