Hello Michael
-----Original Message----- From: Michael Walle michael@walle.cc Sent: Tuesday, November 23, 2021 4:22 PM To: Gaurav Jain gaurav.jain@nxp.com Cc: ZHIZHIKIN Andrey andrey.zhizhikin@leica-geosystems.com; u- boot@lists.denx.de; Stefano Babic sbabic@denx.de; Fabio Estevam festevam@gmail.com; Peng Fan peng.fan@nxp.com; Simon Glass sjg@chromium.org; Priyanka Jain priyanka.jain@nxp.com; Ye Li ye.li@nxp.com; Horia Geanta horia.geanta@nxp.com; Ji Luo ji.luo@nxp.com; Franck Lenormand franck.lenormand@nxp.com; Silvano Di Ninno silvano.dininno@nxp.com; Sahil Malhotra sahil.malhotra@nxp.com; Pankaj Gupta pankaj.gupta@nxp.com; Varun Sethi V.Sethi@nxp.com; dl-uboot-imx uboot-imx@nxp.com; Shengzhou Liu shengzhou.liu@nxp.com; Mingkai Hu mingkai.hu@nxp.com; Rajesh Bhagat rajesh.bhagat@nxp.com; Meenakshi Aggarwal meenakshi.aggarwal@nxp.com; Wasim Khan wasim.khan@nxp.com; Alison Wang alison.wang@nxp.com; Pramod Kumar pramod.kumar_1@nxp.com; Andy Tang andy.tang@nxp.com; Adrian Alonso adrian.alonso@nxp.com; Vladimir Oltean olteanv@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