[U-Boot] [PATCH v2 1/3] x86: congatec: add secureboot enabled defconfig for conga-qeval20-qa3-e3845

[U-Boot] [PATCH v2 1/3] x86: congatec: add secureboot enabled defconfig for conga-qeval20-qa3-e3845

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

Signed-off-by: Markus Valentin <mv@denx.de>
[agust: rebased, fixed to build with v2017.05]
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
 - add Reviewed-by tag

 ...0-qa3-e3845-internal-uart-secure-boot_defconfig | 77 ++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig

diff --git a/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig b/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
new file mode 100644
index 0000000..b78f90e
--- /dev/null
+++ b/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
@@ -0,0 +1,77 @@
+CONFIG_X86=y
+CONFIG_VENDOR_CONGATEC=y
+CONFIG_TARGET_CONGA_QEVAL20_QA3_E3845=y
+CONFIG_DEFAULT_DEVICE_TREE="conga-qeval20-qa3-e3845"
+CONFIG_INTERNAL_UART=y
+CONFIG_HAVE_INTEL_ME=y
+CONFIG_BAYTRAIL_SECURE_BOOT=y
+CONFIG_ENABLE_MRC_CACHE=y
+CONFIG_SMP=y
+CONFIG_HAVE_VGA_BIOS=y
+CONFIG_VGA_BIOS_ADDR=0xfffa0000
+CONFIG_GENERATE_PIRQ_TABLE=y
+CONFIG_GENERATE_MP_TABLE=y
+CONFIG_GENERATE_ACPI_TABLE=y
+CONFIG_SEABIOS=y
+CONFIG_FIT=y
+CONFIG_FIT_SIGNATURE=y
+CONFIG_BOOTSTAGE=y
+CONFIG_BOOTSTAGE_REPORT=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_HUSH_PARSER=y
+CONFIG_CMD_CPU=y
+# CONFIG_CMD_IMLS is not set
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_GPIO=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_DHCP=y
+# CONFIG_CMD_NFS is not set
+CONFIG_CMD_PING=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_BOOTSTAGE=y
+CONFIG_CMD_EXT2=y
+CONFIG_CMD_EXT4=y
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_FAT=y
+CONFIG_CMD_FS_GENERIC=y
+CONFIG_OF_CONTROL=y
+CONFIG_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_CPU=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_INTEL=y
+CONFIG_WINBOND_W83627=y
+CONFIG_MMC=y
+CONFIG_MMC_PCI=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_SPI_FLASH=y
+CONFIG_SPI_FLASH_GIGADEVICE=y
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_SPI_FLASH_STMICRO=y
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_DM_ETH=y
+CONFIG_E1000=y
+CONFIG_DM_PCI=y
+CONFIG_DM_RTC=y
+CONFIG_DEBUG_UART=y
+CONFIG_DEBUG_UART_BASE=0x3f8
+CONFIG_DEBUG_UART_CLOCK=1843200
+CONFIG_SYS_NS16550=y
+CONFIG_ICH_SPI=y
+CONFIG_TIMER=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_KEYBOARD=y
+CONFIG_DM_VIDEO=y
+CONFIG_VIDEO_VESA=y
+CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
+CONFIG_FRAMEBUFFER_VESA_MODE_114=y
+CONFIG_CONSOLE_SCROLL_LINES=5
+CONFIG_USE_PRIVATE_LIBGCC=y
-- 
2.7.4

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

[Anatolij Gustschin]

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
+	bool "Enable Secure Boot on BayTrail"
+	depends on HAVE_FSP
+	default n
+	help
+	  Use the SecureBoot Features of the BayTrail 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 SecureBoot.
+
 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
 
 
 /**
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");
+	}
+
 	/* Copy default data from Flash */
 	memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
 	       sizeof(struct upd_region));
-- 
2.7.4

[U-Boot] [PATCH 3/3] x86: baytrail: secureboot: Add functions for verification of U-Boot

[Anatolij Gustschin]

