[EXT] RE: [PATCH v5 11/16] crypto/fsl: Fix kick_trng

[Gaurav Jain]

Hello Michael

> -----Original Message-----
> From: Michael Walle <michael at walle.cc>
> Sent: Tuesday, November 23, 2021 4:22 PM
> To: Gaurav Jain <gaurav.jain at nxp.com>
> Cc: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>; u-
> boot at lists.denx.de; Stefano Babic <sbabic at denx.de>; Fabio Estevam
> <festevam at gmail.com>; Peng Fan <peng.fan at nxp.com>; Simon Glass
> <sjg at chromium.org>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li
> <ye.li at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji Luo
> <ji.luo at nxp.com>; Franck Lenormand <franck.lenormand at nxp.com>;
> Silvano Di Ninno <silvano.dininno at nxp.com>; Sahil Malhotra
> <sahil.malhotra at nxp.com>; Pankaj Gupta <pankaj.gupta at nxp.com>; Varun
> Sethi <V.Sethi at nxp.com>; dl-uboot-imx <uboot-imx at nxp.com>; Shengzhou
> Liu <shengzhou.liu at nxp.com>; Mingkai Hu <mingkai.hu at nxp.com>; Rajesh
> Bhagat <rajesh.bhagat at nxp.com>; Meenakshi Aggarwal
> <meenakshi.aggarwal at nxp.com>; Wasim Khan <wasim.khan at nxp.com>;
> Alison Wang <alison.wang at nxp.com>; Pramod Kumar
> <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>; Adrian
> Alonso <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> Subject: Re: [EXT] RE: [PATCH v5 11/16] crypto/fsl: Fix kick_trng
> 
> Caution: EXT Email
> 
> Hi Gaurav,
> 
> Am 2021-11-23 11:44, schrieb Gaurav Jain:
> >> > fix hwrng performance issue in kernel.
> >>
> >> This patch is missing some context information, specifically which
> >> performance issue does exist in the Kernel (with some
> >> quantification), and how is it addressed here.
> >>
> >> This function introduced with this patch already exist in the Kernel
> >> [1], and the implementation does differ from Kernel one.
> >> Specifically, this patch lowers the number of test samples that are
> >> run to decide whether the entropy generated by TRNG is sufficiently
> >> random: it reduces the monobit count range, poker test limits, and
> >> number or runs for consecutive 0's and 1's.
> >>
> >> Considering the fact that after TRNG is initialized - JDKEK, TDKEK
> >> and TDSK are preloaded from the RNG and are locked until the next
> >> PoR, Kernel will not re- initialize the TRNG (in fact, there is a
> >> check that is done in the Kernel not to touch RNG if it is already
> >> initialized [2]), and this would leave the Crypto facilities running
> >> in the Kernel to use entropy model that is defined here. In this
> >> case, at least a justification of this change should be made clear -
> >> e.g.
> >> significant speed
> >> improvement over reduced entropy (with quantifiable numbers).
> >>
> >> In addition, with those new parameter set, would the RNG pass FIPS
> >> 140-2 test?
> > TRNG is configured to pass FIPS certification, but will double check
> > and confirm you.
> >
> > You are correct if RNG is instantiated in Uboot then kernel will not
> > reinitialize.
> > 77% performance drop was observed on IMX6/7/8 platforms (0.3 kB/s)
> > compared to 1.3kB/s.
> > With this change hwrng performance improved to 1.3 kB/s.
> 
> Did you test on other platforms like layerscape, too? Can we be sure there
> will no impact with this change on other platforms which uses the CAAM
> TRNG?
> 
Yes I tested Layerscape as well. I tested hwrng, blob encap/decap which works good.

> I have to agree with Andrey, there is little information *why* this is done in
> exactly this way. I'd love to see a proper commit description and comments
> here. I just see a bunch of magic numbers in the code.
> 
Will update the commit description in next version of this patch series.

Regards
Gaurav Jain
> -michael