Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Gaurav Jain <gaurav.jain@nxp.com>
To:     Horia Geanta <horia.geanta@nxp.com>,
        Pankaj Gupta <pankaj.gupta@nxp.com>,
        "\"Herbert Xu\"" <herbert@gondor.apana.org.au>,
        =?iso-8859-2?Q?=22Michal_Vok=E1=E8=22?= <michal.vokac@ysoft.com>
CC:     "\"David S. Miller\"" <davem@davemloft.net>,
        "\"Petr Benes\"" <petr.benes@ysoft.com>,
        "petrben@gmail.com" <petrben@gmail.com>,
        "l.stach@pengutronix.de" <l.stach@pengutronix.de>,
        "robert.hancock@calian.com" <robert.hancock@calian.com>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Varun Sethi <V.Sethi@nxp.com>,
        Vabhav Sharma <vabhav.sharma@nxp.com>
Subject: RE: [EXT] [RFC PATCH] crypto: caam - restore retry count after HW RNG
 failure
Thread-Topic: [EXT] [RFC PATCH] crypto: caam - restore retry count after HW
 RNG failure
Thread-Index: AQHX19neHowqaTNf00Kheozf5e35Yqxg5qfggAAANIA=
Date:   Thu, 13 Jan 2022 07:23:04 +0000
Message-ID: <VI1PR04MB5342237C91A2ADF239B05711E7539@VI1PR04MB5342.eurprd04.prod.outlook.com>
References: <ea3906acc2c8d6fb56eacd94b9531c18fa7cb4dc.camel@calian.com>
 <20211112152705.90513-1-michal.vokac@ysoft.com>
 <DU2PR04MB863088F218774551FA254EB795539@DU2PR04MB8630.eurprd04.prod.outlook.com>
