[U-Boot] [PATCH 0/4] Improve HAB support for SPL targets
This patch set is improving the HAB support for targets that are using IH_TYPE_FIRMWARE_IVT image type.
Patch 0001 is migrating CONFIG_CSF_SIZE to Kconfig so value can be configurable and used by U-Boot tools.
Patch 0002 is fixing secure boot support for targets using IH_TYPE_FIRMWARE_IVT image type.
Patch 0003 is verifying the SPL size to avoid a possible HAB failure event.
Patch 0004 is cleaning up the code as CONFIG_SECURE_BOOT is now enabled through Kconfig.
Breno Lima (4): Kconfig: Migrate CONFIG_CSF_SIZE to Kconfig habv4: tools: Avoid hardcoded CSF size for SPL targets imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled imx: configs: Cleanup CONFIG_SECURE_BOOT comments
arch/arm/mach-imx/Kconfig | 7 +++++++ common/image.c | 8 +++++--- configs/imx8mq_evk_defconfig | 1 + include/configs/cl-som-imx7.h | 4 ---- include/configs/imx8mq_evk.h | 4 ---- include/configs/mx6_common.h | 4 ---- include/configs/mx6sllevk.h | 6 ------ include/configs/mx6ullevk.h | 6 ------ include/configs/mx7_common.h | 4 ---- include/configs/mx7ulp_evk.h | 9 --------- scripts/config_whitelist.txt | 1 - tools/default_image.c | 5 ++++- tools/spl_size_limit.c | 3 +++ 13 files changed, 20 insertions(+), 42 deletions(-)
Move CONFIG_CSF_SIZE to Kconfig and define default value as 0x4000.
mx8mqevk requires 0x2000 add this configuration in imx8mq_evk_defconfig file.
Signed-off-by: Breno Lima breno.lima@nxp.com --- arch/arm/mach-imx/Kconfig | 7 +++++++ configs/imx8mq_evk_defconfig | 1 + include/configs/cl-som-imx7.h | 1 - include/configs/imx8mq_evk.h | 4 ---- include/configs/mx6_common.h | 4 ---- include/configs/mx6sllevk.h | 6 ------ include/configs/mx6ullevk.h | 6 ------ include/configs/mx7_common.h | 4 ---- include/configs/mx7ulp_evk.h | 6 ------ scripts/config_whitelist.txt | 1 - 10 files changed, 8 insertions(+), 32 deletions(-)
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index b6fd1595f0..3df96d570b 100644 --- a/arch/arm/mach-imx/Kconfig +++ b/arch/arm/mach-imx/Kconfig @@ -43,6 +43,13 @@ config SECURE_BOOT This option enables the support for secure boot (HAB). See doc/README.mxc_hab for more details.
+config CSF_SIZE + hex "Maximum size for Command Sequence File (CSF) binary" + default 0x4000 + help + Define the maximum size for Command Sequence File (CSF) binary + this information is used to define the image boot data. + config CMD_BMODE bool "Support the 'bmode' command" default y diff --git a/configs/imx8mq_evk_defconfig b/configs/imx8mq_evk_defconfig index 8417c3ba54..534f4b39f0 100644 --- a/configs/imx8mq_evk_defconfig +++ b/configs/imx8mq_evk_defconfig @@ -6,6 +6,7 @@ CONFIG_SYS_TEXT_BASE=0x40200000 CONFIG_TARGET_IMX8MQ_EVK=y CONFIG_SPL_SERIAL_SUPPORT=y CONFIG_SPL=y +CONFIG_CSF_SIZE=0x2000 CONFIG_FIT=y CONFIG_FIT_EXTERNAL_OFFSET=0x3000 CONFIG_SPL_LOAD_FIT=y diff --git a/include/configs/cl-som-imx7.h b/include/configs/cl-som-imx7.h index 4c93fc6cbe..73fbceec06 100644 --- a/include/configs/cl-som-imx7.h +++ b/include/configs/cl-som-imx7.h @@ -19,7 +19,6 @@
/* Uncomment to enable secure boot support */ /* #define CONFIG_SECURE_BOOT */ -#define CONFIG_CSF_SIZE 0x4000
/* Network */ #define CONFIG_FEC_MXC diff --git a/include/configs/imx8mq_evk.h b/include/configs/imx8mq_evk.h index 16e4136fa9..8ecdf9eb67 100644 --- a/include/configs/imx8mq_evk.h +++ b/include/configs/imx8mq_evk.h @@ -9,10 +9,6 @@ #include <linux/sizes.h> #include <asm/arch/imx-regs.h>
-#ifdef CONFIG_SECURE_BOOT -#define CONFIG_CSF_SIZE 0x2000 /* 8K region */ -#endif - #define CONFIG_SPL_MAX_SIZE (124 * 1024) #define CONFIG_SYS_MONITOR_LEN (512 * 1024) #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h index 2b8ce9d71d..25e27da54b 100644 --- a/include/configs/mx6_common.h +++ b/include/configs/mx6_common.h @@ -57,12 +57,8 @@ /* MMC */ #define CONFIG_FSL_USDHC
-/* Secure boot (HAB) support */ -#ifdef CONFIG_SECURE_BOOT -#define CONFIG_CSF_SIZE 0x4000 #ifdef CONFIG_SPL_BUILD #define CONFIG_SPL_DRIVERS_MISC_SUPPORT #endif -#endif
#endif diff --git a/include/configs/mx6sllevk.h b/include/configs/mx6sllevk.h index fb8f44684b..b96e63198d 100644 --- a/include/configs/mx6sllevk.h +++ b/include/configs/mx6sllevk.h @@ -10,12 +10,6 @@
#include "mx6_common.h"
-#ifdef CONFIG_SECURE_BOOT -#ifndef CONFIG_CSF_SIZE -#define CONFIG_CSF_SIZE 0x4000 -#endif -#endif - /* Size of malloc() pool */ #define CONFIG_SYS_MALLOC_LEN (16 * SZ_1M)
diff --git a/include/configs/mx6ullevk.h b/include/configs/mx6ullevk.h index 1fc5c24dec..7416ccf3ae 100644 --- a/include/configs/mx6ullevk.h +++ b/include/configs/mx6ullevk.h @@ -13,12 +13,6 @@ #include "mx6_common.h" #include <asm/mach-imx/gpio.h>
-#ifdef CONFIG_SECURE_BOOT -#ifndef CONFIG_CSF_SIZE -#define CONFIG_CSF_SIZE 0x4000 -#endif -#endif - #define PHYS_SDRAM_SIZE SZ_512M
/* Size of malloc() pool */ diff --git a/include/configs/mx7_common.h b/include/configs/mx7_common.h index 4f822ef9a0..01a625b135 100644 --- a/include/configs/mx7_common.h +++ b/include/configs/mx7_common.h @@ -46,13 +46,9 @@
#define CONFIG_ARMV7_PSCI_1_0
-/* Secure boot (HAB) support */ -#ifdef CONFIG_SECURE_BOOT -#define CONFIG_CSF_SIZE 0x4000 #ifdef CONFIG_SPL_BUILD #define CONFIG_SPL_DRIVERS_MISC_SUPPORT #endif -#endif
/* * If we have defined the OPTEE ram size and not OPTEE it means that we were diff --git a/include/configs/mx7ulp_evk.h b/include/configs/mx7ulp_evk.h index 2af5a4fe3e..c53270d3ad 100644 --- a/include/configs/mx7ulp_evk.h +++ b/include/configs/mx7ulp_evk.h @@ -14,12 +14,6 @@ /*Uncomment it to use secure boot*/ /*#define CONFIG_SECURE_BOOT*/
-#ifdef CONFIG_SECURE_BOOT -#ifndef CONFIG_CSF_SIZE -#define CONFIG_CSF_SIZE 0x4000 -#endif -#endif - #define CONFIG_BOARD_POSTCLK_INIT #define CONFIG_SYS_BOOTM_LEN 0x1000000
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index 2c9cfb450d..9684f1784a 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -276,7 +276,6 @@ CONFIG_CPU_VR41XX CONFIG_CQSPI_REF_CLK CONFIG_CS8900_BUS16 CONFIG_CS8900_BUS32 -CONFIG_CSF_SIZE CONFIG_CTL_JTAG CONFIG_CTL_TBE CONFIG_CUSTOMER_BOARD_SUPPORT
On Thu, Jul 18, 2019 at 9:34 AM Breno Matheus Lima breno.lima@nxp.com wrote:
Move CONFIG_CSF_SIZE to Kconfig and define default value as 0x4000.
mx8mqevk requires 0x2000 add this configuration in imx8mq_evk_defconfig file.
Signed-off-by: Breno Lima breno.lima@nxp.com
Reviewed-by: Fabio Estevam festevam@gmail.com
Currently it's not possible to authenticate the U-Boot proper of mx6ul_14x14_evk_defconfig target:
Authenticate image from DDR location 0x877fffc0... bad magic magic=0x0 length=0x00 version=0x3 bad length magic=0x0 length=0x00 version=0x3 bad version magic=0x0 length=0x00 version=0x3 spl: ERROR: image authentication fail
Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and i.MX7 devices") has increased CSF_SIZE to avoid a possible issue when booting encrypted boot images.
Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type for HAB verification") is hardcoding the CSF and IVT sizes, the new CSF size is not being considered and u-boot-ivt.img fails to boot.
Avoid hardcoded CSF and IVT size to fix this issue.
Signed-off-by: Breno Lima breno.lima@nxp.com --- common/image.c | 8 +++++--- tools/default_image.c | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/common/image.c b/common/image.c index 9f9538fac2..fc19dfdd9c 100644 --- a/common/image.c +++ b/common/image.c @@ -54,6 +54,8 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch, #endif /* !USE_HOSTCC*/
#include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h>
#ifndef CONFIG_SYS_BARGSIZE #define CONFIG_SYS_BARGSIZE 512 @@ -369,9 +371,9 @@ void image_print_contents(const void *ptr) } } else if (image_check_type(hdr, IH_TYPE_FIRMWARE_IVT)) { printf("HAB Blocks: 0x%08x 0x0000 0x%08x\n", - image_get_load(hdr) - image_get_header_size(), - image_get_size(hdr) + image_get_header_size() - - 0x1FE0); + image_get_load(hdr) - image_get_header_size(), + (int)(image_get_size(hdr) + image_get_header_size() + + sizeof(flash_header_v2_t) - CONFIG_CSF_SIZE)); } }
diff --git a/tools/default_image.c b/tools/default_image.c index 4b7d1ed4a1..7a26232779 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -19,6 +19,8 @@ #include <image.h> #include <tee/optee.h> #include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h>
static image_header_t header;
@@ -106,7 +108,8 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,
if (params->type == IH_TYPE_FIRMWARE_IVT) /* Add size of CSF minus IVT */ - imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0; + imagesize = sbuf->st_size - sizeof(image_header_t) + + CONFIG_CSF_SIZE - sizeof(flash_header_v2_t); else imagesize = sbuf->st_size - sizeof(image_header_t);
On Thu, Jul 18, 2019 at 9:34 AM Breno Matheus Lima breno.lima@nxp.com wrote:
Currently it's not possible to authenticate the U-Boot proper of mx6ul_14x14_evk_defconfig target:
Authenticate image from DDR location 0x877fffc0... bad magic magic=0x0 length=0x00 version=0x3 bad length magic=0x0 length=0x00 version=0x3 bad version magic=0x0 length=0x00 version=0x3 spl: ERROR: image authentication fail
Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and i.MX7 devices") has increased CSF_SIZE to avoid a possible issue when booting encrypted boot images.
Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type for HAB verification") is hardcoding the CSF and IVT sizes, the new CSF size is not being considered and u-boot-ivt.img fails to boot.
Avoid hardcoded CSF and IVT size to fix this issue.
Signed-off-by: Breno Lima breno.lima@nxp.com
Reviewed-by: Fabio Estevam festevam@gmail.com
Hi Breno,
Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets
I saw this patch in imx/master, not in Tom's tree. But this patch breaks build for other archs, such as arc and etc.
Regards, Peng.
Currently it's not possible to authenticate the U-Boot proper of mx6ul_14x14_evk_defconfig target:
Authenticate image from DDR location 0x877fffc0... bad magic magic=0x0 length=0x00 version=0x3 bad length magic=0x0 length=0x00 version=0x3 bad version magic=0x0 length=0x00 version=0x3 spl: ERROR: image authentication fail
Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and i.MX7 devices") has increased CSF_SIZE to avoid a possible issue when booting encrypted boot images.
Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type for HAB verification") is hardcoding the CSF and IVT sizes, the new CSF size is not being considered and u-boot-ivt.img fails to boot.
Avoid hardcoded CSF and IVT size to fix this issue.
Signed-off-by: Breno Lima breno.lima@nxp.com
common/image.c | 8 +++++--- tools/default_image.c | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/common/image.c b/common/image.c index 9f9538fac2..fc19dfdd9c 100644 --- a/common/image.c +++ b/common/image.c @@ -54,6 +54,8 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch, #endif /* !USE_HOSTCC*/
#include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h>
#ifndef CONFIG_SYS_BARGSIZE #define CONFIG_SYS_BARGSIZE 512 @@ -369,9 +371,9 @@ void image_print_contents(const void *ptr) } } else if (image_check_type(hdr, IH_TYPE_FIRMWARE_IVT)) { printf("HAB Blocks: 0x%08x 0x0000 0x%08x\n",
image_get_load(hdr) - image_get_header_size(),image_get_size(hdr) + image_get_header_size()- 0x1FE0);
image_get_load(hdr) - image_get_header_size(),(int)(image_get_size(hdr) + image_get_header_size()+ sizeof(flash_header_v2_t) - CONFIG_CSF_SIZE));}
diff --git a/tools/default_image.c b/tools/default_image.c index 4b7d1ed4a1..7a26232779 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -19,6 +19,8 @@ #include <image.h> #include <tee/optee.h> #include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h>
static image_header_t header;
@@ -106,7 +108,8 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,
if (params->type == IH_TYPE_FIRMWARE_IVT) /* Add size of CSF minus IVT */
imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0;
imagesize = sbuf->st_size - sizeof(image_header_t)+ CONFIG_CSF_SIZE - sizeof(flash_header_v2_t);-- 2.17.1
Hi Peng,
Em qua, 11 de set de 2019 às 22:07, Peng Fan peng.fan@nxp.com escreveu:
Hi Breno,
Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets
I saw this patch in imx/master, not in Tom's tree. But this patch breaks build for other archs, such as arc and etc.
Thanks for reporting the issue, I will submit a fix.
Best regards, Breno Lima
Hi Breno,
On 12/09/19 03:07, Peng Fan wrote:
Hi Breno,
Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets
I saw this patch in imx/master, not in Tom's tree. But this patch breaks build for other archs, such as arc and etc.
Any news on this ? I dropped it from u-boot-imx due the breakage, but I can easy pick it up again if it will be fixed.
Regards, Stefano
Regards, Peng.
Currently it's not possible to authenticate the U-Boot proper of mx6ul_14x14_evk_defconfig target:
Authenticate image from DDR location 0x877fffc0... bad magic magic=0x0 length=0x00 version=0x3 bad length magic=0x0 length=0x00 version=0x3 bad version magic=0x0 length=0x00 version=0x3 spl: ERROR: image authentication fail
Commit 0633e134784a ("imx: hab: Increase CSF_SIZE for i.MX6 and i.MX7 devices") has increased CSF_SIZE to avoid a possible issue when booting encrypted boot images.
Commit d21bd69b6e95 ("tools: mkimage: add firmware-ivt image type for HAB verification") is hardcoding the CSF and IVT sizes, the new CSF size is not being considered and u-boot-ivt.img fails to boot.
Avoid hardcoded CSF and IVT size to fix this issue.
Signed-off-by: Breno Lima breno.lima@nxp.com
common/image.c | 8 +++++--- tools/default_image.c | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/common/image.c b/common/image.c index 9f9538fac2..fc19dfdd9c 100644 --- a/common/image.c +++ b/common/image.c @@ -54,6 +54,8 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch, #endif /* !USE_HOSTCC*/
#include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h>
#ifndef CONFIG_SYS_BARGSIZE #define CONFIG_SYS_BARGSIZE 512 @@ -369,9 +371,9 @@ void image_print_contents(const void *ptr) } } else if (image_check_type(hdr, IH_TYPE_FIRMWARE_IVT)) { printf("HAB Blocks: 0x%08x 0x0000 0x%08x\n",
image_get_load(hdr) - image_get_header_size(),image_get_size(hdr) + image_get_header_size()- 0x1FE0);
image_get_load(hdr) - image_get_header_size(),(int)(image_get_size(hdr) + image_get_header_size()+ sizeof(flash_header_v2_t) - CONFIG_CSF_SIZE));}
diff --git a/tools/default_image.c b/tools/default_image.c index 4b7d1ed4a1..7a26232779 100644 --- a/tools/default_image.c +++ b/tools/default_image.c @@ -19,6 +19,8 @@ #include <image.h> #include <tee/optee.h> #include <u-boot/crc.h> +#include <imximage.h> +#include <generated/autoconf.h>
static image_header_t header;
@@ -106,7 +108,8 @@ static void image_set_header(void *ptr, struct stat *sbuf, int ifd,
if (params->type == IH_TYPE_FIRMWARE_IVT) /* Add size of CSF minus IVT */
imagesize = sbuf->st_size - sizeof(image_header_t) + 0x1FE0;
imagesize = sbuf->st_size - sizeof(image_header_t)+ CONFIG_CSF_SIZE - sizeof(flash_header_v2_t);-- 2.17.1
Hi Stefano,
Em seg, 16 de set de 2019 às 05:17, Stefano Babic sbabic@denx.de escreveu:
Hi Breno,
On 12/09/19 03:07, Peng Fan wrote:
Hi Breno,
Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets
I saw this patch in imx/master, not in Tom's tree. But this patch breaks build for other archs, such as arc and etc.
Any news on this ? I dropped it from u-boot-imx due the breakage, but I can easy pick it up again if it will be fixed.
Sorry the delay. I'm still trying to reproduce the issue, probably something is missing in my buildman setup.
One workaround would be to enclose the code with CONFIG_SECURE_BOOT, this code is only used by IH_TYPE_FIRMWARE_IVT which requires HAB to be enabled. I can send a patch but I would like to confirm before.
Thanks, Breno Lima
On 17/09/19 05:26, Breno Matheus Lima wrote:
Hi Stefano,
Em seg, 16 de set de 2019 às 05:17, Stefano Babic sbabic@denx.de escreveu:
Hi Breno,
On 12/09/19 03:07, Peng Fan wrote:
Hi Breno,
Subject: [PATCH 2/4] habv4: tools: Avoid hardcoded CSF size for SPL targets
I saw this patch in imx/master, not in Tom's tree. But this patch breaks build for other archs, such as arc and etc.
Any news on this ? I dropped it from u-boot-imx due the breakage, but I can easy pick it up again if it will be fixed.
Sorry the delay. I'm still trying to reproduce the issue, probably something is missing in my buildman setup.
It looks like that a header is missing, maybe this came together with the moving to a common place for gzip & Co (patch for Simon). I think it could be easy for you to reproduce if you set current u-boot-imx.
One workaround would be to enclose the code with CONFIG_SECURE_BOOT, this code is only used by IH_TYPE_FIRMWARE_IVT which requires HAB to be enabled. I can send a patch but I would like to confirm before.
Nevertheless, HAB code should be put in just if needed. I agree on the patch independently from the issue.
Regards, Stefano
Thanks, Breno Lima
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com --- tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE) + spl_size_limit -= CONFIG_CSF_SIZE; +#endif #ifdef CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD spl_size_limit -= GENERATED_GBL_DATA_SIZE; #endif
On Thu, Jul 18, 2019 at 9:34 AM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
Reviewed-by: Fabio Estevam festevam@gmail.com
Hi Breno,
On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
spl_size_limit -= CONFIG_CSF_SIZE;+#endif
But, if the target enable HAB on SPL the size would be part of SPL limit, isn't ?
Just now I have looked at this, since one of my board (imx6_mamoj) fails to build.
Jagan.
Hi Jagan, Breno,
On 17/09/19 09:13, Jagan Teki wrote:
Hi Breno,
On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
spl_size_limit -= CONFIG_CSF_SIZE;+#endif
But, if the target enable HAB on SPL the size would be part of SPL limit, isn't ?
Indeed - it is not clear to me, too, if it is correct, even if CSF is added later by the NXP signing tools. The patch reduces significantly the available space for SPL, I just wondering why just mamoj is affected. Jagan, does it work without this patch applied ?
Regards, Stefano
Just now I have looked at this, since one of my board (imx6_mamoj) fails to build.
Jagan.
HI Stefano and Jagan,
Em qua, 18 de set de 2019 às 04:59, Stefano Babic sbabic@denx.de escreveu:
Hi Jagan, Breno,
On 17/09/19 09:13, Jagan Teki wrote:
Hi Breno,
On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
spl_size_limit -= CONFIG_CSF_SIZE;+#endif
But, if the target enable HAB on SPL the size would be part of SPL limit, isn't ?
Indeed - it is not clear to me, too, if it is correct, even if CSF is added later by the NXP signing tools. The patch reduces significantly the available space for SPL, I just wondering why just mamoj is affected. Jagan, does it work without this patch applied ?
When enabling CONFIG_SECURE_BOOT we increase the image length in boot data by the size defined in CONFIG_CSF_SIZE. The HAB code will parse the boot data structure and copy the image length defined (SPL image plus CSF appended) to its respective load address.
HAB code is checking if the image length defined can fit in OCRAM free region, and logs the following HAB event in case not:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
HAB closed targets would then fail to boot, so for that reason we added CONFIG_CSF_SIZE into consideration.
We can reduce the default CONFIG_CSF_SIZE but it depends on the user specific HAB setup. I did a quick test with RSA 4K keys and couldn't achieve 0x2000 length.
Do you think we should decrease default CONFIG_CSF_SIZE? Perhaps 0x2000 plus the maximum dek blob size (0x60) would be enough for most uses cases, users requiring more space can modify their CONFIG_CSF_SIZE.
Thanks, Breno Lima
Hi Breno,
On 19/09/19 03:31, Breno Matheus Lima wrote:
HI Stefano and Jagan,
Em qua, 18 de set de 2019 às 04:59, Stefano Babic sbabic@denx.de escreveu:
Hi Jagan, Breno,
On 17/09/19 09:13, Jagan Teki wrote:
Hi Breno,
On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
spl_size_limit -= CONFIG_CSF_SIZE;+#endif
But, if the target enable HAB on SPL the size would be part of SPL limit, isn't ?
Indeed - it is not clear to me, too, if it is correct, even if CSF is added later by the NXP signing tools. The patch reduces significantly the available space for SPL, I just wondering why just mamoj is affected. Jagan, does it work without this patch applied ?
When enabling CONFIG_SECURE_BOOT we increase the image length in boot data by the size defined in CONFIG_CSF_SIZE. The HAB code will parse the boot data structure and copy the image length defined (SPL image plus CSF appended) to its respective load address.
HAB code is checking if the image length defined can fit in OCRAM free region, and logs the following HAB event in case not:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
HAB closed targets would then fail to boot, so for that reason we added CONFIG_CSF_SIZE into consideration.
Clear - thanks for detailed explanation.
We can reduce the default CONFIG_CSF_SIZE but it depends on the user specific HAB setup. I did a quick test with RSA 4K keys and couldn't achieve 0x2000 length.
That is much less as we have now.
Do you think we should decrease default CONFIG_CSF_SIZE?
I think yes - if we set it for the worst case, we reduce the SPL size so much that most boards, if they enable SECURE_BOOT, won't build. I cannot say that imx6dl_mamoj has dead code in its SPL, it is one of the board with the "state of art" in U-Boot, with DM and OF in SPL. But this is also something we decided to push into U-Boot. Anyway, every board maintainer can change it and add it to the own defconfig.
Jagan, after setting CONFIG_CSF_SIZE to 0x2060 as suggested by Breno, board builds fine - but I have no idea if it can boots. Can you check this ?
Perhaps 0x2000 plus the maximum dek blob size (0x60) would be enough for most uses cases, users requiring more space can modify their CONFIG_CSF_SIZE.
Best regards, Stefano
Hi Stefano,
On Wed, Sep 18, 2019 at 1:29 PM Stefano Babic sbabic@denx.de wrote:
Hi Jagan, Breno,
On 17/09/19 09:13, Jagan Teki wrote:
Hi Breno,
On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
spl_size_limit -= CONFIG_CSF_SIZE;+#endif
But, if the target enable HAB on SPL the size would be part of SPL limit, isn't ?
Indeed - it is not clear to me, too, if it is correct, even if CSF is added later by the NXP signing tools. The patch reduces significantly the available space for SPL, I just wondering why just mamoj is affected. Jagan, does it work without this patch applied ?
mamoj is affected since the board enables SPL_DM, SPL_OF_CONTROL. Yes, the build look fine without this patch.
On 19/09/19 07:37, Jagan Teki wrote:
Hi Stefano,
On Wed, Sep 18, 2019 at 1:29 PM Stefano Babic sbabic@denx.de wrote:
Hi Jagan, Breno,
On 17/09/19 09:13, Jagan Teki wrote:
Hi Breno,
On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
spl_size_limit -= CONFIG_CSF_SIZE;+#endif
But, if the target enable HAB on SPL the size would be part of SPL limit, isn't ?
Indeed - it is not clear to me, too, if it is correct, even if CSF is added later by the NXP signing tools. The patch reduces significantly the available space for SPL, I just wondering why just mamoj is affected. Jagan, does it work without this patch applied ?
mamoj is affected since the board enables SPL_DM, SPL_OF_CONTROL. Yes, the build look fine without this patch.
Anyway, SPL size does not seem to much. But dropping 0x4000 to the available size is really a lot, and I hope we can reduce this.
Regards, Stefano
Hi Stefano and Jagan,
Em qui, 19 de set de 2019 às 05:27, Stefano Babic sbabic@denx.de escreveu:
On 19/09/19 07:37, Jagan Teki wrote:
Hi Stefano,
On Wed, Sep 18, 2019 at 1:29 PM Stefano Babic sbabic@denx.de wrote:
Hi Jagan, Breno,
On 17/09/19 09:13, Jagan Teki wrote:
Hi Breno,
On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima breno.lima@nxp.com wrote:
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to avoid a possible HAB failure event:
--------- HAB Event 1 ----------------- event data: 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00 0x00 0x01 0x10 0x00 STS = HAB_FAILURE (0x33) RSN = HAB_INV_ADDRESS (0x22) CTX = HAB_CTX_TARGET (0x33) ENG = HAB_ENG_ANY (0x00)
As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT default") the i.MX6 SPL size limit is 68KB.
The ROM code is copying the image size defined in boot data to its respective load address, in case we exceed the OCRAM free region a HAB invalid address failure event is generated.
The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size limit based on this configuration.
Signed-off-by: Breno Lima breno.lima@nxp.com
tools/spl_size_limit.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c index 98ff491867..8902e30129 100644 --- a/tools/spl_size_limit.c +++ b/tools/spl_size_limit.c @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
#ifdef CONFIG_SPL_SIZE_LIMIT spl_size_limit = CONFIG_SPL_SIZE_LIMIT; +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
spl_size_limit -= CONFIG_CSF_SIZE;+#endif
But, if the target enable HAB on SPL the size would be part of SPL limit, isn't ?
Indeed - it is not clear to me, too, if it is correct, even if CSF is added later by the NXP signing tools. The patch reduces significantly the available space for SPL, I just wondering why just mamoj is affected. Jagan, does it work without this patch applied ?
mamoj is affected since the board enables SPL_DM, SPL_OF_CONTROL. Yes, the build look fine without this patch.
Anyway, SPL size does not seem to much. But dropping 0x4000 to the available size is really a lot, and I hope we can reduce this.
Thanks for submitting a fix for mamoj board.
We should also reduce CSF_SIZE in default_image.c and image.c to avoid a U-Boot proper authentication failure in HAB closed devices. The current U-Boot tools code is hardcoding CSF_SIZE as 0x2000 and mx6ul_14x14_defconfig target is failing to boot with error below:
Authenticate image from DDR location 0x877fffc0... bad magic magic=0x32 length=0x6131 version=0x38 bad length magic=0x32 length=0x6131 version=0x38 bad version magic=0x32 length=0x6131 version=0x38 spl: ERROR: image authentication fail
The intent of "habv4: tools: Avoid hardcoded CSF size for SPL targets" is to avoid such issue. I tried to apply this patch back but I'm seeing gunzip related errors as reported by Igor Opaniuk. We may need to understand better this dependency.
I have just submitted a patch reducing default CSF_SIZE to 0x2060, this patch is also modifying default_image.c and image.c but CSF_SIZE still hardcoded in U-Boot tools code.
Thanks, Breno Lima
Since commit 6e1f4d2652e7 ("arm: imx-common: add SECURE_BOOT option to Kconfig") the SECURE_BOOT option is selected through Kconfig.
Cleanup comments in code to align with this change.
Signed-off-by: Breno Lima breno.lima@nxp.com --- include/configs/cl-som-imx7.h | 3 --- include/configs/mx7ulp_evk.h | 3 --- 2 files changed, 6 deletions(-)
diff --git a/include/configs/cl-som-imx7.h b/include/configs/cl-som-imx7.h index 73fbceec06..3ca16c5201 100644 --- a/include/configs/cl-som-imx7.h +++ b/include/configs/cl-som-imx7.h @@ -17,9 +17,6 @@
#define CONFIG_BOARD_LATE_INIT
-/* Uncomment to enable secure boot support */ -/* #define CONFIG_SECURE_BOOT */ - /* Network */ #define CONFIG_FEC_MXC #define CONFIG_FEC_XCV_TYPE RGMII diff --git a/include/configs/mx7ulp_evk.h b/include/configs/mx7ulp_evk.h index c53270d3ad..437b2758e8 100644 --- a/include/configs/mx7ulp_evk.h +++ b/include/configs/mx7ulp_evk.h @@ -11,9 +11,6 @@ #include <linux/sizes.h> #include <asm/arch/imx-regs.h>
-/*Uncomment it to use secure boot*/ -/*#define CONFIG_SECURE_BOOT*/ - #define CONFIG_BOARD_POSTCLK_INIT #define CONFIG_SYS_BOOTM_LEN 0x1000000
On Thu, Jul 18, 2019 at 9:34 AM Breno Matheus Lima breno.lima@nxp.com wrote:
Since commit 6e1f4d2652e7 ("arm: imx-common: add SECURE_BOOT option to Kconfig") the SECURE_BOOT option is selected through Kconfig.
Cleanup comments in code to align with this change.
Signed-off-by: Breno Lima breno.lima@nxp.com
Reviewed-by: Fabio Estevam festevam@gmail.com
participants (6)
-
Breno Matheus Lima -
Breno Matheus Lima -
Fabio Estevam -
Jagan Teki -
Peng Fan -
Stefano Babic