MdeModulePkg/NvmExpressDxe: Fix some bugs

1) The Queue size field in create I/O submission/completion queue cmds is 0-based. the current code is 1-based.
2) a typo on allocated memory page size. it's inconsistent that some places is using 4 pages, but a place is using 6 pages.
3) a typo on PRP/SGL mechanism judgment. should directly use Psdt field rather than Opc field.
4) some platforms may not support UINT64 width access on MMIO register. Fix it to use two 32-bit width access.

Signed-off-by: Feng Tian <feng.tian@intel.com>
Reviewed-by: Kinney Michael <michael.d.kinney@intel.com>



git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@14657 6f19259b-4bc3-4df7-8a09-765794883524

5 files changed, 95 insertions, 38 deletions

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index b6729ce711..ed1c4bf391 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -215,6 +215,15 @@ EnumerateNvmeDevNamespace (
            );

 

     //

+    // Dump NvmExpress Identify Namespace Data

+    //

+    DEBUG ((EFI_D_INFO, " == NVME IDENTIFY NAMESPACE [%d] DATA ==\n", NamespaceId));

+    DEBUG ((EFI_D_INFO, "    NSZE        : 0x%x\n", NamespaceData->Nsze));

+    DEBUG ((EFI_D_INFO, "    NCAP        : 0x%x\n", NamespaceData->Ncap));

+    DEBUG ((EFI_D_INFO, "    NUSE        : 0x%x\n", NamespaceData->Nuse));

+    DEBUG ((EFI_D_INFO, "    LBAF0.LBADS : 0x%x\n", (NamespaceData->LbaFormat[0].Lbads)));

+

+    //

     // Build controller name for Component Name (2) protocol.

     //

     UnicodeSPrintAsciiFormat (Device->ModelName, sizeof (Device->ModelName), "%a-%a-%x", Private->ControllerData->Sn, Private->ControllerData->Mn, NamespaceData->Eui64);

@@ -657,7 +666,7 @@ NvmExpressDriverBindingStart (
                       PciIo,

                       AllocateAnyPages,

                       EfiBootServicesData,

-                      6,

+                      4,

                       (VOID**)&Private->Buffer,

                       0

                       );

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
index d39b62e102..79ab927edb 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
@@ -56,11 +56,11 @@ extern EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL  gNvmExpressDriverSupportedEfiV
 #define PCI_CLASS_MASS_STORAGE_NVM                0x08  // mass storage sub-class non-volatile memory.

 #define PCI_IF_NVMHCI                             0x02  // mass storage programming interface NVMHCI.

 

-#define NVME_ASQ_SIZE                             2     // Number of admin submission queue entries

-#define NVME_ACQ_SIZE                             2     // Number of admin completion queue entries

+#define NVME_ASQ_SIZE                             1     // Number of admin submission queue entries, which is 0-based

+#define NVME_ACQ_SIZE                             1     // Number of admin completion queue entries, which is 0-based

 

-#define NVME_CSQ_SIZE                             2     // Number of I/O submission queue entries

-#define NVME_CCQ_SIZE                             2     // Number of I/O completion queue entries

+#define NVME_CSQ_SIZE                             1     // Number of I/O submission queue entries, which is 0-based

