diff mbox series

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

Message ID 20220419114444.586113-1-festevam@gmail.com (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series [v4] crypto: caam - fix i.MX6SX entropy delay value | expand

Commit Message

Fabio Estevam April 19, 2022, 11:44 a.m. UTC
From: Fabio Estevam <festevam@denx.de>

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/20220415111049.2565744-1-gaurav.jain@nxp.com/

Cc: <stable@vger.kernel.org>
Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG") 
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v3:
- Add a needs_extended_entropy_delay() function
- Assign the 'ent_delay' inside the the RNG initialization area. (Horia)
- Change the terminology as per Horia's explanation.

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

Comments

Horia Geanta April 19, 2022, 8:38 p.m. UTC | #1
On 4/19/2022 2:45 PM, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> 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/20220415111049.2565744-1-gaurav.jain@nxp.com/
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG") 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

Thanks,
Horia
Vabhav Sharma April 20, 2022, 11:34 a.m. UTC | #2
> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Tuesday, April 19, 2022 5:15 PM
> To: herbert@gondor.apana.org.au
> Cc: Horia Geanta <horia.geanta@nxp.com>; Gaurav Jain
> <gaurav.jain@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; linux-
> crypto@vger.kernel.org; Fabio Estevam <festevam@denx.de>;
> stable@vger.kernel.org
> Subject: [EXT] [PATCH v4] crypto: caam - fix i.MX6SX entropy delay value
> 
> Caution: EXT Email
> 
> From: Fabio Estevam <festevam@denx.de>
> 
> 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%7Cgaurav.jain%40nxp.co
> m%7C9016fe4b6bb8402de08f08da21fa0703%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635%7C0%7C0%7C637859655100732358%7CUnknown%7CTWFpbGZsb3d8
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C3000%7C%7C%7C&amp;sdata=wglpvZBmQUKn7sLRx5xpdwdgFMOc6LE
> qIMqUU0h4UZE%3D&amp;reserved=0
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 358ba762d9f1 ("crypto: caam - enable prediction resistance in HRWNG")
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
> Changes since v3:
> - Add a needs_extended_entropy_delay() function
> - Assign the 'ent_delay' inside the the RNG initialization area. (Horia)
> - Change the terminology as per Horia's explanation.
> 
>  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..5b989a842ee9 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_extended_entropy_delay(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_extended_entropy_delay())
> +                               ent_delay = 12000;
>                         if (!(ctrlpriv->rng4_sh_init || inst_handles)) {
Please note if TRNG state handles are already instantiated e.g. by U-Boot bootloader, It is assumed that entropy is set correctly and kick_trng() will be skipped
Kindly pick the U-Boot changes as detailed in commit message.
>                                 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_extended_entropy_delay())
There is nothing like extended entropy delay and function name seems to be incorrect, The software test were run offline across ranges of temperature, voltage and process to determine the correct entropy delay.
However, value of 12000 is determined by running said test across voltage and temperature only for iMX6SX.
> +                               break;
>                         if (ret == -EAGAIN)
>                                 /*
>                                  * if here, the loop will rerun,
> --
> 2.25.1
Fabio Estevam April 20, 2022, 12:08 p.m. UTC | #3
Hi Vabhav,

On Wed, Apr 20, 2022 at 8:34 AM Vabhav Sharma <vabhav.sharma@nxp.com> wrote:

> There is nothing like extended entropy delay and function name seems to be incorrect, The software test were run offline across ranges of temperature, voltage and process to determine the correct entropy delay.
> However, value of 12000 is determined by running said test across voltage and temperature only for iMX6SX.

Ok, I did as you suggested and sent a v5.

Just realized that I forgot to add you on Cc. Sorry about that.

Thanks
diff mbox series

Patch

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..5b989a842ee9 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_extended_entropy_delay(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_extended_entropy_delay())
+				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_extended_entropy_delay())
+				break;
 			if (ret == -EAGAIN)
 				/*
 				 * if here, the loop will rerun,