[PATCH v3 0/6] crypto/fsl: add RNG support

[PATCH v3 0/6] crypto/fsl: add RNG support

[Michael Walle]

First, improve the compatibility on newer Era CAAMs. These introduced new
version registers. Secondly, add RNG support for the CAAM. This way we get
random number generator support for EFI for free and KASLR will work with
ARM64 kernels booted with bootefi.

Changes since v2:
 - fix descriptor size of the deinstantiate descriptor
 - use malloc_cache_aligned() instead of memalign(ARCH_DMA_MINALIGN..)
 - don't check the RDSTA_PR() bit
 - fix memcpy() length
 - flush the dcache where the rng job description resides

Changes since v1:
 - instantiate RNG with prediction resistance
 - add annotation for caam_get_era()
 - fix all prints to match u-boots status lines
 - remove superfluous #ifdef CONFIG_DM_RNG
 - changed kconfig help text

Michael Walle (6):
  crypto/fsl: make SEC%u status line consistent
  crypto/fsl: export caam_get_era()
  crypto/fsl: support newer SEC modules
  crypto/fsl: don't regenerate secure keys
  crypto/fsl: instantiate the RNG with prediciton resistance
  crypto/fsl: add RNG support

 drivers/crypto/fsl/Kconfig   |  14 +++++
 drivers/crypto/fsl/Makefile  |   1 +
 drivers/crypto/fsl/desc.h    |   2 +
 drivers/crypto/fsl/jobdesc.c |  26 ++++++++-
 drivers/crypto/fsl/jobdesc.h |   7 ++-
 drivers/crypto/fsl/jr.c      | 110 +++++++++++++++++++++++++++++------
 drivers/crypto/fsl/rng.c     |  86 +++++++++++++++++++++++++++
 drivers/crypto/fsl/sec.c     |  10 +++-
 include/fsl_sec.h            |  61 +++++++++++++++----
 9 files changed, 282 insertions(+), 35 deletions(-)
 create mode 100644 drivers/crypto/fsl/rng.c

-- 
2.20.1

[PATCH v3 1/6] crypto/fsl: make SEC%u status line consistent

[Michael Walle]

Align the status line with all the other output in U-Boot.

Before the change:
DDR    3.9 GiB (DDR3, 32-bit, CL=11, ECC on)
SEC0: RNG instantiated
WDT:   Started with servicing (60s timeout)

After the change:
DDR    3.9 GiB (DDR3, 32-bit, CL=11, ECC on)
SEC0:  RNG instantiated
WDT:   Started with servicing (60s timeout)

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Horia Geant? <horia.geanta@nxp.com>
---
 drivers/crypto/fsl/jr.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index e2d9216cfc..bbdbcb8e58 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -478,8 +478,8 @@ static int instantiate_rng(uint8_t sec_idx)
 		ret = run_descriptor_jr_idx(desc, sec_idx);
 
 		if (ret)
-			printf("RNG: Instantiation failed with error 0x%x\n",
-			       ret);
+			printf("SEC%u:  RNG4 SH%d instantiation failed with error 0x%x\n",
+			       sec_idx, sh_idx, ret);
 
 		rdsta_val = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
 		if (!(rdsta_val & (1 << sh_idx))) {
@@ -569,7 +569,7 @@ static int rng_init(uint8_t sec_idx)
 		ret = instantiate_rng(sec_idx);
 	} while ((ret == -1) && (ent_delay < RTSDCTL_ENT_DLY_MAX));
 	if (ret) {
-		printf("RNG: Failed to instantiate RNG\n");
+		printf("SEC%u:  Failed to instantiate RNG\n", sec_idx);
 		return ret;
 	}
 
@@ -592,7 +592,7 @@ int sec_init_idx(uint8_t sec_idx)
 #endif
 
 	if (!(sec_idx < CONFIG_SYS_FSL_MAX_NUM_OF_SEC)) {
-		printf("SEC initialization failed\n");
+		printf("SEC%u:  initialization failed\n", sec_idx);
 		return -1;
 	}
 
@@ -640,7 +640,7 @@ int sec_init_idx(uint8_t sec_idx)
 
 	ret = jr_init(sec_idx);
 	if (ret < 0) {
-		printf("SEC initialization failed\n");
+		printf("SEC%u:  initialization failed\n", sec_idx);
 		return -1;
 	}
 
@@ -654,10 +654,10 @@ int sec_init_idx(uint8_t sec_idx)
 #ifndef CONFIG_SPL_BUILD
 	if (get_rng_vid(sec_idx) >= 4) {
 		if (rng_init(sec_idx) < 0) {
-			printf("SEC%u: RNG instantiation failed\n", sec_idx);
+			printf("SEC%u:  RNG instantiation failed\n", sec_idx);
 			return -1;
 		}
-		printf("SEC%u: RNG instantiated\n", sec_idx);
+		printf("SEC%u:  RNG instantiated\n", sec_idx);
 	}
 #endif
 	return ret;
-- 
2.20.1

[PATCH v3 2/6] crypto/fsl: export caam_get_era()

[Michael Walle]

We need the era in other modules, too. For example, to get the RNG
version.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Horia Geant? <horia.geanta@nxp.com>
---
 drivers/crypto/fsl/sec.c | 10 +++++++++-
 include/fsl_sec.h        |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/fsl/sec.c b/drivers/crypto/fsl/sec.c
index a2c0bfaf44..f0a4a63d88 100644
--- a/drivers/crypto/fsl/sec.c
+++ b/drivers/crypto/fsl/sec.c
@@ -98,7 +98,15 @@ void fdt_fixup_crypto_node(void *blob, int sec_rev)
 		       fdt_strerror(err));
 }
 #elif CONFIG_SYS_FSL_SEC_COMPAT >= 4  /* SEC4 */
