[PATCH v2 00/10] Fix some bugs in NvmExpressDxe driver

[PATCH v2 00/10] Fix some bugs in NvmExpressDxe driver

[Hao Wu]

Compared with V1 of the series, the following changes are made:
1. Add NamespaceId validity check in
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru().

2. Fixes the issue that the caller event passed to
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru() will not be signaled for
   NVME Admin commands.

3. Set the non-blocking I/O feature support bit in structure
   EFI_NVM_EXPRESS_PASS_THRU_MODE

Hao Wu (10):
  MdeModulePkg NvmExpressDxe: Avoid crashing 'Mode' during OpenProtocol
  MdeModulePkg NvmExpressDxe: Refine BuildDevicePath API to follow spec
  MdeModulePkg NvmExpressDxe: Refine GetNameSpace API to follow spec
  MdeModulePkg NvmExpressDxe: Refine GetNextNamespace API to follow spec
  MdeModulePkg NvmExpressDxe: Add buffer alignment check in PassThru API
  MdeModulePkg NvmExpressDxe: Add check on the attributes of NVME
    controller
  MdeModulePkg NvmExpressDxe: Add check for command packet in PassThru
  MdeModulePkg NvmExpressDxe: Add NamespaceId validity check in PassThru
  MdeModulePkg NvmExpressDxe: Fix 'Event' won't be signaled for Admin
    cmds
  MdeModulePkg NvmExpressDxe: Set the non-blocking I/O feature support
    bit

 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c    |  15 ++-
 .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c     | 141 ++++++++++++++++-----
 2 files changed, 118 insertions(+), 38 deletions(-)

-- 
1.9.5.msysgit.0

[PATCH v2 01/10] MdeModulePkg NvmExpressDxe: Avoid crashing 'Mode' during OpenProtocol

[Hao Wu]

The gBS->OpenProtocol() calls to open EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL
in NvmExpress.c will crash the data in 'Mode' field of
'Private->Passthru'.

