Skip to contentSkip to conversation

x86: baytrail: Add fsp-header verification for secure boot FSP

PATCH New Anatolij Gustschin 3 participants 4 messages Nov 17, 2017 – Nov 28, 2017 x86

lore
Share & tools
lore

Anatolij Gustschin

author
#0 lore raw

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

From: Markus Valentin <[email protected]>

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 <[email protected]>
Signed-off-by Anatolij Gustschin <[email protected]>

---
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(-)
arch/x86/Kconfig +2 -1
index 98c56ad7dc..6755e92748 100644
@@ -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
 	help
 	  The filename of the file to use as Firmware Support Package binary
 	  in the board directory.
arch/x86/cpu/baytrail/Kconfig +10 -0
index 1d876b1927..ff244fcd68 100644
@@ -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
arch/x86/include/asm/fsp/fsp_support.h +2 -0
index df3add008c..124a4148a0 100644
@@ -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
 
 
 /**
arch/x86/lib/fsp/fsp_support.c +24 -0
index e0c49be635..d79a6e900a 100644
@@ -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
+
 	/* Copy default data from Flash */
 	memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
 	       sizeof(struct upd_region));
Signature · 1 line
2.11.0
#1 lore raw

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

Quoted 22 lines · Anatolij Gustschin

From: Markus Valentin <[email protected]>

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 <[email protected]>
Signed-off-by Anatolij Gustschin <[email protected]>

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

Quoted 5 lines
 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(-)
Reviewed-by Simon Glass <[email protected]>
#2 lore raw

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

In reply to Anatolij Gustschin

Hi Anatolij,

Quoted 33 lines · Anatolij Gustschin

From: Markus Valentin <[email protected]>

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 <[email protected]>
Signed-off-by Anatolij Gustschin <[email protected]>

---
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(-)
arch/x86/Kconfig +0 -0
index 98c56ad7dc..6755e92748 100644

arch/x86/Kconfig config HAVE_FSP · line 354

Quoted 6 lines
arch/x86/Kconfig +2 -1
@@ -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?

Quoted 71 lines
arch/x86/Kconfig +0 -0
        help
          The filename of the file to use as Firmware Support Package binary
          in the board directory.
arch/x86/cpu/baytrail/Kconfig +10 -0
index 1d876b1927..ff244fcd68 100644
@@ -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
arch/x86/include/asm/fsp/fsp_support.h +2 -0
index df3add008c..124a4148a0 100644
@@ -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
 
 
 /**
arch/x86/lib/fsp/fsp_support.c +14 -0
index e0c49be635..d79a6e900a 100644
@@ -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.
+        */

arch/x86/lib/fsp/fsp_support.c void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf) · line 156

Quoted 10 lines
arch/x86/lib/fsp/fsp_support.c +10 -0
+       #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?

arch/x86/lib/fsp/fsp_support.c void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)

Quoted 4 lines
arch/x86/lib/fsp/fsp_support.c +0 -1
        /* Copy default data from Flash */
        memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
               sizeof(struct upd_region));
--

Regards,
Bin

author
#3 lore raw

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

Hi Bin,

On Tue, 21 Nov 2017 23:01:08 +0800
Bin Meng bmeng.cn at gmail.com wrote:
...

Quoted 13 lines

arch/x86/Kconfig config HAVE_FSP · line 354

Quoted 9 lines
arch/x86/Kconfig +2 -1
@@ -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:

Quoted 8 lines

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

Ok, will rework in v4.

Quoted 1 line

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

No, currently there is not such generic option. But CONFIG_SECURE_BOOT is used in some powerpc and arm configs. I'll add the generic option to the top level Kconfig.

...

Quoted 13 lines
Quoted 13 lines
arch/x86/Kconfig +9 -0
@@ -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
Quoted 8 lines

arch/x86/Kconfig void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf) · line 156

Quoted 5 lines
arch/x86/Kconfig +5 -0
+        */
+       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?

this makes sense, thanks. I'll add it as fsp_verify_boot_image() API function that is empty when SECURE_BOOT not enabled.

Thanks,
Anatolij