-static u8 caam_get_era(void)
+/**
+ * caam_get_era() - fetch the CAAM's era
+ *
+ * The SEC module povides an "Era" which can be used to differentiate
+ * between different revisions.
+ *
+ * Return: era of the SEC.
+ */
+u8 caam_get_era(void)
 {
 	static const struct {
 		u16 ip_id;
diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index c0d2c7e866..2ebb75c9b2 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -316,6 +316,8 @@ int blob_dek(const u8 *src, u8 *dst, u8 len);
 int sec_init_idx(uint8_t);
 #endif
 int sec_init(void);
+
+u8 caam_get_era(void);
 #endif
 
 #endif /* __FSL_SEC_H */
-- 
2.20.1

[PATCH v3 3/6] crypto/fsl: support newer SEC modules

[Michael Walle]

Since Era 10, the version registers changed. Add the version registers
and use them on newer modules.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Horia Geant? <horia.geanta@nxp.com>
---
 drivers/crypto/fsl/jr.c | 12 ++++++++--
 include/fsl_sec.h       | 51 +++++++++++++++++++++++++++++++++++------
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index bbdbcb8e58..5275c50e8b 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -498,9 +498,17 @@ static int instantiate_rng(uint8_t sec_idx)
 static u8 get_rng_vid(uint8_t sec_idx)
 {
 	ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
-	u32 cha_vid = sec_in32(&sec->chavid_ls);
+	u8 vid;
 
-	return (cha_vid & SEC_CHAVID_RNG_LS_MASK) >> SEC_CHAVID_LS_RNG_SHIFT;
+	if (caam_get_era() < 10) {
+		vid = (sec_in32(&sec->chavid_ls) & SEC_CHAVID_RNG_LS_MASK)
+		       >> SEC_CHAVID_LS_RNG_SHIFT;
+	} else {
+		vid = (sec_in32(&sec->vreg.rng) & CHA_VER_VID_MASK)
+		       >> CHA_VER_VID_SHIFT;
+	}
+
+	return vid;
 }
 
 /*
diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index 2ebb75c9b2..8dce0bbb1b 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -73,6 +73,41 @@ struct rng4tst {
 	u32 rsvd2[15];
 };
 
+/* Version registers (Era 10+) */
+struct version_regs {
+	u32 crca;	/* CRCA_VERSION */
+	u32 afha;	/* AFHA_VERSION */
+	u32 kfha;	/* KFHA_VERSION */
+	u32 pkha;	/* PKHA_VERSION */
+	u32 aesa;	/* AESA_VERSION */
+	u32 mdha;	/* MDHA_VERSION */
+	u32 desa;	/* DESA_VERSION */
+	u32 snw8a;	/* SNW8A_VERSION */
+	u32 snw9a;	/* SNW9A_VERSION */
+	u32 zuce;	/* ZUCE_VERSION */
+	u32 zuca;	/* ZUCA_VERSION */
+	u32 ccha;	/* CCHA_VERSION */
+	u32 ptha;	/* PTHA_VERSION */
+	u32 rng;	/* RNG_VERSION */
+	u32 trng;	/* TRNG_VERSION */
+	u32 aaha;	/* AAHA_VERSION */
+	u32 rsvd[10];
+	u32 sr;		/* SR_VERSION */
+	u32 dma;	/* DMA_VERSION */
+	u32 ai;		/* AI_VERSION */
+	u32 qi;		/* QI_VERSION */
+	u32 jr;		/* JR_VERSION */
+	u32 deco;	/* DECO_VERSION */
+};
+
+#define CHA_VER_NUM_MASK	0x000000ff
+#define CHA_VER_MISC_SHIFT	8
+#define CHA_VER_MISC_MASK	0x0000ff00
+#define CHA_VER_REV_SHIFT	16
+#define CHA_VER_REV_MASK	0x00ff0000
+#define CHA_VER_VID_SHIFT	24
+#define CHA_VER_VID_MASK	0xff000000
+
 typedef struct ccsr_sec {
 	u32	res0;
 	u32	mcfgr;		/* Master CFG Register */
@@ -98,17 +133,19 @@ typedef struct ccsr_sec {
 	u32	drr;		/* DECO Reset Register */
 	u8	res5[0x4d8];
 	struct rng4tst rng;	/* RNG Registers */
-	u8	res6[0x8a0];
+	u8	res6[0x780];
+	struct version_regs vreg; /* version registers since era 10 */
+	u8	res7[0xa0];
 	u32	crnr_ms;	/* CHA Revision Number Register, MS */
 	u32	crnr_ls;	/* CHA Revision Number Register, LS */
 	u32	ctpr_ms;	/* Compile Time Parameters Register, MS */
 	u32	ctpr_ls;	/* Compile Time Parameters Register, LS */
-	u8	res7[0x10];
+	u8	res8[0x10];
 	u32	far_ms;		/* Fault Address Register, MS */
 	u32	far_ls;		/* Fault Address Register, LS */
 	u32	falr;		/* Fault Address LIODN Register */
 	u32	fadr;		/* Fault Address Detail Register */
-	u8	res8[0x4];
+	u8	res9[0x4];
 	u32	csta;		/* CAAM Status Register */
 	u32	smpart;		/* Secure Memory Partition Parameters */
 	u32	smvid;		/* Secure Memory Version ID */
@@ -121,16 +158,16 @@ typedef struct ccsr_sec {
 	u32	secvid_ms;	/* SEC Version ID Register, MS */
 	u32	secvid_ls;	/* SEC Version ID Register, LS */
 #if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
-	u8	res9[0x6f020];
+	u8	res10[0x6f020];
 #else
-	u8	res9[0x6020];
+	u8	res10[0x6020];
 #endif
 	u32	qilcr_ms;	/* Queue Interface LIODN CFG Register, MS */
 	u32	qilcr_ls;	/* Queue Interface LIODN CFG Register, LS */
 #if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
-	u8	res10[0x8ffd8];
+	u8	res11[0x8ffd8];
 #else
-	u8	res10[0x8fd8];
+	u8	res11[0x8fd8];
 #endif
 } ccsr_sec_t;
 
-- 
2.20.1

[PATCH v3 4/6] crypto/fsl: don't regenerate secure keys

[Michael Walle]

The secure keys (TDKEK, JDKEK, TDSK) can only be generated once after a
POR. Otherwise the RNG4 will throw an error.

Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Horia Geant? <horia.geanta@nxp.com>
---
 drivers/crypto/fsl/jobdesc.c | 4 ++--
 drivers/crypto/fsl/jobdesc.h | 2 +-
 drivers/crypto/fsl/jr.c      | 9 +++++----
 include/fsl_sec.h            | 1 +
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/fsl/jobdesc.c b/drivers/crypto/fsl/jobdesc.c
index 2f35e0c90b..6102e9c06b 100644
--- a/drivers/crypto/fsl/jobdesc.c
+++ b/drivers/crypto/fsl/jobdesc.c
@@ -258,7 +258,7 @@ void inline_cnstr_jobdesc_blob_decap(uint32_t *desc, uint8_t *key_idnfr,
  * Descriptor to instantiate RNG State Handle 0 in normal mode and
  * load the JDKEK, TDKEK and TDSK registers
  */
-void inline_cnstr_jobdesc_rng_instantiation(uint32_t *desc, int handle)
+void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk)
 {
 	u32 *jump_cmd;
 
@@ -269,7 +269,7 @@ void inline_cnstr_jobdesc_rng_instantiation(uint32_t *desc, int handle)
 			(handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT);
 
 	/* For SH0, Secure Keys must be generated as well */
-	if (handle == 0) {
+	if (!handle && do_sk) {
 		/* wait for done */
 		jump_cmd = append_jump(desc, JUMP_CLASS_CLASS1);
 		set_jump_tgt_here(desc, jump_cmd);
diff --git a/drivers/crypto/fsl/jobdesc.h b/drivers/crypto/fsl/jobdesc.h
index d782c46b9d..14b2a119d7 100644
--- a/drivers/crypto/fsl/jobdesc.h
+++ b/drivers/crypto/fsl/jobdesc.h
@@ -39,7 +39,7 @@ void inline_cnstr_jobdesc_blob_decap(uint32_t *desc, uint8_t *key_idnfr,
 				     uint8_t *enc_blob, uint8_t *plain_txt,
 				     uint32_t out_sz);
 
-void inline_cnstr_jobdesc_rng_instantiation(uint32_t *desc, int handle);
+void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk);
 
 void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
 				      struct pk_in_params *pkin, uint8_t *out,
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index 5275c50e8b..42865a6cd7 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -446,7 +446,7 @@ int sec_reset(void)
 	return sec_reset_idx(0);
 }
 #ifndef CONFIG_SPL_BUILD
-static int instantiate_rng(uint8_t sec_idx)
+static int instantiate_rng(u8 sec_idx, int gen_sk)
 {
 	u32 *desc;
 	u32 rdsta_val;
@@ -470,7 +470,7 @@ static int instantiate_rng(uint8_t sec_idx)
 		if (rdsta_val & (1 << sh_idx))
 			continue;
 
-		inline_cnstr_jobdesc_rng_instantiation(desc, sh_idx);
+		inline_cnstr_jobdesc_rng_instantiation(desc, sh_idx, gen_sk);
 		size = roundup(sizeof(uint32_t) * 6, ARCH_DMA_MINALIGN);
 		flush_dcache_range((unsigned long)desc,
 				   (unsigned long)desc + size);
@@ -546,12 +546,13 @@ static void kick_trng(int ent_delay, uint8_t sec_idx)
 
 static int rng_init(uint8_t sec_idx)
 {
-	int ret, ent_delay = RTSDCTL_ENT_DLY_MIN;
+	int ret, gen_sk, ent_delay = RTSDCTL_ENT_DLY_MIN;
 	ccsr_sec_t __iomem *sec = (ccsr_sec_t __iomem *)SEC_ADDR(sec_idx);
 	struct rng4tst __iomem *rng =
 			(struct rng4tst __iomem *)&sec->rng;
 	u32 inst_handles;
 
+	gen_sk = !(sec_in32(&rng->rdsta) & RDSTA_SKVN);
 	do {
 		inst_handles = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
 
@@ -574,7 +575,7 @@ static int rng_init(uint8_t sec_idx)
 		 * interval, leading to a sucessful initialization of
 		 * the RNG.
 		 */
-		ret = instantiate_rng(sec_idx);
+		ret = instantiate_rng(sec_idx, gen_sk);
 	} while ((ret == -1) && (ent_delay < RTSDCTL_ENT_DLY_MAX));
 	if (ret) {
 		printf("SEC%u:  Failed to instantiate RNG\n", sec_idx);
diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index 8dce0bbb1b..64b8751f2d 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -69,6 +69,7 @@ struct rng4tst {
 #define RNG_STATE1_HANDLE_INSTANTIATED	0x00000002
 #define RNG_STATE_HANDLE_MASK	\
 	(RNG_STATE0_HANDLE_INSTANTIATED | RNG_STATE1_HANDLE_INSTANTIATED)
+#define RDSTA_SKVN 0x40000000
 	u32 rdsta;		/*RNG DRNG Status Register*/
 	u32 rsvd2[15];
 };
-- 
2.20.1

[PATCH v3 5/6] crypto/fsl: instantiate the RNG with prediciton resistance

[Michael Walle]

If it is already instantiated tear it down first and then reinstanciate
it again with prediction resistance.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/crypto/fsl/desc.h    |  2 ++
 drivers/crypto/fsl/jobdesc.c | 12 ++++++-
 drivers/crypto/fsl/jobdesc.h |  2 ++
 drivers/crypto/fsl/jr.c      | 67 ++++++++++++++++++++++++++++++++----
 include/fsl_sec.h            |  7 ++--
 5 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/fsl/desc.h b/drivers/crypto/fsl/desc.h
index 11ad506829..3589e6ea02 100644
--- a/drivers/crypto/fsl/desc.h
+++ b/drivers/crypto/fsl/desc.h
@@ -520,6 +520,8 @@
 #define OP_ALG_ICV_OFF		(0 << OP_ALG_ICV_SHIFT)
 #define OP_ALG_ICV_ON		(1 << OP_ALG_ICV_SHIFT)
 
+#define OP_ALG_PR_ON		0x02
+
 #define OP_ALG_DIR_SHIFT	0
 #define OP_ALG_DIR_MASK		1
 #define OP_ALG_DECRYPT		0
diff --git a/drivers/crypto/fsl/jobdesc.c b/drivers/crypto/fsl/jobdesc.c
index 6102e9c06b..d9554c550b 100644
--- a/drivers/crypto/fsl/jobdesc.c
+++ b/drivers/crypto/fsl/jobdesc.c
@@ -266,7 +266,8 @@ void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk)
 
 	/* INIT RNG in non-test mode */
 	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
-			(handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT);
+			 (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT |
+			 OP_ALG_PR_ON);
 
 	/* For SH0, Secure Keys must be generated as well */
 	if (!handle && do_sk) {
@@ -286,6 +287,15 @@ void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk)
 	}
 }
 
+/* Descriptor for deinstantiation of the RNG block. */
+void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle)
+{
+	init_job_desc(desc, 0);
+
+	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
+			 (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INITFINAL);
+}
+
 /* Change key size to bytes form bits in calling function*/
 void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
 				      struct pk_in_params *pkin, uint8_t *out,
diff --git a/drivers/crypto/fsl/jobdesc.h b/drivers/crypto/fsl/jobdesc.h
index 14b2a119d7..5185ddd535 100644
--- a/drivers/crypto/fsl/jobdesc.h
+++ b/drivers/crypto/fsl/jobdesc.h
@@ -41,6 +41,8 @@ void inline_cnstr_jobdesc_blob_decap(uint32_t *desc, uint8_t *key_idnfr,
 
 void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk);
 
+void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle);
+
 void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
 				      struct pk_in_params *pkin, uint8_t *out,
 				      uint32_t out_siz);
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index 42865a6cd7..00a3244b18 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -9,6 +9,7 @@
 #include <cpu_func.h>
 #include <log.h>
 #include <malloc.h>
+#include <memalign.h>
 #include "fsl_sec.h"
 #include "jr.h"
 #include "jobdesc.h"
@@ -446,6 +447,51 @@ int sec_reset(void)
 	return sec_reset_idx(0);
 }
 #ifndef CONFIG_SPL_BUILD
+static int deinstantiate_rng(u8 sec_idx, int state_handle_mask)
+{
+	u32 *desc;
+	int sh_idx, ret = 0;
+	int desc_size = sizeof(u32) * 2;
+
+	desc = malloc_cache_aligned(desc_size);
+	if (!desc) {
+		debug("cannot allocate RNG init descriptor memory\n");
+		return -ENOMEM;
+	}
+
+	for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
+		/*
+		 * If the corresponding bit is set, then it means the state
+		 * handle was initialized by us, and thus it needs to be
+		 * deinitialized as well
+		 */
+
+		if (state_handle_mask & RDSTA_IF(sh_idx)) {
+			/*
+			 * Create the descriptor for deinstantating this state
+			 * handle.
+			 */
+			inline_cnstr_jobdesc_rng_deinstantiation(desc, sh_idx);
+			flush_dcache_range((unsigned long)desc,
+					   (unsigned long)desc + desc_size);
+
+			ret = run_descriptor_jr_idx(desc, sec_idx);
+			if (ret) {
+				printf("SEC%u:  RNG4 SH%d deinstantiation failed with error 0x%x\n",
+				       sec_idx, sh_idx, ret);
+				ret = -EIO;
+				break;
+			}
+
+			printf("SEC%u:  Deinstantiated RNG4 SH%d\n",
+			       sec_idx, sh_idx);
+		}
+	}
+
+	free(desc);
+	return ret;
+}
+
 static int instantiate_rng(u8 sec_idx, int gen_sk)
 {
 	u32 *desc;
@@ -466,9 +512,18 @@ static int instantiate_rng(u8 sec_idx, int gen_sk)
 		 * If the corresponding bit is set, this state handle
 		 * was initialized by somebody else, so it's left alone.
 		 */
-		rdsta_val = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
-		if (rdsta_val & (1 << sh_idx))
-			continue;
+		rdsta_val = sec_in32(&rng->rdsta);
+		if (rdsta_val & (RDSTA_IF(sh_idx))) {
+			if (rdsta_val & RDSTA_PR(sh_idx))
+				continue;
+
+			printf("SEC%u:  RNG4 SH%d was instantiated w/o prediction resistance. Tearing it down\n",
+			       sec_idx, sh_idx);
+
+			ret = deinstantiate_rng(sec_idx, RDSTA_IF(sh_idx));
+			if (ret)
+				break;
+		}
 
 		inline_cnstr_jobdesc_rng_instantiation(desc, sh_idx, gen_sk);
 		size = roundup(sizeof(uint32_t) * 6, ARCH_DMA_MINALIGN);
@@ -481,8 +536,8 @@ static int instantiate_rng(u8 sec_idx, int gen_sk)
 			printf("SEC%u:  RNG4 SH%d instantiation failed with error 0x%x\n",
 			       sec_idx, sh_idx, ret);
 
-		rdsta_val = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
-		if (!(rdsta_val & (1 << sh_idx))) {
+		rdsta_val = sec_in32(&rng->rdsta);
+		if (!(rdsta_val & RDSTA_IF(sh_idx))) {
 			free(desc);
 			return -1;
 		}
@@ -554,7 +609,7 @@ static int rng_init(uint8_t sec_idx)
 
 	gen_sk = !(sec_in32(&rng->rdsta) & RDSTA_SKVN);
 	do {
-		inst_handles = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
+		inst_handles = sec_in32(&rng->rdsta) & RDSTA_MASK;
 
 		/*
 		 * If either of the SH's were instantiated by somebody else
diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index 64b8751f2d..1c6f1eb23e 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -65,10 +65,9 @@ struct rng4tst {
 		u32 rtfreqcnt;	/* PRGM=0: freq. count register */
 	};
 	u32 rsvd1[40];
-#define RNG_STATE0_HANDLE_INSTANTIATED	0x00000001
-#define RNG_STATE1_HANDLE_INSTANTIATED	0x00000002
-#define RNG_STATE_HANDLE_MASK	\
-	(RNG_STATE0_HANDLE_INSTANTIATED | RNG_STATE1_HANDLE_INSTANTIATED)
+#define RDSTA_IF(idx) (0x00000001 << (idx))
+#define RDSTA_PR(idx) (0x00000010 << (idx))
+#define RDSTA_MASK (RDSTA_PR(1) | RDSTA_PR(0) | RDSTA_IF(1) | RDSTA_IF(0))
 #define RDSTA_SKVN 0x40000000
 	u32 rdsta;		/*RNG DRNG Status Register*/
 	u32 rsvd2[15];
-- 
2.20.1

[PATCH v3 5/6] crypto/fsl: instantiate the RNG with prediciton resistance

[Heinrich Schuchardt]

On 6/25/20 2:19 PM, Michael Walle wrote:
> If it is already instantiated tear it down first and then reinstanciate
> it again with prediction resistance.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/crypto/fsl/desc.h    |  2 ++
>  drivers/crypto/fsl/jobdesc.c | 12 ++++++-
>  drivers/crypto/fsl/jobdesc.h |  2 ++
>  drivers/crypto/fsl/jr.c      | 67 ++++++++++++++++++++++++++++++++----
>  include/fsl_sec.h            |  7 ++--
>  5 files changed, 79 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/crypto/fsl/desc.h b/drivers/crypto/fsl/desc.h
> index 11ad506829..3589e6ea02 100644
> --- a/drivers/crypto/fsl/desc.h
> +++ b/drivers/crypto/fsl/desc.h
> @@ -520,6 +520,8 @@
>  #define OP_ALG_ICV_OFF		(0 << OP_ALG_ICV_SHIFT)
>  #define OP_ALG_ICV_ON		(1 << OP_ALG_ICV_SHIFT)
>
> +#define OP_ALG_PR_ON		0x02
> +
>  #define OP_ALG_DIR_SHIFT	0
>  #define OP_ALG_DIR_MASK		1
>  #define OP_ALG_DECRYPT		0
> diff --git a/drivers/crypto/fsl/jobdesc.c b/drivers/crypto/fsl/jobdesc.c
> index 6102e9c06b..d9554c550b 100644
> --- a/drivers/crypto/fsl/jobdesc.c
> +++ b/drivers/crypto/fsl/jobdesc.c
> @@ -266,7 +266,8 @@ void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk)
>
>  	/* INIT RNG in non-test mode */
>  	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
> -			(handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT);
> +			 (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT |
> +			 OP_ALG_PR_ON);
>
>  	/* For SH0, Secure Keys must be generated as well */
>  	if (!handle && do_sk) {
> @@ -286,6 +287,15 @@ void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk)
>  	}
>  }
>
> +/* Descriptor for deinstantiation of the RNG block. */
> +void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle)
> +{
> +	init_job_desc(desc, 0);
> +
> +	append_operation(desc, OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
> +			 (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INITFINAL);
> +}
> +
>  /* Change key size to bytes form bits in calling function*/
>  void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
>  				      struct pk_in_params *pkin, uint8_t *out,
> diff --git a/drivers/crypto/fsl/jobdesc.h b/drivers/crypto/fsl/jobdesc.h
> index 14b2a119d7..5185ddd535 100644
> --- a/drivers/crypto/fsl/jobdesc.h
> +++ b/drivers/crypto/fsl/jobdesc.h
> @@ -41,6 +41,8 @@ void inline_cnstr_jobdesc_blob_decap(uint32_t *desc, uint8_t *key_idnfr,
>
>  void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk);
>
> +void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle);
> +
>  void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
>  				      struct pk_in_params *pkin, uint8_t *out,
>  				      uint32_t out_siz);
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index 42865a6cd7..00a3244b18 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -9,6 +9,7 @@
>  #include <cpu_func.h>
>  #include <log.h>
>  #include <malloc.h>
> +#include <memalign.h>
>  #include "fsl_sec.h"
>  #include "jr.h"
>  #include "jobdesc.h"
> @@ -446,6 +447,51 @@ int sec_reset(void)
>  	return sec_reset_idx(0);
>  }
>  #ifndef CONFIG_SPL_BUILD
> +static int deinstantiate_rng(u8 sec_idx, int state_handle_mask)
> +{
> +	u32 *desc;
> +	int sh_idx, ret = 0;
> +	int desc_size = sizeof(u32) * 2;
> +
> +	desc = malloc_cache_aligned(desc_size);
> +	if (!desc) {
> +		debug("cannot allocate RNG init descriptor memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
> +		/*
> +		 * If the corresponding bit is set, then it means the state
> +		 * handle was initialized by us, and thus it needs to be
> +		 * deinitialized as well
> +		 */
> +
> +		if (state_handle_mask & RDSTA_IF(sh_idx)) {
> +			/*
> +			 * Create the descriptor for deinstantating this state
> +			 * handle.
> +			 */
> +			inline_cnstr_jobdesc_rng_deinstantiation(desc, sh_idx);
> +			flush_dcache_range((unsigned long)desc,
> +					   (unsigned long)desc + desc_size);

flush_dcache_range() tests that start and stop address are multiples of
CONFIG_SYS_CACHELINE_SIZE (= ARCH_DMA_MINALIGN on ARM). See
check_cache_range(start, stop).

So you should use ALIGN(desc_size, ARCH_DMA_MINALIGN) instead of desc_size.

Best regards

Heinrich

> +
> +			ret = run_descriptor_jr_idx(desc, sec_idx);
> +			if (ret) {
> +				printf("SEC%u:  RNG4 SH%d deinstantiation failed with error 0x%x\n",
> +				       sec_idx, sh_idx, ret);
> +				ret = -EIO;
> +				break;
> +			}
> +
> +			printf("SEC%u:  Deinstantiated RNG4 SH%d\n",
> +			       sec_idx, sh_idx);
> +		}
> +	}
> +
> +	free(desc);
> +	return ret;
> +}
> +
>  static int instantiate_rng(u8 sec_idx, int gen_sk)
>  {
>  	u32 *desc;
> @@ -466,9 +512,18 @@ static int instantiate_rng(u8 sec_idx, int gen_sk)
>  		 * If the corresponding bit is set, this state handle
>  		 * was initialized by somebody else, so it's left alone.
>  		 */
> -		rdsta_val = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
> -		if (rdsta_val & (1 << sh_idx))
> -			continue;
> +		rdsta_val = sec_in32(&rng->rdsta);
> +		if (rdsta_val & (RDSTA_IF(sh_idx))) {
> +			if (rdsta_val & RDSTA_PR(sh_idx))
> +				continue;
> +
> +			printf("SEC%u:  RNG4 SH%d was instantiated w/o prediction resistance. Tearing it down\n",
> +			       sec_idx, sh_idx);
> +
> +			ret = deinstantiate_rng(sec_idx, RDSTA_IF(sh_idx));
> +			if (ret)
> +				break;
> +		}
>
>  		inline_cnstr_jobdesc_rng_instantiation(desc, sh_idx, gen_sk);
>  		size = roundup(sizeof(uint32_t) * 6, ARCH_DMA_MINALIGN);
> @@ -481,8 +536,8 @@ static int instantiate_rng(u8 sec_idx, int gen_sk)
>  			printf("SEC%u:  RNG4 SH%d instantiation failed with error 0x%x\n",
>  			       sec_idx, sh_idx, ret);
>
> -		rdsta_val = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
> -		if (!(rdsta_val & (1 << sh_idx))) {
> +		rdsta_val = sec_in32(&rng->rdsta);
> +		if (!(rdsta_val & RDSTA_IF(sh_idx))) {
>  			free(desc);
>  			return -1;
>  		}
> @@ -554,7 +609,7 @@ static int rng_init(uint8_t sec_idx)
>
>  	gen_sk = !(sec_in32(&rng->rdsta) & RDSTA_SKVN);
>  	do {
> -		inst_handles = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
> +		inst_handles = sec_in32(&rng->rdsta) & RDSTA_MASK;
>
>  		/*
>  		 * If either of the SH's were instantiated by somebody else
> diff --git a/include/fsl_sec.h b/include/fsl_sec.h
> index 64b8751f2d..1c6f1eb23e 100644
> --- a/include/fsl_sec.h
> +++ b/include/fsl_sec.h
> @@ -65,10 +65,9 @@ struct rng4tst {
>  		u32 rtfreqcnt;	/* PRGM=0: freq. count register */
>  	};
>  	u32 rsvd1[40];
> -#define RNG_STATE0_HANDLE_INSTANTIATED	0x00000001
> -#define RNG_STATE1_HANDLE_INSTANTIATED	0x00000002
> -#define RNG_STATE_HANDLE_MASK	\
> -	(RNG_STATE0_HANDLE_INSTANTIATED | RNG_STATE1_HANDLE_INSTANTIATED)
> +#define RDSTA_IF(idx) (0x00000001 << (idx))
> +#define RDSTA_PR(idx) (0x00000010 << (idx))
> +#define RDSTA_MASK (RDSTA_PR(1) | RDSTA_PR(0) | RDSTA_IF(1) | RDSTA_IF(0))
>  #define RDSTA_SKVN 0x40000000
>  	u32 rdsta;		/*RNG DRNG Status Register*/
>  	u32 rsvd2[15];
>

[PATCH v3 6/6] crypto/fsl: add RNG support

[Michael Walle]

Register the random number generator with the rng subsystem in u-boot.
This way it can be used by EFI as well as for the 'rng' command.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/crypto/fsl/Kconfig   | 14 ++++++
 drivers/crypto/fsl/Makefile  |  1 +
 drivers/crypto/fsl/jobdesc.c | 10 +++++
 drivers/crypto/fsl/jobdesc.h |  3 ++
 drivers/crypto/fsl/jr.c      |  9 ++++
 drivers/crypto/fsl/rng.c     | 86 ++++++++++++++++++++++++++++++++++++
 6 files changed, 123 insertions(+)
 create mode 100644 drivers/crypto/fsl/rng.c

diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
index 181a1e5e99..5ed6140da3 100644
--- a/drivers/crypto/fsl/Kconfig
+++ b/drivers/crypto/fsl/Kconfig
@@ -45,3 +45,17 @@ config SYS_FSL_SEC_COMPAT
 
 config SYS_FSL_SEC_LE
 	bool "Little-endian access to Freescale Secure Boot"
+
+if FSL_CAAM
+
+config FSL_CAAM_RNG
+	bool "Enable Random Number Generator support"
+	depends on DM_RNG
+	default y
+	help
+	  Enable support for the hardware based random number generator
+	  module of the CAAM. The random data is fetched from the DRGB
+	  using the prediction resistance flag which means the DRGB is
+	  reseeded from the TRNG every time random data is generated.
+
+endif
diff --git a/drivers/crypto/fsl/Makefile b/drivers/crypto/fsl/Makefile
index cfb36f3bb9..a5e8d38e38 100644
--- a/drivers/crypto/fsl/Makefile
+++ b/drivers/crypto/fsl/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_FSL_CAAM) += jr.o fsl_hash.o jobdesc.o error.o
 obj-$(CONFIG_CMD_BLOB) += fsl_blob.o
 obj-$(CONFIG_CMD_DEKBLOB) += fsl_blob.o
 obj-$(CONFIG_RSA_FREESCALE_EXP) += fsl_rsa.o
+obj-$(CONFIG_FSL_CAAM_RNG) += rng.o
diff --git a/drivers/crypto/fsl/jobdesc.c b/drivers/crypto/fsl/jobdesc.c
index d9554c550b..decde64078 100644
--- a/drivers/crypto/fsl/jobdesc.c
+++ b/drivers/crypto/fsl/jobdesc.c
@@ -296,6 +296,16 @@ void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle)
 			 (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INITFINAL);
 }
 
