[PATCH 0/4] i.mx: hab/ocotop: extend field return to i.MX6

[PATCH 0/4] i.mx: hab/ocotop: extend field return to i.MX6

[Fabian Pflug]

Field return handling was only implemented for the i.MX8* processors.
Extend this with support for i.MX6 and extend the hab command with an
option to burn the field return fuse in case the field return sticky bit
might be unlocked.

Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
Fabian Pflug (4):
      arm: mach-imx6: use kconfig for field return
      nvmem: ocotp: extend support to query the sticky bit
      i.MX: HAB: extend field_return support to imx6
      commands: hab: extend by field_return fuse burn

 arch/arm/mach-imx/Kconfig                     |  6 +++++-
 commands/hab.c                                | 20 +++++++++++++++++---
 drivers/hab/hab.c                             | 10 ++++++----
 drivers/nvmem/ocotp.c                         | 12 ++++++++----
 include/mach/imx/habv4-imx6-gencsf-template.h | 11 +++--------
 include/mach/imx/ocotp-fusemap.h              |  1 +
 6 files changed, 40 insertions(+), 20 deletions(-)
---
base-commit: d6f0974673c0e3da00f8d0789d6302a43f3e478d
change-id: 20251218-v2025-11-0-topic-imx6-field-return-d3b4f2e55afb

Best regards,
-- 
Fabian Pflug <f.pflug@pengutronix.de>

[PATCH 1/4] arm: mach-imx6: use kconfig for field return

[Fabian Pflug]

There is a kConfig option for the field return, that is also documented,
so using it here instead of providing a headerfile to patch.

Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
 include/mach/imx/habv4-imx6-gencsf-template.h | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/include/mach/imx/habv4-imx6-gencsf-template.h b/include/mach/imx/habv4-imx6-gencsf-template.h
index 45da2981cb..c24bf84b85 100644
--- a/include/mach/imx/habv4-imx6-gencsf-template.h
+++ b/include/mach/imx/habv4-imx6-gencsf-template.h
@@ -39,17 +39,12 @@ hab Engine = SETUP_HABV4_ENGINE
 hab Features = SETUP_HABV4_FEATURES
 #endif
 
-/*
-// allow fusing FIELD_RETURN
-// # ocotp0.permanent_write_enable=1
-// # mw -l -d /dev/imx-ocotp 0xb8 0x1
+#if defined(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN)
 hab [Unlock]
 hab Engine = OCOTP
 hab Features = FIELD RETURN
-// device-specific UID:
-// $ dd if=/sys/bus/nvmem/devices/imx-ocotp0/nvmem bs=4 skip=1 count=2 status=none | hexdump -ve '1/1 "0x%.2x, "' | sed 's/, $//'
-hab UID = 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08
-*/
+hab UID = HABV4_CSF_UNLOCK_UID
+#endif
 
 hab [Install Key]
 /* verification key index in key store (0, 2...4) */

-- 
2.47.3

[PATCH 2/4] nvmem: ocotp: extend support to query the sticky bit

[Fabian Pflug]

The i.MX* devices do have an sticky bit which indicates if the
field-return fuse can be written. Before only support for i.MX8* was
provided. Extend this for the i.MX6* series.

Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
 drivers/nvmem/ocotp.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/nvmem/ocotp.c b/drivers/nvmem/ocotp.c
index 7bca275404..1f74fddb60 100644
--- a/drivers/nvmem/ocotp.c
+++ b/drivers/nvmem/ocotp.c
@@ -294,7 +294,7 @@ static void imx8m_lock_srk_revoke(struct ocotp_priv *priv)
 	writel(val, priv->base + OCOTP_SW_STICKY);
 }
 
-static bool imx8m_field_return_locked(struct ocotp_priv *priv)
+static bool imx_field_return_locked(struct ocotp_priv *priv)
 {
 	return readl(priv->base + OCOTP_SW_STICKY) & OCOTP_SW_STICKY_FIELD_RETURN_LOCK;
 }
