From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anatolij Gustschin Date: Fri, 19 May 2017 08:20:37 +0200 Subject: [U-Boot] [PATCH v2 2/3] x86: baytrail: Add fsp-header verification for secure boot fsp In-Reply-To: References: <1494921350-803-1-git-send-email-agust@denx.de> <1494921350-803-2-git-send-email-agust@denx.de> Message-ID: <20170519082037.5a223e16@crub> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bin, On Tue, 16 May 2017 22:39:23 +0800 Bin Meng bmeng.cn at gmail.com wrote: > Hi Anatolij, > > On Tue, May 16, 2017 at 3:55 PM, Anatolij Gustschin wrote: > > From: Markus Valentin > > > > Introduce a new Kconfig variable for secure boot on baytrail based > > platforms. If this variable is set the build process tries to use > > fsp-sb.bin instead of fsp.bin (-sb is the secure boot enabled fsp). > > > > Also check the two fsp headers against each other and print if secure > > boot is enabled or not. > > > > Signed-off-by: Markus Valentin > > Signed-off-by: Anatolij Gustschin > > --- > > Changes in v2: > > - use if (IS_ENABLED(CONFIG_*)) instead of #ifdef > > - s/SB/Secure Boot/ > > - minor Kconfig help cleanup > > > > arch/x86/Kconfig | 13 ++++++++++++- > > arch/x86/include/asm/fsp/fsp_support.h | 2 ++ > > arch/x86/lib/fsp/fsp_support.c | 18 ++++++++++++++++++ > > 3 files changed, 32 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index 9ead3eb..8cea393 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -348,7 +348,8 @@ config HAVE_FSP > > config FSP_FILE > > string "Firmware Support Package binary filename" > > depends on HAVE_FSP > > - default "fsp.bin" > > + default "fsp.bin" if !BAYTRAIL_SECURE_BOOT > > + default "fsp-sb.bin" if BAYTRAIL_SECURE_BOOT > > help > > The filename of the file to use as Firmware Support Package binary > > in the board directory. > > @@ -400,6 +401,16 @@ config FSP_BROKEN_HOB > > do not overwrite the important boot service data which is used by > > FSP, otherwise the subsequent call to fsp_notify() will fail. > > > > +config BAYTRAIL_SECURE_BOOT > > Should this be in arch/x86/cpu/baytrail/Kconfig instead? right, I'll move it to baytrail subdir. > > > + bool "Enable Secure Boot on BayTrail" > > + depends on HAVE_FSP > > + default n > > + help > > + Use the SecureBoot Features of the BayTrail platform. This switch > > nits: secure boot feature OK. > > + enables the usage of the secure-boot enabled fsp.bin (fsp-sb.bin) > > + for your board you need to provide this yourself. You can reconfigure > > + your fsp with the Intel BCT tool to enable SecureBoot. > > nits: secure boot OK. > > config ENABLE_MRC_CACHE > > bool "Enable MRC cache" > > depends on !EFI && !SYS_COREBOOT > > diff --git a/arch/x86/include/asm/fsp/fsp_support.h b/arch/x86/include/asm/fsp/fsp_support.h > > index 61d811f..bae17bc 100644 > > --- a/arch/x86/include/asm/fsp/fsp_support.h > > +++ b/arch/x86/include/asm/fsp/fsp_support.h > > @@ -21,6 +21,8 @@ > > #define FSP_LOWMEM_BASE 0x100000UL > > #define FSP_HIGHMEM_BASE 0x100000000ULL > > #define UPD_TERMINATOR 0x55AA > > +#define FSP_FIRST_HEADER_OFFSET 0x94 > > +#define FSP_SECOND_HEADER_OFFSET 0x20494 > > Are these two offsets common to all FSP, or BayTrail-specific values? I think that 0x204094 is BayTrail-specific. 0x94 is common, I have seen this value in all FSP integration guide files, when it is documented there. > > diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c > > index a480361..0bbd9ae 100644 > > --- a/arch/x86/lib/fsp/fsp_support.c > > +++ b/arch/x86/lib/fsp/fsp_support.c > > @@ -120,6 +120,14 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf) > > panic("Invalid FSP header"); > > } > > > > + if (IS_ENABLED(CONFIG_BAYTRAIL_SECURE_BOOT)) { > > + /* compare primary and secondary header */ > > + if (memcmp((void *)(CONFIG_FSP_ADDR + FSP_FIRST_HEADER_OFFSET), > > + (void *)(CONFIG_FSP_ADDR + FSP_SECOND_HEADER_OFFSET), > > + fsp_hdr->hdr_len)) > > + panic("Secure Boot: 1st & 2nd FSP headers don't match"); > > + } > > + > > config_data.common.fsp_hdr = fsp_hdr; > > config_data.common.stack_top = stack_top; > > config_data.common.boot_mode = boot_mode; > > @@ -134,6 +142,16 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf) > > > > fsp_upd = &config_data.fsp_upd; > > > > + if (IS_ENABLED(CONFIG_BAYTRAIL_SECURE_BOOT)) { > > + /* > > + * if the enable secure boot flag is not 1, secure boot has not > > + * been activated in the FSP which results in the TXE-Engine not > > + * getting loaded > > + */ > > + printf("FSP: Secure Boot %sabled\n", > > + fsp_vpd->enable_secure_boot == 1 ? "en" : "dis"); > > I believe this won't build for other FSP platforms due to no > enable_secure_boot member in fsp_vpd structure. Yes, this breaks crownbay_defconfig build. AFAIK, we do not have support for multiple platforms in a single image, so I have to use ifdef here to avoid build issues. Thanks for review and comments! -- Anatolij