+#define NVME_CCQ_SIZE                             1     // Number of I/O completion queue entries, which is 0-based

 

 #define NVME_MAX_IO_QUEUES                        2     // Number of I/O queues supported by the driver

 

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c
index 458afd232b..c1aad6b813 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressBlockIo.c
@@ -172,10 +172,12 @@ NvmeRead (
   UINT32                           BlockSize;

   NVME_CONTROLLER_PRIVATE_DATA     *Controller;

   UINT32                           MaxTransferBlocks;

+  UINTN                            OrginalBlocks;

 

-  Status     = EFI_SUCCESS;

-  Controller = Device->Controller;

-  BlockSize  = Device->Media.BlockSize;

+  Status        = EFI_SUCCESS;

+  Controller    = Device->Controller;

+  BlockSize     = Device->Media.BlockSize;

+  OrginalBlocks = Blocks;

 

   if (Controller->ControllerData->Mdts != 0) {

     MaxTransferBlocks = (1 << (Controller->ControllerData->Mdts)) * (1 << (Controller->Cap.Mpsmin + 12)) / BlockSize;

@@ -200,7 +202,7 @@ NvmeRead (
     }

   }

 

-  DEBUG ((EFI_D_INFO, "NvmeRead()  Lba = %8d, Blocks = %8d, BlockSize = %d Status = %r\n", Lba, Blocks, BlockSize, Status));

+  DEBUG ((EFI_D_INFO, "NvmeRead()  Lba = 0x%08x, Original = 0x%08x, Remaining = 0x%08x, BlockSize = 0x%x Status = %r\n", Lba, OrginalBlocks, Blocks, BlockSize, Status));

 

   return Status;

 }

@@ -229,10 +231,12 @@ NvmeWrite (
   UINT32                           BlockSize;

   NVME_CONTROLLER_PRIVATE_DATA     *Controller;

   UINT32                           MaxTransferBlocks;

+  UINTN                            OrginalBlocks;

 

-  Status     = EFI_SUCCESS;

-  Controller = Device->Controller;

-  BlockSize  = Device->Media.BlockSize;

+  Status        = EFI_SUCCESS;

+  Controller    = Device->Controller;

+  BlockSize     = Device->Media.BlockSize;

+  OrginalBlocks = Blocks;

 

   if (Controller->ControllerData->Mdts != 0) {

     MaxTransferBlocks = (1 << (Controller->ControllerData->Mdts)) * (1 << (Controller->Cap.Mpsmin + 12)) / BlockSize;

@@ -257,7 +261,7 @@ NvmeWrite (
     }

   }

 

-  DEBUG ((EFI_D_INFO, "NvmeWrite() Lba = %8d, Blocks = %8d, BlockSize = %d Status = %r\n", Lba, Blocks, BlockSize, Status));

+  DEBUG ((EFI_D_INFO, "NvmeWrite() Lba = 0x%08x, Original = 0x%08x, Remaining = 0x%08x, BlockSize = 0x%x Status = %r\n", Lba, OrginalBlocks, Blocks, BlockSize, Status));

 

   return Status;

 }

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
index 157e10127a..7b46870d5a 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
@@ -33,21 +33,23 @@ ReadNvmeControllerCapabilities (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT64                Data;

 

   PciIo  = Private->PciIo;

   Status = PciIo->Mem.Read (

                         PciIo,

-                        EfiPciIoWidthUint64,

+                        EfiPciIoWidthUint32,

                         NVME_BAR,

                         NVME_CAP_OFFSET,

-                        1,

-                        Cap

+                        2,

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

     return Status;

   }

 

+  WriteUnaligned64 ((UINT64*)Cap, Data);

   return EFI_SUCCESS;

 }

 