From: Markus Valentin <mv@denx.de>

Introduce functions that check the integrity of U-Boot by utilising
the hashes stored in the oem-data block.

The verification functions get called in fsp_init()

Signed-off-by: Markus Valentin <mv@denx.de>
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
Changes in v2:
 - use 'const void *' for fdt property ptr, drop cast
 - s/u-boot/U-Boot/
 - fix function comment style and move comments to header with
   prototypes. Use fsp_ prefix
 - fix multiline comment style
 - s/SB:/Secure Boot/ for non-debug messages

 arch/x86/cpu/baytrail/Makefile                     |   1 +
 arch/x86/cpu/baytrail/secure_boot.c                | 105 +++++++++++++++++++++
 .../include/asm/arch-baytrail/fsp/fsp_configs.h    |  22 +++++
 arch/x86/lib/fsp/fsp_support.c                     |  17 ++++
 4 files changed, 145 insertions(+)
 create mode 100644 arch/x86/cpu/baytrail/secure_boot.c

diff --git a/arch/x86/cpu/baytrail/Makefile b/arch/x86/cpu/baytrail/Makefile
index a0216f3..dbf9a82 100644
--- a/arch/x86/cpu/baytrail/Makefile
+++ b/arch/x86/cpu/baytrail/Makefile
@@ -8,4 +8,5 @@ obj-y += cpu.o
 obj-y += early_uart.o
 obj-y += fsp_configs.o
 obj-y += valleyview.o
+obj-$(CONFIG_BAYTRAIL_SECURE_BOOT) += secure_boot.o
 obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o
diff --git a/arch/x86/cpu/baytrail/secure_boot.c b/arch/x86/cpu/baytrail/secure_boot.c
new file mode 100644
index 0000000..97acea8
--- /dev/null
+++ b/arch/x86/cpu/baytrail/secure_boot.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2017 Markus Valentin <mv@denx.de>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+
+#define SB_MANIFEST_BASE		0xFFFE0000
+#define SB_MANIFEST_SIZE		0x400
+#define SB_MANIFEST_OEM_DATA_OFFSET	0x58
+#define SB_MANIFEST_OEM_HASH_OFFSET	(SB_MANIFEST_OEM_DATA_OFFSET + 4)
+#define SB_MANIFEST_OEM_HASH_BASE	(SB_MANIFEST_BASE +\
+					 SB_MANIFEST_OEM_HASH_OFFSET)
+#define SB_MANIFEST_END			(SB_MANIFEST_BASE + SB_MANIFEST_SIZE)
+
+#define PUB_KEY_MODULUS_SIZE		0x100
+#define U_BOOT_STAGE_SIZE		0xDD360
+#define U_BOOT_OFFSET			0x2CA0
+
+#define U_BOOT_STAGE_START		(CONFIG_SYS_TEXT_BASE + U_BOOT_OFFSET)
+#define U_BOOT_STAGE_END		(U_BOOT_STAGE_START + U_BOOT_STAGE_SIZE)
+
+#define SHA256_U_BOOT_STAGE_ID		0
+#define SHA256_FSP_STAGE2_ID		1
+#define SHA256_FIT_PUB_KEY_ID		2
+
+#define FIT_KEY_NAME			"dev"
+
+/**
+ * verify_oem_sha256() - oem data block verification
+ *
+ * This function compares a hash which gets retrieved from the oem data block
+ * with the runtime calculated hash of start_address+size. If they match,
+ * this function returns true. If not, it returns false.
+ *
+ * @hash_id:		offset of oem-data block for hash to compare
+ * @start_address:	address where the hash calculation should start
+ * @size:		length of the region for hash calculation
+ *
+ * @retval:		true on success, false on error
+ */
+static bool verify_oem_sha256(unsigned int hash_id,
+			      const void *start_address,
+			      size_t size)
+{
+	uint8_t value[SHA256_SUM_LEN];
+	int value_len;
+
+	/* calculate address of hash to compare in the oemdata block*/
+	void *hash_to_verify = (void *)SB_MANIFEST_OEM_HASH_BASE +
+			       (SHA256_SUM_LEN * hash_id);
+#ifdef DEBUG
+	unsigned int i = 0;
+	uint8_t oem_value[SHA256_SUM_LEN];
+
+	memcpy(oem_value, hash_to_verify, SHA256_SUM_LEN);
+	printf("SB: Hash to verify:\t");
+	for (i = 0; i < SHA256_SUM_LEN; i++)
+		printf("%X", oem_value[i]);
+	printf("\n");
+#endif
+
+	/* calculate the hash of the binary */
+	calculate_hash(start_address, size, "sha256", value, &value_len);
+
+#ifdef DEBUG
+	printf("SB: calculated hash:\t");
+	for (i = 0; i < SHA256_SUM_LEN; i++)
+		printf("%X", value[i]);
+	printf("\n");
+#endif
+	/* compare the two hash  values */
+	if (memcmp(hash_to_verify, value, SHA256_SUM_LEN))
+		return false;
+	return true;
+}
+
+bool fsp_verify_u_boot_bin(void)
+{
+	return verify_oem_sha256(SHA256_U_BOOT_STAGE_ID,
+				 (const void *)U_BOOT_STAGE_START,
+				 U_BOOT_STAGE_SIZE);
+}
+
+bool fsp_verify_public_key(void)
+{
+	const void *fit_public_key_modulus;
+
+	int offset = fdt_node_offset_by_prop_value(gd->fdt_blob, -1,
+						   "key-name-hint",
+						   FIT_KEY_NAME,
+						   4);
+
+	fit_public_key_modulus =  fdt_getprop(gd->fdt_blob, offset,
+					      "rsa,modulus", NULL);
+	if (!fit_public_key_modulus) {
+		debug("SB: Could not fetch public key from U-Boot Devicetree\n");
+		return false;
+	}
+
+	return verify_oem_sha256(SHA256_FIT_PUB_KEY_ID,
+				 fit_public_key_modulus,
+				 PUB_KEY_MODULUS_SIZE);
+}
diff --git a/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
index e539890..a07853c 100644
--- a/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
+++ b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
@@ -16,4 +16,26 @@ struct fspinit_rtbuf {
 	struct common_buf	common;	/* FSP common runtime data structure */
 };
 
