2022-04-16 15:22:11

by Fabio Estevam

[permalink] [raw]
Subject: [PATCH v2] crypto: caam - fix i.MX6SX entropy delay value

From: Fabio Estevam <[email protected]>

Since commit 358ba762d9f1 ("crypto: caam - enable prediction resistance
in HRWNG") the following CAAM errors can be seen on i.MX6SX:

caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
hwrng: no data available
caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
...

This error is due to an incorrect entropy delay for i.MX6SX.

Fix it by increasing the minimum entropy delay for i.MX6SX
as done in U-Boot:
https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/

Signed-off-by: Fabio Estevam <[email protected]>
---
Change since v1:
- Align the fix with U-Boot.

drivers/crypto/caam/ctrl.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..c515c20442d5 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -648,6 +648,8 @@ static int caam_probe(struct platform_device *pdev)
return ret;
}

+ if (of_machine_is_compatible("fsl,imx6sx"))
+ ent_delay = 12000;

/* Get configuration properties from device tree */
/* First, get register page */
@@ -871,6 +873,15 @@ static int caam_probe(struct platform_device *pdev)
*/
ret = instantiate_rng(dev, inst_handles,
gen_sk);
+ /*
+ * Entropy delay is calculated via self-test method.
+ * Self-test is run across different voltages and
+ * temperatures.
+ * If worst case value for ent_dly is identified,
+ * the loop can be skipped for that platform.
+ */
+ if (of_machine_is_compatible("fsl,imx6sx"))
+ break;
if (ret == -EAGAIN)
/*
* if here, the loop will rerun,
--
2.25.1


2022-04-16 21:14:09

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: caam - fix i.MX6SX entropy delay value

Hi Horia and Varun,

On Sat, Apr 16, 2022 at 10:54 AM Fabio Estevam <[email protected]> wrote:
>
> From: Fabio Estevam <[email protected]>
>
> Since commit 358ba762d9f1 ("crypto: caam - enable prediction resistance
> in HRWNG") the following CAAM errors can be seen on i.MX6SX:
>
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
> hwrng: no data available
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
> ...
>
> This error is due to an incorrect entropy delay for i.MX6SX.
>
> Fix it by increasing the minimum entropy delay for i.MX6SX
> as done in U-Boot:
> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
>
> Signed-off-by: Fabio Estevam <[email protected]>
> ---
> Change since v1:
> - Align the fix with U-Boot.

Actually, after thinking more about it, I realize that this issue is
not i.MX6SX specific as
I have seen reports of the same failures on i.MX6D as well.

Would it make sense to fix it like this instead?

--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -516,7 +516,7 @@ struct rng4tst {
};
#define RTSDCTL_ENT_DLY_SHIFT 16
#define RTSDCTL_ENT_DLY_MASK (0xffff << RTSDCTL_ENT_DLY_SHIFT)
-#define RTSDCTL_ENT_DLY_MIN 3200
+#define RTSDCTL_ENT_DLY_MIN 12000
#define RTSDCTL_ENT_DLY_MAX 12800
u32 rtsdctl; /* seed control register */
union {

Any drawbacks in using this generic approach?

Please advise.

2022-04-18 06:24:12

by Varun Sethi

[permalink] [raw]
Subject: RE: [EXT] [PATCH v2] crypto: caam - fix i.MX6SX entropy delay value

Hi Fabio,
Vabhav is working on a fix for the Linux driver. He would be introducing a new property in the CAAM device tree node, which would be used for specifying the entropy delay value. This would make the solution generic. This property is optional.
Also, please note that the entropy delay value has been derived for i.MX6SX after a silicon characterization study.


Regards
Varun

> -----Original Message-----
> From: Fabio Estevam <[email protected]>
> Sent: Saturday, April 16, 2022 7:24 PM
> To: [email protected]
> Cc: Horia Geanta <[email protected]>; Gaurav Jain
> <[email protected]>; Varun Sethi <[email protected]>; linux-
> [email protected]; Fabio Estevam <[email protected]>
> Subject: [EXT] [PATCH v2] crypto: caam - fix i.MX6SX entropy delay value
>
> Caution: EXT Email
>
> From: Fabio Estevam <[email protected]>
>
> Since commit 358ba762d9f1 ("crypto: caam - enable prediction resistance in
> HRWNG") the following CAAM errors can be seen on i.MX6SX:
>
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
> hwrng: no data available
> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error ...
>
> This error is due to an incorrect entropy delay for i.MX6SX.
>
> Fix it by increasing the minimum entropy delay for i.MX6SX as done in U-Boot:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwo
> rk.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F20220415111049.2565744-1-
> gaurav.jain%40nxp.com%2F&amp;data=04%7C01%7CV.Sethi%40nxp.com%7C7
> 9659c64ea334807ca2c08da1fb0b1b3%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C637857141005203180%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000
> &amp;sdata=%2BpR3ecPQNZ001zRGWe%2FWo%2FayfltO6zYK1p%2BwQ57c6J
> U%3D&amp;reserved=0
>
> Signed-off-by: Fabio Estevam <[email protected]>
> ---
> Change since v1:
> - Align the fix with U-Boot.
>
> drivers/crypto/caam/ctrl.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> ca0361b2dbb0..c515c20442d5 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -648,6 +648,8 @@ static int caam_probe(struct platform_device *pdev)
> return ret;
> }
>
> + if (of_machine_is_compatible("fsl,imx6sx"))
> + ent_delay = 12000;
>
> /* Get configuration properties from device tree */
> /* First, get register page */
> @@ -871,6 +873,15 @@ static int caam_probe(struct platform_device *pdev)
> */
> ret = instantiate_rng(dev, inst_handles,
> gen_sk);
> + /*
> + * Entropy delay is calculated via self-test method.
> + * Self-test is run across different voltages and
> + * temperatures.
> + * If worst case value for ent_dly is identified,
> + * the loop can be skipped for that platform.
> + */
> + if (of_machine_is_compatible("fsl,imx6sx"))
> + break;
> if (ret == -EAGAIN)
> /*
> * if here, the loop will rerun,
> --
> 2.25.1

2022-04-19 08:54:53

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: caam - fix i.MX6SX entropy delay value

On Mon, Apr 18, 2022 at 11:43 AM Horia Geantă <[email protected]> wrote:

> Someone will need to check whether this solves the issue on i.MX6D,
> the root cause might be different.
>
> Besides this, as Varun said, we should check with the HW team,
> such that the value used for entropy delay is optimal.
> IOW we need TRNG characterization for i.MX6D.

Let's focus on the i.MX6SX for now then.

Please check v3 of the patch that I sent today.

2022-04-19 16:21:48

by Horia Geanta

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: caam - fix i.MX6SX entropy delay value

On 4/16/2022 5:34 PM, Fabio Estevam wrote:
> Hi Horia and Varun,
>
> On Sat, Apr 16, 2022 at 10:54 AM Fabio Estevam <[email protected]> wrote:
>>
>> From: Fabio Estevam <[email protected]>
>>
>> Since commit 358ba762d9f1 ("crypto: caam - enable prediction resistance
>> in HRWNG") the following CAAM errors can be seen on i.MX6SX:
>>
>> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
>> hwrng: no data available
>> caam_jr 2101000.jr: 20003c5b: CCB: desc idx 60: RNG: Hardware error
>> ...
>>
>> This error is due to an incorrect entropy delay for i.MX6SX.
>>
>> Fix it by increasing the minimum entropy delay for i.MX6SX
>> as done in U-Boot:
>> https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/
>>
>> Signed-off-by: Fabio Estevam <[email protected]>
>> ---
>> Change since v1:
>> - Align the fix with U-Boot.
>
> Actually, after thinking more about it, I realize that this issue is
> not i.MX6SX specific as
> I have seen reports of the same failures on i.MX6D as well.
>
Someone will need to check whether this solves the issue on i.MX6D,
the root cause might be different.

Besides this, as Varun said, we should check with the HW team,
such that the value used for entropy delay is optimal.
IOW we need TRNG characterization for i.MX6D.

> Would it make sense to fix it like this instead?
>
> --- a/drivers/crypto/caam/regs.h
> +++ b/drivers/crypto/caam/regs.h
> @@ -516,7 +516,7 @@ struct rng4tst {
> };
> #define RTSDCTL_ENT_DLY_SHIFT 16
> #define RTSDCTL_ENT_DLY_MASK (0xffff << RTSDCTL_ENT_DLY_SHIFT)
> -#define RTSDCTL_ENT_DLY_MIN 3200
> +#define RTSDCTL_ENT_DLY_MIN 12000
> #define RTSDCTL_ENT_DLY_MAX 12800
> u32 rtsdctl; /* seed control register */
> union {
>
> Any drawbacks in using this generic approach?
>
One drawback is performance, not sure if there are others.

Horia