@@ -990,6 +990,7 @@ static struct imx_ocotp_data imx6q_ocotp_data = {
 	.fuse_blow = imx6_fuse_blow_addr,
 	.fuse_read = imx6_fuse_read_addr,
 	.ctrl = &ocotp_ctrl_reg_default,
+	.field_return_locked = imx_field_return_locked,
 };
 
 static struct imx_ocotp_data imx6sl_ocotp_data = {
@@ -1002,6 +1003,7 @@ static struct imx_ocotp_data imx6sl_ocotp_data = {
 	.fuse_blow = imx6_fuse_blow_addr,
 	.fuse_read = imx6_fuse_read_addr,
 	.ctrl = &ocotp_ctrl_reg_default,
+	.field_return_locked = imx_field_return_locked,
 };
 
 static struct imx_ocotp_data imx6ul_ocotp_data = {
@@ -1014,6 +1016,7 @@ static struct imx_ocotp_data imx6ul_ocotp_data = {
 	.fuse_blow = imx6_fuse_blow_addr,
 	.fuse_read = imx6_fuse_read_addr,
 	.ctrl = &ocotp_ctrl_reg_default,
+	.field_return_locked = imx_field_return_locked,
 };
 
 static struct imx_ocotp_data imx6ull_ocotp_data = {
@@ -1026,6 +1029,7 @@ static struct imx_ocotp_data imx6ull_ocotp_data = {
 	.fuse_blow = imx6_fuse_blow_addr,
 	.fuse_read = imx6_fuse_read_addr,
 	.ctrl = &ocotp_ctrl_reg_default,
+	.field_return_locked = imx_field_return_locked,
 };
 
 static struct imx_ocotp_data vf610_ocotp_data = {
@@ -1063,7 +1067,7 @@ static struct imx_ocotp_data imx8mp_ocotp_data = {
 	.fuse_read = imx6_fuse_read_addr,
 	.srk_revoke_locked = imx8m_srk_revoke_locked,
 	.lock_srk_revoke = imx8m_lock_srk_revoke,
-	.field_return_locked = imx8m_field_return_locked,
+	.field_return_locked = imx_field_return_locked,
 	.ctrl = &ocotp_ctrl_reg_8mp,
 };
 
@@ -1095,7 +1099,7 @@ static struct imx_ocotp_data imx8mm_ocotp_data = {
 	.fuse_read = imx6_fuse_read_addr,
 	.srk_revoke_locked = imx8m_srk_revoke_locked,
 	.lock_srk_revoke = imx8m_lock_srk_revoke,
-	.field_return_locked = imx8m_field_return_locked,
+	.field_return_locked = imx_field_return_locked,
 	.feat = &imx8mm_featctrl_data,
 	.ctrl = &ocotp_ctrl_reg_default,
 };
@@ -1116,7 +1120,7 @@ static struct imx_ocotp_data imx8mn_ocotp_data = {
 	.fuse_read = imx6_fuse_read_addr,
 	.srk_revoke_locked = imx8m_srk_revoke_locked,
 	.lock_srk_revoke = imx8m_lock_srk_revoke,
-	.field_return_locked = imx8m_field_return_locked,
+	.field_return_locked = imx_field_return_locked,
 	.feat = &imx8mn_featctrl_data,
 	.ctrl = &ocotp_ctrl_reg_default,
 };

-- 
2.47.3

[PATCH 3/4] i.MX: HAB: extend field_return support to imx6

[Fabian Pflug]

Extend the helper for imx_fuse_burn with support for i.MX6 devices.

Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
 drivers/hab/hab.c                | 10 ++++++----
 include/mach/imx/ocotp-fusemap.h |  1 +
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/hab/hab.c b/drivers/hab/hab.c
index 1c747e8a3e..281645f79e 100644
--- a/drivers/hab/hab.c
+++ b/drivers/hab/hab.c
@@ -262,7 +262,7 @@ static int imx8m_hab_revoke_key_ocotp(unsigned key_idx)
  */
 #define MX8MP_FIELD_RETURN_PATTERN	0x28001401
 
-static int imx8m_hab_field_return_ocotp(void)
+static int imx_hab_field_return_ocotp(void)
 {
 	int ret;
 
@@ -274,7 +274,9 @@ static int imx8m_hab_field_return_ocotp(void)
 	if (ret == 1)
 		return -EINVAL;
 
-	if (cpu_is_mx8mp())
+	if (cpu_is_mx6())
+		ret = imx_ocotp_write_field(MX6_OCOTP_FIELD_RETURN, 1);
+	else if (cpu_is_mx8mp())
 		ret = imx_ocotp_write_field(MX8MP_OCOTP_FIELD_RETURN,
 					    MX8MP_FIELD_RETURN_PATTERN);
 	else
@@ -282,7 +284,6 @@ static int imx8m_hab_field_return_ocotp(void)
 
 	return ret;
 }
-
 struct imx_hab_ops {
 	int (*write_srk_hash)(const u8 *srk, unsigned flags);
 	int (*read_srk_hash)(u8 *srk);
@@ -310,6 +311,7 @@ static struct imx_hab_ops imx6_hab_ops_ocotp = {
 	.device_locked_down = imx6_hab_device_locked_down_ocotp,
 	.permanent_write_enable = imx_hab_permanent_write_enable_ocotp,
 	.print_status = imx6_hab_print_status,
+	.field_return = imx_hab_field_return_ocotp,
 };
 
 static struct imx_hab_ops imx8m_hab_ops_ocotp = {
@@ -320,7 +322,7 @@ static struct imx_hab_ops imx8m_hab_ops_ocotp = {
 	.permanent_write_enable = imx_hab_permanent_write_enable_ocotp,
 	.print_status = imx8m_hab_print_status,
 	.revoke_key = imx8m_hab_revoke_key_ocotp,
-	.field_return = imx8m_hab_field_return_ocotp,
+	.field_return = imx_hab_field_return_ocotp,
 };
 
 static int imx_ahab_write_srk_hash(const u8 *__newsrk, unsigned flags)
diff --git a/include/mach/imx/ocotp-fusemap.h b/include/mach/imx/ocotp-fusemap.h
index ae10dcef2a..3fd9d6df24 100644
--- a/include/mach/imx/ocotp-fusemap.h
+++ b/include/mach/imx/ocotp-fusemap.h
@@ -103,6 +103,7 @@
 #define MX8M_OCOTP_TZASC_EN		(OCOTP_WORD(0x480) | OCOTP_BIT(11) | OCOTP_WIDTH(1))
 #define MX8MP_OCOTP_ROM_NO_LOG		(OCOTP_WORD(0x480) | OCOTP_BIT(22) | OCOTP_WIDTH(1))
 #define MX8M_OCOTP_RECOVERY_SDMMC_BOOT_DIS	(OCOTP_WORD(0x490) | OCOTP_BIT(23) | OCOTP_WIDTH(1))
+#define MX6_OCOTP_FIELD_RETURN		(OCOTP_WORD(0x6E0) | OCOTP_BIT(0) | OCOTP_WIDTH(1))
 #define MX8M_OCOTP_FIELD_RETURN		(OCOTP_WORD(0x630) | OCOTP_BIT(0) | OCOTP_WIDTH(1))
 #define MX8MP_OCOTP_FIELD_RETURN	(OCOTP_WORD(0x630) | OCOTP_BIT(0) | OCOTP_WIDTH(32))
 #define MX8M_OCOTP_SRK_REVOKE		(OCOTP_WORD(0x670) | OCOTP_BIT(0) | OCOTP_WIDTH(4))

-- 
2.47.3

[PATCH 4/4] commands: hab: extend by field_return fuse burn

[Fabian Pflug]

Extend hab command with an additional parameter to burn the field return
fuse, but only if it is unlocked via the kconfig option.
Without the kconfig option, the extra argument makes no sense, as it
would not be possible to access the FIELD_RETURN fuse.

Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
---
 arch/arm/mach-imx/Kconfig |  6 +++++-
 commands/hab.c            | 20 +++++++++++++++++---
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 5f50d1a823..5fea0bbbca 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -926,13 +926,17 @@ config HABV4_CSF_UNLOCK_UID
           feature. This value must match the per device UNIQUE_ID fuses.
 
 	  The below example shows the expected format. The UNIQUE_ID is
-	  queried by Linux via:
+	  printed during boot by barebox:
+	    i.MX___ unique ID: 7766554433221100
+	  or it can be queried by Linux via:
             - cat /sys/devices/soc0/serial_number
 	      7766554433221100
 
 	  So this value have to be set:
 	    - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77
 
+	  Afterwards, the `hab -p -r` command can be used to burn the fuse.
+
 config HABV4_IMG_CRT_PEM
 	string "Path to IMG certificate"
 	default "../crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem"
diff --git a/commands/hab.c b/commands/hab.c
index 8ae943a4c8..435c20f4d3 100644
--- a/commands/hab.c
+++ b/commands/hab.c
@@ -16,9 +16,9 @@ static int do_hab(int argc, char *argv[])
 	char *srkhashfile = NULL, *srkhash = NULL;
 	unsigned flags = 0;
 	u8 srk[SRK_HASH_SIZE];
-	int lockdown = 0, info = 0;
+	int lockdown = 0, info = 0, field_return = 0;
 
-	while ((opt = getopt(argc, argv, "s:fpx:li")) > 0) {
+	while ((opt = getopt(argc, argv, "s:fpx:lir")) > 0) {
 		switch (opt) {
 		case 's':
 			srkhashfile = optarg;
@@ -38,12 +38,16 @@ static int do_hab(int argc, char *argv[])
 		case 'i':
 			info = 1;
 			break;
+		case 'r':
+			field_return = 1;
+			break;
 		default:
 			return COMMAND_ERROR_USAGE;
 		}
 	}
 
-	if (!info && !lockdown && !srkhashfile && !srkhash) {
+	if (!info && !lockdown && !srkhashfile && !srkhash &&
+	    !(IS_ENABLED(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN) && field_return)) {
 		printf("Nothing to do\n");
 		return COMMAND_ERROR_USAGE;
 	}
@@ -94,6 +98,13 @@ static int do_hab(int argc, char *argv[])
 		printf("Device successfully locked down\n");
 	}
 
+	if (IS_ENABLED(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN) && field_return) {
+		ret = imx_hab_field_return(flags & IMX_SRK_HASH_WRITE_PERMANENT);
+		if (ret)
+			return ret;
+		printf("Field return fuse successfully burnt\n");
+	}
+
 	return 0;
 }
 
@@ -105,6 +116,9 @@ BAREBOX_CMD_HELP_OPT ("-x <sha256>",  "Burn Super Root Key hash from hex string"
 BAREBOX_CMD_HELP_OPT ("-i",  "Print HAB info")
 BAREBOX_CMD_HELP_OPT ("-f",  "Force. Write even when a key is already written")
 BAREBOX_CMD_HELP_OPT ("-l",  "Lockdown device. Dangerous! After executing only signed images can be booted")
+#ifdef CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN
+BAREBOX_CMD_HELP_OPT ("-r",  "Field Return. Dangerous! After executing signed images are disabled forever.")
+#endif // CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN
 BAREBOX_CMD_HELP_OPT ("-p",  "Permanent. Really burn fuses. Be careful!")
 BAREBOX_CMD_HELP_END
 

-- 
2.47.3

Re: [PATCH 1/4] arm: mach-imx6: use kconfig for field return

[Marco Felsch]

On 25-12-18, Fabian Pflug wrote:
> There is a kConfig option for the field return, that is also documented,
		^
	    nit: Kconfig?

> so using it here instead of providing a headerfile to patch.
> 
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

Re: [PATCH 2/4] nvmem: ocotp: extend support to query the sticky bit

[Marco Felsch]

On 25-12-18, Fabian Pflug wrote:
> The i.MX* devices do have an sticky bit which indicates if the
> field-return fuse can be written. Before only support for i.MX8* was
> provided. Extend this for the i.MX6* series.
> 
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
>  drivers/nvmem/ocotp.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvmem/ocotp.c b/drivers/nvmem/ocotp.c
> index 7bca275404..1f74fddb60 100644
> --- a/drivers/nvmem/ocotp.c
> +++ b/drivers/nvmem/ocotp.c
> @@ -294,7 +294,7 @@ static void imx8m_lock_srk_revoke(struct ocotp_priv *priv)
>  	writel(val, priv->base + OCOTP_SW_STICKY);
>  }
>  
> -static bool imx8m_field_return_locked(struct ocotp_priv *priv)
> +static bool imx_field_return_locked(struct ocotp_priv *priv)

Nit: I would mention the rename within the commit message at least.

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

>  {
>  	return readl(priv->base + OCOTP_SW_STICKY) & OCOTP_SW_STICKY_FIELD_RETURN_LOCK;
>  }
> @@ -990,6 +990,7 @@ static struct imx_ocotp_data imx6q_ocotp_data = {
>  	.fuse_blow = imx6_fuse_blow_addr,
>  	.fuse_read = imx6_fuse_read_addr,
>  	.ctrl = &ocotp_ctrl_reg_default,
> +	.field_return_locked = imx_field_return_locked,
>  };
>  
>  static struct imx_ocotp_data imx6sl_ocotp_data = {
> @@ -1002,6 +1003,7 @@ static struct imx_ocotp_data imx6sl_ocotp_data = {
>  	.fuse_blow = imx6_fuse_blow_addr,
>  	.fuse_read = imx6_fuse_read_addr,
>  	.ctrl = &ocotp_ctrl_reg_default,
> +	.field_return_locked = imx_field_return_locked,
>  };
>  
>  static struct imx_ocotp_data imx6ul_ocotp_data = {
> @@ -1014,6 +1016,7 @@ static struct imx_ocotp_data imx6ul_ocotp_data = {
>  	.fuse_blow = imx6_fuse_blow_addr,
>  	.fuse_read = imx6_fuse_read_addr,
>  	.ctrl = &ocotp_ctrl_reg_default,
> +	.field_return_locked = imx_field_return_locked,
>  };
>  
>  static struct imx_ocotp_data imx6ull_ocotp_data = {
> @@ -1026,6 +1029,7 @@ static struct imx_ocotp_data imx6ull_ocotp_data = {
>  	.fuse_blow = imx6_fuse_blow_addr,
>  	.fuse_read = imx6_fuse_read_addr,
>  	.ctrl = &ocotp_ctrl_reg_default,
> +	.field_return_locked = imx_field_return_locked,
>  };
>  
>  static struct imx_ocotp_data vf610_ocotp_data = {
> @@ -1063,7 +1067,7 @@ static struct imx_ocotp_data imx8mp_ocotp_data = {
>  	.fuse_read = imx6_fuse_read_addr,
>  	.srk_revoke_locked = imx8m_srk_revoke_locked,
>  	.lock_srk_revoke = imx8m_lock_srk_revoke,
> -	.field_return_locked = imx8m_field_return_locked,
> +	.field_return_locked = imx_field_return_locked,
>  	.ctrl = &ocotp_ctrl_reg_8mp,
>  };
>  
> @@ -1095,7 +1099,7 @@ static struct imx_ocotp_data imx8mm_ocotp_data = {
>  	.fuse_read = imx6_fuse_read_addr,
>  	.srk_revoke_locked = imx8m_srk_revoke_locked,
>  	.lock_srk_revoke = imx8m_lock_srk_revoke,
> -	.field_return_locked = imx8m_field_return_locked,
> +	.field_return_locked = imx_field_return_locked,
>  	.feat = &imx8mm_featctrl_data,
>  	.ctrl = &ocotp_ctrl_reg_default,
>  };
> @@ -1116,7 +1120,7 @@ static struct imx_ocotp_data imx8mn_ocotp_data = {
>  	.fuse_read = imx6_fuse_read_addr,
>  	.srk_revoke_locked = imx8m_srk_revoke_locked,
>  	.lock_srk_revoke = imx8m_lock_srk_revoke,
> -	.field_return_locked = imx8m_field_return_locked,
> +	.field_return_locked = imx_field_return_locked,
>  	.feat = &imx8mn_featctrl_data,
>  	.ctrl = &ocotp_ctrl_reg_default,
>  };
> 
> -- 
> 2.47.3
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH 3/4] i.MX: HAB: extend field_return support to imx6

[Marco Felsch]

On 25-12-18, Fabian Pflug wrote:
> Extend the helper for imx_fuse_burn with support for i.MX6 devices.
> 
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
>  drivers/hab/hab.c                | 10 ++++++----
>  include/mach/imx/ocotp-fusemap.h |  1 +
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hab/hab.c b/drivers/hab/hab.c
> index 1c747e8a3e..281645f79e 100644
> --- a/drivers/hab/hab.c
> +++ b/drivers/hab/hab.c
> @@ -262,7 +262,7 @@ static int imx8m_hab_revoke_key_ocotp(unsigned key_idx)
>   */
>  #define MX8MP_FIELD_RETURN_PATTERN	0x28001401
>  
> -static int imx8m_hab_field_return_ocotp(void)
> +static int imx_hab_field_return_ocotp(void)
>  {
>  	int ret;
>  
> @@ -274,7 +274,9 @@ static int imx8m_hab_field_return_ocotp(void)
>  	if (ret == 1)
>  		return -EINVAL;
>  
> -	if (cpu_is_mx8mp())
> +	if (cpu_is_mx6())

The driver has hooks to abstract the different i.MX HAB SoC families.
Therefore please add a imx6_hab_field_return_ocotp() hook.

Regards,
  Marco

> +		ret = imx_ocotp_write_field(MX6_OCOTP_FIELD_RETURN, 1);
> +	else if (cpu_is_mx8mp())
>  		ret = imx_ocotp_write_field(MX8MP_OCOTP_FIELD_RETURN,
>  					    MX8MP_FIELD_RETURN_PATTERN);
>  	else
> @@ -282,7 +284,6 @@ static int imx8m_hab_field_return_ocotp(void)
>  
>  	return ret;
>  }
> -
>  struct imx_hab_ops {
>  	int (*write_srk_hash)(const u8 *srk, unsigned flags);
>  	int (*read_srk_hash)(u8 *srk);
> @@ -310,6 +311,7 @@ static struct imx_hab_ops imx6_hab_ops_ocotp = {
>  	.device_locked_down = imx6_hab_device_locked_down_ocotp,
>  	.permanent_write_enable = imx_hab_permanent_write_enable_ocotp,
>  	.print_status = imx6_hab_print_status,
> +	.field_return = imx_hab_field_return_ocotp,
>  };
>  
>  static struct imx_hab_ops imx8m_hab_ops_ocotp = {
> @@ -320,7 +322,7 @@ static struct imx_hab_ops imx8m_hab_ops_ocotp = {
>  	.permanent_write_enable = imx_hab_permanent_write_enable_ocotp,
>  	.print_status = imx8m_hab_print_status,
>  	.revoke_key = imx8m_hab_revoke_key_ocotp,
> -	.field_return = imx8m_hab_field_return_ocotp,
> +	.field_return = imx_hab_field_return_ocotp,
>  };
>  
>  static int imx_ahab_write_srk_hash(const u8 *__newsrk, unsigned flags)
> diff --git a/include/mach/imx/ocotp-fusemap.h b/include/mach/imx/ocotp-fusemap.h
> index ae10dcef2a..3fd9d6df24 100644
> --- a/include/mach/imx/ocotp-fusemap.h
> +++ b/include/mach/imx/ocotp-fusemap.h
> @@ -103,6 +103,7 @@
>  #define MX8M_OCOTP_TZASC_EN		(OCOTP_WORD(0x480) | OCOTP_BIT(11) | OCOTP_WIDTH(1))
>  #define MX8MP_OCOTP_ROM_NO_LOG		(OCOTP_WORD(0x480) | OCOTP_BIT(22) | OCOTP_WIDTH(1))
>  #define MX8M_OCOTP_RECOVERY_SDMMC_BOOT_DIS	(OCOTP_WORD(0x490) | OCOTP_BIT(23) | OCOTP_WIDTH(1))
> +#define MX6_OCOTP_FIELD_RETURN		(OCOTP_WORD(0x6E0) | OCOTP_BIT(0) | OCOTP_WIDTH(1))
>  #define MX8M_OCOTP_FIELD_RETURN		(OCOTP_WORD(0x630) | OCOTP_BIT(0) | OCOTP_WIDTH(1))
>  #define MX8MP_OCOTP_FIELD_RETURN	(OCOTP_WORD(0x630) | OCOTP_BIT(0) | OCOTP_WIDTH(32))
>  #define MX8M_OCOTP_SRK_REVOKE		(OCOTP_WORD(0x670) | OCOTP_BIT(0) | OCOTP_WIDTH(4))
> 
> -- 
> 2.47.3
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH 4/4] commands: hab: extend by field_return fuse burn

[Marco Felsch]

On 25-12-18, Fabian Pflug wrote:
> Extend hab command with an additional parameter to burn the field return
> fuse, but only if it is unlocked via the kconfig option.
> Without the kconfig option, the extra argument makes no sense, as it
> would not be possible to access the FIELD_RETURN fuse.
> 
> Signed-off-by: Fabian Pflug <f.pflug@pengutronix.de>
> ---
>  arch/arm/mach-imx/Kconfig |  6 +++++-
>  commands/hab.c            | 20 +++++++++++++++++---
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 5f50d1a823..5fea0bbbca 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -926,13 +926,17 @@ config HABV4_CSF_UNLOCK_UID
>            feature. This value must match the per device UNIQUE_ID fuses.
>  
>  	  The below example shows the expected format. The UNIQUE_ID is
> -	  queried by Linux via:
> +	  printed during boot by barebox:
> +	    i.MX___ unique ID: 7766554433221100
> +	  or it can be queried by Linux via:
>              - cat /sys/devices/soc0/serial_number
>  	      7766554433221100
>  
>  	  So this value have to be set:
>  	    - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77
>  
> +	  Afterwards, the `hab -p -r` command can be used to burn the fuse.

This Kconfig update should be done in a separate patch or at least
mentioned within the commit message.

> +
>  config HABV4_IMG_CRT_PEM
>  	string "Path to IMG certificate"
>  	default "../crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem"
> diff --git a/commands/hab.c b/commands/hab.c
> index 8ae943a4c8..435c20f4d3 100644
> --- a/commands/hab.c
> +++ b/commands/hab.c
> @@ -16,9 +16,9 @@ static int do_hab(int argc, char *argv[])
>  	char *srkhashfile = NULL, *srkhash = NULL;
>  	unsigned flags = 0;
>  	u8 srk[SRK_HASH_SIZE];
> -	int lockdown = 0, info = 0;
> +	int lockdown = 0, info = 0, field_return = 0;
>  
> -	while ((opt = getopt(argc, argv, "s:fpx:li")) > 0) {
> +	while ((opt = getopt(argc, argv, "s:fpx:lir")) > 0) {
>  		switch (opt) {
>  		case 's':
>  			srkhashfile = optarg;
> @@ -38,12 +38,16 @@ static int do_hab(int argc, char *argv[])
>  		case 'i':
>  			info = 1;
>  			break;
> +		case 'r':
> +			field_return = 1;
> +			break;
>  		default:
>  			return COMMAND_ERROR_USAGE;
>  		}
>  	}
>  
> -	if (!info && !lockdown && !srkhashfile && !srkhash) {
> +	if (!info && !lockdown && !srkhashfile && !srkhash &&
> +	    !(IS_ENABLED(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN) && field_return)) {
		^
I would drop the CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN check here and
instead just check the: !field_return option...

>  		printf("Nothing to do\n");
>  		return COMMAND_ERROR_USAGE;
>  	}
> @@ -94,6 +98,13 @@ static int do_hab(int argc, char *argv[])
>  		printf("Device successfully locked down\n");
>  	}
>  
> +	if (IS_ENABLED(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN) && field_return) {

same here..

	if (field_return) {

> +		ret = imx_hab_field_return(flags & IMX_SRK_HASH_WRITE_PERMANENT);
> +		if (ret)
> +			return ret;

and instead check the return value here. If -EINVAL is returned, the
sticky bit is still locked because either the CSF SOC_UID is wrong or
the CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN wasn't enabled at all.
Therefore:

		if (ret) {
			if (ret == -EINVAL && IS_ENABLED(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN))
				printf("Field-return burn failed, check HABV4_CSF_UNLOCK_UID!\n");
			else if (ret == -EINVAL && !IS_ENABLED(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN))
				printf("Field-return burn failed because CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN=n\n");
			else
				printf("Field-return burn failed\n");

			return ret;
		}

> +		printf("Field return fuse successfully burnt\n");
> +	}
> +
>  	return 0;
>  }
>  
> @@ -105,6 +116,9 @@ BAREBOX_CMD_HELP_OPT ("-x <sha256>",  "Burn Super Root Key hash from hex string"
>  BAREBOX_CMD_HELP_OPT ("-i",  "Print HAB info")
>  BAREBOX_CMD_HELP_OPT ("-f",  "Force. Write even when a key is already written")
>  BAREBOX_CMD_HELP_OPT ("-l",  "Lockdown device. Dangerous! After executing only signed images can be booted")
> +#ifdef CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN

IMHO we shouldn't add conditional compilation here.

Regards,
  Marco


> +BAREBOX_CMD_HELP_OPT ("-r",  "Field Return. Dangerous! After executing signed images are disabled forever.")
> +#endif // CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN
>  BAREBOX_CMD_HELP_OPT ("-p",  "Permanent. Really burn fuses. Be careful!")
>  BAREBOX_CMD_HELP_END
>  
> 
> -- 
> 2.47.3
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |