On 10/18/2018 2:41 PM, Hao Wu wrote:
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1142

According to the the NVM Express spec Revision 1.1, for some commands
(like Get/Set Feature Command, Figure 89 & 90 of the spec), the Memory
Buffer maybe optional although the command opcode indicates there is a
data transfer between host & controller (Get/Set Feature Command, Figure
38 of the spec).

Hence, this commit refine the checks for the 'TransferLength' and
'TransferBuffer' field of the EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET
structure to address this issue.

Cc: Liangcheng Tang <[email protected]>
Cc: Ruiyu Ni <[email protected]>
Cc: Star Zeng <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <[email protected]>
---
  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 33 
+++++++++++---------
  1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c 
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
index 2468871322..bfcd349794 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
@@ -595,7 +595,8 @@ NvmExpressPassThru (
    //
    if (((Sq->Opc & (BIT0 | BIT1)) != 0) &&
        !((Packet->QueueType == NVME_ADMIN_QUEUE) && ((Sq->Opc == 
NVME_ADMIN_CRIOCQ_CMD) || (Sq->Opc == NVME_ADMIN_CRIOSQ_CMD)))) {
-    if ((Packet->TransferLength == 0) || (Packet->TransferBuffer == NULL)) {
+    if (((Packet->TransferLength != 0) && (Packet->TransferBuffer == NULL)) ||
+        ((Packet->TransferLength == 0) && (Packet->TransferBuffer != NULL))) {
        return EFI_INVALID_PARAMETER;
      }
@@ -605,21 +606,23 @@ NvmExpressPassThru (
        Flag = EfiPciIoOperationBusMasterWrite;
      }
- MapLength = Packet->TransferLength;
-    Status = PciIo->Map (
-                      PciIo,
-                      Flag,
-                      Packet->TransferBuffer,
-                      &MapLength,
-                      &PhyAddr,
-                      &MapData
-                      );
-    if (EFI_ERROR (Status) || (Packet->TransferLength != MapLength)) {
-      return EFI_OUT_OF_RESOURCES;
-    }
+    if ((Packet->TransferLength != 0) && (Packet->TransferBuffer != NULL)) {
+      MapLength = Packet->TransferLength;
+      Status = PciIo->Map (
+                        PciIo,
+                        Flag,
+                        Packet->TransferBuffer,
+                        &MapLength,
+                        &PhyAddr,
+                        &MapData
+                        );
+      if (EFI_ERROR (Status) || (Packet->TransferLength != MapLength)) {
+        return EFI_OUT_OF_RESOURCES;
+      }
- Sq->Prp[0] = PhyAddr;
-    Sq->Prp[1] = 0;
+      Sq->Prp[0] = PhyAddr;
+      Sq->Prp[1] = 0;
+    }
if((Packet->MetadataLength != 0) && (Packet->MetadataBuffer != NULL)) {
        MapLength = Packet->MetadataLength;

Reviewed-by: Ruiyu Ni <[email protected]>

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

Reply via email to