+/**
+ * fsp_verify_u_boot_bin() - U-Boot binary verification
+ *
+ * This function verifies the integrity for U-Boot, its devicetree and the ucode
+ * appended or inserted to the devicetree.
+ *
+ * @retval:	true on success, false on error
+ */
+bool fsp_verify_u_boot_bin(void);
+
+/**
+ * fsp_verify_public_key() - integrity of public key for fit image verification
+ *
+ * This function verifies the integrity for the modulus of the public key which
+ * is stored in the U-Boot devicetree for fit image verification. It tries to
+ * find the "rsa,modulus" property in the dtb and then verifies it with the
+ * checksum stored in the oem-data block
+ *
+ * @retval:	true on success, false on error
+ */
+bool fsp_verify_public_key(void);
+
 #endif /* __FSP_CONFIGS_H__ */
diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
index 0bbd9ae..5e3ce13 100644
--- a/arch/x86/lib/fsp/fsp_support.c
+++ b/arch/x86/lib/fsp/fsp_support.c
@@ -150,6 +150,23 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
 		 */
 		printf("FSP: Secure Boot %sabled\n",
 		       fsp_vpd->enable_secure_boot == 1 ? "en" : "dis");
+
+		if (!fsp_verify_u_boot_bin()) {
+			/*
+			 * If our U-Boot binary checksum isn't equal to
+			 * our expected checksum we need to stop booting
+			 */
+			puts("Secure Boot: Failed to verify U-Boot and dtb\n");
+			hang();
+		}
+
+		/*
+		 * Verification of the public key happens with verification of
+		 * the devicetree binary (that's where it's stored), this check
+		 * is not necessary, but nice to see it's integer
+		 */
+		if (!fsp_verify_public_key())
+			puts("Secure Boot: Failed to verify public key for fit-image\n");
 	}
 
 	/* Copy default data from Flash */
