[U-Boot] [PATCH v3 1/6] x86: baytrail: Add fsp-header verification for secure boot FSP

[Bin Meng]

Hi Anatolij,

On Fri, Nov 17, 2017 at 9:13 AM, Anatolij Gustschin <agust at denx.de> wrote:
> From: Markus Valentin <mv at denx.de>
>
> Introduce a new Kconfig variable for secure boot on Bay Trail 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 at denx.de>
> Signed-off-by: Anatolij Gustschin <agust at denx.de>
> ---
> Changes in v3:
>  - move BAYTRAIL_SECURE_BOOT to arch/x86/cpu/baytrail/Kconfig
>  - Kconfig help text improvements
>  - fix crownbay build breakage
>
> Changes in v2:
>  - use if (IS_ENABLED(CONFIG_*)) instead of #ifdef
>  - s/SB/Secure Boot/
>  - minor Kconfig help cleanup
>
>  arch/x86/Kconfig                       |  3 ++-
>  arch/x86/cpu/baytrail/Kconfig          | 10 ++++++++++
>  arch/x86/include/asm/fsp/fsp_support.h |  2 ++
>  arch/x86/lib/fsp/fsp_support.c         | 24 ++++++++++++++++++++++++
>  4 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 98c56ad7dc..6755e92748 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -350,7 +350,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

Since this Kconfig is generic to all x86, can we introduce a generic
name here, something like:

default "fsp-sb.bin" if SECURE_BOOT

Then in the arch/x86/cpu/baytrail/Kconfig, we can have:

config BAYTRAIL_SECURE_BOOT
        depends on HAVE_FSP
        depends on SECURE_BOOT
        default y if SECURE_BOOT

Does U-Boot have any generic Kconfig option for secure boot?

>         help
>           The filename of the file to use as Firmware Support Package binary
>           in the board directory.
> diff --git a/arch/x86/cpu/baytrail/Kconfig b/arch/x86/cpu/baytrail/Kconfig
> index 1d876b1927..ff244fcd68 100644
> --- a/arch/x86/cpu/baytrail/Kconfig
> +++ b/arch/x86/cpu/baytrail/Kconfig
> @@ -39,4 +39,14 @@ config DEBUG_UART
>         bool
>         select DEBUG_UART_BOARD_INIT
>
> +config BAYTRAIL_SECURE_BOOT
> +       bool "Enable Secure Boot on Bay Trail"
> +       depends on HAVE_FSP
> +       default n
> +       help
> +         Use the secure boot feature of the Bay Trail platform. This switch
> +         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 secure boot.
> +
>  endif
> diff --git a/arch/x86/include/asm/fsp/fsp_support.h b/arch/x86/include/asm/fsp/fsp_support.h
> index df3add008c..124a4148a0 100644
> --- a/arch/x86/include/asm/fsp/fsp_support.h
> +++ b/arch/x86/include/asm/fsp/fsp_support.h
> @@ -22,6 +22,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
>
>
>  /**
> diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
> index e0c49be635..d79a6e900a 100644
> --- a/arch/x86/lib/fsp/fsp_support.c
> +++ b/arch/x86/lib/fsp/fsp_support.c
> @@ -97,6 +97,8 @@ void fsp_continue(u32 status, void *hob_list)
>         fsp_init_done(hob_list);
>  }
>
> +#define SB_PRFX                "Secure Boot:"
> +
>  void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>  {
>         struct fsp_config_data config_data;
> @@ -116,6 +118,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("%s 1st & 2nd FSP headers don't match", SB_PRFX);
> +       }
> +
>         config_data.common.fsp_hdr = fsp_hdr;
>         config_data.common.stack_top = stack_top;
>         config_data.common.boot_mode = boot_mode;
> @@ -130,6 +140,20 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>
>         fsp_upd = &config_data.fsp_upd;
>
> +       /*
> +        * On some platforms there is no 'enable_secure_boot' field
> +        * in VPD region struct, so we have to use ifdef here.
> +        */
> +       #ifdef 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");
> +       #endif
> +

Can we introduce some APIs like fsp_secure_boot_check() and move the
above 2 blocks into baytrail directory that implements the API?

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

Regards,
Bin