[PATCH v8 10/15] crypto/fsl: Improve hwrng performance in kernel
ZHIZHIKIN Andrey
andrey.zhizhikin at leica-geosystems.com
Mon Jan 10 15:01:13 CET 2022
Hello Gaurav,
Cc: Michael Walle
> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Gaurav Jain
> Sent: Monday, January 10, 2022 1:27 PM
> To: u-boot at lists.denx.de
> Cc: 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>; NXP i . MX U-Boot Team <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>; Tang Yuantian <andy.tang at nxp.com>; Adrian Alonso
> <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> Subject: [PATCH v8 10/15] crypto/fsl: Improve hwrng performance in kernel
>
> From: Ye Li <ye.li at nxp.com>
>
> RNG parameters are reconfigured.
> - For TRNG to generate 256 bits of entropy, RNG TRNG Seed Control register
> is configured to have reduced SAMP_SIZE from default 2500 to 512. it is
> number of entropy samples that will be taken during Entropy generation.
> - self-test registers(Monobit Limit, Poker Range, Run Length Limit)
> are synchronized with new RTSDCTL[SAMP_SIZE] of 512.
>
> TRNG time is caluculated based on sample size.
Typo: caluculated -> calculated
> time required to generate entropy is reduced and
> hwrng performance improved from 0.3 kB/s to 1.3 kB/s.
Is there any degradation in passed/failed FIPS 140-2 test count? Can you
provide some results from at least rngtest run?
>
> Signed-off-by: Ye Li <ye.li at nxp.com>
> Acked-by: Gaurav Jain <gaurav.jain at nxp.com>>
> ---
> drivers/crypto/fsl/jr.c | 102 +++++++++++++++++++++++++++++++++-------
> include/fsl_sec.h | 1 +
> 2 files changed, 87 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index a84440ab10..e5346a84a4 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -603,30 +603,100 @@ static u8 get_rng_vid(ccsr_sec_t *sec)
> */
> static void kick_trng(int ent_delay, ccsr_sec_t *sec)
> {
> + u32 samples = 512; /* number of bits to generate and test */
> + u32 mono_min = 195;
> + u32 mono_max = 317;
> + u32 mono_range = mono_max - mono_min;
> + u32 poker_min = 1031;
> + u32 poker_max = 1600;
> + u32 poker_range = poker_max - poker_min + 1;
> + u32 retries = 2;
> + u32 lrun_max = 32;
> + s32 run_1_min = 27;
> + s32 run_1_max = 107;
> + s32 run_1_range = run_1_max - run_1_min;
> + s32 run_2_min = 7;
> + s32 run_2_max = 62;
> + s32 run_2_range = run_2_max - run_2_min;
> + s32 run_3_min = 0;
> + s32 run_3_max = 39;
> + s32 run_3_range = run_3_max - run_3_min;
> + s32 run_4_min = -1;
> + s32 run_4_max = 26;
> + s32 run_4_range = run_4_max - run_4_min;
> + s32 run_5_min = -1;
> + s32 run_5_max = 18;
> + s32 run_5_range = run_5_max - run_5_min;
> + s32 run_6_min = -1;
> + s32 run_6_max = 17;
> + s32 run_6_range = run_6_max - run_6_min;
I have a feeling that this whole block of local variables can be
simplified. I'm not sure it is required to list this so detailed.
You can attempt to define those values in header file and use
macros to compute bound conditions, rather than allocating this on
the stack here.
> + u32 val;
> +
> struct rng4tst __iomem *rng =
> (struct rng4tst __iomem *)&sec->rng;
> - u32 val;
>
> - /* put RNG4 into program mode */
> - sec_setbits32(&rng->rtmctl, RTMCTL_PRGM);
> - /* rtsdctl bits 0-15 contain "Entropy Delay, which defines the
> - * length (in system clocks) of each Entropy sample taken
> - * */
> + /* Put RNG in program mode */
> + /* Setting both RTMCTL:PRGM and RTMCTL:TRNG_ACC causes TRNG to
> + * properly invalidate the entropy in the entropy register and
> + * force re-generation.
> + */
> + sec_setbits32(&rng->rtmctl, RTMCTL_PRGM | RTMCTL_ACC);
> +
> + /* Configure the RNG Entropy Delay
> + * Performance-wise, it does not make sense to
> + * set the delay to a value that is lower
> + * than the last one that worked (i.e. the state handles
> + * were instantiated properly. Thus, instead of wasting
> + * time trying to set the values controlling the sample
> + * frequency, the function simply returns.
> + */
> val = sec_in32(&rng->rtsdctl);
> - val = (val & ~RTSDCTL_ENT_DLY_MASK) |
> - (ent_delay << RTSDCTL_ENT_DLY_SHIFT);
> + val &= RTSDCTL_ENT_DLY_MASK;
> + val >>= RTSDCTL_ENT_DLY_SHIFT;
> + if (ent_delay < val) {
> + /* Put RNG4 into run mode */
> + sec_clrbits32(&rng->rtmctl, RTMCTL_PRGM | RTMCTL_ACC);
> + return;
> + }
> +
> + val = (ent_delay << RTSDCTL_ENT_DLY_SHIFT) | samples;
> sec_out32(&rng->rtsdctl, val);
> - /* min. freq. count, equal to 1/4 of the entropy sample length */
> - sec_out32(&rng->rtfreqmin, ent_delay >> 2);
> - /* disable maximum frequency count */
> - sec_out32(&rng->rtfreqmax, RTFRQMAX_DISABLE);
> +
> + /*
> + * Recommended margins (min,max) for freq. count:
> + * freq_mul = RO_freq / TRNG_clk_freq
> + * rtfrqmin = (ent_delay x freq_mul) >> 1;
> + * rtfrqmax = (ent_delay x freq_mul) << 3;
> + * Given current deployments of CAAM in i.MX SoCs, and to simplify
> + * the configuration, we consider [1,16] to be a safe interval
Statement "... we consider ..." should be perhaps extended with justification
on how this conclusion was made. Some test results (either here in the comment
or in commit message) would be beneficial to see.
> + * for the freq_mul and the limits of the interval are used to compute
> + * rtfrqmin, rtfrqmax
> + */
> + sec_out32(&rng->rtfreqmin, ent_delay >> 1);
> + sec_out32(&rng->rtfreqmax, ent_delay << 7);
> +
> + sec_out32(&rng->rtscmisc, (retries << 16) | lrun_max);
> + sec_out32(&rng->rtpkrmax, poker_max);
> + sec_out32(&rng->rtpkrrng, poker_range);
> + sec_out32(&rng->rsvd1[0], (mono_range << 16) | mono_max);
> + sec_out32(&rng->rsvd1[1], (run_1_range << 16) | run_1_max);
> + sec_out32(&rng->rsvd1[2], (run_2_range << 16) | run_2_max);
> + sec_out32(&rng->rsvd1[3], (run_3_range << 16) | run_3_max);
> + sec_out32(&rng->rsvd1[4], (run_4_range << 16) | run_4_max);
> + sec_out32(&rng->rsvd1[5], (run_5_range << 16) | run_5_max);
> + sec_out32(&rng->rsvd1[6], (run_6_range << 16) | run_6_max);
Writing in reserved area is not a good idea. Those registers are defined
in HW, can you please define them also in the header file?
> +
> + val = sec_in32(&rng->rtmctl);
> /*
> - * select raw sampling in both entropy shifter
> + * Select raw sampling in both entropy shifter
This change is not needed, otherwise please adjust all comments in
original file, as for example comment below also starts with small
letter...
> * and statistical checker
> */
> - sec_setbits32(&rng->rtmctl, RTMCTL_SAMP_MODE_RAW_ES_SC);
The only usage of RTMCTL_SAMP_MODE_RAW_ES_SC is dropped here, please
remove it from include/fsl_sec.h as well.
> - /* put RNG4 into run mode */
> - sec_clrbits32(&rng->rtmctl, RTMCTL_PRGM);
> + val &= ~RTMCTL_SAMP_MODE_INVALID;
> + val |= RTMCTL_SAMP_MODE_RAW_ES_SC;
> + /* Put RNG4 into run mode */
> + val &= ~(RTMCTL_PRGM | RTMCTL_ACC);
BIT() macro for both new and existing defines?
> + /*test with sample mode only */
> + sec_out32(&rng->rtmctl, val);
> }
>
> static int rng_init(uint8_t sec_idx, ccsr_sec_t *sec)
> diff --git a/include/fsl_sec.h b/include/fsl_sec.h
> index 7b6e3e2c20..2b3239414a 100644
> --- a/include/fsl_sec.h
> +++ b/include/fsl_sec.h
> @@ -34,6 +34,7 @@
> #if CONFIG_SYS_FSL_SEC_COMPAT >= 4
> /* RNG4 TRNG test registers */
> struct rng4tst {
> +#define RTMCTL_ACC 0x20
> #define RTMCTL_PRGM 0x00010000 /* 1 -> program mode, 0 -> run mode */
> #define RTMCTL_SAMP_MODE_VON_NEUMANN_ES_SC 0 /* use von Neumann data in
> both entropy shifter and
> --
> 2.17.1
-- andrey
More information about the U-Boot
mailing list