[PATCH v3 0/2] imx: hab: Make imx_hab_is_enabled dependent on FIELD_RETURN
The imx_hab_is_enabled function makes sure SPL and U-Boot only blocks the boot process when HAB is actually enabled. Currently only the SEC_CONFIG fuse is checked for this, as this is the fuse that closes the board for HAB. The Field return fuse however is used to permanently disable HAB. This fuse is not taken into account.
Take the FIELD_RETURN fuse into account as well when deciding whether HAB is enabled.
v2: - Split up into two commits v3: - Fixed support for i.MX8M Plus, which has a fuse pattern
The imx_sec_config_fuse_t structure is not specific to the sec_config fuse, but can be used for all fuse words.
Rename the structure to a more generic name to be reused for other fuses.
Signed-off-by: Paul Geurts paul.geurts@prodrive-technologies.com --- arch/arm/include/asm/mach-imx/hab.h | 4 ++-- arch/arm/mach-imx/hab.c | 4 ++-- arch/arm/mach-imx/imx8m/soc.c | 2 +- arch/arm/mach-imx/mx6/soc.c | 2 +- arch/arm/mach-imx/mx7/soc.c | 2 +- arch/arm/mach-imx/mx7ulp/soc.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 2abf28ea45bc..52cd8aff87e1 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -132,13 +132,13 @@ enum hab_target { HAB_TGT_ANY = 0x55, };
-struct imx_sec_config_fuse_t { +struct imx_fuse_t { int bank; int word; };
#if defined(CONFIG_IMX_HAB) -extern struct imx_sec_config_fuse_t const imx_sec_config_fuse; +extern struct imx_fuse_t const imx_sec_config_fuse; #endif
/*Function prototype description*/ diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index a8107f46ae5e..13598e3181a1 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -870,8 +870,8 @@ static int validate_ivt(struct ivt *ivt_initial)
bool imx_hab_is_enabled(void) { - struct imx_sec_config_fuse_t *fuse = - (struct imx_sec_config_fuse_t *)&imx_sec_config_fuse; + struct imx_fuse_t *fuse = + (struct imx_fuse_t *)&imx_sec_config_fuse; uint32_t reg; int ret;
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index a72329ea919d..355c704505fd 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -37,7 +37,7 @@ DECLARE_GLOBAL_DATA_PTR;
#if defined(CONFIG_IMX_HAB) -struct imx_sec_config_fuse_t const imx_sec_config_fuse = { +struct imx_fuse_t const imx_sec_config_fuse = { .bank = 1, .word = 3, }; diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c index 9b40fe9235a4..a6d673ee8e91 100644 --- a/arch/arm/mach-imx/mx6/soc.c +++ b/arch/arm/mach-imx/mx6/soc.c @@ -51,7 +51,7 @@ U_BOOT_DRVINFO(imx6_thermal) = { #endif
#if defined(CONFIG_IMX_HAB) -struct imx_sec_config_fuse_t const imx_sec_config_fuse = { +struct imx_fuse_t const imx_sec_config_fuse = { .bank = 0, .word = 6, }; diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c index 1b891a2db3d4..4b9b2bbc8620 100644 --- a/arch/arm/mach-imx/mx7/soc.c +++ b/arch/arm/mach-imx/mx7/soc.c @@ -127,7 +127,7 @@ static void isolate_resource(void) #endif
#if defined(CONFIG_IMX_HAB) -struct imx_sec_config_fuse_t const imx_sec_config_fuse = { +struct imx_fuse_t const imx_sec_config_fuse = { .bank = 1, .word = 3, }; diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach-imx/mx7ulp/soc.c index 980e02261563..10dbce388cb4 100644 --- a/arch/arm/mach-imx/mx7ulp/soc.c +++ b/arch/arm/mach-imx/mx7ulp/soc.c @@ -38,7 +38,7 @@ static char *get_reset_cause(char *);
#if defined(CONFIG_IMX_HAB) -struct imx_sec_config_fuse_t const imx_sec_config_fuse = { +struct imx_fuse_t const imx_sec_config_fuse = { .bank = 29, .word = 6, };
On 10/17/24 9:50 AM, Paul Geurts wrote:
The imx_sec_config_fuse_t structure is not specific to the sec_config fuse, but can be used for all fuse words.
Rename the structure to a more generic name to be reused for other fuses.
Signed-off-by: Paul Geurts paul.geurts@prodrive-technologies.com
arch/arm/include/asm/mach-imx/hab.h | 4 ++-- arch/arm/mach-imx/hab.c | 4 ++-- arch/arm/mach-imx/imx8m/soc.c | 2 +- arch/arm/mach-imx/mx6/soc.c | 2 +- arch/arm/mach-imx/mx7/soc.c | 2 +- arch/arm/mach-imx/mx7ulp/soc.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 2abf28ea45bc..52cd8aff87e1 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -132,13 +132,13 @@ enum hab_target { HAB_TGT_ANY = 0x55, };
-struct imx_sec_config_fuse_t { +struct imx_fuse_t {
Please drop the _t suffix from it, otherwise it resembles all those downstream typedef struct {} stuff_t; constructs.
Thanks !
The decision on whether HAB is enabled is solely based on the SEC_CONFIG fuse. The HAB FIELD_RETURN feature is able to permanently disable HAB on a CPU, after which it is able to boot unsigned firmware. U-Boot however does not take into account the FIELD_RETURN mode, and refuses to boot unsigned software when the feature is enabled.
Also take the FIELD_RETURN fuse into account when deciding whether HAB is enabled. When The FIELD_RETURN fuse is blown, HAB is not enabled.
Tested on i.MX8M Mini, i.MX8M Plus, i.MX8M Nano and i.MX6ULL
Signed-off-by: Paul Geurts paul.geurts@prodrive-technologies.com --- arch/arm/include/asm/mach-imx/hab.h | 1 + arch/arm/mach-imx/hab.c | 26 +++++++++++++++++++++++--- arch/arm/mach-imx/imx8m/soc.c | 5 +++++ arch/arm/mach-imx/mx6/soc.c | 5 +++++ arch/arm/mach-imx/mx7/soc.c | 5 +++++ arch/arm/mach-imx/mx7ulp/soc.c | 5 +++++ 6 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h index 52cd8aff87e1..d70e8eac1358 100644 --- a/arch/arm/include/asm/mach-imx/hab.h +++ b/arch/arm/include/asm/mach-imx/hab.h @@ -139,6 +139,7 @@ struct imx_fuse_t {
#if defined(CONFIG_IMX_HAB) extern struct imx_fuse_t const imx_sec_config_fuse; +extern struct imx_fuse_t const imx_field_return_fuse; #endif
/*Function prototype description*/ diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c index 13598e3181a1..d3bdf6d76f73 100644 --- a/arch/arm/mach-imx/hab.c +++ b/arch/arm/mach-imx/hab.c @@ -26,6 +26,14 @@ DECLARE_GLOBAL_DATA_PTR; #define IS_HAB_ENABLED_BIT \ (is_soc_type(MXC_SOC_MX7ULP) ? 0x80000000 : \ ((is_soc_type(MXC_SOC_MX7) || is_soc_type(MXC_SOC_IMX8M)) ? 0x2000000 : 0x2)) +#define FIELD_RETURN_FUSE_MASK \ + (is_imx8mp() ? 0xFFFFFFFF : 0x00000001) +/* + * The fuse pattern for i.MX8M Plus is 0x28001401, but bit 2 is already set from factory. + * This means when field return is set, the fuse word value reads 0x28001405 + */ +#define FIELD_RETURN_PATTERN \ + (is_imx8mp() ? 0x28001405 : 0x00000001)
#ifdef CONFIG_MX7ULP #define HAB_M4_PERSISTENT_START ((soc_rev() >= CHIP_REV_2_0) ? 0x20008040 : \ @@ -870,18 +878,30 @@ static int validate_ivt(struct ivt *ivt_initial)
bool imx_hab_is_enabled(void) { - struct imx_fuse_t *fuse = + struct imx_fuse_t *sec_config = (struct imx_fuse_t *)&imx_sec_config_fuse; + struct imx_fuse_t *field_return = + (struct imx_fuse_t *)&imx_field_return_fuse; uint32_t reg; + bool is_enabled; int ret;
- ret = fuse_read(fuse->bank, fuse->word, ®); + ret = fuse_read(sec_config->bank, sec_config->word, ®); if (ret) { puts("\nSecure boot fuse read error\n"); return ret; } + is_enabled = (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT; + if (is_enabled) { + ret = fuse_read(field_return->bank, field_return->word, ®); + if (ret) { + puts("\nField return fuse read error\n"); + return ret; + } + is_enabled = !((reg & FIELD_RETURN_FUSE_MASK) == FIELD_RETURN_PATTERN); + }
- return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT; + return is_enabled; }
int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t image_size, diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 355c704505fd..31583b5fbcad 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -41,6 +41,11 @@ struct imx_fuse_t const imx_sec_config_fuse = { .bank = 1, .word = 3, }; + +struct imx_fuse_t const imx_field_return_fuse = { + .bank = 8, + .word = 3, +}; #endif
int timer_init(void) diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c index a6d673ee8e91..6822ab8d2432 100644 --- a/arch/arm/mach-imx/mx6/soc.c +++ b/arch/arm/mach-imx/mx6/soc.c @@ -55,6 +55,11 @@ struct imx_fuse_t const imx_sec_config_fuse = { .bank = 0, .word = 6, }; + +struct imx_fuse_t const imx_field_return_fuse = { + .bank = 5, + .word = 6, +}; #endif
u32 get_nr_cpus(void) diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c index 4b9b2bbc8620..7a231d3bc8dd 100644 --- a/arch/arm/mach-imx/mx7/soc.c +++ b/arch/arm/mach-imx/mx7/soc.c @@ -131,6 +131,11 @@ struct imx_fuse_t const imx_sec_config_fuse = { .bank = 1, .word = 3, }; + +struct imx_fuse_t const imx_field_return_fuse = { + .bank = 8, + .word = 3, +}; #endif
static bool is_mx7d(void) diff --git a/arch/arm/mach-imx/mx7ulp/soc.c b/arch/arm/mach-imx/mx7ulp/soc.c index 10dbce388cb4..c83fe016f6ee 100644 --- a/arch/arm/mach-imx/mx7ulp/soc.c +++ b/arch/arm/mach-imx/mx7ulp/soc.c @@ -42,6 +42,11 @@ struct imx_fuse_t const imx_sec_config_fuse = { .bank = 29, .word = 6, }; + +struct imx_fuse_t const imx_field_return_fuse = { + .bank = 9, + .word = 6, +}; #endif
#define ROM_VERSION_ADDR 0x80
On 10/17/24 9:50 AM, Paul Geurts wrote:
[...]
bool imx_hab_is_enabled(void) {
- struct imx_fuse_t *fuse =
- struct imx_fuse_t *sec_config = (struct imx_fuse_t *)&imx_sec_config_fuse;
- struct imx_fuse_t *field_return =
(struct imx_fuse_t *)&imx_field_return_fuse;- bool is_enabled; int ret;
- ret = fuse_read(fuse->bank, fuse->word, ®);
- ret = fuse_read(sec_config->bank, sec_config->word, ®); if (ret) { puts("\nSecure boot fuse read error\n");
Drop the leading \n please.
return ret;}
- is_enabled = (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT;
is_enabled = reg & IS_HAB_ENABLED_BIT; is enough here.
- if (is_enabled) {
ret = fuse_read(field_return->bank, field_return->word, ®);if (ret) {puts("\nField return fuse read error\n");
Drop the leading \n please.
return ret;}is_enabled = !((reg & FIELD_RETURN_FUSE_MASK) == FIELD_RETURN_PATTERN);
is_enabled = (reg & FIELD_RETURN_FUSE_MASK) != FIELD_RETURN_PATTERN;
- }
- return (reg & IS_HAB_ENABLED_BIT) == IS_HAB_ENABLED_BIT;
- return is_enabled; }
[...]
participants (2)
-
Marek Vasut -
Paul Geurts