[PATCH v2 0/9] i.MX8M HAB and OCOTP additions and fixes

[PATCH v2 0/9] i.MX8M HAB and OCOTP additions and fixes

[Marco Felsch]

Hi,

this fixes the HABv4 locking on i.MX8M devices and add additional
features like:
 - key revocation
 - field-return burning
 - additional security related fuse defintions

Changes in v2:
- Link to v1: https://lore.barebox.org/barebox/Zmv2KyQq3ACxksc_@pengutronix.de/
- adapt patch-1 commit message
- make use of 'bool permanent' parameter instead of introducing new
  IMX_SRK_* flags.

---
Marco Felsch (9):
      i.MX: HABv4: fix SRK_LOCK for i.MX8M devices
      nvmem: ocotp: add support to get/set srk_revoke sticky bit
      nvmem: ocotp: add support to query the field-return sticky bit
      hab: convert flags to use BIT() macro
      i.MX: HAB: add imx_hab_revoke_key support
      i.MX: HABv4: add more i.MX8M fuse defines
      i.MX8M: HABv4: add an option to allow key revocation
      i.MX8M: HABv4: add option to allow burning the field-return fuse
      i.MX: HAB: add imx_hab_field_return support

 arch/arm/mach-imx/Kconfig            |  33 ++++++++++
 drivers/hab/hab.c                    | 120 ++++++++++++++++++++++++++++++++++-
 drivers/nvmem/ocotp.c                |  79 +++++++++++++++++++++++
 include/hab.h                        |   9 ++-
 include/mach/imx/habv4-imx8-gencsf.h |  13 ++++
 include/mach/imx/ocotp-fusemap.h     |  13 ++++
 include/mach/imx/ocotp.h             |   3 +
 scripts/Makefile.lib                 |   1 +
 8 files changed, 265 insertions(+), 6 deletions(-)
---
base-commit: bafdf4b35d777d159ac4058efc86d36622ce5ccf
change-id: 20240703-v2024-05-0-topic-hab-218bab0e829c

Best regards,
-- 
Marco Felsch <m.felsch@pengutronix.de>

[PATCH v2 1/9] i.MX: HABv4: fix SRK_LOCK for i.MX8M devices

[Marco Felsch]

The fuse to lock the SRK hash on i.MX8M* SoCs is different than the one
used for the i.MX6 SoCs. Fix this by refactoring
imx_hab_write_srk_hash_ocotp() and make the lock fusing SoC specific.

Fixes: 6c4d5bb5acfe ("i.MX: HABv4: implement interface for i.MX8MQ")
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/hab/hab.c                | 34 +++++++++++++++++++++++++++++++---
 include/mach/imx/ocotp-fusemap.h |  1 +
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/hab/hab.c b/drivers/hab/hab.c
index ed091058d8fb..28a091841a69 100644
--- a/drivers/hab/hab.c
+++ b/drivers/hab/hab.c
@@ -128,7 +128,7 @@ static int imx_hab_read_srk_hash_ocotp(u8 *__srk)
 	return 0;
 }
 
-static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk, unsigned flags)
+static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk)
 {
 	u32 *newsrk = (u32 *)__newsrk;
 	int ret, i;
@@ -139,6 +139,17 @@ static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk, unsigned flags)
 			return ret;
 	}
 
+	return 0;
+}
+
+static int imx6_hab_write_srk_hash_ocotp(const u8 *newsrk, unsigned flags)
+{
+	int ret;
+
+	ret = imx_hab_write_srk_hash_ocotp(newsrk);
+	if (ret)
+		return ret;
+
 	if (flags & IMX_SRK_HASH_WRITE_LOCK) {
 		ret = imx_ocotp_write_field(OCOTP_SRK_LOCK, 1);
 		if (ret < 0)
@@ -148,6 +159,23 @@ static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk, unsigned flags)
 	return 0;
 }
 
