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

[gaoliming via groups.io]

Hao and Weipu:
  I have no other comments for this change.

Thanks
Liming
> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Wu, Hao A
> 发送时间: 2023年2月2日 10:41
> 收件人: devel at edk2.groups.io; Zhu, Weipu <weipu.zhu at intel.com>
> 抄送: Kinney, Michael D <michael.d.kinney at intel.com>; Gao, Liming
> <gaoliming at byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu at intel.com>
> 主题: Re: [edk2-devel] [PATCH 2/2] MdePkg: Add NVMe boot partition
> header definition
> 
> Add MdePkg maintainers & reviewers.
> 
> One inline comment below:
> 
> 
> > -----Original Message-----
> > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Weipu
> > Zhu
> > Sent: Wednesday, February 1, 2023 2:27 PM
> > To: devel at edk2.groups.io
> > Cc: Zhu, Weipu <weipu.zhu at 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 at 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
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99713): https://edk2.groups.io/g/devel/message/99713
Mute This Topic: https://groups.io/mt/96798882/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-