Re: [edk2-devel] [PATCH 2/2] MdePkg: Add NVMe boot partition header definition

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

Add MdePkg maintainers & reviewers.

One inline comment below:


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Weipu
> Zhu
> Sent: Wednesday, February 1, 2023 2:27 PM
> To: devel@edk2.groups.io
> Cc: Zhu, Weipu <weipu.zhu@intel.com>
> Subject: [edk2-devel] [PATCH 2/2] MdePkg: Add NVMe boot partition header
> definition
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4304
> 
> Add NVMe boot partition header definition to NVMe.h
> according to NVMe spec 2.0.
> 
> Signed-off-by: Weipu Zhu <weipu.zhu@intel.com>
> ---
>  MdePkg/Include/IndustryStandard/Nvme.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Nvme.h
> b/MdePkg/Include/IndustryStandard/Nvme.h
> index c37cd762b9..41d40edbba 100644
> --- a/MdePkg/Include/IndustryStandard/Nvme.h
> +++ b/MdePkg/Include/IndustryStandard/Nvme.h
> @@ -546,6 +546,17 @@ typedef struct {
>  #define RPMB_RESULT_AUTHKEY_NOT_PROGRAMMED  0x07
>  #define RPMB_RESULT_INVALID_DCB             0x08
> 
> +//
> +// Get Log Page - Boot Partition Log Header.
> +// (ref. NVMe Base spec. v2.0 Figure 262).
> +//
> +typedef struct {
> +  UINT8          LogIdentifier; /* Log Identifier, shall be set to 15h */
> +  UINT8          Rsvd1[3];      /* Reserved as of Nvm Express 2.0 Spec */
> +  NVME_BPINFO    BpInfo;        /* Boot Partition Information, Brs in
> NVME_BPINFO is not available here */


I suggest to not directly use NVME_BPINFO here.

Since the 'Boot Partition Information' within 'Boot Partition Header' is not
exactly the same with the one defined for controller properties (no BRS field
like you mentioned in the comment).

How about adding field definition for 'ABPID' & 'BPSZ' instead?

Best Regards,
Hao Wu


> +  UINT8          Rsvd2[8];      /* Reserved as of Nvm Express 2.0 Spec */
> +} NVME_BOOT_PARTITION_HEADER;
> +
>  //
>  // NvmExpress Admin Identify Cmd
>  //
> --
> 2.37.1.windows.1
> 
> 
> 
> 
>