In-Reply-To: <DU2PR04MB863088F218774551FA254EB795539@DU2PR04MB8630.eurprd04.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-2?Q?c76tuF1It9T95CK+gIEs/r2zCTjH6HFWqnc+a1P26h8rE+7v0IUKK3LbZi?=
 =?iso-8859-2?Q?HM7lKnWUDhGpp3+k/c2SdxSoDccHNWmHGkRmiAyitZWc4FlVr4qhZsk3iF?=
 =?iso-8859-2?Q?PBvkAieETCCCny4kDNLIvNAN8BFC8nYXt66Cljq3opUDUXeH+/P4CRma4V?=
 =?iso-8859-2?Q?sz9nHQgg4MYiQCY3/x+UH8CVf0vlc1+mIFitP5ShYU5qbJoz3gmMTxHWnh?=
 =?iso-8859-2?Q?mBSNhWs1bk7z4JV0P7ldAd3/AyPdBeXRirTWSlsCu907O3obYvTSV9sAri?=
 =?iso-8859-2?Q?vt9aPmSPiNrE/qN+F1CWFMTVzZnf0boNmH9M4ZdIsSP5gdhw/bhRH0bnXI?=
 =?iso-8859-2?Q?qwlZVksKs8gttbXXx56hoWa56rQwB0cu7g00QQKQTo3A2QZo3zo5yYbv8X?=
 =?iso-8859-2?Q?g+z4NFMjwUKeGG5tv9Jl61Hjhoz96XWCY6boDMGVGdvKuT2kCg1IwlSVKO?=
 =?iso-8859-2?Q?geRN+de1Ih5+CANz0l5OKGAFbDjg6yYkNJ3P1qnpslqmXGbQECm64ehZdE?=
 =?iso-8859-2?Q?FFaSUW6mIsauM3OJvM3Tw1ucxX9MNj9Wyq1DGB55G2qqVjXUh0RIGRjDcl?=
 =?iso-8859-2?Q?960pVjRkKBRLtyLLJzEnoZgiRiUQwJLysLz4yAB2LBRAHj3MpuvXxpafVt?=
 =?iso-8859-2?Q?6bBgILg8cMnxX4y2eYVWgkJlw26o8I+/B9hPAzMZHXvBya48g6n6HGfoKu?=
 =?iso-8859-2?Q?m+usFIa8M9QrXHOVtIPGYUwdUtbF+KLpaRevsouptEOZ5EGNhcrCMoygio?=
 =?iso-8859-2?Q?dgn6iyGkJrOjFPB/Y9KMVVLs5TXl5N4pDIRFPLttfUXZRHZ+AccrXnk7CG?=
 =?iso-8859-2?Q?q+JbIiGRlmJGtfQgUc9gXITUQ8lxKBOEOUX5wWyfdEm6tDPtYIii25Ipql?=
 =?iso-8859-2?Q?MK85Um7/0v9KbguyL1o01yX1pjscFdVmxGZus3xAjawCcvyHnAmbR3YktI?=
 =?iso-8859-2?Q?guYxIdsgCtXUmDcfbuu+gx/R2sTfqRhcD0eLHp82xPt1F5HN8yKZbDX5ma?=
 =?iso-8859-2?Q?pN3rOtG6rkzT6M/Bq2ieJVQQKlHyftlFvALUBZJDSudDlSqUM4Owtc5ynA?=
 =?iso-8859-2?Q?XHzQja5gS8ZCQKJqOU71/Fj7yIJ6oN9uTAdhZwBX0zUIulu9bbvaEKOaKQ?=
 =?iso-8859-2?Q?jUgRVLbPnSfyZL3TPCgZ+mfhJn2/UJSCDS1hj8Ju9qBMPue4hfJObZP9p7?=
 =?iso-8859-2?Q?3BnEIEWSKmykqN9unB4zlObQXkgN7RK75aaNED5/oov4IIlMP1trrIQ938?=
 =?iso-8859-2?Q?ucXVCdi0Tw4Qxg995aJH7HQfO8qd+BdHZHxVBrlHN7z96pZPf6JMDsE0S3?=
 =?iso-8859-2?Q?Sovq3zNu1eHNy60h12n9R6xs4HGmtHc1SDGA0b9KAEcMW1+Z5uYoxGyAXG?=
 =?iso-8859-2?Q?eT1BI1BysMR2TWsMWhK5sxmIgECyt1hXDVUZ7xjdzUrtopzRRJlpDw3f94?=
 =?iso-8859-2?Q?qgnA/uOpHoW4jnC5SjkwM+0HfIzFYz9aNiwthlHTdCSs2KvtyuSRVO7zGI?=
 =?iso-8859-2?Q?2bU9EPWH/X0WWfkO5GA/Kr4uMPGF3peTxtNu8gB01E571pRyk6coafqizr?=
 =?iso-8859-2?Q?JWe2DIt18ajZ58tRJNZBQ7cOH2hchue/XnQ9DkPNYpxIhBXk+5LscDLDoY?=
 =?iso-8859-2?Q?YCLFsUOjMGPfi/wadk/F66qALqD9qtOWJA7t1Bkmazkaulm5dR+rU4cosH?=
 =?iso-8859-2?Q?XZm35FcUMNU7+uYtMNE=3D?=
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5342.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e1c144b9-d25a-4ddc-2a0b-08d9d6658a26
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jan 2022 07:23:04.4152
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 73VIBbYaYbBcOdbqn6rvCF1x7zB0uXMFGNCWHK7xCAgbzxwosA4m75tKuf068JcEyf58cB5plHVvWO672VsWBw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB4345
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

Hello Michal
=20
>=20
> -----Original Message-----
> From: Michal Vok=E1=E8 <michal.vokac@ysoft.com>
> Sent: Friday, November 12, 2021 8:57 PM
> To: Horia Geanta <horia.geanta@nxp.com>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; Herbert Xu <herbert@gondor.apana.org.au>
> Cc: David S. Miller <davem@davemloft.net>; linux-crypto@vger.kernel.org;
> linux-kernel@vger.kernel.org; l.stach@pengutronix.de;
> robert.hancock@calian.com; Petr Benes <petr.benes@ysoft.com>;
> petrben@gmail.com; Michal Vok=E1=E8 <michal.vokac@ysoft.com>
> Subject: [EXT] [RFC PATCH] crypto: caam - restore retry count after HW RN=
G
> failure
>=20
> Caution: EXT Email
>=20
> From: Petr Benes <petr.benes@ysoft.com>
>=20
> Each time TRNG generates entropy, statistical tests are run.
> If they fail, RETRY_COUNT value is decremented. Once it reaches 0, HW RNG
> returns an error, and needs to be reset.

