Re: [PATCH v2] MdeModulePkg: Add a check for metadata size in NvmExpress Driver

["Wu, Hao A" <hao.a.wu@intel.com>]

Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: Ma, Hua <hua.ma@intel.com>
> Sent: Thursday, March 3, 2022 1:06 PM
> 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: [PATCH v2] 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.
> 
> v2 changes:
>  - Change debug log level from INFO to ERROR
>  - Change to if (NamespaceData->LbaFormat[LbaFmtIdx].Ms != 0)
> 
> v1: https://edk2.groups.io/g/devel/message/87242
> 
> 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..388583e4d5 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 != 0) {
> +      DEBUG ((
> +        DEBUG_ERROR,
> +        "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..e8a29f23c7 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 != 0) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "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