-- 
2.7.4

[U-Boot] [PATCH v2 1/3] x86: congatec: add secureboot enabled defconfig for conga-qeval20-qa3-e3845

[Bin Meng]

Hi Anatolij,

On Tue, May 16, 2017 at 3:55 PM, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@denx.de>
>

Can you add a simple sentence for this commit?

> Signed-off-by: Markus Valentin <mv@denx.de>
> [agust: rebased, fixed to build with v2017.05]
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
>  - add Reviewed-by tag
>
>  ...0-qa3-e3845-internal-uart-secure-boot_defconfig | 77 ++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
>  create mode 100644 configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
>
> diff --git a/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig b/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
> new file mode 100644
> index 0000000..b78f90e
> --- /dev/null
> +++ b/configs/conga-qeval20-qa3-e3845-internal-uart-secure-boot_defconfig
> @@ -0,0 +1,77 @@
> +CONFIG_X86=y
> +CONFIG_VENDOR_CONGATEC=y
> +CONFIG_TARGET_CONGA_QEVAL20_QA3_E3845=y
> +CONFIG_DEFAULT_DEVICE_TREE="conga-qeval20-qa3-e3845"
> +CONFIG_INTERNAL_UART=y
> +CONFIG_HAVE_INTEL_ME=y
> +CONFIG_BAYTRAIL_SECURE_BOOT=y

This commit should come in later in this patch series, as
CONFIG_BAYTRAIL_SECURE_BOOT is not available yet at this point.

> +CONFIG_ENABLE_MRC_CACHE=y
> +CONFIG_SMP=y
> +CONFIG_HAVE_VGA_BIOS=y
> +CONFIG_VGA_BIOS_ADDR=0xfffa0000
> +CONFIG_GENERATE_PIRQ_TABLE=y
> +CONFIG_GENERATE_MP_TABLE=y
> +CONFIG_GENERATE_ACPI_TABLE=y
> +CONFIG_SEABIOS=y
> +CONFIG_FIT=y
> +CONFIG_FIT_SIGNATURE=y
> +CONFIG_BOOTSTAGE=y
> +CONFIG_BOOTSTAGE_REPORT=y
> +CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_HUSH_PARSER=y
> +CONFIG_CMD_CPU=y
> +# CONFIG_CMD_IMLS is not set
> +# CONFIG_CMD_FLASH is not set
> +CONFIG_CMD_MMC=y
> +CONFIG_CMD_SF=y
> +CONFIG_CMD_SPI=y
> +CONFIG_CMD_I2C=y
> +CONFIG_CMD_USB=y
> +CONFIG_CMD_GPIO=y
> +# CONFIG_CMD_SETEXPR is not set
> +CONFIG_CMD_DHCP=y
> +# CONFIG_CMD_NFS is not set
> +CONFIG_CMD_PING=y
> +CONFIG_CMD_TIME=y
> +CONFIG_CMD_BOOTSTAGE=y
> +CONFIG_CMD_EXT2=y
> +CONFIG_CMD_EXT4=y
> +CONFIG_CMD_EXT4_WRITE=y
> +CONFIG_CMD_FAT=y
> +CONFIG_CMD_FS_GENERIC=y
> +CONFIG_OF_CONTROL=y
> +CONFIG_REGMAP=y
> +CONFIG_SYSCON=y
> +CONFIG_CPU=y
> +CONFIG_DM_I2C=y
> +CONFIG_SYS_I2C_INTEL=y
> +CONFIG_WINBOND_W83627=y
> +CONFIG_MMC=y
> +CONFIG_MMC_PCI=y
> +CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_SDMA=y
> +CONFIG_SPI_FLASH=y
> +CONFIG_SPI_FLASH_GIGADEVICE=y
> +CONFIG_SPI_FLASH_MACRONIX=y
> +CONFIG_SPI_FLASH_STMICRO=y
> +CONFIG_SPI_FLASH_WINBOND=y
> +CONFIG_DM_ETH=y
> +CONFIG_E1000=y
> +CONFIG_DM_PCI=y
> +CONFIG_DM_RTC=y
> +CONFIG_DEBUG_UART=y
> +CONFIG_DEBUG_UART_BASE=0x3f8
> +CONFIG_DEBUG_UART_CLOCK=1843200
> +CONFIG_SYS_NS16550=y
> +CONFIG_ICH_SPI=y
> +CONFIG_TIMER=y
> +CONFIG_USB=y
> +CONFIG_DM_USB=y
> +CONFIG_USB_STORAGE=y
> +CONFIG_USB_KEYBOARD=y
> +CONFIG_DM_VIDEO=y
> +CONFIG_VIDEO_VESA=y
> +CONFIG_FRAMEBUFFER_SET_VESA_MODE=y
> +CONFIG_FRAMEBUFFER_VESA_MODE_114=y
> +CONFIG_CONSOLE_SCROLL_LINES=5
> +CONFIG_USE_PRIVATE_LIBGCC=y