+static int imx8m_hab_write_srk_hash_ocotp(const u8 *newsrk, unsigned flags)
+{
+	int ret;
+
+	ret = imx_hab_write_srk_hash_ocotp(newsrk);
+	if (ret)
+		return ret;
+
+	if (flags & IMX_SRK_HASH_WRITE_LOCK) {
+		ret = imx_ocotp_write_field(MX8M_OCOTP_SRK_LOCK, 1);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int imx_hab_permanent_write_enable_ocotp(int enable)
 {
 	return imx_ocotp_permanent_write(enable);
@@ -222,7 +250,7 @@ static struct imx_hab_ops imx_hab_ops_iim = {
 };
 
 static struct imx_hab_ops imx6_hab_ops_ocotp = {
-	.write_srk_hash = imx_hab_write_srk_hash_ocotp,
+	.write_srk_hash = imx6_hab_write_srk_hash_ocotp,
 	.read_srk_hash =  imx_hab_read_srk_hash_ocotp,
 	.lockdown_device = imx6_hab_lockdown_device_ocotp,
 	.device_locked_down = imx6_hab_device_locked_down_ocotp,
@@ -231,7 +259,7 @@ static struct imx_hab_ops imx6_hab_ops_ocotp = {
 };
 
 static struct imx_hab_ops imx8m_hab_ops_ocotp = {
-	.write_srk_hash = imx_hab_write_srk_hash_ocotp,
+	.write_srk_hash = imx8m_hab_write_srk_hash_ocotp,
 	.read_srk_hash =  imx_hab_read_srk_hash_ocotp,
 	.lockdown_device = imx8m_hab_lockdown_device_ocotp,
 	.device_locked_down = imx8m_hab_device_locked_down_ocotp,
diff --git a/include/mach/imx/ocotp-fusemap.h b/include/mach/imx/ocotp-fusemap.h
index 823273895502..c4f94e61e8f8 100644
--- a/include/mach/imx/ocotp-fusemap.h
+++ b/include/mach/imx/ocotp-fusemap.h
@@ -54,6 +54,7 @@
 #define OCOTP_GP2			(OCOTP_WORD(0x670) | OCOTP_BIT(0) | OCOTP_WIDTH(32))
 #define OCOTP_PAD_SETTINGS		(OCOTP_WORD(0x6d0) | OCOTP_BIT(0) | OCOTP_WIDTH(6))
 /* i.MX8M moved the security related fuses */
+#define MX8M_OCOTP_SRK_LOCK		(OCOTP_WORD(0x400) | OCOTP_BIT(9) | OCOTP_WIDTH(1))
 #define MX8M_OCOTP_SEC_CONFIG_1		(OCOTP_WORD(0x470) | OCOTP_BIT(25) | OCOTP_WIDTH(1))
 #define MX8MQ_OCOTP_DIR_BT_DIS		(OCOTP_WORD(0x470) | OCOTP_BIT(27) | OCOTP_WIDTH(1))
 

-- 
2.39.2

[PATCH v2 2/9] nvmem: ocotp: add support to get/set srk_revoke sticky bit

[Marco Felsch]

The i.MX8M* devices do have an sticky bit which indicates if the
srk_revoke fuse can be written. Add support to query and to set the lock
bit.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/nvmem/ocotp.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/mach/imx/ocotp.h |  2 ++
 2 files changed, 57 insertions(+)

diff --git a/drivers/nvmem/ocotp.c b/drivers/nvmem/ocotp.c
index c282efefa824..c0517980fb18 100644
--- a/drivers/nvmem/ocotp.c
+++ b/drivers/nvmem/ocotp.c
@@ -52,6 +52,7 @@
 #define OCOTP_DATA			0x20
 #define OCOTP_READ_CTRL			0x30
 #define OCOTP_READ_FUSE_DATA		0x40
+#define OCOTP_SW_STICKY			0x50
 
 #define MX7_OCOTP_DATA0			0x20
 #define MX7_OCOTP_DATA1			0x30
@@ -89,6 +90,8 @@
 #define OCOTP_TIMING_STROBE_PROG	GENMASK(11, 0)
 #define OCOTP_TIMING_WAIT		GENMASK(27, 22)
 
+#define OCOTP_SW_STICKY_SRK_REVOKE_LOCK		BIT(1)
+
 #define OCOTP_READ_CTRL_READ_FUSE	BIT(1)
 
 #define OCOTP_OFFSET_TO_ADDR(o) (OCOTP_OFFSET_TO_INDEX(o) * 4)
@@ -147,6 +150,8 @@ struct imx_ocotp_data {
 	int (*set_timing)(struct ocotp_priv *priv);
 	int (*fuse_read)(struct ocotp_priv *priv, u32 addr, u32 *pdata);
 	int (*fuse_blow)(struct ocotp_priv *priv, u32 addr, u32 value);
+	bool (*srk_revoke_locked)(struct ocotp_priv *priv);
+	void (*lock_srk_revoke)(struct ocotp_priv *priv);
 	u8  mac_offsets[MAX_MAC_OFFSETS];
 	u8  mac_offsets_num;
 	struct imx8m_featctrl_data *feat;
@@ -273,6 +278,20 @@ static int imx6_ocotp_prepare(struct ocotp_priv *priv)
 	return 0;
 }
 
+static bool imx8m_srk_revoke_locked(struct ocotp_priv *priv)
+{
+	return readl(priv->base + OCOTP_SW_STICKY) & OCOTP_SW_STICKY_SRK_REVOKE_LOCK;
+}
+
+static void imx8m_lock_srk_revoke(struct ocotp_priv *priv)
+{
+	u32 val;
+
+	val = readl(priv->base + OCOTP_SW_STICKY);
+	val |= OCOTP_SW_STICKY_SRK_REVOKE_LOCK;
+	writel(val, priv->base + OCOTP_SW_STICKY);
+}
+
 static int imx6_fuse_read_addr(struct ocotp_priv *priv, u32 addr, u32 *pdata)
 {
 	const u32 bm_ctrl_error = priv->data->ctrl->bm_error;
@@ -625,6 +644,36 @@ int imx_ocotp_sense_enable(bool enable)
 	return old_value;
 }
 
+int imx_ocotp_srk_revoke_locked(void)
+{
+	int ret;
+
+	ret = imx_ocotp_ensure_probed();
+	if (ret)
+		return ret;
+
+	if (imx_ocotp->data->srk_revoke_locked)
+		return imx_ocotp->data->srk_revoke_locked(imx_ocotp);
+
+	return -ENOSYS;
+}
+
+int imx_ocotp_lock_srk_revoke(void)
+{
+	int ret;
+
+	ret = imx_ocotp_ensure_probed();
+	if (ret)
+		return ret;
+
+	if (imx_ocotp->data->lock_srk_revoke) {
+		imx_ocotp->data->lock_srk_revoke(imx_ocotp);
+		return 0;
+	}
+
+	return -ENOSYS;
+}
+
 static void imx_ocotp_format_mac(u8 *dst, const u8 *src,
 				 enum imx_ocotp_format_mac_direction dir)
 {
@@ -985,6 +1034,8 @@ static struct imx_ocotp_data imx8mp_ocotp_data = {
 	.set_timing = imx6_ocotp_set_timing,
 	.fuse_blow = imx6_fuse_blow_addr,
 	.fuse_read = imx6_fuse_read_addr,
+	.srk_revoke_locked = imx8m_srk_revoke_locked,
+	.lock_srk_revoke = imx8m_lock_srk_revoke,
 	.ctrl = &ocotp_ctrl_reg_8mp,
 };
 
@@ -1014,6 +1065,8 @@ static struct imx_ocotp_data imx8mm_ocotp_data = {
 	.set_timing = imx6_ocotp_set_timing,
 	.fuse_blow = imx6_fuse_blow_addr,
 	.fuse_read = imx6_fuse_read_addr,
+	.srk_revoke_locked = imx8m_srk_revoke_locked,
+	.lock_srk_revoke = imx8m_lock_srk_revoke,
 	.feat = &imx8mm_featctrl_data,
 	.ctrl = &ocotp_ctrl_reg_default,
 };
@@ -1032,6 +1085,8 @@ static struct imx_ocotp_data imx8mn_ocotp_data = {
 	.set_timing = imx6_ocotp_set_timing,
 	.fuse_blow = imx6_fuse_blow_addr,
 	.fuse_read = imx6_fuse_read_addr,
+	.srk_revoke_locked = imx8m_srk_revoke_locked,
+	.lock_srk_revoke = imx8m_lock_srk_revoke,
 	.feat = &imx8mn_featctrl_data,
 	.ctrl = &ocotp_ctrl_reg_default,
 };
diff --git a/include/mach/imx/ocotp.h b/include/mach/imx/ocotp.h
index 5f7b88f716a7..7a516ff789b9 100644
--- a/include/mach/imx/ocotp.h
+++ b/include/mach/imx/ocotp.h
@@ -36,6 +36,8 @@ int imx_ocotp_read_field(uint32_t field, unsigned *value);
 int imx_ocotp_write_field(uint32_t field, unsigned value);
 int imx_ocotp_permanent_write(int enable);
 int imx_ocotp_sense_enable(bool enable);
+int imx_ocotp_srk_revoke_locked(void);
+int imx_ocotp_lock_srk_revoke(void);
 
 static inline u64 imx_ocotp_read_uid(void __iomem *ocotp)
 {

-- 
2.39.2

[PATCH v2 3/9] nvmem: ocotp: add support to query the field-return sticky bit

[Marco Felsch]

The i.MX8M* devices do have an sticky bit which indicates if the
field-return fuse can be written. Add support to query the lock bit.

To make it easy to read align the STICKY bit definitions as well.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/nvmem/ocotp.c    | 24 ++++++++++++++++++++++++
 include/mach/imx/ocotp.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/nvmem/ocotp.c b/drivers/nvmem/ocotp.c
index c0517980fb18..87cc95a636f3 100644
--- a/drivers/nvmem/ocotp.c
+++ b/drivers/nvmem/ocotp.c
@@ -91,6 +91,7 @@
 #define OCOTP_TIMING_WAIT		GENMASK(27, 22)
 
 #define OCOTP_SW_STICKY_SRK_REVOKE_LOCK		BIT(1)
+#define OCOTP_SW_STICKY_FIELD_RETURN_LOCK	BIT(2)
 
 #define OCOTP_READ_CTRL_READ_FUSE	BIT(1)
 
@@ -152,6 +153,7 @@ struct imx_ocotp_data {
 	int (*fuse_blow)(struct ocotp_priv *priv, u32 addr, u32 value);
 	bool (*srk_revoke_locked)(struct ocotp_priv *priv);
 	void (*lock_srk_revoke)(struct ocotp_priv *priv);
+	bool (*field_return_locked)(struct ocotp_priv *priv);
 	u8  mac_offsets[MAX_MAC_OFFSETS];
 	u8  mac_offsets_num;
 	struct imx8m_featctrl_data *feat;
@@ -292,6 +294,11 @@ 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)
+{
+	return readl(priv->base + OCOTP_SW_STICKY) & OCOTP_SW_STICKY_FIELD_RETURN_LOCK;
+}
+
 static int imx6_fuse_read_addr(struct ocotp_priv *priv, u32 addr, u32 *pdata)
 {
 	const u32 bm_ctrl_error = priv->data->ctrl->bm_error;
@@ -674,6 +681,20 @@ int imx_ocotp_lock_srk_revoke(void)
 	return -ENOSYS;
 }
 
+int imx_ocotp_field_return_locked(void)
+{
+	int ret;
+
+	ret = imx_ocotp_ensure_probed();
+	if (ret)
+		return ret;
+
+	if (imx_ocotp->data->field_return_locked)
+		return imx_ocotp->data->field_return_locked(imx_ocotp);
+
+	return -ENOSYS;
+}
+
 static void imx_ocotp_format_mac(u8 *dst, const u8 *src,
 				 enum imx_ocotp_format_mac_direction dir)
 {
@@ -1036,6 +1057,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,
 	.ctrl = &ocotp_ctrl_reg_8mp,
 };
 
@@ -1067,6 +1089,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,
 	.feat = &imx8mm_featctrl_data,
 	.ctrl = &ocotp_ctrl_reg_default,
 };
@@ -1087,6 +1110,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,
 	.feat = &imx8mn_featctrl_data,
 	.ctrl = &ocotp_ctrl_reg_default,
 };
diff --git a/include/mach/imx/ocotp.h b/include/mach/imx/ocotp.h
index 7a516ff789b9..e6f62a9da48c 100644
--- a/include/mach/imx/ocotp.h
+++ b/include/mach/imx/ocotp.h
@@ -38,6 +38,7 @@ int imx_ocotp_permanent_write(int enable);
 int imx_ocotp_sense_enable(bool enable);
 int imx_ocotp_srk_revoke_locked(void);
 int imx_ocotp_lock_srk_revoke(void);
+int imx_ocotp_field_return_locked(void);
 
 static inline u64 imx_ocotp_read_uid(void __iomem *ocotp)
 {

-- 
2.39.2

[PATCH v2 4/9] hab: convert flags to use BIT() macro

[Marco Felsch]

Make use of the BIT() macro to define the flags, no functional change.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 include/hab.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/hab.h b/include/hab.h
index da79a8ffea36..2cef3841d149 100644
--- a/include/hab.h
+++ b/include/hab.h
@@ -7,6 +7,7 @@
 #define __HABV4_H
 
 #include <errno.h>
+#include <linux/bits.h>
 
 /* State definitions */
 enum habv4_state {
@@ -32,15 +33,15 @@ static inline int habv4_get_state(void)
 #define SRK_HASH_SIZE	32
 
 /* Force writing of key, even when a key is already written */
-#define IMX_SRK_HASH_FORCE		(1 << 0)
+#define IMX_SRK_HASH_FORCE		BIT(0)
 /* Permanently write fuses, without this flag only the shadow registers
  * are written.
  */
-#define IMX_SRK_HASH_WRITE_PERMANENT	(1 << 1)
+#define IMX_SRK_HASH_WRITE_PERMANENT	BIT(1)
 /* When writing the super root key hash, also burn the write protection
  * fuses so that the key hash can not be modified.
  */
-#define IMX_SRK_HASH_WRITE_LOCK		(1 << 2)
+#define IMX_SRK_HASH_WRITE_LOCK		BIT(2)
 
 bool imx_hab_srk_hash_valid(const void *buf);
 int imx_hab_write_srk_hash(const void *buf, unsigned flags);

-- 
2.39.2

[PATCH v2 5/9] i.MX: HAB: add imx_hab_revoke_key support

[Marco Felsch]

Add an helper to revoke an i.MX SRK key. At the moment the helper
supprts i.MX8M devices only, but adding support for other SoCs can be
done easily by providing the .revoke_key() hook.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/hab/hab.c                | 45 ++++++++++++++++++++++++++++++++++++++++
 include/hab.h                    |  1 +
 include/mach/imx/ocotp-fusemap.h |  1 +
 3 files changed, 47 insertions(+)

diff --git a/drivers/hab/hab.c b/drivers/hab/hab.c
index 28a091841a69..7c840303d395 100644
--- a/drivers/hab/hab.c
+++ b/drivers/hab/hab.c
@@ -231,6 +231,27 @@ static int imx8m_hab_device_locked_down_ocotp(void)
 	return v;
 }
 
+static int imx8m_hab_revoke_key_ocotp(unsigned key_idx)
+{
+	int ret;
+
+	/* Prohibit revocation of last possible key */
+	if (key_idx >= 4)
+		return -EINVAL;
+
+	ret = imx_ocotp_srk_revoke_locked();
+	if (ret < 0)
+		return ret;
+
+	/* Return -EINVAL in case the SRK_REVOKE write is locked */
+	if (ret == 1)
+		return -EINVAL;
+
+	ret = imx_ocotp_write_field(MX8M_OCOTP_SRK_REVOKE, BIT(key_idx));
+
+	return ret;
+}
+
 struct imx_hab_ops {
 	int (*write_srk_hash)(const u8 *srk, unsigned flags);
 	int (*read_srk_hash)(u8 *srk);
@@ -238,6 +259,7 @@ struct imx_hab_ops {
 	int (*lockdown_device)(unsigned flags);
 	int (*device_locked_down)(void);
 	int (*print_status)(void);
+	int (*revoke_key)(unsigned key_idx);
 };
 
 static struct imx_hab_ops imx_hab_ops_iim = {
@@ -265,6 +287,7 @@ static struct imx_hab_ops imx8m_hab_ops_ocotp = {
 	.device_locked_down = imx8m_hab_device_locked_down_ocotp,
 	.permanent_write_enable = imx_hab_permanent_write_enable_ocotp,
 	.print_status = imx8m_hab_print_status,
+	.revoke_key = imx8m_hab_revoke_key_ocotp,
 };
 
 static int imx_ahab_write_srk_hash(const u8 *__newsrk, unsigned flags)
@@ -536,3 +559,25 @@ static int init_imx_hab_print_status(void)
 	return 0;
 }
 postmmu_initcall(init_imx_hab_print_status);
+
+int imx_hab_revoke_key(unsigned key_idx, bool permanent)
+{
+	struct imx_hab_ops *ops = imx_get_hab_ops();
+	int ret;
+
+	if (!ops || !ops->revoke_key)
+		return -ENOSYS;
+
+	if (permanent) {
+		ret = ops->permanent_write_enable(1);
+		if (ret)
+			return ret;
+	}
+
+	ret = ops->revoke_key(key_idx);
+
+	if (permanent)
+		ops->permanent_write_enable(0);
+
+	return ret;
+}
diff --git a/include/hab.h b/include/hab.h
index 2cef3841d149..7a70c67f0488 100644
--- a/include/hab.h
+++ b/include/hab.h
@@ -51,5 +51,6 @@ int imx_hab_read_srk_hash(void *buf);
 int imx_hab_lockdown_device(unsigned flags);
 int imx_hab_device_locked_down(void);
 int imx_hab_print_status(void);
+int imx_hab_revoke_key(unsigned key_idx, bool permanent);
 
 #endif /* __HABV4_H */
diff --git a/include/mach/imx/ocotp-fusemap.h b/include/mach/imx/ocotp-fusemap.h
index c4f94e61e8f8..e6fa96b40ba9 100644
--- a/include/mach/imx/ocotp-fusemap.h
+++ b/include/mach/imx/ocotp-fusemap.h
@@ -57,5 +57,6 @@
 #define MX8M_OCOTP_SRK_LOCK		(OCOTP_WORD(0x400) | OCOTP_BIT(9) | OCOTP_WIDTH(1))
 #define MX8M_OCOTP_SEC_CONFIG_1		(OCOTP_WORD(0x470) | OCOTP_BIT(25) | OCOTP_WIDTH(1))
 #define MX8MQ_OCOTP_DIR_BT_DIS		(OCOTP_WORD(0x470) | OCOTP_BIT(27) | OCOTP_WIDTH(1))
+#define MX8M_OCOTP_SRK_REVOKE		(OCOTP_WORD(0x670) | OCOTP_BIT(0) | OCOTP_WIDTH(4))
 
 #endif /* __MACH_IMX_OCOTP_FUSEMAP_H */

-- 
2.39.2

[PATCH v2 6/9] i.MX: HABv4: add more i.MX8M fuse defines

[Marco Felsch]

Add more common i.MX8M fuse defines so they can be reused by board code.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 include/mach/imx/ocotp-fusemap.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/mach/imx/ocotp-fusemap.h b/include/mach/imx/ocotp-fusemap.h
index e6fa96b40ba9..1aece9195ff9 100644
--- a/include/mach/imx/ocotp-fusemap.h
+++ b/include/mach/imx/ocotp-fusemap.h
@@ -55,8 +55,18 @@
 #define OCOTP_PAD_SETTINGS		(OCOTP_WORD(0x6d0) | OCOTP_BIT(0) | OCOTP_WIDTH(6))
 /* i.MX8M moved the security related fuses */
 #define MX8M_OCOTP_SRK_LOCK		(OCOTP_WORD(0x400) | OCOTP_BIT(9) | OCOTP_WIDTH(1))
+#define MX8M_OCOTP_SJC_RESP_LOCK	(OCOTP_WORD(0x400) | OCOTP_BIT(10) | OCOTP_WIDTH(1))
+#define MX8M_OCOTP_USB_ID_LOCK		(OCOTP_WORD(0x400) | OCOTP_BIT(12) | OCOTP_WIDTH(2))
+#define MX8M_OCOTP_KTE			(OCOTP_WORD(0x470) | OCOTP_BIT(20) | OCOTP_WIDTH(1))
+#define MX8M_OCOTP_SJC_DISABLE		(OCOTP_WORD(0x470) | OCOTP_BIT(21) | OCOTP_WIDTH(1))
+#define MX8M_OCOTP_JTAG_SMODE		(OCOTP_WORD(0x470) | OCOTP_BIT(22) | OCOTP_WIDTH(2))
 #define MX8M_OCOTP_SEC_CONFIG_1		(OCOTP_WORD(0x470) | OCOTP_BIT(25) | OCOTP_WIDTH(1))
+#define MX8M_OCOTP_JTAG_HEO		(OCOTP_WORD(0x470) | OCOTP_BIT(26) | OCOTP_WIDTH(1))
 #define MX8MQ_OCOTP_DIR_BT_DIS		(OCOTP_WORD(0x470) | OCOTP_BIT(27) | OCOTP_WIDTH(1))
+#define MX8M_OCOTP_FORCE_COLD_BOOT	(OCOTP_WORD(0x470) | OCOTP_BIT(29) | OCOTP_WIDTH(1))
+#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 MX8M_OCOTP_SRK_REVOKE		(OCOTP_WORD(0x670) | OCOTP_BIT(0) | OCOTP_WIDTH(4))
 
 #endif /* __MACH_IMX_OCOTP_FUSEMAP_H */

-- 
2.39.2

[PATCH v2 7/9] i.MX8M: HABv4: add an option to allow key revocation

[Marco Felsch]

The HAB code needs an special [Unlock] instruction to keep the
SRK_REVOKE fuse bank unlocked. This is required if a key needs to be
revoked.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm/mach-imx/Kconfig            | 8 ++++++++
 include/mach/imx/habv4-imx8-gencsf.h | 6 ++++++
 2 files changed, 14 insertions(+)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 61258137736f..68f55971506b 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -835,6 +835,14 @@ config HABV4_QSPI
 	help
 	  Enable this option to build signed QSPI/FlexSPI images.
 
+config HABV4_CSF_UNLOCK_SRK_REVOKE
+	depends on HABV4
+	bool "Unlock SRK revocation"
+	help
+	  Enable this option to instruct the HAB code to not lock
+	  the SRK_REVOKE_LOCK sticky bit. This is required for key
+	  revocation. Don't enable this if you are unsure.
+
 config HAB_CERTS_ENV
 	depends on HAB
 	bool "Specify certificates in environment"
diff --git a/include/mach/imx/habv4-imx8-gencsf.h b/include/mach/imx/habv4-imx8-gencsf.h
index 5f92ceceab00..56d9ef2de92f 100644
--- a/include/mach/imx/habv4-imx8-gencsf.h
+++ b/include/mach/imx/habv4-imx8-gencsf.h
@@ -36,6 +36,12 @@ hab [Unlock]
 hab Engine = CAAM
 hab Features = RNG, MID
 
+#if defined(CONFIG_HABV4_CSF_UNLOCK_SRK_REVOKE)
+hab [Unlock]
+hab Engine = OCOTP
+hab Features = SRK REVOKE
+#endif
+
 hab [Install Key]
 /* verification key index in key store (0, 2...4) */
 hab Verification index = 0

-- 
2.39.2

[PATCH v2 8/9] i.MX8M: HABv4: add option to allow burning the field-return fuse

[Marco Felsch]

This adds the required Kconfig options which need to be enabled and
correctly set to build a custom device specific barebox image which can
be used to burn the FIELD_RETURN fuse.

The CST tool can't handle quoted UID strings so we need to define it on
the cmdline by using the -D switch. This removes the quotes within the
CSF file and the CST is happy.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm/mach-imx/Kconfig            | 25 +++++++++++++++++++++++++
 include/mach/imx/habv4-imx8-gencsf.h |  7 +++++++
 scripts/Makefile.lib                 |  1 +
 3 files changed, 33 insertions(+)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 68f55971506b..8bbde38e0d9a 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -843,6 +843,31 @@ config HABV4_CSF_UNLOCK_SRK_REVOKE
 	  the SRK_REVOKE_LOCK sticky bit. This is required for key
 	  revocation. Don't enable this if you are unsure.
 
+config HABV4_CSF_UNLOCK_FIELD_RETURN
+	depends on HABV4
+	bool "Unlock field return"
+	help
+	  Enable this option to instruct the HAB code to not lock
+	  the FIELD_RETURN_LOCK sticky bit. This is required to be
+	  able to fuse the FIELD_RETURN fuse. It is also required
+	  that the CONFIG_HABV4_CSF_UNLOCK_UID is set correct as
+	  well.
+
+config HABV4_CSF_UNLOCK_UID
+	depends on HABV4 && HABV4_CSF_UNLOCK_FIELD_RETURN
+	string "CSF Unlock UID"
+	help
+	  Device specific 64-bit UID Required to unlock the field-return
+          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:
+            - cat /sys/devices/soc0/serial_number
+	      7766554433221100
+
+	  So this value have to be set:
+	    - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77
+
 config HAB_CERTS_ENV
 	depends on HAB
 	bool "Specify certificates in environment"
diff --git a/include/mach/imx/habv4-imx8-gencsf.h b/include/mach/imx/habv4-imx8-gencsf.h
index 56d9ef2de92f..3106facc511f 100644
--- a/include/mach/imx/habv4-imx8-gencsf.h
+++ b/include/mach/imx/habv4-imx8-gencsf.h
@@ -42,6 +42,13 @@ hab Engine = OCOTP
 hab Features = SRK REVOKE
 #endif
 
+#if defined(CONFIG_HABV4_CSF_UNLOCK_FIELD_RETURN)
+hab [Unlock]
+hab Engine = OCOTP
+hab Features = FIELD RETURN
+hab UID = HABV4_CSF_UNLOCK_UID
+#endif
+
 hab [Install Key]
 /* verification key index in key store (0, 2...4) */
 hab Verification index = 0
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 6b1f0ccbc003..ffbd5360cba0 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -566,6 +566,7 @@ imxcfg_cpp_flags  = -Wp,-MD,$(depfile) -nostdinc -x assembler-with-cpp \
       $(call overwrite-hab-env,CONFIG_HABV4_TABLE_BIN) \
       $(call overwrite-hab-env,CONFIG_HABV4_CSF_CRT_PEM) \
       $(call overwrite-hab-env,CONFIG_HABV4_IMG_CRT_PEM) \
+      -DHABV4_CSF_UNLOCK_UID=$(CONFIG_HABV4_CSF_UNLOCK_UID) \
       $(call overwrite-fit-env,CONFIG_BOOTM_FITIMAGE_PUBKEY) \
 
 dcd-tmp = $(subst $(comma),_,$(dot-target).dcd.tmp)

-- 
2.39.2

[PATCH v2 9/9] i.MX: HAB: add imx_hab_field_return support

[Marco Felsch]

Add a convenient helper to burn the field-return fuse which wraps the
platform specific hook. At the moment only i.MX8M devices are supported.
Adding support for other platforms can be done by providing the platform
specific hook.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/hab/hab.c                | 41 ++++++++++++++++++++++++++++++++++++++++
 include/hab.h                    |  1 +
 include/mach/imx/ocotp-fusemap.h |  1 +
 3 files changed, 43 insertions(+)

diff --git a/drivers/hab/hab.c b/drivers/hab/hab.c
index 7c840303d395..73d470be131e 100644
--- a/drivers/hab/hab.c
+++ b/drivers/hab/hab.c
@@ -252,6 +252,23 @@ static int imx8m_hab_revoke_key_ocotp(unsigned key_idx)
 	return ret;
 }
 
+static int imx8m_hab_field_return_ocotp(void)
+{
+	int ret;
+
+	ret = imx_ocotp_field_return_locked();
+	if (ret < 0)
+		return ret;
+
+	/* Return -EINVAL in case the FIELD_RETURN write is locked */
+	if (ret == 1)
+		return -EINVAL;
+
+	ret = imx_ocotp_write_field(MX8M_OCOTP_FIELD_RETURN, 1);
+
+	return ret;
+}
+
 struct imx_hab_ops {
 	int (*write_srk_hash)(const u8 *srk, unsigned flags);
 	int (*read_srk_hash)(u8 *srk);
@@ -260,6 +277,7 @@ struct imx_hab_ops {
 	int (*device_locked_down)(void);
 	int (*print_status)(void);
 	int (*revoke_key)(unsigned key_idx);
+	int (*field_return)(void);
 };
 
 static struct imx_hab_ops imx_hab_ops_iim = {
@@ -288,6 +306,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,
 };
 
 static int imx_ahab_write_srk_hash(const u8 *__newsrk, unsigned flags)
@@ -581,3 +600,25 @@ int imx_hab_revoke_key(unsigned key_idx, bool permanent)
 
 	return ret;
 }
+
+int imx_hab_field_return(bool permanent)
+{
+	struct imx_hab_ops *ops = imx_get_hab_ops();
+	int ret;
+
+	if (!ops || !ops->field_return)
+		return -ENOSYS;
+
+	if (permanent) {
+		ret = ops->permanent_write_enable(1);
+		if (ret)
+			return ret;
+	}
+
+	ret = ops->field_return();
+
+	if (permanent)
+		ops->permanent_write_enable(0);
+
+	return ret;
+}
diff --git a/include/hab.h b/include/hab.h
index 7a70c67f0488..393cf0513e0c 100644
--- a/include/hab.h
+++ b/include/hab.h
@@ -52,5 +52,6 @@ int imx_hab_lockdown_device(unsigned flags);
 int imx_hab_device_locked_down(void);
 int imx_hab_print_status(void);
 int imx_hab_revoke_key(unsigned key_idx, bool permanent);
+int imx_hab_field_return(bool permanent);
 
 #endif /* __HABV4_H */
diff --git a/include/mach/imx/ocotp-fusemap.h b/include/mach/imx/ocotp-fusemap.h
index 1aece9195ff9..37f1ee8298c2 100644
--- a/include/mach/imx/ocotp-fusemap.h
+++ b/include/mach/imx/ocotp-fusemap.h
@@ -67,6 +67,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 MX8M_OCOTP_FIELD_RETURN		(OCOTP_WORD(0x630) | OCOTP_BIT(0) | OCOTP_WIDTH(1))
 #define MX8M_OCOTP_SRK_REVOKE		(OCOTP_WORD(0x670) | OCOTP_BIT(0) | OCOTP_WIDTH(4))
 
 #endif /* __MACH_IMX_OCOTP_FUSEMAP_H */

-- 
2.39.2

Re: [PATCH v2 7/9] i.MX8M: HABv4: add an option to allow key revocation

[Ahmad Fatoum]

Hello Marco,

On 03.07.24 19:20, Marco Felsch wrote:
> The HAB code needs an special [Unlock] instruction to keep the
> SRK_REVOKE fuse bank unlocked. This is required if a key needs to be
> revoked.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm/mach-imx/Kconfig            | 8 ++++++++
>  include/mach/imx/habv4-imx8-gencsf.h | 6 ++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index 61258137736f..68f55971506b 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -835,6 +835,14 @@ config HABV4_QSPI
>  	help
>  	  Enable this option to build signed QSPI/FlexSPI images.
>  
> +config HABV4_CSF_UNLOCK_SRK_REVOKE
> +	depends on HABV4
> +	bool "Unlock SRK revocation"
> +	help
> +	  Enable this option to instruct the HAB code to not lock
> +	  the SRK_REVOKE_LOCK sticky bit. This is required for key
> +	  revocation. Don't enable this if you are unsure.

I think for added safety we should have an extra option that prompts
for the key to be revoked and an initcall that is activated depending
on it, e.g.:

config HABV4_CSF_SRK_REVOKE_INDEX
        int "SRK to revoke"
        range 0 3
        default 0
        depends on HABV4_CSF_SRK_REVOKE_UNLOCK
        help
          Which of the first three SRKs to revoke. The SRK indices are
          1-based. Saying 0 here will just print the SRK Revocation
          register without modification. SRK #4 is immutable.
                                                                       
          Proceed with caution, revoking a SRK is irreversible and
          manual manipulation of this code can brick the board!
                                                                       
if HABV4_CSF_SRK_REVOKE_INDEX = HABV4_SRK_INDEX
comment "Can't revoke same SRK used for signing"
comment "Attempts to build a signed barebox image will fail"
endif

and then some code that checks the same above condition during final
assembly of the signed image.

What do you think?

> +
>  config HAB_CERTS_ENV
>  	depends on HAB
>  	bool "Specify certificates in environment"
> diff --git a/include/mach/imx/habv4-imx8-gencsf.h b/include/mach/imx/habv4-imx8-gencsf.h
> index 5f92ceceab00..56d9ef2de92f 100644
> --- a/include/mach/imx/habv4-imx8-gencsf.h
> +++ b/include/mach/imx/habv4-imx8-gencsf.h
> @@ -36,6 +36,12 @@ hab [Unlock]
>  hab Engine = CAAM
>  hab Features = RNG, MID
>  
> +#if defined(CONFIG_HABV4_CSF_UNLOCK_SRK_REVOKE)
> +hab [Unlock]
> +hab Engine = OCOTP
> +hab Features = SRK REVOKE
> +#endif
> +
>  hab [Install Key]
>  /* verification key index in key store (0, 2...4) */
>  hab Verification index = 0
> 

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

Re: [PATCH v2 1/9] i.MX: HABv4: fix SRK_LOCK for i.MX8M devices

[Ahmad Fatoum]

Hi,

On 03.07.24 19:20, Marco Felsch wrote:
> The fuse to lock the SRK hash on i.MX8M* SoCs is different than the one
> used for the i.MX6 SoCs. Fix this by refactoring
> imx_hab_write_srk_hash_ocotp() and make the lock fusing SoC specific.
> 
> Fixes: 6c4d5bb5acfe ("i.MX: HABv4: implement interface for i.MX8MQ")
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Sascha, can this go into master?

Thanks,
Ahmad

> ---
>  drivers/hab/hab.c                | 34 +++++++++++++++++++++++++++++++---
>  include/mach/imx/ocotp-fusemap.h |  1 +
>  2 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hab/hab.c b/drivers/hab/hab.c
> index ed091058d8fb..28a091841a69 100644
> --- a/drivers/hab/hab.c
> +++ b/drivers/hab/hab.c
> @@ -128,7 +128,7 @@ static int imx_hab_read_srk_hash_ocotp(u8 *__srk)
>  	return 0;
>  }
>  
> -static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk, unsigned flags)
> +static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk)
>  {
>  	u32 *newsrk = (u32 *)__newsrk;
>  	int ret, i;
> @@ -139,6 +139,17 @@ static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk, unsigned flags)
>  			return ret;
>  	}
>  
> +	return 0;
> +}
> +
> +static int imx6_hab_write_srk_hash_ocotp(const u8 *newsrk, unsigned flags)
> +{
> +	int ret;
> +
> +	ret = imx_hab_write_srk_hash_ocotp(newsrk);
> +	if (ret)
> +		return ret;
> +
>  	if (flags & IMX_SRK_HASH_WRITE_LOCK) {
>  		ret = imx_ocotp_write_field(OCOTP_SRK_LOCK, 1);
>  		if (ret < 0)
> @@ -148,6 +159,23 @@ static int imx_hab_write_srk_hash_ocotp(const u8 *__newsrk, unsigned flags)
>  	return 0;
>  }
>  
> +static int imx8m_hab_write_srk_hash_ocotp(const u8 *newsrk, unsigned flags)
> +{
> +	int ret;
> +
> +	ret = imx_hab_write_srk_hash_ocotp(newsrk);
> +	if (ret)
> +		return ret;
> +
> +	if (flags & IMX_SRK_HASH_WRITE_LOCK) {
> +		ret = imx_ocotp_write_field(MX8M_OCOTP_SRK_LOCK, 1);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int imx_hab_permanent_write_enable_ocotp(int enable)
>  {
>  	return imx_ocotp_permanent_write(enable);
> @@ -222,7 +250,7 @@ static struct imx_hab_ops imx_hab_ops_iim = {
>  };
>  
>  static struct imx_hab_ops imx6_hab_ops_ocotp = {
> -	.write_srk_hash = imx_hab_write_srk_hash_ocotp,
> +	.write_srk_hash = imx6_hab_write_srk_hash_ocotp,
>  	.read_srk_hash =  imx_hab_read_srk_hash_ocotp,
>  	.lockdown_device = imx6_hab_lockdown_device_ocotp,
>  	.device_locked_down = imx6_hab_device_locked_down_ocotp,
> @@ -231,7 +259,7 @@ static struct imx_hab_ops imx6_hab_ops_ocotp = {
>  };
>  
>  static struct imx_hab_ops imx8m_hab_ops_ocotp = {
> -	.write_srk_hash = imx_hab_write_srk_hash_ocotp,
> +	.write_srk_hash = imx8m_hab_write_srk_hash_ocotp,
>  	.read_srk_hash =  imx_hab_read_srk_hash_ocotp,
>  	.lockdown_device = imx8m_hab_lockdown_device_ocotp,
>  	.device_locked_down = imx8m_hab_device_locked_down_ocotp,
> diff --git a/include/mach/imx/ocotp-fusemap.h b/include/mach/imx/ocotp-fusemap.h
> index 823273895502..c4f94e61e8f8 100644
> --- a/include/mach/imx/ocotp-fusemap.h
> +++ b/include/mach/imx/ocotp-fusemap.h
> @@ -54,6 +54,7 @@
>  #define OCOTP_GP2			(OCOTP_WORD(0x670) | OCOTP_BIT(0) | OCOTP_WIDTH(32))
>  #define OCOTP_PAD_SETTINGS		(OCOTP_WORD(0x6d0) | OCOTP_BIT(0) | OCOTP_WIDTH(6))
>  /* i.MX8M moved the security related fuses */
> +#define MX8M_OCOTP_SRK_LOCK		(OCOTP_WORD(0x400) | OCOTP_BIT(9) | OCOTP_WIDTH(1))
>  #define MX8M_OCOTP_SEC_CONFIG_1		(OCOTP_WORD(0x470) | OCOTP_BIT(25) | OCOTP_WIDTH(1))
>  #define MX8MQ_OCOTP_DIR_BT_DIS		(OCOTP_WORD(0x470) | OCOTP_BIT(27) | OCOTP_WIDTH(1))
>  
> 

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

Re: [PATCH v2 7/9] i.MX8M: HABv4: add an option to allow key revocation

[Marco Felsch]

Hi Ahmad,

On 24-07-03, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 03.07.24 19:20, Marco Felsch wrote:
> > The HAB code needs an special [Unlock] instruction to keep the
> > SRK_REVOKE fuse bank unlocked. This is required if a key needs to be
> > revoked.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  arch/arm/mach-imx/Kconfig            | 8 ++++++++
> >  include/mach/imx/habv4-imx8-gencsf.h | 6 ++++++
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> > index 61258137736f..68f55971506b 100644
> > --- a/arch/arm/mach-imx/Kconfig
> > +++ b/arch/arm/mach-imx/Kconfig
> > @@ -835,6 +835,14 @@ config HABV4_QSPI
> >  	help
> >  	  Enable this option to build signed QSPI/FlexSPI images.
> >  
> > +config HABV4_CSF_UNLOCK_SRK_REVOKE
> > +	depends on HABV4
> > +	bool "Unlock SRK revocation"
> > +	help
> > +	  Enable this option to instruct the HAB code to not lock
> > +	  the SRK_REVOKE_LOCK sticky bit. This is required for key
> > +	  revocation. Don't enable this if you are unsure.
> 
> I think for added safety we should have an extra option that prompts
> for the key to be revoked and an initcall that is activated depending
> on it, e.g.:
> 
> config HABV4_CSF_SRK_REVOKE_INDEX
>         int "SRK to revoke"
>         range 0 3
>         default 0
>         depends on HABV4_CSF_SRK_REVOKE_UNLOCK
>         help
>           Which of the first three SRKs to revoke. The SRK indices are
>           1-based. Saying 0 here will just print the SRK Revocation
>           register without modification. SRK #4 is immutable.
>                                                                        
>           Proceed with caution, revoking a SRK is irreversible and
>           manual manipulation of this code can brick the board!
>                                                                        
> if HABV4_CSF_SRK_REVOKE_INDEX = HABV4_SRK_INDEX
> comment "Can't revoke same SRK used for signing"
> comment "Attempts to build a signed barebox image will fail"
> endif
> 
> and then some code that checks the same above condition during final
> assembly of the signed image.
> 
> What do you think?

That's an good idea to make it more user-friendly for most users :)
Regarding this patchset I do see it more as an addition since for my
project the revocation is checked on every startup and we do allow the
revocation of multiple SRK slots at the same time.

Regards,
  Marco

> >  config HAB_CERTS_ENV
> >  	depends on HAB
> >  	bool "Specify certificates in environment"
> > diff --git a/include/mach/imx/habv4-imx8-gencsf.h b/include/mach/imx/habv4-imx8-gencsf.h
> > index 5f92ceceab00..56d9ef2de92f 100644
> > --- a/include/mach/imx/habv4-imx8-gencsf.h
> > +++ b/include/mach/imx/habv4-imx8-gencsf.h
> > @@ -36,6 +36,12 @@ hab [Unlock]
> >  hab Engine = CAAM
> >  hab Features = RNG, MID
> >  
> > +#if defined(CONFIG_HABV4_CSF_UNLOCK_SRK_REVOKE)
> > +hab [Unlock]
> > +hab Engine = OCOTP
> > +hab Features = SRK REVOKE
> > +#endif
> > +
> >  hab [Install Key]
> >  /* verification key index in key store (0, 2...4) */
> >  hab Verification index = 0
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

Re: (subset) [PATCH v2 0/9] i.MX8M HAB and OCOTP additions and fixes

[Sascha Hauer]

On Wed, 03 Jul 2024 19:20:17 +0200, Marco Felsch wrote:
> this fixes the HABv4 locking on i.MX8M devices and add additional
> features like:
>  - key revocation
>  - field-return burning
>  - additional security related fuse defintions
> 
> Changes in v2:
> - Link to v1: https://lore.barebox.org/barebox/Zmv2KyQq3ACxksc_@pengutronix.de/
> - adapt patch-1 commit message
> - make use of 'bool permanent' parameter instead of introducing new
>   IMX_SRK_* flags.
> 
> [...]

Applied, thanks!

[1/9] i.MX: HABv4: fix SRK_LOCK for i.MX8M devices
      https://git.pengutronix.de/cgit/barebox/commit/?id=2f5856ee7ea9 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH v2 7/9] i.MX8M: HABv4: add an option to allow key revocation

[Ahmad Fatoum]

Hello Marco,

On 7/4/24 10:15, Marco Felsch wrote:
> Hi Ahmad,
> 
> On 24-07-03, Ahmad Fatoum wrote:
>> Hello Marco,
>>
>> On 03.07.24 19:20, Marco Felsch wrote:
>>> The HAB code needs an special [Unlock] instruction to keep the
>>> SRK_REVOKE fuse bank unlocked. This is required if a key needs to be
>>> revoked.
>>>
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>> ---
>>>  arch/arm/mach-imx/Kconfig            | 8 ++++++++
>>>  include/mach/imx/habv4-imx8-gencsf.h | 6 ++++++
>>>  2 files changed, 14 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
>>> index 61258137736f..68f55971506b 100644
>>> --- a/arch/arm/mach-imx/Kconfig
>>> +++ b/arch/arm/mach-imx/Kconfig
>>> @@ -835,6 +835,14 @@ config HABV4_QSPI
>>>  	help
>>>  	  Enable this option to build signed QSPI/FlexSPI images.
>>>  
>>> +config HABV4_CSF_UNLOCK_SRK_REVOKE
>>> +	depends on HABV4
>>> +	bool "Unlock SRK revocation"
>>> +	help
>>> +	  Enable this option to instruct the HAB code to not lock
>>> +	  the SRK_REVOKE_LOCK sticky bit. This is required for key
>>> +	  revocation. Don't enable this if you are unsure.
>>
>> I think for added safety we should have an extra option that prompts
>> for the key to be revoked and an initcall that is activated depending
>> on it, e.g.:
>>
>> config HABV4_CSF_SRK_REVOKE_INDEX
>>         int "SRK to revoke"
>>         range 0 3
>>         default 0
>>         depends on HABV4_CSF_SRK_REVOKE_UNLOCK
>>         help
>>           Which of the first three SRKs to revoke. The SRK indices are
>>           1-based. Saying 0 here will just print the SRK Revocation
>>           register without modification. SRK #4 is immutable.
>>                                                                        
>>           Proceed with caution, revoking a SRK is irreversible and
>>           manual manipulation of this code can brick the board!
>>                                                                        
>> if HABV4_CSF_SRK_REVOKE_INDEX = HABV4_SRK_INDEX
>> comment "Can't revoke same SRK used for signing"
>> comment "Attempts to build a signed barebox image will fail"
>> endif
>>
>> and then some code that checks the same above condition during final
>> assembly of the signed image.
>>
>> What do you think?
> 
> That's an good idea to make it more user-friendly for most users :)
> Regarding this patchset I do see it more as an addition since for my
> project the revocation is checked on every startup and we do allow the
> revocation of multiple SRK slots at the same time.

Ok. I just have a single nit then, can you rename
HABV4_CSF_UNLOCK_SRK_REVOKE to HABV4_CSF_SRK_REVOKE_UNLOCK, so we can
add HABV4_CSF_SRK_REVOKE_INDEX in future with the same prefix?

Thanks,
Ahmad

> 
> Regards,
>   Marco
> 
>>>  config HAB_CERTS_ENV
>>>  	depends on HAB
>>>  	bool "Specify certificates in environment"
>>> diff --git a/include/mach/imx/habv4-imx8-gencsf.h b/include/mach/imx/habv4-imx8-gencsf.h
>>> index 5f92ceceab00..56d9ef2de92f 100644
>>> --- a/include/mach/imx/habv4-imx8-gencsf.h
>>> +++ b/include/mach/imx/habv4-imx8-gencsf.h
>>> @@ -36,6 +36,12 @@ hab [Unlock]
>>>  hab Engine = CAAM
>>>  hab Features = RNG, MID
>>>  
>>> +#if defined(CONFIG_HABV4_CSF_UNLOCK_SRK_REVOKE)
>>> +hab [Unlock]
>>> +hab Engine = OCOTP
>>> +hab Features = SRK REVOKE
>>> +#endif
>>> +
>>>  hab [Install Key]
>>>  /* verification key index in key store (0, 2...4) */
>>>  hab Verification index = 0
>>>
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
>>
> 

Re: [PATCH v2 7/9] i.MX8M: HABv4: add an option to allow key revocation

[Marco Felsch]

On 24-07-30, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 7/4/24 10:15, Marco Felsch wrote:
> > Hi Ahmad,
> > 
> > On 24-07-03, Ahmad Fatoum wrote:
> >> Hello Marco,
> >>
> >> On 03.07.24 19:20, Marco Felsch wrote:
> >>> The HAB code needs an special [Unlock] instruction to keep the
> >>> SRK_REVOKE fuse bank unlocked. This is required if a key needs to be
> >>> revoked.
> >>>
> >>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> >>> ---
> >>>  arch/arm/mach-imx/Kconfig            | 8 ++++++++
> >>>  include/mach/imx/habv4-imx8-gencsf.h | 6 ++++++
> >>>  2 files changed, 14 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> >>> index 61258137736f..68f55971506b 100644
> >>> --- a/arch/arm/mach-imx/Kconfig
> >>> +++ b/arch/arm/mach-imx/Kconfig
> >>> @@ -835,6 +835,14 @@ config HABV4_QSPI
> >>>  	help
> >>>  	  Enable this option to build signed QSPI/FlexSPI images.
> >>>  
> >>> +config HABV4_CSF_UNLOCK_SRK_REVOKE
> >>> +	depends on HABV4
> >>> +	bool "Unlock SRK revocation"
> >>> +	help
> >>> +	  Enable this option to instruct the HAB code to not lock
> >>> +	  the SRK_REVOKE_LOCK sticky bit. This is required for key
> >>> +	  revocation. Don't enable this if you are unsure.
> >>
> >> I think for added safety we should have an extra option that prompts
> >> for the key to be revoked and an initcall that is activated depending
> >> on it, e.g.:
> >>
> >> config HABV4_CSF_SRK_REVOKE_INDEX
> >>         int "SRK to revoke"
> >>         range 0 3
> >>         default 0
> >>         depends on HABV4_CSF_SRK_REVOKE_UNLOCK
> >>         help
> >>           Which of the first three SRKs to revoke. The SRK indices are
> >>           1-based. Saying 0 here will just print the SRK Revocation
> >>           register without modification. SRK #4 is immutable.
> >>                                                                        
> >>           Proceed with caution, revoking a SRK is irreversible and
> >>           manual manipulation of this code can brick the board!
> >>                                                                        
> >> if HABV4_CSF_SRK_REVOKE_INDEX = HABV4_SRK_INDEX
> >> comment "Can't revoke same SRK used for signing"
> >> comment "Attempts to build a signed barebox image will fail"
> >> endif
> >>
> >> and then some code that checks the same above condition during final
> >> assembly of the signed image.
> >>
> >> What do you think?
> > 
> > That's an good idea to make it more user-friendly for most users :)
> > Regarding this patchset I do see it more as an addition since for my
> > project the revocation is checked on every startup and we do allow the
> > revocation of multiple SRK slots at the same time.
> 
> Ok. I just have a single nit then, can you rename
> HABV4_CSF_UNLOCK_SRK_REVOKE to HABV4_CSF_SRK_REVOKE_UNLOCK, so we can
> add HABV4_CSF_SRK_REVOKE_INDEX in future with the same prefix?

Sure, I can do this.

Regards,
  Marco

> 
> Thanks,
> Ahmad
> 
> > 
> > Regards,
> >   Marco
> > 
> >>>  config HAB_CERTS_ENV
> >>>  	depends on HAB
> >>>  	bool "Specify certificates in environment"
> >>> diff --git a/include/mach/imx/habv4-imx8-gencsf.h b/include/mach/imx/habv4-imx8-gencsf.h
> >>> index 5f92ceceab00..56d9ef2de92f 100644
> >>> --- a/include/mach/imx/habv4-imx8-gencsf.h
> >>> +++ b/include/mach/imx/habv4-imx8-gencsf.h
> >>> @@ -36,6 +36,12 @@ hab [Unlock]
> >>>  hab Engine = CAAM
> >>>  hab Features = RNG, MID
> >>>  
> >>> +#if defined(CONFIG_HABV4_CSF_UNLOCK_SRK_REVOKE)
> >>> +hab [Unlock]
> >>> +hab Engine = OCOTP
> >>> +hab Features = SRK REVOKE
> >>> +#endif
> >>> +
> >>>  hab [Install Key]
> >>>  /* verification key index in key store (0, 2...4) */
> >>>  hab Verification index = 0
> >>>
> >>
> >> -- 
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>
> >>
> > 
>