The third parameter of gBS->OpenProtocol() is an output parameter that
stores the address where a pointer to the corresponding Protocol
Interface. The current code mistakenly pass '&Private->Passthru' (a
pointer of the EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL) as the third
parameter. This will crash the data in 'Mode' filed.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index cb25b3e..255fa2b 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -76,6 +76,7 @@ EnumerateNvmeDevNamespace (
   UINT32                                LbaFmtIdx;
   UINT8                                 Sn[21];
   UINT8                                 Mn[41];
+  VOID                                  *DummyInterface;
 
   NewDevicePathNode = NULL;
   DevicePath        = NULL;
@@ -264,7 +265,7 @@ EnumerateNvmeDevNamespace (
     gBS->OpenProtocol (
            Private->ControllerHandle,
            &gEfiNvmExpressPassThruProtocolGuid,
-           (VOID **) &Private->Passthru,
+           (VOID **) &DummyInterface,
            Private->DriverBindingHandle,
            Device->DeviceHandle,
            EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
@@ -392,10 +393,10 @@ UnregisterNvmeNamespace (
   EFI_STATUS                               Status;
   EFI_BLOCK_IO_PROTOCOL                    *BlockIo;
   NVME_DEVICE_PRIVATE_DATA                 *Device;
-  NVME_CONTROLLER_PRIVATE_DATA             *Private;
   EFI_STORAGE_SECURITY_COMMAND_PROTOCOL    *StorageSecurity;
   BOOLEAN                                  IsEmpty;
   EFI_TPL                                  OldTpl;
+  VOID                                     *DummyInterface;
 
   BlockIo = NULL;
 
@@ -412,7 +413,6 @@ UnregisterNvmeNamespace (
   }
 
   Device  = NVME_DEVICE_PRIVATE_DATA_FROM_BLOCK_IO (BlockIo);
-  Private = Device->Controller;
 
   //
   // Wait for the device's asynchronous I/O queue to become empty.
@@ -460,7 +460,7 @@ UnregisterNvmeNamespace (
     gBS->OpenProtocol (
            Controller,
            &gEfiNvmExpressPassThruProtocolGuid,
-           (VOID **) &Private->Passthru,
+           (VOID **) &DummyInterface,
            This->DriverBindingHandle,
            Handle,
            EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
@@ -490,7 +490,7 @@ UnregisterNvmeNamespace (
       gBS->OpenProtocol (
         Controller,
         &gEfiNvmExpressPassThruProtocolGuid,
-        (VOID **) &Private->Passthru,
+        (VOID **) &DummyInterface,
         This->DriverBindingHandle,
         Handle,
         EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
-- 
1.9.5.msysgit.0

[PATCH v2 02/10] MdeModulePkg NvmExpressDxe: Refine BuildDevicePath API to follow spec

[Hao Wu]

According to the UEFI spec,
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.BuildDevicePath() should return
EFI_NOT_FOUND when the input NamespaceId is not valid. However, current
code returns EFI_DEVICE_ERROR instead. This commit modifies the check for
input NamespaceId to return the correct status.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 0221921..ccff4f6 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -3,7 +3,7 @@
   NVM Express specification.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
-  Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
   which accompanies this distribution.  The full text of the license may be found at
@@ -895,13 +895,17 @@ NvmExpressBuildDevicePath (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (NamespaceId == 0) {
-    return EFI_NOT_FOUND;
-  }
-
   Status  = EFI_SUCCESS;
   Private = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
 
+  //
+  // Check NamespaceId is valid or not.
+  //
+  if ((NamespaceId == 0) ||
+    (NamespaceId > Private->ControllerData->Nn)) {
+    return EFI_NOT_FOUND;
+  }
+
   Node = (NVME_NAMESPACE_DEVICE_PATH *)AllocateZeroPool (sizeof (NVME_NAMESPACE_DEVICE_PATH));
   if (Node == NULL) {
     return EFI_OUT_OF_RESOURCES;
-- 
1.9.5.msysgit.0

[PATCH v2 03/10] MdeModulePkg NvmExpressDxe: Refine GetNameSpace API to follow spec

[Hao Wu]

According to the UEFI spec,
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.GetNamespace() should return
EFI_NOT_FOUND when the input DevicePath is a device path node type that
the NVM Express Pass Thru driver supports, but there is not a valid
translation from DevicePath to a namespace ID. Current code will return
EFI_SUCCESS. This commit adds additional check in the GetNameSpace API to
make sure correct status is returned.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index ccff4f6..f0d2f5a 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -820,6 +820,7 @@ NvmExpressGetNamespace (
   )
 {
   NVME_NAMESPACE_DEVICE_PATH       *Node;
+  NVME_CONTROLLER_PRIVATE_DATA     *Private;
 
   if ((This == NULL) || (DevicePath == NULL) || (NamespaceId == NULL)) {
     return EFI_INVALID_PARAMETER;
@@ -829,13 +830,22 @@ NvmExpressGetNamespace (
     return EFI_UNSUPPORTED;
   }
 
-  Node = (NVME_NAMESPACE_DEVICE_PATH *)DevicePath;
+  Node    = (NVME_NAMESPACE_DEVICE_PATH *)DevicePath;
+  Private = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
 
   if (DevicePath->SubType == MSG_NVME_NAMESPACE_DP) {
     if (DevicePathNodeLength(DevicePath) != sizeof(NVME_NAMESPACE_DEVICE_PATH)) {
       return EFI_NOT_FOUND;
     }
 
+    //
+    // Check NamespaceId in the device path node is valid or not.
+    //
+    if ((Node->NamespaceId == 0) ||
+      (Node->NamespaceId > Private->ControllerData->Nn)) {
+      return EFI_NOT_FOUND;
+    }
+
     *NamespaceId = Node->NamespaceId;
 
     return EFI_SUCCESS;
-- 
1.9.5.msysgit.0

[PATCH v2 04/10] MdeModulePkg NvmExpressDxe: Refine GetNextNamespace API to follow spec

[Hao Wu]

According to the UEFI spec,
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.GetNextNamespace() should return
EFI_NOT_FOUND when the value pointed to by NamespaceId is the namespace ID
of the last namespace on the NVM Express controller. This commit modifies
the check for NamespaceId to follow this rule.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index f0d2f5a..ec7507e 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -758,11 +758,15 @@ NvmExpressGetNextNamespace (
 
     *NamespaceId = NextNamespaceId;
   } else {
-    if (*NamespaceId >= Private->ControllerData->Nn) {
+    if (*NamespaceId > Private->ControllerData->Nn) {
       return EFI_INVALID_PARAMETER;
     }
 
     NextNamespaceId = *NamespaceId + 1;
+    if (NextNamespaceId > Private->ControllerData->Nn) {
+      return EFI_NOT_FOUND;
+    }
+
     //
     // Allocate buffer for Identify Namespace data.
     //
-- 
1.9.5.msysgit.0

[PATCH v2 05/10] MdeModulePkg NvmExpressDxe: Add buffer alignment check in PassThru API

[Hao Wu]

According to the UEFI spec, the 'TransferBuffer' and 'MetadataBuffer' used
in a data transfer should be aligned on the boundary specified by the
IoAlign field in the EFI_NVM_EXPRESS_PASS_THRU_MODE structure.

This commit adds this check to follow the spec.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c     | 55 +++++++++++++---------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index ec7507e..6b29260 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -357,27 +357,28 @@ NvmExpressPassThru (
   IN     EFI_EVENT                                   Event OPTIONAL
   )
 {
-  NVME_CONTROLLER_PRIVATE_DATA  *Private;
-  EFI_STATUS                    Status;
-  EFI_PCI_IO_PROTOCOL           *PciIo;
-  NVME_SQ                       *Sq;
-  NVME_CQ                       *Cq;
-  UINT16                        QueueId;
-  UINT32                        Bytes;
-  UINT16                        Offset;
-  EFI_EVENT                     TimerEvent;
-  EFI_PCI_IO_PROTOCOL_OPERATION Flag;
-  EFI_PHYSICAL_ADDRESS          PhyAddr;
-  VOID                          *MapData;
-  VOID                          *MapMeta;
-  VOID                          *MapPrpList;
-  UINTN                         MapLength;
-  UINT64                        *Prp;
-  VOID                          *PrpListHost;
-  UINTN                         PrpListNo;
-  UINT32                        Data;
-  NVME_PASS_THRU_ASYNC_REQ      *AsyncRequest;
-  EFI_TPL                       OldTpl;
+  NVME_CONTROLLER_PRIVATE_DATA   *Private;
+  EFI_STATUS                     Status;
+  EFI_PCI_IO_PROTOCOL            *PciIo;
+  NVME_SQ                        *Sq;
+  NVME_CQ                        *Cq;
+  UINT16                         QueueId;
+  UINT32                         Bytes;
+  UINT16                         Offset;
+  EFI_EVENT                      TimerEvent;
+  EFI_PCI_IO_PROTOCOL_OPERATION  Flag;
+  EFI_PHYSICAL_ADDRESS           PhyAddr;
+  VOID                           *MapData;
+  VOID                           *MapMeta;
+  VOID                           *MapPrpList;
+  UINTN                          MapLength;
+  UINT64                         *Prp;
+  VOID                           *PrpListHost;
+  UINTN                          PrpListNo;
+  UINT32                         IoAlign;
+  UINT32                         Data;
+  NVME_PASS_THRU_ASYNC_REQ       *AsyncRequest;
+  EFI_TPL                        OldTpl;
 
   //
   // check the data fields in Packet parameter.
@@ -394,6 +395,18 @@ NvmExpressPassThru (
     return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // Buffer alignment check for TransferBuffer & MetadataBuffer.
+  //
+  IoAlign = This->Mode->IoAlign;
+  if (IoAlign > 0 && (((UINTN) Packet->TransferBuffer & (IoAlign - 1)) != 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (IoAlign > 0 && (((UINTN) Packet->MetadataBuffer & (IoAlign - 1)) != 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
   Private     = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
   PciIo       = Private->PciIo;
   MapData     = NULL;
-- 
1.9.5.msysgit.0

[PATCH v2 06/10] MdeModulePkg NvmExpressDxe: Add check on the attributes of NVME controller

[Hao Wu]

According to UEFI spec, an EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL with neither
EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL nor
EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL set in the Attributes field
is an illegal configuration.

This commit adds this check in the PassThru API to follow the spec.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 6b29260..c7ead21 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -375,6 +375,7 @@ NvmExpressPassThru (
   UINT64                         *Prp;
   VOID                           *PrpListHost;
   UINTN                          PrpListNo;
+  UINT32                         Attributes;
   UINT32                         IoAlign;
   UINT32                         Data;
   NVME_PASS_THRU_ASYNC_REQ       *AsyncRequest;
@@ -396,9 +397,20 @@ NvmExpressPassThru (
   }
 
   //
+  // 'Attributes' with neither EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL nor
+  // EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL set is an illegal
+  // configuration.
+  //
+  Attributes  = This->Mode->Attributes;
+  if ((Attributes & (EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL |
+    EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL)) == 0) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
   // Buffer alignment check for TransferBuffer & MetadataBuffer.
   //
-  IoAlign = This->Mode->IoAlign;
+  IoAlign     = This->Mode->IoAlign;
   if (IoAlign > 0 && (((UINTN) Packet->TransferBuffer & (IoAlign - 1)) != 0)) {
     return EFI_INVALID_PARAMETER;
   }
-- 
1.9.5.msysgit.0

[PATCH v2 07/10] MdeModulePkg NvmExpressDxe: Add check for command packet in PassThru

[Hao Wu]

This commit adds serveral checks for the 'Packet' parameter passed to the
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru() API:

The check for the 'TransferLength' field in
EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET to make sure the value will not
exceed the maximum data transfer size allowed by a controller.

The check for the 'TransferBuffer' and 'TransferLength' fields in
EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET when the Opcode of an NVME
command indicates a data transfer between controller and host.

The check for the 'MetadataLength' field in
EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET to make sure the value is not 0
when the corresponding 'MetadataBuffer' field has a non-NULL value.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c      | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index c7ead21..2209ee6 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -377,6 +377,7 @@ NvmExpressPassThru (
   UINTN                          PrpListNo;
   UINT32                         Attributes;
   UINT32                         IoAlign;
+  UINT32                         MaxTransLen;
   UINT32                         Data;
   NVME_PASS_THRU_ASYNC_REQ       *AsyncRequest;
   EFI_TPL                        OldTpl;
@@ -420,6 +421,19 @@ NvmExpressPassThru (
   }
 
   Private     = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
+
+  //
+  // Check whether TransferLength exceeds the maximum data transfer size.
+  //
+  if (Private->ControllerData->Mdts != 0) {
+    MaxTransLen = (1 << (Private->ControllerData->Mdts)) *
+                  (1 << (Private->Cap.Mpsmin + 12));
+    if (Packet->TransferLength > MaxTransLen) {
+      Packet->TransferLength = MaxTransLen;
+      return EFI_BAD_BUFFER_SIZE;
+    }
+  }
+
   PciIo       = Private->PciIo;
   MapData     = NULL;
   MapMeta     = NULL;
@@ -477,6 +491,10 @@ NvmExpressPassThru (
   // processor and a PCI Bus Master. It's caller's responsbility to ensure this.
   //
   if (((Sq->Opc & (BIT0 | BIT1)) != 0) && (Sq->Opc != NVME_ADMIN_CRIOCQ_CMD) && (Sq->Opc != NVME_ADMIN_CRIOSQ_CMD)) {
+    if ((Packet->TransferLength == 0) || (Packet->TransferBuffer == NULL)) {
+      return EFI_INVALID_PARAMETER;
+    }
+
     if ((Sq->Opc & BIT0) != 0) {
       Flag = EfiPciIoOperationBusMasterRead;
     } else {
@@ -499,8 +517,7 @@ NvmExpressPassThru (
     Sq->Prp[0] = PhyAddr;
     Sq->Prp[1] = 0;
 
-    MapLength = Packet->MetadataLength;
-    if(Packet->MetadataBuffer != NULL) {
+    if((Packet->MetadataLength != 0) && (Packet->MetadataBuffer != NULL)) {
       MapLength = Packet->MetadataLength;
       Status = PciIo->Map (
                         PciIo,
-- 
1.9.5.msysgit.0

[PATCH v2 08/10] MdeModulePkg NvmExpressDxe: Add NamespaceId validity check in PassThru

[Hao Wu]

According to the UEFI spec, EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru()
should return EFI_INVALID_PARAMETER if the input 'NamespaceId' is invalid
for the NVM Express controller. This commit adds check in PassThru() to
follow this rule.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 2209ee6..96e9d88 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -423,6 +423,14 @@ NvmExpressPassThru (
   Private     = NVME_CONTROLLER_PRIVATE_DATA_FROM_PASS_THRU (This);
 
   //
+  // Check NamespaceId is valid or not.
+  //
+  if ((NamespaceId > Private->ControllerData->Nn) &&
+      (NamespaceId != (UINT32) -1)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
   // Check whether TransferLength exceeds the maximum data transfer size.
   //
   if (Private->ControllerData->Mdts != 0) {
-- 
1.9.5.msysgit.0

[PATCH v2 09/10] MdeModulePkg NvmExpressDxe: Fix 'Event' won't be signaled for Admin cmds

[Hao Wu]

This commit fixes the issue that the caller event passed to
EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru() will not be signaled for
NVME Admin commands.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 96e9d88..2c30009 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -596,7 +596,7 @@ NvmExpressPassThru (
   //
   // Ring the submission queue doorbell.
   //
-  if (Event != NULL) {
+  if ((Event != NULL) && (QueueId != 0)) {
     Private->SqTdbl[QueueId].Sqt =
       (Private->SqTdbl[QueueId].Sqt + 1) % (NVME_ASYNC_CSQ_SIZE + 1);
   } else {
@@ -616,7 +616,7 @@ NvmExpressPassThru (
   // For non-blocking requests, return directly if the command is placed
   // in the submission queue.
   //
-  if (Event != NULL) {
+  if ((Event != NULL) && (QueueId != 0)) {
     AsyncRequest = AllocateZeroPool (sizeof (NVME_PASS_THRU_ASYNC_REQ));
     if (AsyncRequest == NULL) {
       Status = EFI_DEVICE_ERROR;
@@ -699,6 +699,15 @@ NvmExpressPassThru (
                &Data
                );
 
+  //
+  // For now, the code does not support the non-blocking feature for admin queue.
+  // If Event is not NULL for admin queue, signal the caller's event here.
+  //
+  if (Event != NULL) {
+    ASSERT (QueueId == 0);
+    gBS->SignalEvent (Event);
+  }
+
 EXIT:
   if (MapData != NULL) {
     PciIo->Unmap (
-- 
1.9.5.msysgit.0

[PATCH v2 10/10] MdeModulePkg NvmExpressDxe: Set the non-blocking I/O feature support bit

[Hao Wu]

Since current codes in NvmExpressDxe already support the non-blocking I/O
feature for EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL interface, the relative bit
in the 'Attributes' field of EFI_NVM_EXPRESS_PASS_THRU_MODE should be set
to reflect this.

Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index 255fa2b..39f49bd 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -39,7 +39,10 @@ EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL gNvmExpressDriverSupportedEfiVersion =
 // Template for NVM Express Pass Thru Mode data structure.
 //
 GLOBAL_REMOVE_IF_UNREFERENCED EFI_NVM_EXPRESS_PASS_THRU_MODE gEfiNvmExpressPassThruMode = {
-  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL | EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL | EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_CMD_SET_NVM,
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_PHYSICAL   |
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_LOGICAL    |
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_NONBLOCKIO |
+  EFI_NVM_EXPRESS_PASS_THRU_ATTRIBUTES_CMD_SET_NVM,
   sizeof (UINTN),
   0x10100
 };
-- 
1.9.5.msysgit.0

Re: [PATCH v2 00/10] Fix some bugs in NvmExpressDxe driver

[Tian, Feng]

Looks good to me

Reviewed-by: Feng Tian <feng.tian@Intel.com>

Thanks
Feng

-----Original Message-----
From: Wu, Hao A 
Sent: Thursday, September 1, 2016 10:33 AM
To: edk2-devel@lists.01.org; Tian, Feng <feng.tian@intel.com>
Cc: Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH v2 00/10] Fix some bugs in NvmExpressDxe driver

Compared with V1 of the series, the following changes are made:
1. Add NamespaceId validity check in
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru().

2. Fixes the issue that the caller event passed to
   EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL.PassThru() will not be signaled for
   NVME Admin commands.

3. Set the non-blocking I/O feature support bit in structure
   EFI_NVM_EXPRESS_PASS_THRU_MODE

Hao Wu (10):
  MdeModulePkg NvmExpressDxe: Avoid crashing 'Mode' during OpenProtocol
  MdeModulePkg NvmExpressDxe: Refine BuildDevicePath API to follow spec
  MdeModulePkg NvmExpressDxe: Refine GetNameSpace API to follow spec
  MdeModulePkg NvmExpressDxe: Refine GetNextNamespace API to follow spec
  MdeModulePkg NvmExpressDxe: Add buffer alignment check in PassThru API
  MdeModulePkg NvmExpressDxe: Add check on the attributes of NVME
    controller
  MdeModulePkg NvmExpressDxe: Add check for command packet in PassThru
  MdeModulePkg NvmExpressDxe: Add NamespaceId validity check in PassThru
  MdeModulePkg NvmExpressDxe: Fix 'Event' won't be signaled for Admin
    cmds
  MdeModulePkg NvmExpressDxe: Set the non-blocking I/O feature support
    bit

 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c    |  15 ++-
 .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c     | 141 ++++++++++++++++-----
 2 files changed, 118 insertions(+), 38 deletions(-)

-- 
1.9.5.msysgit.0