Regards,
Bin

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

[Bin Meng]

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

[U-Boot] [PATCH 3/3] x86: baytrail: secureboot: Add functions for verification of U-Boot

[Bin Meng]

Hi Anatolij,

On Tue, May 16, 2017 at 3:55 PM, Anatolij Gustschin <agust@denx.de> wrote:
> From: Markus Valentin <mv@denx.de>
>
> Introduce functions that check the integrity of U-Boot by utilising
> the hashes stored in the oem-data block.
>
> The verification functions get called in fsp_init()
>
> Signed-off-by: Markus Valentin <mv@denx.de>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> Changes in v2:
>  - use 'const void *' for fdt property ptr, drop cast
>  - s/u-boot/U-Boot/
>  - fix function comment style and move comments to header with
>    prototypes. Use fsp_ prefix
>  - fix multiline comment style
>  - s/SB:/Secure Boot/ for non-debug messages
>
>  arch/x86/cpu/baytrail/Makefile                     |   1 +
>  arch/x86/cpu/baytrail/secure_boot.c                | 105 +++++++++++++++++++++
>  .../include/asm/arch-baytrail/fsp/fsp_configs.h    |  22 +++++
>  arch/x86/lib/fsp/fsp_support.c                     |  17 ++++
>  4 files changed, 145 insertions(+)
>  create mode 100644 arch/x86/cpu/baytrail/secure_boot.c
>
> diff --git a/arch/x86/cpu/baytrail/Makefile b/arch/x86/cpu/baytrail/Makefile
> index a0216f3..dbf9a82 100644
> --- a/arch/x86/cpu/baytrail/Makefile
> +++ b/arch/x86/cpu/baytrail/Makefile
> @@ -8,4 +8,5 @@ obj-y += cpu.o
>  obj-y += early_uart.o
>  obj-y += fsp_configs.o
>  obj-y += valleyview.o
> +obj-$(CONFIG_BAYTRAIL_SECURE_BOOT) += secure_boot.o
>  obj-$(CONFIG_GENERATE_ACPI_TABLE) += acpi.o
> diff --git a/arch/x86/cpu/baytrail/secure_boot.c b/arch/x86/cpu/baytrail/secure_boot.c
> new file mode 100644
> index 0000000..97acea8
> --- /dev/null
> +++ b/arch/x86/cpu/baytrail/secure_boot.c
> @@ -0,0 +1,105 @@
> +/*
> + * Copyright (C) 2017 Markus Valentin <mv@denx.de>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +
> +#define SB_MANIFEST_BASE               0xFFFE0000

nits: please use lower case hex and fix this globally in this file

> +#define SB_MANIFEST_SIZE               0x400
> +#define SB_MANIFEST_OEM_DATA_OFFSET    0x58
> +#define SB_MANIFEST_OEM_HASH_OFFSET    (SB_MANIFEST_OEM_DATA_OFFSET + 4)
> +#define SB_MANIFEST_OEM_HASH_BASE      (SB_MANIFEST_BASE +\
> +                                        SB_MANIFEST_OEM_HASH_OFFSET)
> +#define SB_MANIFEST_END                        (SB_MANIFEST_BASE + SB_MANIFEST_SIZE)
> +
> +#define PUB_KEY_MODULUS_SIZE           0x100
> +#define U_BOOT_STAGE_SIZE              0xDD360

What is this?

> +#define U_BOOT_OFFSET                  0x2CA0

and this?

> +
> +#define U_BOOT_STAGE_START             (CONFIG_SYS_TEXT_BASE + U_BOOT_OFFSET)
> +#define U_BOOT_STAGE_END               (U_BOOT_STAGE_START + U_BOOT_STAGE_SIZE)
> +
> +#define SHA256_U_BOOT_STAGE_ID         0
> +#define SHA256_FSP_STAGE2_ID           1
> +#define SHA256_FIT_PUB_KEY_ID          2
> +

I believe a comment block is needed for explaining things like number
0/1/2, at least where are they coming?

> +#define FIT_KEY_NAME                   "dev"
> +
> +/**
> + * verify_oem_sha256() - oem data block verification
> + *
> + * This function compares a hash which gets retrieved from the oem data block
> + * with the runtime calculated hash of start_address+size. If they match,
> + * this function returns true. If not, it returns false.
> + *
> + * @hash_id:           offset of oem-data block for hash to compare
> + * @start_address:     address where the hash calculation should start
> + * @size:              length of the region for hash calculation
> + *
> + * @retval:            true on success, false on error
> + */
> +static bool verify_oem_sha256(unsigned int hash_id,
> +                             const void *start_address,
> +                             size_t size)
> +{
> +       uint8_t value[SHA256_SUM_LEN];
> +       int value_len;
> +
> +       /* calculate address of hash to compare in the oemdata block*/
> +       void *hash_to_verify = (void *)SB_MANIFEST_OEM_HASH_BASE +
> +                              (SHA256_SUM_LEN * hash_id);
> +#ifdef DEBUG
> +       unsigned int i = 0;
> +       uint8_t oem_value[SHA256_SUM_LEN];
> +
> +       memcpy(oem_value, hash_to_verify, SHA256_SUM_LEN);
> +       printf("SB: Hash to verify:\t");
> +       for (i = 0; i < SHA256_SUM_LEN; i++)
> +               printf("%X", oem_value[i]);
> +       printf("\n");
> +#endif
> +
> +       /* calculate the hash of the binary */
> +       calculate_hash(start_address, size, "sha256", value, &value_len);
> +
> +#ifdef DEBUG
> +       printf("SB: calculated hash:\t");
> +       for (i = 0; i < SHA256_SUM_LEN; i++)
> +               printf("%X", value[i]);
> +       printf("\n");
> +#endif
> +       /* compare the two hash  values */

nits: two spaces after 'hash'

> +       if (memcmp(hash_to_verify, value, SHA256_SUM_LEN))
> +               return false;
> +       return true;
> +}
> +
> +bool fsp_verify_u_boot_bin(void)
> +{
> +       return verify_oem_sha256(SHA256_U_BOOT_STAGE_ID,
> +                                (const void *)U_BOOT_STAGE_START,
> +                                U_BOOT_STAGE_SIZE);
> +}
> +
> +bool fsp_verify_public_key(void)
> +{
> +       const void *fit_public_key_modulus;
> +
> +       int offset = fdt_node_offset_by_prop_value(gd->fdt_blob, -1,
> +                                                  "key-name-hint",
> +                                                  FIT_KEY_NAME,
> +                                                  4);
> +
> +       fit_public_key_modulus =  fdt_getprop(gd->fdt_blob, offset,
> +                                             "rsa,modulus", NULL);
> +       if (!fit_public_key_modulus) {
> +               debug("SB: Could not fetch public key from U-Boot Devicetree\n");
> +               return false;
> +       }
> +
> +       return verify_oem_sha256(SHA256_FIT_PUB_KEY_ID,
> +                                fit_public_key_modulus,
> +                                PUB_KEY_MODULUS_SIZE);
> +}
> diff --git a/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> index e539890..a07853c 100644
> --- a/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> +++ b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> @@ -16,4 +16,26 @@ struct fspinit_rtbuf {
>         struct common_buf       common; /* FSP common runtime data structure */
>  };
>
> +/**
> + * fsp_verify_u_boot_bin() - U-Boot binary verification
> + *
> + * This function verifies the integrity for U-Boot, its devicetree and the ucode
> + * appended or inserted to the devicetree.
> + *
> + * @retval:    true on success, false on error
> + */
> +bool fsp_verify_u_boot_bin(void);
> +
> +/**
> + * fsp_verify_public_key() - integrity of public key for fit image verification
> + *
> + * This function verifies the integrity for the modulus of the public key which
> + * is stored in the U-Boot devicetree for fit image verification. It tries to

nits: device tree

> + * find the "rsa,modulus" property in the dtb and then verifies it with the
> + * checksum stored in the oem-data block
> + *
> + * @retval:    true on success, false on error
> + */
> +bool fsp_verify_public_key(void);