+void inline_cnstr_jobdesc_rng(u32 *desc, void *data_out, u32 size)
+{
+	dma_addr_t dma_data_out = virt_to_phys(data_out);
+
+	init_job_desc(desc, 0);
+	append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
+			 OP_ALG_PR_ON);
+	append_fifo_store(desc, dma_data_out, size, FIFOST_TYPE_RNGSTORE);
+}
+
 /* Change key size to bytes form bits in calling function*/
 void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
 				      struct pk_in_params *pkin, uint8_t *out,
diff --git a/drivers/crypto/fsl/jobdesc.h b/drivers/crypto/fsl/jobdesc.h
index 5185ddd535..c4501abd26 100644
--- a/drivers/crypto/fsl/jobdesc.h
+++ b/drivers/crypto/fsl/jobdesc.h
@@ -43,7 +43,10 @@ void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk);
 
 void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle);
 
+void inline_cnstr_jobdesc_rng(u32 *desc, void *data_out, u32 size);
+
 void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
 				      struct pk_in_params *pkin, uint8_t *out,
 				      uint32_t out_siz);
+
 #endif
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index 00a3244b18..9ada1d625b 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -20,6 +20,7 @@
 #include <asm/cache.h>
 #include <asm/fsl_pamu.h>
 #endif
+#include <dm/lists.h>
 
 #define CIRC_CNT(head, tail, size)	(((head) - (tail)) & (size - 1))
 #define CIRC_SPACE(head, tail, size)	CIRC_CNT((tail), (head) + 1, (size))
