Hi, On Thu, 7 Jan 2021 at 18:11, Lim, Elly Siew Chin <[email protected]> wrote: > > Hi Simon, > > > -----Original Message----- > > From: Simon Glass <[email protected]> > > Sent: Thursday, January 7, 2021 8:36 PM > > To: Lim, Elly Siew Chin <[email protected]> > > Cc: U-Boot Mailing List <[email protected]>; Marek Vasut > > <[email protected]>; Tan, Ley Foon <[email protected]>; See, Chin Liang > > <[email protected]>; Simon Goldschmidt > > <[email protected]>; Chee, Tien Fong > > <[email protected]>; Westergreen, Dalon > > <[email protected]>; Gan, Yau Wai <[email protected]> > > Subject: Re: [v2 2/6] arm: socfpga: soc64: Support Vendor Authorized Boot > > (VAB) > > > > On Thu, 7 Jan 2021 at 03:03, Siew Chin Lim <[email protected]> > > 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 <[email protected]> > > > > > > --- > > > 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."?
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