The least-significant 16 bits of the RTSTATUS register reflect the result o=
f each of statistical tests.
Can you dump RTSTATUS to see which test has failed? Please dump RTMCTL as w=
ell.

Have you tried to build this patch for armv8? For me it is failing.

Regards
Gaurav Jain

> RETRY_COUNT could be programmed in RTSCMISC register and is set to 1 by
> default. Hence, we are left without hwrng after the first error, which co=
uld
> happen even under normal conditions.
>=20
> Cc: petrben@gmail.com
> Signed-off-by: Petr Benes <petr.benes@ysoft.com>
> Signed-off-by: Michal Vok=E1=E8 <michal.vokac@ysoft.com>
> ---
> Hi,
> we are also experiencing this issue:
>=20
> caam_jr 2101000.jr0: 20003c5b: CCB: desc idx 60: RNG: Hardware error.
>=20
> It is happening on both i.MX6S and i.MX6DL SoCs we use.
> On Solo I can reproduce it really fast. Sometimes it happens right after =
the board
> is NFS booted, sometimes I need to stress the HWRNG for a while (generate=
 few
> hundred KBs of random data). On some DualLite SoCs it is happening at lea=
st
> once a day.
>=20
> We are using the v5.10 LTS branch but I can confirm that this is happenin=
g on all
> kernels since v5.7 to the latest linux-next.
>=20
> We also tried to increase the RTSDCTL_ENT_DLY_MIN delay as suggested in t=
his
> thread [1]. It helped and the issue never occurred since then but we are =
looking
> for more universal and permanent solution suitable for upstream, hence we
> came up with this patch.
>=20
> Any comments will be appreciated.
> Thanks, Michal
>=20
> [1]
> https://eur01.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Flkml.=
org
> %2Flkml%2F2021%2F8%2F30%2F296&amp;data=3D04%7C01%7Cpankaj.gupta%40
> nxp.com%7C5d6bf460b260415aeda008d9a5f0ffab%7C686ea1d3bc2b4c6fa92cd
> 99c5c301635%7C0%7C0%7C637723276775699794%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7C1000&amp;sdata=3DVxN9MqDFbISptW3OX9XTtZ%2FwQTsEbF6dETxXB%2
> BHrywg%3D&amp;reserved=3D0
>=20
>  drivers/crypto/caam/caamrng.c | 42 ++++++++++++++++++++++++++++++++---
>  drivers/crypto/caam/ctrl.c    | 13 +++++++++++
>  drivers/crypto/caam/ctrl.h    |  2 ++
>  3 files changed, 54 insertions(+), 3 deletions(-)
>=20
> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.=
c
> index 77d048dfe5d0..2be5584ae591 100644
> --- a/drivers/crypto/caam/caamrng.c
> +++ b/drivers/crypto/caam/caamrng.c
> @@ -21,6 +21,7 @@
>  #include "desc_constr.h"
>  #include "jr.h"
>  #include "error.h"
> +#include "ctrl.h"
>=20
>  #define CAAM_RNG_MAX_FIFO_STORE_SIZE   16
>=20
> @@ -113,6 +114,35 @@ static int caam_rng_read_one(struct device *jrdev,
>         return err ?: (ret ?: len);
>  }
>=20
> +static void caam_rng_retry_reset(struct caam_rng_ctx *context) {
> +       struct device *ctrldev =3D context->ctrldev;
> +       struct caam_drv_private *ctrlpriv =3D dev_get_drvdata(ctrldev);
> +       struct caam_ctrl __iomem *ctrl;
> +       struct rng4tst __iomem *r4tst;
> +       u32 __iomem *rtstatus;
> +       u32 retry_count;
> +
> +       ctrl =3D (struct caam_ctrl __iomem *)ctrlpriv->ctrl;
> +       r4tst =3D &ctrl->r4tst[0];
> +
> +       /*
> +        * There is unfortunately no member for RTSTATUS register in
> +        * struct rng4tst and the structure doesn't look stable
> +        */
> +       rtstatus =3D (u32 *)((char *)&ctrl->r4tst[0] + 0x3C);
> +       retry_count =3D (rd_reg32(rtstatus) >> 16) & 0xf;
> +       dev_dbg(ctrldev, "CAAM RNG retry count %d\n", retry_count);
> +       if (retry_count =3D=3D 0) {
> +               dev_err(ctrldev, "CAAM RNG resetting retry count to 1\n")=
;
> +               clrsetbits_32(&r4tst->rtmctl, 0, RTMCTL_PRGM | RTMCTL_ACC=
);
> +               wr_reg32(&r4tst->rtscmisc, (rd_reg32(&r4tst->rtscmisc) & =
0x7f) | (1 <<
> 16));
> +               clrsetbits_32(&r4tst->rtmctl, RTMCTL_PRGM | RTMCTL_ACC,
> +                               RTMCTL_SAMP_MODE_RAW_ES_SC);
> +               caam_reinstantiate_rng(ctrldev);
> +       }
> +}
> +
>  static void caam_rng_fill_async(struct caam_rng_ctx *ctx)  {
>         struct scatterlist sg[1];
> @@ -129,8 +159,10 @@ static void caam_rng_fill_async(struct caam_rng_ctx
> *ctx)
>                                 sg[0].length,
>                                 ctx->desc_async,
>                                 &done);
> -       if (len < 0)
> +       if (len < 0) {
> +               caam_rng_retry_reset(ctx);
>                 return;
> +       }
>=20
>         kfifo_dma_in_finish(&ctx->fifo, len);  } @@ -145,13 +177,17 @@ st=
atic void
> caam_rng_worker(struct work_struct *work)  static int caam_read(struct hw=
rng
> *rng, void *dst, size_t max, bool wait)  {
>         struct caam_rng_ctx *ctx =3D to_caam_rng_ctx(rng);
> -       int out;
> +       int out, ret;
>=20
>         if (wait) {
>                 struct completion done;
>=20
> -               return caam_rng_read_one(ctx->jrdev, dst, max,
> +               ret =3D caam_rng_read_one(ctx->jrdev, dst, max,
>                                          ctx->desc_sync, &done);
> +               if (ret < 0)
> +                       caam_rng_retry_reset(ctx);
> +
> +               return ret;
>         }
>=20
>         out =3D kfifo_out(&ctx->fifo, dst, max); diff --git a/drivers/cry=
pto/caam/ctrl.c
> b/drivers/crypto/caam/ctrl.c index ca0361b2dbb0..e421f8d1982b 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -339,6 +339,19 @@ static int instantiate_rng(struct device *ctrldev, i=
nt
> state_handle_mask,
>         return devm_add_action_or_reset(ctrldev, devm_deinstantiate_rng,
> ctrldev);  }
>=20
> +/*
> + * caam_reinstantiate_rng - reinstantiates RNG. Intended for a case when=
 RNG
> falls into
> + *                         HW error condition. That happens if TRNG fail=
s statistical
> + *                         check and RTY_CNT value set in RTSCMISC decre=
ments to zero.
> + *                         It is exported to caamrng.c
> + * @ctrldev - pointer to device
> + */
> +
> +int caam_reinstantiate_rng(struct device *ctrldev) {
> +       return instantiate_rng(ctrldev, 0, 0); }
> +
>  /*
>   * kick_trng - sets the various parameters for enabling the initializati=
on
>   *            of the RNG4 block in CAAM
> diff --git a/drivers/crypto/caam/ctrl.h b/drivers/crypto/caam/ctrl.h inde=
x
> f3ecd67922a7..26ff5a49a865 100644
> --- a/drivers/crypto/caam/ctrl.h
> +++ b/drivers/crypto/caam/ctrl.h
> @@ -8,6 +8,8 @@
>  #ifndef CTRL_H
>  #define CTRL_H
>=20
> +int caam_reinstantiate_rng(struct device *ctrldev);
> +
>  /* Prototypes for backend-level services exposed to APIs */  extern bool
> caam_dpaa2;
>=20
> --
> 2.25.1