@@ -69,6 +71,7 @@ ReadNvmeControllerConfiguration (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT32                Data;

 

   PciIo  = Private->PciIo;

   Status = PciIo->Mem.Read (

@@ -77,13 +80,14 @@ ReadNvmeControllerConfiguration (
                         NVME_BAR,

                         NVME_CC_OFFSET,

                         1,

-                        Cc

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

     return Status;

   }

 

+  WriteUnaligned32 ((UINT32*)Cc, Data);

   return EFI_SUCCESS;

 }

 

@@ -105,15 +109,17 @@ WriteNvmeControllerConfiguration (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT32                Data;

 

   PciIo  = Private->PciIo;

+  Data   = ReadUnaligned32 ((UINT32*)Cc);

   Status = PciIo->Mem.Write (

                         PciIo,

                         EfiPciIoWidthUint32,

                         NVME_BAR,

                         NVME_CC_OFFSET,

                         1,

-                        Cc

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

@@ -149,6 +155,7 @@ ReadNvmeControllerStatus (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT32                Data;

 

   PciIo  = Private->PciIo;

   Status = PciIo->Mem.Read (

@@ -157,13 +164,14 @@ ReadNvmeControllerStatus (
                         NVME_BAR,

                         NVME_CSTS_OFFSET,

                         1,

-                        Csts

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

     return Status;

   }

 

+  WriteUnaligned32 ((UINT32*)Csts, Data);

   return EFI_SUCCESS;

 }

 

@@ -185,6 +193,7 @@ ReadNvmeAdminQueueAttributes (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT32                Data;

 

   PciIo  = Private->PciIo;

   Status = PciIo->Mem.Read (

@@ -193,13 +202,14 @@ ReadNvmeAdminQueueAttributes (
                         NVME_BAR,

                         NVME_AQA_OFFSET,

                         1,

-                        Aqa

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

     return Status;

   }

 

+  WriteUnaligned32 ((UINT32*)Aqa, Data);

   return EFI_SUCCESS;

 }

 

@@ -221,15 +231,17 @@ WriteNvmeAdminQueueAttributes (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT32                Data;

 

   PciIo  = Private->PciIo;

+  Data   = ReadUnaligned32 ((UINT32*)Aqa);

   Status = PciIo->Mem.Write (

                         PciIo,

                         EfiPciIoWidthUint32,

                         NVME_BAR,

                         NVME_AQA_OFFSET,

                         1,

-                        Aqa

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

@@ -260,21 +272,23 @@ ReadNvmeAdminSubmissionQueueBaseAddress (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT64                Data;

 

   PciIo  = Private->PciIo;

   Status = PciIo->Mem.Read (

                         PciIo,

-                        EfiPciIoWidthUint64,

+                        EfiPciIoWidthUint32,

                         NVME_BAR,

                         NVME_ASQ_OFFSET,

-                        1,

-                        Asq

+                        2,

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

     return Status;

   }

 

+  WriteUnaligned64 ((UINT64*)Asq, Data);

   return EFI_SUCCESS;

 }

 

@@ -296,15 +310,18 @@ WriteNvmeAdminSubmissionQueueBaseAddress (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT64                Data;

 

   PciIo  = Private->PciIo;

+  Data   = ReadUnaligned64 ((UINT64*)Asq);

+

   Status = PciIo->Mem.Write (

                         PciIo,

-                        EfiPciIoWidthUint64,

+                        EfiPciIoWidthUint32,

                         NVME_BAR,

                         NVME_ASQ_OFFSET,

-                        1,

-                        Asq

+                        2,

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

@@ -334,21 +351,24 @@ ReadNvmeAdminCompletionQueueBaseAddress (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT64                Data;

 

   PciIo  = Private->PciIo;

+

   Status = PciIo->Mem.Read (

                         PciIo,

-                        EfiPciIoWidthUint64,

+                        EfiPciIoWidthUint32,

                         NVME_BAR,

                         NVME_ACQ_OFFSET,

-                        1,

-                        Acq

+                        2,

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

     return Status;

   }

 

+  WriteUnaligned64 ((UINT64*)Acq, Data);

   return EFI_SUCCESS;

 }

 

@@ -370,15 +390,18 @@ WriteNvmeAdminCompletionQueueBaseAddress (
 {

   EFI_PCI_IO_PROTOCOL   *PciIo;

   EFI_STATUS            Status;

+  UINT64                Data;

 

   PciIo  = Private->PciIo;

+  Data   = ReadUnaligned64 ((UINT64*)Acq);

+

   Status = PciIo->Mem.Write (

                         PciIo,

-                        EfiPciIoWidthUint64,

+                        EfiPciIoWidthUint32,

                         NVME_BAR,

                         NVME_ACQ_OFFSET,

-                        1,

-                        Acq

+                        2,

+                        &Data

                         );

 

   if (EFI_ERROR(Status)) {

@@ -921,6 +944,25 @@ NvmeControllerInit (
     Private->ControllerData = NULL;

     return EFI_NOT_FOUND;

   }

+

+  //

+  // Dump NvmExpress Identify Controller Data

+  //

+  Private->ControllerData->Sn[19] = 0;

+  Private->ControllerData->Mn[39] = 0;

+  DEBUG ((EFI_D_INFO, " == NVME IDENTIFY CONTROLLER DATA ==\n"));

+  DEBUG ((EFI_D_INFO, "    PCI VID   : 0x%x\n", Private->ControllerData->Vid));

+  DEBUG ((EFI_D_INFO, "    PCI SSVID : 0x%x\n", Private->ControllerData->Ssvid));

+  DEBUG ((EFI_D_INFO, "    SN        : %a\n",   (CHAR8 *)(Private->ControllerData->Sn)));

+  DEBUG ((EFI_D_INFO, "    MN        : %a\n",   (CHAR8 *)(Private->ControllerData->Mn)));

+  DEBUG ((EFI_D_INFO, "    FR        : 0x%x\n", *((UINT64*)Private->ControllerData->Fr)));

+  DEBUG ((EFI_D_INFO, "    RAB       : 0x%x\n", Private->ControllerData->Rab));

+  DEBUG ((EFI_D_INFO, "    IEEE      : 0x%x\n", *(UINT32*)Private->ControllerData->Ieee_oiu));

+  DEBUG ((EFI_D_INFO, "    AERL      : 0x%x\n", Private->ControllerData->Aerl));

+  DEBUG ((EFI_D_INFO, "    SQES      : 0x%x\n", Private->ControllerData->Sqes));

+  DEBUG ((EFI_D_INFO, "    CQES      : 0x%x\n", Private->ControllerData->Cqes));

+  DEBUG ((EFI_D_INFO, "    NN        : 0x%x\n", Private->ControllerData->Nn));

+

   return Status;

 }

 

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index d1231aca89..4320549ed4 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -387,6 +387,7 @@ NvmExpressPassThru (
   UINT64                        *Prp;

   VOID                          *PrpListHost;

   UINTN                         PrpListNo;

+  UINT32                        Data;

 

   //

   // check the data fields in Packet parameter.

@@ -431,8 +432,8 @@ NvmExpressPassThru (
   //

   // Currently we only support PRP for data transfer, SGL is NOT supported.

   //

-  ASSERT ((Sq->Opc & BIT15) == 0);

-  if ((Sq->Opc & BIT15) != 0) {

+  ASSERT (Sq->Psdt == 0);

+  if (Sq->Psdt != 0) {

     DEBUG ((EFI_D_ERROR, "NvmExpressPassThru: doesn't support SGL mechanism\n"));

     return EFI_UNSUPPORTED;

   }

@@ -534,14 +535,14 @@ NvmExpressPassThru (
   // Ring the submission queue doorbell.

   //

   Private->SqTdbl[Qid].Sqt ^= 1;

-

+  Data = ReadUnaligned32 ((UINT32*)&Private->SqTdbl[Qid]);

   PciIo->Mem.Write (

                PciIo,

                EfiPciIoWidthUint32,

                NVME_BAR,

                NVME_SQTDBL_OFFSET(Qid, Private->Cap.Dstrd),

                1,

-               &Private->SqTdbl[Qid]

+               &Data

                );

 

   Status = gBS->CreateEvent (

@@ -591,13 +592,14 @@ NvmExpressPassThru (
     NvmeDumpStatus(Cq);

   DEBUG_CODE_END();

 

+  Data = ReadUnaligned32 ((UINT32*)&Private->CqHdbl[Qid]);

   PciIo->Mem.Write (

                PciIo,

                EfiPciIoWidthUint32,

                NVME_BAR,

                NVME_CQHDBL_OFFSET(Qid, Private->Cap.Dstrd),

                1,

-               &Private->CqHdbl[Qid]

+               &Data

                );

 

 EXIT: