[U-Boot] [PATCH v2 2/3] x86: baytrail: Add fsp-header verification for secure boot fsp

Bin Meng
2017-05-16T14:39:23Z

Hi Anatolij,

On Tue, May 16, 2017 at 3:55 PM, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@denx.de>
>
> 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 <mv@denx.de>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> 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?

> + 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

> + 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

> +
> 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?

>
>
> /**
> 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.

> + }
> +
> /* Copy default data from Flash */
> memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
> sizeof(struct upd_region));
> --

Regards,
Bin