Again, are these two APIs common for all FSP based platforms?

> +
>  #endif /* __FSP_CONFIGS_H__ */
> diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
> index 0bbd9ae..5e3ce13 100644
> --- a/arch/x86/lib/fsp/fsp_support.c
> +++ b/arch/x86/lib/fsp/fsp_support.c
> @@ -150,6 +150,23 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>                  */
>                 printf("FSP: Secure Boot %sabled\n",
>                        fsp_vpd->enable_secure_boot == 1 ? "en" : "dis");
> +
> +               if (!fsp_verify_u_boot_bin()) {
> +                       /*
> +                        * If our U-Boot binary checksum isn't equal to
> +                        * our expected checksum we need to stop booting
> +                        */
> +                       puts("Secure Boot: Failed to verify U-Boot and dtb\n");
> +                       hang();
> +               }
> +
> +               /*
> +                * Verification of the public key happens with verification of
> +                * the devicetree binary (that's where it's stored), this check

nits: device tree

> +                * is not necessary, but nice to see it's integer
> +                */
> +               if (!fsp_verify_public_key())
> +                       puts("Secure Boot: Failed to verify public key for fit-image\n");

As the comments say it's not necessary, which means it will never
fail, right? But in case it fails, that means either hardware is doing
wrong, or software is doing wrong? Can we adjust the output message to
indicate some hints?