@@ -721,6 +722,14 @@ int sec_init_idx(uint8_t sec_idx)
 			printf("SEC%u:  RNG instantiation failed\n", sec_idx);
 			return -1;
 		}
+
+		if (IS_ENABLED(CONFIG_DM_RNG)) {
+			ret = device_bind_driver(NULL, "caam-rng", "caam-rng",
+						 NULL);
+			if (ret)
+				printf("Couldn't bind rng driver (%d)\n", ret);
+		}
+
 		printf("SEC%u:  RNG instantiated\n", sec_idx);
 	}
 #endif
diff --git a/drivers/crypto/fsl/rng.c b/drivers/crypto/fsl/rng.c
new file mode 100644
index 0000000000..136b2cdcd7
--- /dev/null
+++ b/drivers/crypto/fsl/rng.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Michael Walle <michael@walle.cc>
+ *
+ * Driver for Freescale Cryptographic Accelerator and Assurance
+ * Module (CAAM) hardware random number generator.
+ */
+
+#include <asm/cache.h>
+#include <common.h>
+#include <cpu_func.h>
+#include <dm.h>
+#include <rng.h>
+#include "desc_constr.h"
+#include "jobdesc.h"
+#include "jr.h"
+
+#define CAAM_RNG_MAX_FIFO_STORE_SIZE 16
+#define CAAM_RNG_DESC_LEN (3 * CAAM_CMD_SZ + CAAM_PTR_SZ)
+
+struct caam_rng_platdata {
+	u32 desc[CAAM_RNG_DESC_LEN / 4];
+	u8 data[CAAM_RNG_MAX_FIFO_STORE_SIZE] __aligned(ARCH_DMA_MINALIGN);
+};
+
+static int caam_rng_read_one(struct caam_rng_platdata *pdata)
+{
+	int size = CAAM_RNG_MAX_FIFO_STORE_SIZE;
+	int ret;
+
+	ret = run_descriptor_jr(pdata->desc);
+	if (ret < 0)
+		return -EIO;
+
+	invalidate_dcache_range((unsigned long)pdata->data,
+				(unsigned long)pdata->data + size);
+
+	return 0;
+}
+
+static int caam_rng_read(struct udevice *dev, void *data, size_t len)
+{
+	struct caam_rng_platdata *pdata = dev_get_platdata(dev);
+	u8 *buffer = data;
+	size_t size;
+	int ret;
+
+	while (len) {
+		ret = caam_rng_read_one(pdata);
+		if (ret)
+			return ret;
+
+		size = min(len, (size_t)CAAM_RNG_MAX_FIFO_STORE_SIZE);
+
+		memcpy(buffer, pdata->data, size);
+		buffer += size;
+		len -= size;
+	}
+
+	return 0;
+}
+
+static int caam_rng_probe(struct udevice *dev)
+{
+	struct caam_rng_platdata *pdata = dev_get_platdata(dev);
+
+	inline_cnstr_jobdesc_rng(pdata->desc, pdata->data,
+				 CAAM_RNG_MAX_FIFO_STORE_SIZE);
+	flush_dcache_range((unsigned long)pdata->desc,
+			   (unsigned long)pdata->desc + CAAM_RNG_DESC_LEN);
+
+	return 0;
+}
+
+static const struct dm_rng_ops caam_rng_ops = {
+	.read = caam_rng_read,
+};
+
+U_BOOT_DRIVER(caam_rng) = {
+	.name = "caam-rng",
+	.id = UCLASS_RNG,
+	.ops = &caam_rng_ops,
+	.probe = caam_rng_probe,
+	.platdata_auto_alloc_size = sizeof(struct caam_rng_platdata),
+	.flags = DM_FLAG_ALLOC_PRIV_DMA,
+};
-- 
2.20.1

