Hi Simon, > -----Original Message----- > From: Simon Glass <s...@chromium.org> > Sent: Friday, January 8, 2021 11:24 AM > To: Lim, Elly Siew Chin <elly.siew.chin....@intel.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Marek Vasut > <ma...@denx.de>; Tan, Ley Foon <ley.foon....@intel.com>; See, Chin Liang > <chin.liang....@intel.com>; Simon Goldschmidt > <simon.k.r.goldschm...@gmail.com>; Chee, Tien Fong > <tien.fong.c...@intel.com>; Westergreen, Dalon > <dalon.westergr...@intel.com>; Gan, Yau Wai <yau.wai....@intel.com> > Subject: Re: [v2 2/6] arm: socfpga: soc64: Support Vendor Authorized Boot > (VAB) > > Hi, > > On Thu, 7 Jan 2021 at 18:11, Lim, Elly Siew Chin > <elly.siew.chin....@intel.com> > wrote: > > > > Hi Simon, > > > > > -----Original Message----- > > > From: Simon Glass <s...@chromium.org> > > > Sent: Thursday, January 7, 2021 8:36 PM > > > To: Lim, Elly Siew Chin <elly.siew.chin....@intel.com> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Marek Vasut > > > <ma...@denx.de>; Tan, Ley Foon <ley.foon....@intel.com>; See, Chin > > > Liang <chin.liang....@intel.com>; Simon Goldschmidt > > > <simon.k.r.goldschm...@gmail.com>; Chee, Tien Fong > > > <tien.fong.c...@intel.com>; Westergreen, Dalon > > > <dalon.westergr...@intel.com>; Gan, Yau Wai <yau.wai....@intel.com> > > > Subject: Re: [v2 2/6] arm: socfpga: soc64: Support Vendor Authorized > > > Boot > > > (VAB) > > > > > > On Thu, 7 Jan 2021 at 03:03, Siew Chin Lim > > > <elly.siew.chin....@intel.com> > > > wrote: > > > > > > > > Vendor Authorized Boot is a security feature for authenticating > > > > the images such as U-Boot, ARM trusted Firmware, Linux kernel, > > > > device tree blob and etc loaded from FIT. After those images are > > > > loaded from FIT, the VAB certificate and signature block appended > > > > at the end of each image are sent to Secure Device Manager (SDM) for > authentication. > > > > U-Boot will validate the SHA384 of the image against the SHA384 > > > > hash stored in the VAB certificate before sending the image to SDM > > > > for authentication. > > > > > > > > Signed-off-by: Siew Chin Lim <elly.siew.chin....@intel.com> > > > > > > > > --- > > > > v2 > > > > --- > > > > - Renamed SECURE_VAB_AUTH* to SOCFPGA_SECURE_VAB_AUTH* > > > > - Changes in secure_vab.c > > > > - Changed to use SZ_1K for 1024 > > > > - Updated comment in secure_vab.c of "... the certificate for T" > > > > - The code will report error before end of the function if reach > > > > maximum retry. > > > > - In board_prep_linux function, only execute linux_qspi_enable > > > > command if it exists in enviroment variable. It is optional. > > > > --- > > > > arch/arm/mach-socfpga/Kconfig | 15 ++ > > > > arch/arm/mach-socfpga/Makefile | 2 + > > > > arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 1 + > > > > arch/arm/mach-socfpga/include/mach/secure_vab.h | 63 ++++++++ > > > > arch/arm/mach-socfpga/secure_vab.c | 193 > > > +++++++++++++++++++++++ > > > > common/Kconfig.boot | 2 +- > > > > 6 files changed, 275 insertions(+), 1 deletion(-) create mode > > > > 100644 arch/arm/mach-socfpga/include/mach/secure_vab.h > > > > create mode 100644 arch/arm/mach-socfpga/secure_vab.c > > > > > > I think this could use a few more function comments. Also try to use > > > if() instead of #if > > > > Noted. I will change #if in *.c to " if (IS_ENABLED(CONFIG...))". > > > > Besides, can you please help to elaborate more about "I think this could > > use a > few more function comments."?
Noted, I will review this. Thanks. > > Non-trivial functions should have a comment in the standard style that > explains > their purpose/action and documents input and output args and return value. > > Regards, > Simon