>         }
>
>         /* Copy default data from Flash */
> --

BTW: I don't see device tree update for the secure boot enabled board,
like a device tree that has the "rsa,modulus" stuff.

Regards,
Bin

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

[Anatolij Gustschin]

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

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

[U-Boot] [PATCH v2 1/3] x86: congatec: add secureboot enabled defconfig for conga-qeval20-qa3-e3845

[Anatolij Gustschin]

Hi Bin,

On Tue, 16 May 2017 22:39:17 +0800
Bin Meng bmeng.cn at gmail.com wrote:

> Hi Anatolij,
> 
> On Tue, May 16, 2017 at 3:55 PM, Anatolij Gustschin <agust@denx.de> wrote:
> > From: Markus Valentin <mv@denx.de>
> >  
> 
> Can you add a simple sentence for this commit?

yes, will do in next version.

...
> > +CONFIG_BAYTRAIL_SECURE_BOOT=y  
> 
> This commit should come in later in this patch series, as
> CONFIG_BAYTRAIL_SECURE_BOOT is not available yet at this point.

OK, will move it.

--
Anatolij

[U-Boot] [PATCH 3/3] x86: baytrail: secureboot: Add functions for verification of U-Boot

[Anatolij Gustschin]

Hi Bin,

Sorry for long delay, I finally got to prepare v3 series.