[PATCH v3 6/6] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 25.06.20 14:19, Michael Walle wrote:
> Register the random number generator with the rng subsystem in u-boot.
> This way it can be used by EFI as well as for the 'rng' command.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/crypto/fsl/Kconfig   | 14 ++++++
>  drivers/crypto/fsl/Makefile  |  1 +
>  drivers/crypto/fsl/jobdesc.c | 10 +++++
>  drivers/crypto/fsl/jobdesc.h |  3 ++
>  drivers/crypto/fsl/jr.c      |  9 ++++
>  drivers/crypto/fsl/rng.c     | 86 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 123 insertions(+)
>  create mode 100644 drivers/crypto/fsl/rng.c
>
> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
> index 181a1e5e99..5ed6140da3 100644
> --- a/drivers/crypto/fsl/Kconfig
> +++ b/drivers/crypto/fsl/Kconfig
> @@ -45,3 +45,17 @@ config SYS_FSL_SEC_COMPAT
>
>  config SYS_FSL_SEC_LE
>  	bool "Little-endian access to Freescale Secure Boot"
> +
> +if FSL_CAAM
> +
> +config FSL_CAAM_RNG
> +	bool "Enable Random Number Generator support"
> +	depends on DM_RNG
> +	default y
> +	help
> +	  Enable support for the hardware based random number generator
> +	  module of the CAAM. The random data is fetched from the DRGB
> +	  using the prediction resistance flag which means the DRGB is
> +	  reseeded from the TRNG every time random data is generated.
> +
> +endif

Should FSL_CAAM imply DM_RNG? This way you would not have to update the
defconfigs of the NXP boards.

For new files there should be a maintainer. It would be great if
somebody at NXP could take charge of the drivers/crypto/fsl directory.

Best regards

Heinrich

[PATCH v3 6/6] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 6/25/20 2:19 PM, Michael Walle wrote:
> Register the random number generator with the rng subsystem in u-boot.
> This way it can be used by EFI as well as for the 'rng' command.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/crypto/fsl/Kconfig   | 14 ++++++
>  drivers/crypto/fsl/Makefile  |  1 +
>  drivers/crypto/fsl/jobdesc.c | 10 +++++
>  drivers/crypto/fsl/jobdesc.h |  3 ++
>  drivers/crypto/fsl/jr.c      |  9 ++++
>  drivers/crypto/fsl/rng.c     | 86 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 123 insertions(+)
>  create mode 100644 drivers/crypto/fsl/rng.c
>
> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
> index 181a1e5e99..5ed6140da3 100644
> --- a/drivers/crypto/fsl/Kconfig
> +++ b/drivers/crypto/fsl/Kconfig
> @@ -45,3 +45,17 @@ config SYS_FSL_SEC_COMPAT
>
>  config SYS_FSL_SEC_LE
>  	bool "Little-endian access to Freescale Secure Boot"
> +
> +if FSL_CAAM
> +
> +config FSL_CAAM_RNG
> +	bool "Enable Random Number Generator support"
> +	depends on DM_RNG
> +	default y
> +	help
> +	  Enable support for the hardware based random number generator
> +	  module of the CAAM. The random data is fetched from the DRGB
> +	  using the prediction resistance flag which means the DRGB is
> +	  reseeded from the TRNG every time random data is generated.
> +
> +endif
> diff --git a/drivers/crypto/fsl/Makefile b/drivers/crypto/fsl/Makefile
> index cfb36f3bb9..a5e8d38e38 100644
> --- a/drivers/crypto/fsl/Makefile
> +++ b/drivers/crypto/fsl/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_FSL_CAAM) += jr.o fsl_hash.o jobdesc.o error.o
>  obj-$(CONFIG_CMD_BLOB) += fsl_blob.o
>  obj-$(CONFIG_CMD_DEKBLOB) += fsl_blob.o
>  obj-$(CONFIG_RSA_FREESCALE_EXP) += fsl_rsa.o
> +obj-$(CONFIG_FSL_CAAM_RNG) += rng.o
> diff --git a/drivers/crypto/fsl/jobdesc.c b/drivers/crypto/fsl/jobdesc.c
> index d9554c550b..decde64078 100644
> --- a/drivers/crypto/fsl/jobdesc.c
> +++ b/drivers/crypto/fsl/jobdesc.c
> @@ -296,6 +296,16 @@ void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle)
>  			 (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INITFINAL);
>  }
>
> +void inline_cnstr_jobdesc_rng(u32 *desc, void *data_out, u32 size)
> +{
> +	dma_addr_t dma_data_out = virt_to_phys(data_out);
> +
> +	init_job_desc(desc, 0);
> +	append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
> +			 OP_ALG_PR_ON);
> +	append_fifo_store(desc, dma_data_out, size, FIFOST_TYPE_RNGSTORE);
> +}
> +
>  /* Change key size to bytes form bits in calling function*/
>  void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
>  				      struct pk_in_params *pkin, uint8_t *out,
> diff --git a/drivers/crypto/fsl/jobdesc.h b/drivers/crypto/fsl/jobdesc.h
> index 5185ddd535..c4501abd26 100644
> --- a/drivers/crypto/fsl/jobdesc.h
> +++ b/drivers/crypto/fsl/jobdesc.h
> @@ -43,7 +43,10 @@ void inline_cnstr_jobdesc_rng_instantiation(u32 *desc, int handle, int do_sk);
>
>  void inline_cnstr_jobdesc_rng_deinstantiation(u32 *desc, int handle);
>
> +void inline_cnstr_jobdesc_rng(u32 *desc, void *data_out, u32 size);
> +
>  void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
>  				      struct pk_in_params *pkin, uint8_t *out,
>  				      uint32_t out_siz);
> +
>  #endif
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index 00a3244b18..9ada1d625b 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -20,6 +20,7 @@
>  #include <asm/cache.h>
>  #include <asm/fsl_pamu.h>
>  #endif
> +#include <dm/lists.h>
>
>  #define CIRC_CNT(head, tail, size)	(((head) - (tail)) & (size - 1))
>  #define CIRC_SPACE(head, tail, size)	CIRC_CNT((tail), (head) + 1, (size))
> @@ -721,6 +722,14 @@ int sec_init_idx(uint8_t sec_idx)
>  			printf("SEC%u:  RNG instantiation failed\n", sec_idx);
>  			return -1;
>  		}
> +
> +		if (IS_ENABLED(CONFIG_DM_RNG)) {
> +			ret = device_bind_driver(NULL, "caam-rng", "caam-rng",
> +						 NULL);
> +			if (ret)
> +				printf("Couldn't bind rng driver (%d)\n", ret);
> +		}
> +
>  		printf("SEC%u:  RNG instantiated\n", sec_idx);
>  	}
>  #endif
> diff --git a/drivers/crypto/fsl/rng.c b/drivers/crypto/fsl/rng.c
> new file mode 100644
> index 0000000000..136b2cdcd7
> --- /dev/null
> +++ b/drivers/crypto/fsl/rng.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Michael Walle <michael@walle.cc>
> + *
> + * Driver for Freescale Cryptographic Accelerator and Assurance
> + * Module (CAAM) hardware random number generator.
> + */
> +
> +#include <asm/cache.h>
> +#include <common.h>
> +#include <cpu_func.h>
> +#include <dm.h>
> +#include <rng.h>
> +#include "desc_constr.h"
> +#include "jobdesc.h"
> +#include "jr.h"
> +
> +#define CAAM_RNG_MAX_FIFO_STORE_SIZE 16
> +#define CAAM_RNG_DESC_LEN (3 * CAAM_CMD_SZ + CAAM_PTR_SZ)
> +
> +struct caam_rng_platdata {
> +	u32 desc[CAAM_RNG_DESC_LEN / 4];
> +	u8 data[CAAM_RNG_MAX_FIFO_STORE_SIZE] __aligned(ARCH_DMA_MINALIGN);
> +};
> +
> +static int caam_rng_read_one(struct caam_rng_platdata *pdata)
> +{
> +	int size = CAAM_RNG_MAX_FIFO_STORE_SIZE;

This should be

int size = ALIGN(CAAM_RNG_MAX_FIFO_STORE_SIZE, ARCH_DMA_MINALIGN);



> +	int ret;
> +
> +	ret = run_descriptor_jr(pdata->desc);
> +	if (ret < 0)
> +		return -EIO;
> +
> +	invalidate_dcache_range((unsigned long)pdata->data,
> +				(unsigned long)pdata->data + size);

This fails on the i.MX6 Wandboard with

CACHE: Misaligned operation at range [8e596f68, 8e596f78]

because pdata is not ARCH_DMA_MINALIGN aligned.

> +
> +	return 0;
> +}
> +
> +static int caam_rng_read(struct udevice *dev, void *data, size_t len)
> +{
> +	struct caam_rng_platdata *pdata = dev_get_platdata(dev);
> +	u8 *buffer = data;
> +	size_t size;
> +	int ret;
> +
> +	while (len) {
> +		ret = caam_rng_read_one(pdata);
> +		if (ret)
> +			return ret;
> +
> +		size = min(len, (size_t)CAAM_RNG_MAX_FIFO_STORE_SIZE);
> +
> +		memcpy(buffer, pdata->data, size);
> +		buffer += size;
> +		len -= size;
> +	}
> +
> +	return 0;
> +}
> +
> +static int caam_rng_probe(struct udevice *dev)
> +{
> +	struct caam_rng_platdata *pdata = dev_get_platdata(dev);
> +
> +	inline_cnstr_jobdesc_rng(pdata->desc, pdata->data,
> +				 CAAM_RNG_MAX_FIFO_STORE_SIZE);
> +	flush_dcache_range((unsigned long)pdata->desc,
> +			   (unsigned long)pdata->desc + CAAM_RNG_DESC_LEN);

You should round CAAM_RNG_DESC_LEN up to a multiple of ARCH_DMA_MINALIGN.

> +
> +	return 0;
> +}
> +
> +static const struct dm_rng_ops caam_rng_ops = {
> +	.read = caam_rng_read,
> +};
> +
> +U_BOOT_DRIVER(caam_rng) = {
> +	.name = "caam-rng",
> +	.id = UCLASS_RNG,
> +	.ops = &caam_rng_ops,
> +	.probe = caam_rng_probe,
> +	.platdata_auto_alloc_size = sizeof(struct caam_rng_platdata),
> +	.flags = DM_FLAG_ALLOC_PRIV_DMA,

This flag is for private data. It does not provide aligned platform data.

So you should use:

.priv_auto_alloc_size = sizeof(struct caam_rng_platdata),

and

struct caam_rng_priv *priv = dev_get_priv(dev)

Best regards

Heinrich

> +};
>

