1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH v1] MdeModulePkg: Add a check for metadata size in NvmExpress Driver

Ma, Hua posted 1 patch 2 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c   | 15 +++++++++++++++
.../Bus/Pci/NvmExpressPei/NvmExpressPei.c         | 15 +++++++++++++++
2 files changed, 30 insertions(+)
[edk2-devel] [PATCH v1] MdeModulePkg: Add a check for metadata size in NvmExpress Driver
Posted by Ma, Hua 2 years, 2 months ago 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3856

Currently this NvmeExpress Driver do not support metadata handling.
According to the NVME specs, metadata may be transferred to the host after
the logical block data. It can overrun the input buffer which may only
be the size of logical block data.

Add a check to return not support for the namespaces formatted with
metadata.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Hua Ma <hua.ma@intel.com>
---
 MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c   | 15 +++++++++++++++
 .../Bus/Pci/NvmExpressPei/NvmExpressPei.c         | 15 +++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index 5a1eda8e8d..46b7dcba20 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -139,6 +139,21 @@ EnumerateNvmeDevNamespace (
 
     Flbas                   = NamespaceData->Flbas;
     LbaFmtIdx               = Flbas & 0xF;
+
+    //
+    // Currently this NVME driver only suport Metadata Size == 0
+    //
+    if (NamespaceData->LbaFormat[LbaFmtIdx].Ms) {
+      DEBUG ((
+        DEBUG_INFO,
+        "NVME IDENTIFY NAMESPACE [%d] Ms(%d) is not supported.\n",
+        NamespaceId,
+        NamespaceData->LbaFormat[LbaFmtIdx].Ms
+        ));
+      Status = EFI_UNSUPPORTED;
+      goto Exit;
+    }
+
     Lbads                   = NamespaceData->LbaFormat[LbaFmtIdx].Lbads;
     Device->Media.BlockSize = (UINT32)1 << Lbads;
 
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
index f73053fc3f..6e27950648 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
@@ -104,6 +104,21 @@ EnumerateNvmeDevNamespace (
   //
   Flbas     = NamespaceData->Flbas;
   LbaFmtIdx = Flbas & 0xF;
+
+  //
+  // Currently this NVME driver only suport Metadata Size == 0
+  //
+  if (NamespaceData->LbaFormat[LbaFmtIdx].Ms) {
+    DEBUG ((
+      DEBUG_INFO,
+      "NVME IDENTIFY NAMESPACE [%d] Ms(%d) is not supported.\n",
+      NamespaceId,
+      NamespaceData->LbaFormat[LbaFmtIdx].Ms
+      ));
+    Status = EFI_UNSUPPORTED;
+    goto Exit;
+  }
+
   Lbads     = NamespaceData->LbaFormat[LbaFmtIdx].Lbads;
 
   NamespaceInfo->Media.InterfaceType  = MSG_NVME_NAMESPACE_DP;
-- 
2.32.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87242): https://edk2.groups.io/g/devel/message/87242
Mute This Topic: https://groups.io/mt/89517497/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v1] MdeModulePkg: Add a check for metadata size in NvmExpress Driver
Posted by Wu, Hao A 2 years, 2 months ago 
Thanks for the patch, a couple of inline comments below:


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ma, Hua
> Sent: Thursday, March 3, 2022 10:06 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Ma, Hua <hua.ma@intel.com>
> Subject: [edk2-devel] [PATCH v1] MdeModulePkg: Add a check for metadata
> size in NvmExpress Driver
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3856
> 
> Currently this NvmeExpress Driver do not support metadata handling.
> According to the NVME specs, metadata may be transferred to the host after
> the logical block data. It can overrun the input buffer which may only be the
> size of logical block data.
> 
> Add a check to return not support for the namespaces formatted with
> metadata.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> 
> Signed-off-by: Hua Ma <hua.ma@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c   | 15
> +++++++++++++++
>  .../Bus/Pci/NvmExpressPei/NvmExpressPei.c         | 15 +++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> index 5a1eda8e8d..46b7dcba20 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> @@ -139,6 +139,21 @@ EnumerateNvmeDevNamespace (
> 
>      Flbas                   = NamespaceData->Flbas;
>      LbaFmtIdx               = Flbas & 0xF;
> +
> +    //
> +    // Currently this NVME driver only suport Metadata Size == 0
> +    //
> +    if (NamespaceData->LbaFormat[LbaFmtIdx].Ms) {


1. Please help to update to:
if (NamespaceData->LbaFormat[LbaFmtIdx].Ms != 0) {
(Similar comment applies to NvmExpressPei)


> +      DEBUG ((
> +        DEBUG_INFO,


2. My preference is to use DEBUG_ERROR level here. Could you help to update debug output level?
(Similar comment applies to NvmExpressPei)

Best Regards,
Hao Wu


> +        "NVME IDENTIFY NAMESPACE [%d] Ms(%d) is not supported.\n",
> +        NamespaceId,
> +        NamespaceData->LbaFormat[LbaFmtIdx].Ms
> +        ));
> +      Status = EFI_UNSUPPORTED;
> +      goto Exit;
> +    }
> +
>      Lbads                   = NamespaceData->LbaFormat[LbaFmtIdx].Lbads;
>      Device->Media.BlockSize = (UINT32)1 << Lbads;
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> index f73053fc3f..6e27950648 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
> @@ -104,6 +104,21 @@ EnumerateNvmeDevNamespace (
>    //
>    Flbas     = NamespaceData->Flbas;
>    LbaFmtIdx = Flbas & 0xF;
> +
> +  //
> +  // Currently this NVME driver only suport Metadata Size == 0  //  if
> + (NamespaceData->LbaFormat[LbaFmtIdx].Ms) {
> +    DEBUG ((
> +      DEBUG_INFO,
> +      "NVME IDENTIFY NAMESPACE [%d] Ms(%d) is not supported.\n",
> +      NamespaceId,
> +      NamespaceData->LbaFormat[LbaFmtIdx].Ms
> +      ));
> +    Status = EFI_UNSUPPORTED;
> +    goto Exit;
> +  }
> +
>    Lbads     = NamespaceData->LbaFormat[LbaFmtIdx].Lbads;
> 
>    NamespaceInfo->Media.InterfaceType  = MSG_NVME_NAMESPACE_DP;
> --
> 2.32.0.windows.2
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87243): https://edk2.groups.io/g/devel/message/87243
Mute This Topic: https://groups.io/mt/89517497/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.