On Tue, 16 May 2017 22:40:29 +0800
Bin Meng bmeng.cn at gmail.com wrote:
...
> > +#define SB_MANIFEST_BASE               0xFFFE0000  
> 
> nits: please use lower case hex and fix this globally in this file

OK, done in v3.

...
> > +#define PUB_KEY_MODULUS_SIZE           0x100
> > +#define U_BOOT_STAGE_SIZE              0xDD360  
> 
> What is this?
> 
> > +#define U_BOOT_OFFSET                  0x2CA0  
> 
> and this?

I don't know where these values are comming from, but I think they
are both wrong. A big U-Boot RAM stage part from reset vector remains
unprotected with these. I'll fix U_BOOT_STAGE_SIZE, drop U_BOOT_OFFSET
and will add a comment in v3. Will also adapt the image signing script
as needed.

> > +#define U_BOOT_STAGE_START             (CONFIG_SYS_TEXT_BASE + U_BOOT_OFFSET)
> > +#define U_BOOT_STAGE_END               (U_BOOT_STAGE_START + U_BOOT_STAGE_SIZE)
> > +
> > +#define SHA256_U_BOOT_STAGE_ID         0
> > +#define SHA256_FSP_STAGE2_ID           1
> > +#define SHA256_FIT_PUB_KEY_ID          2
> > +  
> 
> I believe a comment block is needed for explaining things like number
> 0/1/2, at least where are they coming?

These are indexes of 32-byte blocks in the manifest containing SHA256 hashes,
the stage order is fixed. Will add some comments here in v3.

...
> > +       /* compare the two hash  values */  
> 
> nits: two spaces after 'hash'

OK, fixed.

...
> > +/**
> > + * fsp_verify_u_boot_bin() - U-Boot binary verification
> > + *
> > + * This function verifies the integrity for U-Boot, its devicetree and the ucode
> > + * appended or inserted to the devicetree.
> > + *
> > + * @retval:    true on success, false on error
> > + */
> > +bool fsp_verify_u_boot_bin(void);
> > +
> > +/**
> > + * fsp_verify_public_key() - integrity of public key for fit image verification
> > + *
> > + * This function verifies the integrity for the modulus of the public key which
> > + * is stored in the U-Boot devicetree for fit image verification. It tries to  
> 
> nits: device tree

OK, fixed.

> > + * find the "rsa,modulus" property in the dtb and then verifies it with the
> > + * checksum stored in the oem-data block
> > + *
> > + * @retval:    true on success, false on error
> > + */
> > +bool fsp_verify_public_key(void);  
> 
> Again, are these two APIs common for all FSP based platforms?

No, these are used only for Bay Trail secure boot, so the prototypes
are in baytrail specific fsp_configs.h.

...
> > +               /*
> > +                * Verification of the public key happens with verification of
> > +                * the devicetree binary (that's where it's stored), this check  
> 
> nits: device tree

OK, fixed.

> > +                * is not necessary, but nice to see it's integer
> > +                */
> > +               if (!fsp_verify_public_key())
> > +                       puts("Secure Boot: Failed to verify public key for fit-image\n");  
> 
> As the comments say it's not necessary, which means it will never
> fail, right? But in case it fails, that means either hardware is doing
> wrong, or software is doing wrong? Can we adjust the output message to
> indicate some hints?

It can fail if the key is missing in the device tree or if the key
hash in the verified boot manifest doesn't match the hash of the key
in the device tree. I'll slightly rework it here to give more hints.

... 
> BTW: I don't see device tree update for the secure boot enabled board,
> like a device tree that has the "rsa,modulus" stuff.

The device tree will be updated by users, they have to supply their
own keys to the mkimage tool and it will insert the signature node
with custom "rsa,modulus" and other properties.

Thanks,

Anatolij