Re: [edk2-devel] 回复: [PATCH v7 00/11] Secure Boot default keys

["Ard Biesheuvel" <ardb@kernel.org>]

On Mon, 2 Aug 2021 at 09:27, Grzegorz Bernacki <gjb@semihalf.com> wrote:
>
> Hi,
>
> I took the template for function header from:
> https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/57_c_programming
> (5.7.1.10)
> It is incorrect? Where I can found the correct one?
> I run CI test locally and it did not show that kind of errors.
> Anyway, I will send a new version soon.
>

Great. Could you please also fix the spurious newlines at then end of
the .uni and .inf files in the first two patches?



>
> pon., 2 sie 2021 o 07:09 gaoliming <gaoliming@byosoft.com.cn> napisał(a):
> >
> > I see most failures are coding style. The function header comment style is /** .. **/.
> >
> > --*/ should be replaced by **/
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: Ard Biesheuvel <ardb@kernel.org>
> > > 发送时间: 2021年8月2日 2:04
> > > 收件人: Grzegorz Bernacki <gjb@semihalf.com>
> > > 抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Leif Lindholm
> > > <leif@nuviainc.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Samer
> > > El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang
> > > <sunny.Wang@arm.com>; Marcin Wojtas <mw@semihalf.com>;
> > > upstream@semihalf.com; Jiewen Yao <jiewen.yao@intel.com>; Jian J Wang
> > > <jian.j.wang@intel.com>; Min Xu <min.m.xu@intel.com>; Laszlo Ersek
> > > <lersek@redhat.com>; Sami Mujawar <sami.mujawar@arm.com>; Andrew
> > > Fish <afish@apple.com>; Ray Ni <ray.ni@intel.com>; Jordan Justen
> > > <jordan.l.justen@intel.com>; Rebecca Cran <rebecca@bsdio.com>; Peter
> > > Grehan <grehan@freebsd.org>; Thomas Abraham
> > > <thomas.abraham@arm.com>; Chasel Chiu <chasel.chiu@intel.com>; Nate
> > > DeSimone <nathaniel.l.desimone@intel.com>; Liming Gao (Byosoft address)
> > > <gaoliming@byosoft.com.cn>; Eric Dong <eric.dong@intel.com>; Michael
> > > Kinney <michael.d.kinney@intel.com>; zailiang.sun@intel.com;
> > > yi.qian@intel.com; Graeme Gregory <graeme@nuviainc.com>; Radoslaw
> > > Biernacki <rad@semihalf.com>; Peter Batard <pete@akeo.ie>
> > > 主题: Re: [PATCH v7 00/11] Secure Boot default keys
> > >
> > > On Fri, 30 Jul 2021 at 12:23, Grzegorz Bernacki <gjb@semihalf.com> wrote:
> > > >
> > > > This patchset adds support for initialization of default
> > > > Secure Boot variables based on keys content embedded in
> > > > flash binary. This feature is active only if Secure Boot
> > > > is enabled and DEFAULT_KEY is defined. The patchset
> > > > consist also application to enroll keys from default
> > > > variables and secure boot menu change to allow user
> > > > to reset key content to default values.
> > > > Discussion on design can be found at:
> > > > https://edk2.groups.io/g/rfc/topic/82139806#600
> > > >
> > > > Built with:
> > > > GCC
> > > > - RISC-V (U500, U540) [requires fixes in dsc to build]
> > > > - Intel (Vlv2TbltDevicePkg (X64/IA32), Quark, MinPlatformPkg,
> > > >   EmulatorPkg (X64), Bhyve, OvmfPkg (X64/IA32))
> > > > - ARM (Sgi75,SbsaQemu,DeveloperBox, RPi3/RPi4)
> > > >
> > > > RISC-V, Quark, Vlv2TbltDevicePkg, Bhyve requires additional fixes to be
> > > built,
> > > > will be post on edk2 maillist later
> > > >
> > > > VS2019
> > > > - Intel (OvmfPkgX64)
> > > >
> > > > Test with:
> > > > GCC5/RPi4
> > > > VS2019/OvmfX64 (requires changes to enable feature)
> > > >
> > > > Tests:
> > > > 1. Try to enroll key in incorrect format.
> > > > 2. Enroll with only PKDefault keys specified.
> > > > 3. Enroll with all keys specified.
> > > > 4. Enroll when keys are enrolled.
> > > > 5. Reset keys values.
> > > > 6. Running signed & unsigned app after enrollment.
> > > >
> > > > Changes since v1:
> > > > - change names:
> > > >   SecBootVariableLib => SecureBootVariableLib
> > > >   SecBootDefaultKeysDxe => SecureBootDefaultKeysDxe
> > > >   SecEnrollDefaultKeysApp => EnrollFromDefaultKeysApp
> > > > - change name of function CheckSetupMode to GetSetupMode
> > > > - remove ShellPkg dependecy from EnrollFromDefaultKeysApp
> > > > - rebase to master
> > > >
> > > > Changes since v2:
> > > > - fix coding style for functions headers in SecureBootVariableLib.h
> > > > - add header to SecureBootDefaultKeys.fdf.inc
> > > > - remove empty line spaces in SecureBootDefaultKeysDxe files
> > > > - revert FAIL macro in EnrollFromDefaultKeysApp
> > > > - remove functions duplicates and  add SecureBootVariableLib
> > > >   to platforms which used it
> > > >
> > > > Changes since v3:
> > > > - move SecureBootDefaultKeys.fdf.inc to ArmPlatformPkg
> > > > - leave duplicate of CreateTimeBasedPayload in PlatformVarCleanupLib
> > > > - fix typo in guid description
> > > >
> > > > Changes since v4:
> > > > - reorder patches to make it bisectable
> > > > - split commits related to more than one platform
> > > > - move edk2-platform commits to separate patchset
> > > >
> > > > Changes since v5:
> > > > - split SecureBootVariableLib into SecureBootVariableLib and
> > > >   SecureBootVariableProvisionLib
> > > >
> > > > Changes since v6:
> > > > - fix problems found by CI
> > > >   - add correct modules to SecurityPkg.dsc
> > > >   - update SecurityPkg.dec
> > > >   - fix coding style issues
> > > >
> > >
> > > This still generates CI errors:
> > >
> > > https://github.com/tianocore/edk2/pull/1850
> > >
> > > Note that you can create PRs against tianocore/edk2 directly from your
> > > own branch, which will result in the CI checks to be performed on the
> > > code, without your branch being merged even if all checks pass (that
> > > requires the push label which only maintainers can set)
> > >
> > >
> > > > NOTE: edk2-platform has not been changed and v6 platform patches
> > > > are still valid
> > > >
> > > > Grzegorz Bernacki (11):
> > > >   SecurityPkg: Create SecureBootVariableLib.
> > > >   SecurityPkg: Create library for enrolling Secure Boot variables.
> > > >   ArmVirtPkg: add SecureBootVariableLib class resolution
> > > >   OvmfPkg: add SecureBootVariableLib class resolution
> > > >   EmulatorPkg: add SecureBootVariableLib class resolution
> > > >   SecurityPkg: Remove duplicated functions from SecureBootConfigDxe.
> > > >   ArmPlatformPkg: Create include file for default key content.
> > > >   SecurityPkg: Add SecureBootDefaultKeysDxe driver
> > > >   SecurityPkg: Add EnrollFromDefaultKeys application.
> > > >   SecurityPkg: Add new modules to Security package.
> > > >   SecurityPkg: Add option to reset secure boot keys.
> > > >
> > > >  SecurityPkg/SecurityPkg.dec
> > > |  22 +
> > > >  ArmVirtPkg/ArmVirt.dsc.inc
> > > |   2 +
> > > >  EmulatorPkg/EmulatorPkg.dsc
> > > |   2 +
> > > >  OvmfPkg/Bhyve/BhyveX64.dsc
> > > |   2 +
> > > >  OvmfPkg/OvmfPkgIa32.dsc
> > > |   2 +
> > > >  OvmfPkg/OvmfPkgIa32X64.dsc
> > > |   2 +
> > > >  OvmfPkg/OvmfPkgX64.dsc
> > > |   2 +
> > > >  SecurityPkg/SecurityPkg.dsc
> > > |   9 +-
> > > >  SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
> > > |  48 ++
> > > >  SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
> > > |  80 +++
> > > >
> > > SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvi
> > > sionLib.inf   |  80 +++
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigD
> > > xe.inf           |   3 +
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
> > > efaultKeysDxe.inf |  46 ++
> > > >  SecurityPkg/Include/Library/SecureBootVariableLib.h
> > > | 153 ++++++
> > > >  SecurityPkg/Include/Library/SecureBootVariableProvisionLib.h
> > > | 134 +++++
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigN
> > > vData.h          |   2 +
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.v
> > > fr              |   6 +
> > > >  SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
> > > | 115 +++++
> > > >  SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
> > > | 510 ++++++++++++++++++++
> > > >
> > > SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvi
> > > sionLib.c     | 482 ++++++++++++++++++
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigI
> > > mpl.c            | 344 ++++++-------
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
> > > efaultKeysDxe.c   |  69 +++
> > > >  ArmPlatformPkg/SecureBootDefaultKeys.fdf.inc
> > > |  70 +++
> > > >  SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.uni
> > > |  17 +
> > > >
> > > SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvi
> > > sionLib.uni   |  16 +
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigS
> > > trings.uni       |   4 +
> > > >
> > > SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
> > > efaultKeysDxe.uni |  16 +
> > > >  27 files changed, 2049 insertions(+), 189 deletions(-)
> > > >  create mode 100644
> > > SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.inf
> > > >  create mode 100644
> > > SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
> > > >  create mode 100644
> > > SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvi
> > > sionLib.inf
> > > >  create mode 100644
> > > SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
> > > efaultKeysDxe.inf
> > > >  create mode 100644
> > > SecurityPkg/Include/Library/SecureBootVariableLib.h
> > > >  create mode 100644
> > > SecurityPkg/Include/Library/SecureBootVariableProvisionLib.h
> > > >  create mode 100644
> > > SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
> > > >  create mode 100644
> > > SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
> > > >  create mode 100644
> > > SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvi
> > > sionLib.c
> > > >  create mode 100644
> > > SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
> > > efaultKeysDxe.c
> > > >  create mode 100644 ArmPlatformPkg/SecureBootDefaultKeys.fdf.inc
> > > >  create mode 100644
> > > SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.uni
> > > >  create mode 100644
> > > SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvi
> > > sionLib.uni
> > > >  create mode 100644
> > > SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootD
> > > efaultKeysDxe.uni
> > > >
> > > > --
> > > > 2.25.1
> > > >
> >
> >
> >
> >
> > 
> >
> >