[v5] crypto: caam - fix i.MX6SX entropy delay value

Commit Message

Fabio Estevam April 20, 2022, 12:06 p.m. UTC

From: Fabio Estevam <[e-mail]>

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

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/[e-mail]/

As explained in the U-Boot patch:

"RNG self tests are run to determine the correct entropy delay.
Such tests are executed with different voltages and temperatures to identify
the worst case value for the entropy delay. For i.MX6SX, it was determined
that after adding a margin value of 1000 the minimum entropy delay should be
at least 12000."

Cc: <[e-mail]>
Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG") 
Signed-off-by: Fabio Estevam <[e-mail]>
Reviewed-by: Horia Geantă <[e-mail]>
---
Changes since v4:
- Change the function name to needs_entropy_delay_adjustment() - Vabhav
- Improve the commit log by adding the explanation from the U-Boot
patch - Vabhav

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

Comments

Vabhav Sharma April 21, 2022, 12:51 p.m. UTC | #1

Reviewed-by: Vabhav Sharma <[e-mail]>

> -----Original Message-----
> From: Fabio Estevam <[e-mail]>
> Sent: Wednesday, April 20, 2022 5:36 PM
> To: [e-mail]
> Cc: Horia Geanta <[e-mail]>; Gaurav Jain
> <[e-mail]>; Varun Sethi <[e-mail]>; linux-
> [e-mail]; Fabio Estevam <[e-mail]>;
> [e-mail]
> Subject: [PATCH v5] crypto: caam - fix i.MX6SX entropy delay value
> 
> From: Fabio Estevam <[e-mail]>
> 
> 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
> 
> 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%2Fpatch
> work.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F20220415111049.2565744-
> 1-
> gaurav.jain%40nxp.com%2F&amp;data=05%7C01%7Cmeenakshi.aggarwal%4
> 0nxp.com%7Caf57d0186dde479aa9cf08da22c687d0%7C686ea1d3bc2b4c6fa92
> cd99c5c301635%7C0%7C0%7C637860533324307730%7CUnknown%7CTWFpb
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
> 6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=UhqjgESpgMhOhJS%2BT4ghI6y
> NIvyybOI8yEv5%2FjKNcDE%3D&amp;reserved=0
> 
> As explained in the U-Boot patch:
> 
> "RNG self tests are run to determine the correct entropy delay.
> Such tests are executed with different voltages and temperatures to identify
> the worst case value for the entropy delay. For i.MX6SX, it was determined
> that after adding a margin value of 1000 the minimum entropy delay should
> be at least 12000."
> 
> Cc: <[e-mail]>
> Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG")
> Signed-off-by: Fabio Estevam <[e-mail]>
> Reviewed-by: Horia Geantă <[e-mail]>
> ---
> Changes since v4:
> - Change the function name to needs_entropy_delay_adjustment() -
> Vabhav
> - Improve the commit log by adding the explanation from the U-Boot patch -
> Vabhav
> 
>  drivers/crypto/caam/ctrl.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> ca0361b2dbb0..f87aa2169e5f 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -609,6 +609,13 @@ static bool check_version(struct fsl_mc_version
> *mc_version, u32 major,  }  #endif
> 
> +static bool needs_entropy_delay_adjustment(void)
> +{
> +	if (of_machine_is_compatible("fsl,imx6sx"))
> +		return true;
> +	return false;
> +}
> +
>  /* Probe routine for CAAM top (controller) level */  static int
> caam_probe(struct platform_device *pdev)  { @@ -855,6 +862,8 @@ static
> int caam_probe(struct platform_device *pdev)
>  			 * Also, if a handle was instantiated, do not change
>  			 * the TRNG parameters.
>  			 */
> +			if (needs_entropy_delay_adjustment())
> +				ent_delay = 12000;
>  			if (!(ctrlpriv->rng4_sh_init || inst_handles)) {
>  				dev_info(dev,
>  					 "Entropy delay = %u\n",
> @@ -871,6 +880,15 @@ static int caam_probe(struct platform_device *pdev)
>  			 */
>  			ret = instantiate_rng(dev, inst_handles,
>  					      gen_sk);
> +			/*
> +			 * Entropy delay is determined via TRNG
> characterization.
> +			 * TRNG characterization 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 (needs_entropy_delay_adjustment())
> +				break;
>  			if (ret == -EAGAIN)
>  				/*
>  				 * if here, the loop will rerun,
> --
> 2.25.1

Gaurav Jain April 27, 2022, 6:05 a.m. UTC | #2

Reviewed-by: Gaurav Jain <[e-mail]>

> -----Original Message-----
> From: Fabio Estevam <[e-mail]>
> Sent: Wednesday, April 20, 2022 5:36 PM
> To: [e-mail]
> Cc: Horia Geanta <[e-mail]>; Gaurav Jain
> <[e-mail]>; Varun Sethi <[e-mail]>; linux-
> [e-mail]; Fabio Estevam <[e-mail]>;
> [e-mail]
> Subject: [EXT] [PATCH v5] crypto: caam - fix i.MX6SX entropy delay value
> 
> Caution: EXT Email
> 
> From: Fabio Estevam <[e-mail]>
> 
> 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
> 
> 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=05%7C01%7Cgaurav.jain%40nxp.com%
> 7Cc2f2316c0db64170b59e08da22c62c4d%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C0%7C637860531804367821%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> 3000%7C%7C%7C&amp;sdata=OTgUAquesUW39%2F0bhSnXHDe4UpCU8dQN%
> 2B7P0hlE6oiE%3D&amp;reserved=0
> 
> As explained in the U-Boot patch:
> 
> "RNG self tests are run to determine the correct entropy delay.
> Such tests are executed with different voltages and temperatures to identify the
> worst case value for the entropy delay. For i.MX6SX, it was determined that
> after adding a margin value of 1000 the minimum entropy delay should be at
> least 12000."
> 
> Cc: <[e-mail]>
> Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG")
> Signed-off-by: Fabio Estevam <[e-mail]>
> Reviewed-by: Horia Geantă <[e-mail]>
> ---
> Changes since v4:
> - Change the function name to needs_entropy_delay_adjustment() - Vabhav
> - Improve the commit log by adding the explanation from the U-Boot patch -
> Vabhav
> 
>  drivers/crypto/caam/ctrl.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index
> ca0361b2dbb0..f87aa2169e5f 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -609,6 +609,13 @@ static bool check_version(struct fsl_mc_version
> *mc_version, u32 major,  }  #endif
> 
> +static bool needs_entropy_delay_adjustment(void)
> +{
> +       if (of_machine_is_compatible("fsl,imx6sx"))
> +               return true;
> +       return false;
> +}
> +
>  /* Probe routine for CAAM top (controller) level */  static int caam_probe(struct
> platform_device *pdev)  { @@ -855,6 +862,8 @@ static int caam_probe(struct
> platform_device *pdev)
>                          * Also, if a handle was instantiated, do not change
>                          * the TRNG parameters.
>                          */
> +                       if (needs_entropy_delay_adjustment())
> +                               ent_delay = 12000;
>                         if (!(ctrlpriv->rng4_sh_init || inst_handles)) {
>                                 dev_info(dev,
>                                          "Entropy delay = %u\n", @@ -871,6 +880,15 @@ static
> int caam_probe(struct platform_device *pdev)
>                          */
>                         ret = instantiate_rng(dev, inst_handles,
>                                               gen_sk);
> +                       /*
> +                        * Entropy delay is determined via TRNG characterization.
> +                        * TRNG characterization 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 (needs_entropy_delay_adjustment())
> +                               break;
>                         if (ret == -EAGAIN)
>                                 /*
>                                  * if here, the loop will rerun,
> --
> 2.25.1

Herbert Xu April 29, 2022, 5:50 a.m. UTC | #3

On Wed, Apr 20, 2022 at 09:06:01AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <[e-mail]>
> 
> 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
> 
> 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/[e-mail]/
> 
> As explained in the U-Boot patch:
> 
> "RNG self tests are run to determine the correct entropy delay.
> Such tests are executed with different voltages and temperatures to identify
> the worst case value for the entropy delay. For i.MX6SX, it was determined
> that after adding a margin value of 1000 the minimum entropy delay should be
> at least 12000."
> 
> Cc: <[e-mail]>
> Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG") 
> Signed-off-by: Fabio Estevam <[e-mail]>
> Reviewed-by: Horia Geantă <[e-mail]>
> ---
> Changes since v4:
> - Change the function name to needs_entropy_delay_adjustment() - Vabhav
> - Improve the commit log by adding the explanation from the U-Boot
> patch - Vabhav
> 
>  drivers/crypto/caam/ctrl.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

Patch applied.  Thanks.

Benjamin Bara June 9, 2022, 1:45 p.m. UTC | #4

Hi!

I found an older discussion on the same topic,
where it is mentioned that the same problem also occurs on imx6dl, imx6d and imx6s:

https://linuxlists.cc/l/4/linux-crypto/t/3843436/caam_rng_trouble#post4282922

I have also observed the problem on an imx6dl, running a 5.10.112 kernel.

Therefore, the problem might persist for the other variants.

BR
BB

Patch

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..f87aa2169e5f 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -609,6 +609,13 @@  static bool check_version(struct fsl_mc_version *mc_version, u32 major,
 }
 #endif
 
+static bool needs_entropy_delay_adjustment(void)
+{
+	if (of_machine_is_compatible("fsl,imx6sx"))
+		return true;
+	return false;
+}
+
 /* Probe routine for CAAM top (controller) level */
 static int caam_probe(struct platform_device *pdev)
 {
@@ -855,6 +862,8 @@  static int caam_probe(struct platform_device *pdev)
 			 * Also, if a handle was instantiated, do not change
 			 * the TRNG parameters.
 			 */
+			if (needs_entropy_delay_adjustment())
+				ent_delay = 12000;
 			if (!(ctrlpriv->rng4_sh_init || inst_handles)) {
 				dev_info(dev,
 					 "Entropy delay = %u\n",
@@ -871,6 +880,15 @@  static int caam_probe(struct platform_device *pdev)
 			 */
 			ret = instantiate_rng(dev, inst_handles,
 					      gen_sk);
+			/*
+			 * Entropy delay is determined via TRNG characterization.
+			 * TRNG characterization 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 (needs_entropy_delay_adjustment())
+				break;
 			if (ret == -EAGAIN)
 				/*
 				 * if here, the loop will rerun,