The patch series is good to me. Please make the patch subjects and description more clear, as Laszlo's comments. And also need to check the RetVal descriptions in your new-added functions.
Best Regards & Thanks, LONG, Qin > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Tuesday, November 3, 2015 4:55 PM > To: Zhang, Chao B > Cc: [email protected]; Zeng, Star; Long, Qin > Subject: Re: [edk2] [PATCH 0/5] Enable ECR1263 Customized Secure Boot > > On 11/03/15 08:34, Zhang, Chao B wrote: > > Enable ECR1263 Customized Secure Boot. Update Variable driver, > > AuthenticationLib & Secure Boot UI accordingly. > > > > Zhang, Chao B (5): > > SecurityPkg: Enable Customized Secure Boot feature > > SecurityPkg: Enable Customized Secure Boot feature > > SecurityPkg: Enable Customized Secure Boot feature > > SecurityPkg: Enable Customized Secure Boot feature > > SecurityPkg: Enable Customized Secure Boot feature > > > > .../VarCheckUefiLib/VarCheckUefiLibNullClass.c | 22 + > > MdePkg/Include/Guid/GlobalVariable.h | 14 + > > MdePkg/Include/Guid/ImageAuthentication.h | 9 +- > > .../Include/Guid/AuthenticatedVariableFormat.h | 1 + > > SecurityPkg/Library/AuthVariableLib/AuthService.c | 1221 > > +++++++++++++++++--- > > .../Library/AuthVariableLib/AuthServiceInternal.h | 73 ++ > > .../Library/AuthVariableLib/AuthVariableLib.c | 153 ++- > > .../Library/AuthVariableLib/AuthVariableLib.inf | 4 + > > .../DxeImageVerificationLib.c | 669 ++++++++++- > > SecurityPkg/SecurityPkg.dec | 6 +- > > .../SecureBootConfigDxe/SecureBootConfig.vfr | 77 +- > > .../SecureBootConfigDxe/SecureBootConfigImpl.c | 427 ++++++- > > .../SecureBootConfigDxe/SecureBootConfigNvData.h | 11 +- > > .../SecureBootConfigStrings.uni | Bin 13086 -> 14876 > > bytes > > 14 files changed, 2366 insertions(+), 321 deletions(-) > > > > I won't try to review this series, but I'll comment on the subjects. > They should be something like: > > MdeModulePkg: VarCheckUefiLib: Add VarCheck for AuditMode, DeployedMode > MdePkg: Add AuditMode, DeployedMode variable names and values > SecurityPkg: Introduce gEfiSecureBootModeGuid > SecurityPkg: Implement Customized Secure Boot > SecurityPkg: SecureBootConfigDxe: Expose Customized Secure Boot > > Furthermore, in addition to naming "UEFI2.5 ECR1263" in each commit > message (which is great), please consider linking the Mantis ticket too. > > Thanks > Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