[PATCH v3 0/6] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 25.06.20 14:18, Michael Walle wrote:
> First, improve the compatibility on newer Era CAAMs. These introduced new
> version registers. Secondly, add RNG support for the CAAM. This way we get
> random number generator support for EFI for free and KASLR will work with
> ARM64 kernels booted with bootefi.
>

It seems that a Kconfig dependency at least on CONFIG_SYS_FSL_HAS_SEC
which itself depends on CONFIG_IMX_HAB is missing:

wandboard_defconfig + FSL_CAAM + DM_RNG gives me a bunch of errors

drivers/crypto/fsl/jr.c: In function ?start_jr0?:
drivers/crypto/fsl/jr.c:47:2: error: unknown type name ?ccsr_sec_t?; did
you mean ?pci_dev_t??
  ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
  ^~~~~~~~~~
  pci_dev_t
In file included from ./arch/arm/include/asm/byteorder.h:29,
                 from include/linux/libfdt_env.h:15,
                 from include/linux/libfdt.h:6,
                 from include/fdtdec.h:17,
                 from include/asm-generic/global_data.h:23,
                 from ./arch/arm/include/asm/global_data.h:87,
                 from include/common.h:26,
                 from drivers/crypto/fsl/jr.c:8:
drivers/crypto/fsl/jr.c:48:29: error: request for member ?ctpr_ms? in
something not a structure or union
  u32 ctpr_ms = sec_in32(&sec->ctpr_ms);
                             ^~

But if I enable IMX_HAB booting fails with: "hab fuse not enabled".

Why should I need to enable the HAB fuse to use the random number
generator on the i.MX6?

Best regards

Heinrich

[PATCH v3 0/6] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 25.06.20 16:36, Heinrich Schuchardt wrote:
> On 25.06.20 14:18, Michael Walle wrote:
>> First, improve the compatibility on newer Era CAAMs. These introduced new
>> version registers. Secondly, add RNG support for the CAAM. This way we get
>> random number generator support for EFI for free and KASLR will work with
>> ARM64 kernels booted with bootefi.
>>
>
> It seems that a Kconfig dependency at least on CONFIG_SYS_FSL_HAS_SEC
> which itself depends on CONFIG_IMX_HAB is missing:
>
> wandboard_defconfig + FSL_CAAM + DM_RNG gives me a bunch of errors
>
> drivers/crypto/fsl/jr.c: In function ?start_jr0?:
> drivers/crypto/fsl/jr.c:47:2: error: unknown type name ?ccsr_sec_t?; did
> you mean ?pci_dev_t??
>   ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
>   ^~~~~~~~~~
>   pci_dev_t
> In file included from ./arch/arm/include/asm/byteorder.h:29,
>                  from include/linux/libfdt_env.h:15,
>                  from include/linux/libfdt.h:6,
>                  from include/fdtdec.h:17,
>                  from include/asm-generic/global_data.h:23,
>                  from ./arch/arm/include/asm/global_data.h:87,
>                  from include/common.h:26,
>                  from drivers/crypto/fsl/jr.c:8:
> drivers/crypto/fsl/jr.c:48:29: error: request for member ?ctpr_ms? in
> something not a structure or union
>   u32 ctpr_ms = sec_in32(&sec->ctpr_ms);
>                              ^~
>
> But if I enable IMX_HAB booting fails with: "hab fuse not enabled".
>
> Why should I need to enable the HAB fuse to use the random number
> generator on the i.MX6?
>

With this change I can build the RNG driver for the i.MX6 Wandboard:

diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
index 5ed6140da3..84783ea987 100644
--- a/drivers/crypto/fsl/Kconfig
+++ b/drivers/crypto/fsl/Kconfig
@@ -37,7 +37,6 @@ config SYS_FSL_SEC_BE

 config SYS_FSL_SEC_COMPAT
        int "Freescale Secure Boot compatibility"
-       depends on SYS_FSL_HAS_SEC
        default 2 if SYS_FSL_SEC_COMPAT_2
        default 4 if SYS_FSL_SEC_COMPAT_4
        default 5 if SYS_FSL_SEC_COMPAT_5

Even if you do not plan to support the i.MX6, I would recommend this
change to separate HAB and RNG.

With the following additional change the RNG driver is loaded on the
Wandboard:

diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
index 19ca382649..e129286065 100644
--- a/arch/arm/mach-imx/mx6/soc.c
+++ b/arch/arm/mach-imx/mx6/soc.c
@@ -22,6 +22,7 @@
 #include <asm/arch/mxc_hdmi.h>
 #include <asm/arch/crm_regs.h>
 #include <dm.h>
+#include <fsl_sec.h>
 #include <imx_thermal.h>
 #include <mmc.h>

@@ -691,6 +692,15 @@ void imx_setup_hdmi(void)
 }
 #endif

