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

[Michael Walle]

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?

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.

-michael