Re: [PATCH v2 5/6] crypto: caam - replace DRNG with TRNG for use with hw_random

[Lucas Stach <l.stach@pengutronix.de>]

On Mo, 2019-11-18 at 07:38 -0800, Andrey Smirnov wrote:
> In order to give CAAM-generated random data highest quarlity
> raiting (999), replace current code that uses DRNG with code that
> fetches data straight out of TRNG used to seed aforementioned DRNG.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Horia Geantă <horia.geanta@nxp.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Iuliana Prodan <iuliana.prodan@nxp.com>
> Cc: linux-imx@nxp.com
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
[...]
> diff --git a/drivers/crypto/caam/trng.c b/drivers/crypto/caam/trng.c
> new file mode 100644
> index 000000000000..ab2af786543e
> --- /dev/null
> +++ b/drivers/crypto/caam/trng.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * hw_random interface for TRNG generator in CAAM RNG block
> + *
> + * Copyright 2019 Zoidac Inflight Innovations
                     ^ Zodiac

> + *
> + */
> +
> +#include <linux/hw_random.h>
> +
> +#include "compat.h"
> +#include "regs.h"
> +#include "intern.h"
> +
> +struct caam_trng_ctx {
> +	struct rng4tst __iomem *r4tst;
> +	struct hwrng rng;
> +};
> +
> +static bool caam_trng_busy(struct caam_trng_ctx *ctx)
> +{
> +	return !(rd_reg32(&ctx->r4tst->rtmctl) & RTMCTL_ENT_VAL);
> +}
> +
> +static int caam_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> +{
> +	struct caam_trng_ctx *ctx = (void *)rng->priv;
> +	u32 rtent[ARRAY_SIZE(ctx->r4tst->rtent)];
> +	size_t residue = max;
> +
> +	clrsetbits_32(&ctx->r4tst->rtmctl, 0, RTMCTL_ACC);
> +
> +	do {
> +		const size_t chunk = min(residue, sizeof(rtent));
> +		unsigned int i;
> +
> +		while (caam_trng_busy(ctx)) {

The CAAM needs quite a bit of time to gather the 384bits of raw
entropy, in my testing it was almost 60ms. A busy loop (even with a
cpu_relax) for such an extended amount of time is probably not
appropriate, better sleep for some time here.

Also in the !wait case we are almost guaranteed to leave this function
without any entropy gathered. Maybe we should just bail out on !wait
without even trying to enable the TRNG access?

Regards,
Lucas

> +			if (wait)
> +				cpu_relax();
> +			else
> +				goto out;
> +		}
> +
> +		for (i = 0; i < DIV_ROUND_UP(chunk, sizeof(u32)); i++)
> +			rtent[i] = rd_reg32(&ctx->r4tst->rtent[i]);
> +
> +		memcpy(data, rtent, chunk);
> +
> +		residue -= chunk;
> +		data    += chunk;
> +	} while (residue);
> +
> +out:
> +	clrsetbits_32(&ctx->r4tst->rtmctl, RTMCTL_ACC, 0);
> +	return max - residue;
> +}
> +
> +int caam_trng_register(struct device *ctrldev)
> +{
> +	struct caam_drv_private *priv = dev_get_drvdata(ctrldev);
> +
> +	if (caam_has_rng(priv)) {
> +		struct caam_trng_ctx *ctx;
> +		int err;
> +
> +		ctx = devm_kzalloc(ctrldev, sizeof(*ctx), GFP_KERNEL);
> +		if (!ctx)
> +			return -ENOMEM;
> +
> +		ctx->r4tst = &priv->ctrl->r4tst[0];
> +
> +		ctx->rng.name = "trng-caam";
> +		ctx->rng.read = caam_trng_read;
> +		ctx->rng.priv = (unsigned long)ctx;
> +		ctx->rng.quality = 999;
> +
> +		dev_info(ctrldev, "registering %s\n", ctx->rng.name);
> +
> +		err = devm_hwrng_register(ctrldev, &ctx->rng);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}