+#ifdef CONFIG_ARCH_MISC_INIT
+int arch_misc_init(void)
+{
+#ifdef CONFIG_FSL_CAAM
+       sec_init();
+#endif
+       return 0;
+}
+#endif

 /*
  * gpr_init() function is common for boards using MX6S, MX6DL, MX6D,


But the RNG driver seems to require some changes to work on the i.MX6:

=> rng
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
ERROR: v7_outer_cache_inval_range - start address is not aligned -
0x8e596f68
ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x8e596f78
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
ERROR: v7_outer_cache_inval_range - start address is not aligned -
0x8e596f68
ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x8e596f78
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
ERROR: v7_outer_cache_inval_range - start address is not aligned -
0x8e596f68
ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x8e596f78
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
CACHE: Misaligned operation at range [8e596f68, 8e596f78]
ERROR: v7_outer_cache_inval_range - start address is not aligned -
0x8e596f68
ERROR: v7_outer_cache_inval_range - stop address is not aligned - 0x8e596f78
00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
=>

Best regards

Heinrich

[PATCH v3 0/6] crypto/fsl: add RNG support

[Michael Walle]

Am 2020-06-25 18:03, schrieb Heinrich Schuchardt:
> On 25.06.20 16:36, Heinrich Schuchardt wrote:
>> On 25.06.20 14:18, Michael Walle wrote:
>>> First, improve the compatibility on newer Era CAAMs. These introduced 
>>> new
>>> version registers. Secondly, add RNG support for the CAAM. This way 
>>> we get
>>> random number generator support for EFI for free and KASLR will work 
>>> with
>>> ARM64 kernels booted with bootefi.
>>> 
>> 
>> It seems that a Kconfig dependency at least on CONFIG_SYS_FSL_HAS_SEC
>> which itself depends on CONFIG_IMX_HAB is missing:
>> 
>> wandboard_defconfig + FSL_CAAM + DM_RNG gives me a bunch of errors
>> 
>> drivers/crypto/fsl/jr.c: In function ?start_jr0?:
>> drivers/crypto/fsl/jr.c:47:2: error: unknown type name ?ccsr_sec_t?; 
>> did
>> you mean ?pci_dev_t??
>>   ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
>>   ^~~~~~~~~~
>>   pci_dev_t
>> In file included from ./arch/arm/include/asm/byteorder.h:29,
>>                  from include/linux/libfdt_env.h:15,
>>                  from include/linux/libfdt.h:6,
>>                  from include/fdtdec.h:17,
>>                  from include/asm-generic/global_data.h:23,
>>                  from ./arch/arm/include/asm/global_data.h:87,
>>                  from include/common.h:26,
>>                  from drivers/crypto/fsl/jr.c:8:
>> drivers/crypto/fsl/jr.c:48:29: error: request for member ?ctpr_ms? in
>> something not a structure or union
>>   u32 ctpr_ms = sec_in32(&sec->ctpr_ms);
>>                              ^~
>> 
>> But if I enable IMX_HAB booting fails with: "hab fuse not enabled".
>> 
>> Why should I need to enable the HAB fuse to use the random number
>> generator on the i.MX6?
>> 
> 
> With this change I can build the RNG driver for the i.MX6 Wandboard:
> 
> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
> index 5ed6140da3..84783ea987 100644
> --- a/drivers/crypto/fsl/Kconfig
> +++ b/drivers/crypto/fsl/Kconfig
> @@ -37,7 +37,6 @@ config SYS_FSL_SEC_BE
> 
>  config SYS_FSL_SEC_COMPAT
>         int "Freescale Secure Boot compatibility"
> -       depends on SYS_FSL_HAS_SEC
>         default 2 if SYS_FSL_SEC_COMPAT_2
>         default 4 if SYS_FSL_SEC_COMPAT_4
>         default 5 if SYS_FSL_SEC_COMPAT_5
> 
> Even if you do not plan to support the i.MX6, I would recommend this
> change to separate HAB and RNG.

I don't think this is the correct place. Rather the architecture should
set SYS_FSL_HAS_SEC if it actually has the SEC. I mean it already sets
the COMPAT level but not the actual config which indicates it has one.
At the moment it depends on IMX_HAB; I don't know the reasoning behind
this. But I don't see how this would be part of this series.

> With the following additional change the RNG driver is loaded on the
> Wandboard:
> 
> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> index 19ca382649..e129286065 100644
> --- a/arch/arm/mach-imx/mx6/soc.c
> +++ b/arch/arm/mach-imx/mx6/soc.c
> @@ -22,6 +22,7 @@
>  #include <asm/arch/mxc_hdmi.h>
>  #include <asm/arch/crm_regs.h>
>  #include <dm.h>
> +#include <fsl_sec.h>
>  #include <imx_thermal.h>
>  #include <mmc.h>
> 
> @@ -691,6 +692,15 @@ void imx_setup_hdmi(void)
>  }
>  #endif
> 
> +#ifdef CONFIG_ARCH_MISC_INIT
> +int arch_misc_init(void)
> +{
> +#ifdef CONFIG_FSL_CAAM
> +       sec_init();
> +#endif
> +       return 0;
> +}
> +#endif
> 
>  /*
>   * gpr_init() function is common for boards using MX6S, MX6DL, MX6D,
> 
> 
> But the RNG driver seems to require some changes to work on the i.MX6:
> 
> => rng
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> ERROR: v7_outer_cache_inval_range - start address is not aligned -
> 0x8e596f68
> ERROR: v7_outer_cache_inval_range - stop address is not aligned - 
> 0x8e596f78
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> ERROR: v7_outer_cache_inval_range - start address is not aligned -
> 0x8e596f68
> ERROR: v7_outer_cache_inval_range - stop address is not aligned - 
> 0x8e596f78
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> ERROR: v7_outer_cache_inval_range - start address is not aligned -
> 0x8e596f68
> ERROR: v7_outer_cache_inval_range - stop address is not aligned - 
> 0x8e596f78
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
> ERROR: v7_outer_cache_inval_range - start address is not aligned -
> 0x8e596f68
> ERROR: v7_outer_cache_inval_range - stop address is not aligned - 
> 0x8e596f78
> 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
> ................
> 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
> ................
> 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
> ................
> 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
> ................
> =>

Could you please trace where this happens? The start address should
be aligned, the end is likely not aligned. I presumed it is part of
the dcache code to take care of the rounding. Of course I can
do the rounding before the invalidation/flushing. It seems to be
another discepancy between the architectures. Still doesn't explain
why the start address is unaligned.

-michael

[PATCH v3 0/6] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 6/25/20 11:01 PM, Michael Walle wrote:
> Am 2020-06-25 18:03, schrieb Heinrich Schuchardt:
>> On 25.06.20 16:36, Heinrich Schuchardt wrote:
>>> On 25.06.20 14:18, Michael Walle wrote:
>>>> First, improve the compatibility on newer Era CAAMs. These
>>>> introduced new
>>>> version registers. Secondly, add RNG support for the CAAM. This way
>>>> we get
>>>> random number generator support for EFI for free and KASLR will work
>>>> with
>>>> ARM64 kernels booted with bootefi.
>>>>
>>>
>>> It seems that a Kconfig dependency at least on CONFIG_SYS_FSL_HAS_SEC
>>> which itself depends on CONFIG_IMX_HAB is missing:
>>>
>>> wandboard_defconfig + FSL_CAAM + DM_RNG gives me a bunch of errors
>>>
>>> drivers/crypto/fsl/jr.c: In function ?start_jr0?:
>>> drivers/crypto/fsl/jr.c:47:2: error: unknown type name ?ccsr_sec_t?; did
>>> you mean ?pci_dev_t??
>>> ? ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
>>> ? ^~~~~~~~~~
>>> ? pci_dev_t
>>> In file included from ./arch/arm/include/asm/byteorder.h:29,
>>> ???????????????? from include/linux/libfdt_env.h:15,
>>> ???????????????? from include/linux/libfdt.h:6,
>>> ???????????????? from include/fdtdec.h:17,
>>> ???????????????? from include/asm-generic/global_data.h:23,
>>> ???????????????? from ./arch/arm/include/asm/global_data.h:87,
>>> ???????????????? from include/common.h:26,
>>> ???????????????? from drivers/crypto/fsl/jr.c:8:
>>> drivers/crypto/fsl/jr.c:48:29: error: request for member ?ctpr_ms? in
>>> something not a structure or union
>>> ? u32 ctpr_ms = sec_in32(&sec->ctpr_ms);
>>> ???????????????????????????? ^~
>>>
>>> But if I enable IMX_HAB booting fails with: "hab fuse not enabled".
>>>
>>> Why should I need to enable the HAB fuse to use the random number
>>> generator on the i.MX6?
>>>
>>
>> With this change I can build the RNG driver for the i.MX6 Wandboard:
>>
>> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
>> index 5ed6140da3..84783ea987 100644
>> --- a/drivers/crypto/fsl/Kconfig
>> +++ b/drivers/crypto/fsl/Kconfig
>> @@ -37,7 +37,6 @@ config SYS_FSL_SEC_BE
>>
>> ?config SYS_FSL_SEC_COMPAT
>> ??????? int "Freescale Secure Boot compatibility"
>> -?????? depends on SYS_FSL_HAS_SEC
>> ??????? default 2 if SYS_FSL_SEC_COMPAT_2
>> ??????? default 4 if SYS_FSL_SEC_COMPAT_4
>> ??????? default 5 if SYS_FSL_SEC_COMPAT_5
>>
>> Even if you do not plan to support the i.MX6, I would recommend this
>> change to separate HAB and RNG.
>
> I don't think this is the correct place. Rather the architecture should
> set SYS_FSL_HAS_SEC if it actually has the SEC. I mean it already sets
> the COMPAT level but not the actual config which indicates it has one.
> At the moment it depends on IMX_HAB; I don't know the reasoning behind
> this. But I don't see how this would be part of this series.

ARCH_MX7 (arch/arm/Kconfig) has:
select SYS_FSL_HAS_SEC if IMX_HAB

So according to your suggestion this should be changed to

select SYS_FSL_HAS_SEC ?

And the same added to ARCH_MX6?

Best regards

Heinrich

>
>> With the following additional change the RNG driver is loaded on the
>> Wandboard:
>>
>> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
>> index 19ca382649..e129286065 100644
>> --- a/arch/arm/mach-imx/mx6/soc.c
>> +++ b/arch/arm/mach-imx/mx6/soc.c
>> @@ -22,6 +22,7 @@
>> ?#include <asm/arch/mxc_hdmi.h>
>> ?#include <asm/arch/crm_regs.h>
>> ?#include <dm.h>
>> +#include <fsl_sec.h>
>> ?#include <imx_thermal.h>
>> ?#include <mmc.h>
>>
>> @@ -691,6 +692,15 @@ void imx_setup_hdmi(void)
>> ?}
>> ?#endif
>>
>> +#ifdef CONFIG_ARCH_MISC_INIT
>> +int arch_misc_init(void)
>> +{
>> +#ifdef CONFIG_FSL_CAAM
>> +?????? sec_init();
>> +#endif
>> +?????? return 0;
>> +}
>> +#endif
>>
>> ?/*
>> ? * gpr_init() function is common for boards using MX6S, MX6DL, MX6D,
>>
>>
>> But the RNG driver seems to require some changes to work on the i.MX6:
>>
>> => rng
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> ERROR: v7_outer_cache_inval_range - start address is not aligned -
>> 0x8e596f68
>> ERROR: v7_outer_cache_inval_range - stop address is not aligned -
>> 0x8e596f78
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> ERROR: v7_outer_cache_inval_range - start address is not aligned -
>> 0x8e596f68
>> ERROR: v7_outer_cache_inval_range - stop address is not aligned -
>> 0x8e596f78
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> ERROR: v7_outer_cache_inval_range - start address is not aligned -
>> 0x8e596f68
>> ERROR: v7_outer_cache_inval_range - stop address is not aligned -
>> 0x8e596f78
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> CACHE: Misaligned operation at range [8e596f68, 8e596f78]
>> ERROR: v7_outer_cache_inval_range - start address is not aligned -
>> 0x8e596f68
>> ERROR: v7_outer_cache_inval_range - stop address is not aligned -
>> 0x8e596f78
>> 00000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00?
>> ................
>> 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00?
>> ................
>> 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00?
>> ................
>> 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00?
>> ................
>> =>
>
> Could you please trace where this happens? The start address should
> be aligned, the end is likely not aligned. I presumed it is part of
> the dcache code to take care of the rounding. Of course I can
> do the rounding before the invalidation/flushing. It seems to be
> another discepancy between the architectures. Still doesn't explain
> why the start address is unaligned.
>
> -michael

[PATCH v3 0/6] crypto/fsl: add RNG support

[Michael Walle]

Am 2020-06-26 18:26, schrieb Heinrich Schuchardt:
> On 6/25/20 11:01 PM, Michael Walle wrote:
>> Am 2020-06-25 18:03, schrieb Heinrich Schuchardt:
>>> On 25.06.20 16:36, Heinrich Schuchardt wrote:
>>>> On 25.06.20 14:18, Michael Walle wrote:
>>>>> First, improve the compatibility on newer Era CAAMs. These
>>>>> introduced new
>>>>> version registers. Secondly, add RNG support for the CAAM. This way
>>>>> we get
>>>>> random number generator support for EFI for free and KASLR will 
>>>>> work
>>>>> with
>>>>> ARM64 kernels booted with bootefi.
>>>>> 
>>>> 
>>>> It seems that a Kconfig dependency at least on 
>>>> CONFIG_SYS_FSL_HAS_SEC
>>>> which itself depends on CONFIG_IMX_HAB is missing:
>>>> 
>>>> wandboard_defconfig + FSL_CAAM + DM_RNG gives me a bunch of errors
>>>> 
>>>> drivers/crypto/fsl/jr.c: In function ?start_jr0?:
>>>> drivers/crypto/fsl/jr.c:47:2: error: unknown type name ?ccsr_sec_t?; 
>>>> did
>>>> you mean ?pci_dev_t??
>>>> ? ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
>>>> ? ^~~~~~~~~~
>>>> ? pci_dev_t
>>>> In file included from ./arch/arm/include/asm/byteorder.h:29,
>>>> ???????????????? from include/linux/libfdt_env.h:15,
>>>> ???????????????? from include/linux/libfdt.h:6,
>>>> ???????????????? from include/fdtdec.h:17,
>>>> ???????????????? from include/asm-generic/global_data.h:23,
>>>> ???????????????? from ./arch/arm/include/asm/global_data.h:87,
>>>> ???????????????? from include/common.h:26,
>>>> ???????????????? from drivers/crypto/fsl/jr.c:8:
>>>> drivers/crypto/fsl/jr.c:48:29: error: request for member ?ctpr_ms? 
>>>> in
>>>> something not a structure or union
>>>> ? u32 ctpr_ms = sec_in32(&sec->ctpr_ms);
>>>> ???????????????????????????? ^~
>>>> 
>>>> But if I enable IMX_HAB booting fails with: "hab fuse not enabled".
>>>> 
>>>> Why should I need to enable the HAB fuse to use the random number
>>>> generator on the i.MX6?
>>>> 
>>> 
>>> With this change I can build the RNG driver for the i.MX6 Wandboard:
>>> 
>>> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
>>> index 5ed6140da3..84783ea987 100644
>>> --- a/drivers/crypto/fsl/Kconfig
>>> +++ b/drivers/crypto/fsl/Kconfig
>>> @@ -37,7 +37,6 @@ config SYS_FSL_SEC_BE
>>> 
>>> ?config SYS_FSL_SEC_COMPAT
>>> ??????? int "Freescale Secure Boot compatibility"
>>> -?????? depends on SYS_FSL_HAS_SEC
>>> ??????? default 2 if SYS_FSL_SEC_COMPAT_2
>>> ??????? default 4 if SYS_FSL_SEC_COMPAT_4
>>> ??????? default 5 if SYS_FSL_SEC_COMPAT_5
>>> 
>>> Even if you do not plan to support the i.MX6, I would recommend this
>>> change to separate HAB and RNG.
>> 
>> I don't think this is the correct place. Rather the architecture 
>> should
>> set SYS_FSL_HAS_SEC if it actually has the SEC. I mean it already sets
>> the COMPAT level but not the actual config which indicates it has one.
>> At the moment it depends on IMX_HAB; I don't know the reasoning behind
>> this. But I don't see how this would be part of this series.
> 
> ARCH_MX7 (arch/arm/Kconfig) has:
> select SYS_FSL_HAS_SEC if IMX_HAB
> 
> So according to your suggestion this should be changed to
> 
> select SYS_FSL_HAS_SEC ?
> And the same added to ARCH_MX6?

yes, because HAS_SEC is a hardware feature, why should that be dependant
on a feature which is selected by the user? But I don't know if there 
are
any side effects.

Also I don't know if the SEC is available in all SoC of the imx6/7 
series.

-michael

[PATCH v3 0/6] crypto/fsl: add RNG support

[Michael Walle]

Am 2020-06-26 18:26, schrieb Heinrich Schuchardt:
> On 6/25/20 11:01 PM, Michael Walle wrote:
>> Am 2020-06-25 18:03, schrieb Heinrich Schuchardt:
>>> On 25.06.20 16:36, Heinrich Schuchardt wrote:
>>>> On 25.06.20 14:18, Michael Walle wrote:
>>>>> First, improve the compatibility on newer Era CAAMs. These
>>>>> introduced new
>>>>> version registers. Secondly, add RNG support for the CAAM. This way
>>>>> we get
>>>>> random number generator support for EFI for free and KASLR will 
>>>>> work
>>>>> with
>>>>> ARM64 kernels booted with bootefi.
>>>>> 
>>>> 
>>>> It seems that a Kconfig dependency at least on 
>>>> CONFIG_SYS_FSL_HAS_SEC
>>>> which itself depends on CONFIG_IMX_HAB is missing:
>>>> 
>>>> wandboard_defconfig + FSL_CAAM + DM_RNG gives me a bunch of errors
>>>> 
>>>> drivers/crypto/fsl/jr.c: In function ?start_jr0?:
>>>> drivers/crypto/fsl/jr.c:47:2: error: unknown type name ?ccsr_sec_t?; 
>>>> did
>>>> you mean ?pci_dev_t??
>>>> ? ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
>>>> ? ^~~~~~~~~~
>>>> ? pci_dev_t
>>>> In file included from ./arch/arm/include/asm/byteorder.h:29,
>>>> ???????????????? from include/linux/libfdt_env.h:15,
>>>> ???????????????? from include/linux/libfdt.h:6,
>>>> ???????????????? from include/fdtdec.h:17,
>>>> ???????????????? from include/asm-generic/global_data.h:23,
>>>> ???????????????? from ./arch/arm/include/asm/global_data.h:87,
>>>> ???????????????? from include/common.h:26,
>>>> ???????????????? from drivers/crypto/fsl/jr.c:8:
>>>> drivers/crypto/fsl/jr.c:48:29: error: request for member ?ctpr_ms? 
>>>> in
>>>> something not a structure or union
>>>> ? u32 ctpr_ms = sec_in32(&sec->ctpr_ms);
>>>> ???????????????????????????? ^~
>>>> 
>>>> But if I enable IMX_HAB booting fails with: "hab fuse not enabled".
>>>> 
>>>> Why should I need to enable the HAB fuse to use the random number
>>>> generator on the i.MX6?
>>>> 
>>> 
>>> With this change I can build the RNG driver for the i.MX6 Wandboard:
>>> 
>>> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
>>> index 5ed6140da3..84783ea987 100644
>>> --- a/drivers/crypto/fsl/Kconfig
>>> +++ b/drivers/crypto/fsl/Kconfig
>>> @@ -37,7 +37,6 @@ config SYS_FSL_SEC_BE
>>> 
>>> ?config SYS_FSL_SEC_COMPAT
>>> ??????? int "Freescale Secure Boot compatibility"
>>> -?????? depends on SYS_FSL_HAS_SEC
>>> ??????? default 2 if SYS_FSL_SEC_COMPAT_2
>>> ??????? default 4 if SYS_FSL_SEC_COMPAT_4
>>> ??????? default 5 if SYS_FSL_SEC_COMPAT_5
>>> 
>>> Even if you do not plan to support the i.MX6, I would recommend this
>>> change to separate HAB and RNG.
>> 
>> I don't think this is the correct place. Rather the architecture 
>> should
>> set SYS_FSL_HAS_SEC if it actually has the SEC. I mean it already sets
>> the COMPAT level but not the actual config which indicates it has one.
>> At the moment it depends on IMX_HAB; I don't know the reasoning behind
>> this. But I don't see how this would be part of this series.
> 
> ARCH_MX7 (arch/arm/Kconfig) has:
> select SYS_FSL_HAS_SEC if IMX_HAB
> 
> So according to your suggestion this should be changed to
> 
> select SYS_FSL_HAS_SEC ?
> 
> And the same added to ARCH_MX6?
> 

Also I don't know why the SYS_FSL_SEC_COMPAT is user selectable:

--- a/drivers/crypto/fsl/Kconfig
+++ b/drivers/crypto/fsl/Kconfig
@@ -36,7 +36,7 @@ config SYS_FSL_SEC_BE
         bool "Big-endian access to Freescale Secure Boot"

  config SYS_FSL_SEC_COMPAT
-       int "Freescale Secure Boot compatibility"
+       int
         depends on SYS_FSL_HAS_SEC
         default 2 if SYS_FSL_SEC_COMPAT_2
         default 4 if SYS_FSL_SEC_COMPAT_4

-michael