Re: [PATCH v2 14/14] crypto: sun8i-ce: Add support for the TRNG
From: LABBE Corentin
Date: Mon Apr 27 2020 - 04:42:41 EST
On Fri, Apr 24, 2020 at 04:34:40PM +0200, Stephan Mueller wrote:
> Am Freitag, 24. April 2020, 16:02:14 CEST schrieb Corentin Labbe:
>
> Hi Corentin,
>
> > This patch had support for the TRNG present in the CE.
> > Note that according to the algorithm ID, 2 version of the TRNG exists,
> > the first present in H3/H5/R40/A64 and the second present in H6.
> > This patch adds support for both, but only the second is working
> > reliabily accoridng to rngtest.
> >
> > Signed-off-by: Corentin Labbe <clabbe@xxxxxxxxxxxx>
> > ---
> > drivers/crypto/allwinner/Kconfig | 8 ++
> > drivers/crypto/allwinner/sun8i-ce/Makefile | 1 +
> > .../crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 18 +++
> > .../crypto/allwinner/sun8i-ce/sun8i-ce-trng.c | 123 ++++++++++++++++++
> > drivers/crypto/allwinner/sun8i-ce/sun8i-ce.h | 18 +++
> > 5 files changed, 168 insertions(+)
> > create mode 100644 drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> >
> > diff --git a/drivers/crypto/allwinner/Kconfig
> > b/drivers/crypto/allwinner/Kconfig index 223a5823867c..6aec31f7d2be 100644
> > --- a/drivers/crypto/allwinner/Kconfig
> > +++ b/drivers/crypto/allwinner/Kconfig
> > @@ -87,6 +87,14 @@ config CRYPTO_DEV_SUN8I_CE_PRNG
> > Select this option if you want to provide kernel-side support for
> > the Pseudo-Random Number Generator found in the Crypto Engine.
> >
> > +config CRYPTO_DEV_SUN8I_CE_TRNG
> > + bool "Support for Allwinner Crypto Engine TRNG"
> > + depends on CRYPTO_DEV_SUN8I_CE
> > + select HW_RANDOM
> > + help
> > + Select this option if you want to provide kernel-side support for
> > + the True Random Number Generator found in the Crypto Engine.
> > +
> > config CRYPTO_DEV_SUN8I_SS
> > tristate "Support for Allwinner Security System cryptographic
> offloader"
> > select CRYPTO_SKCIPHER
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/Makefile
> > b/drivers/crypto/allwinner/sun8i-ce/Makefile index
> > c0ea81da2c7d..0842eb2d9408 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/Makefile
> > +++ b/drivers/crypto/allwinner/sun8i-ce/Makefile
> > @@ -2,3 +2,4 @@ obj-$(CONFIG_CRYPTO_DEV_SUN8I_CE) += sun8i-ce.o
> > sun8i-ce-y += sun8i-ce-core.o sun8i-ce-cipher.o
> > sun8i-ce-$(CONFIG_CRYPTO_DEV_SUN8I_CE_HASH) += sun8i-ce-hash.o
> > sun8i-ce-$(CONFIG_CRYPTO_DEV_SUN8I_CE_PRNG) += sun8i-ce-prng.o
> > +sun8i-ce-$(CONFIG_CRYPTO_DEV_SUN8I_CE_TRNG) += sun8i-ce-trng.o
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> > b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c index
> > 23b9fc67d7ea..86d75789811f 100644
> > --- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
> > @@ -47,6 +47,7 @@ static const struct ce_variant ce_h3_variant = {
> > },
> > .esr = ESR_H3,
> > .prng = CE_ALG_PRNG,
> > + .trng = CE_ID_NOTSUPP,
> > };
> >
> > static const struct ce_variant ce_h5_variant = {
> > @@ -63,6 +64,7 @@ static const struct ce_variant ce_h5_variant = {
> > },
> > .esr = ESR_H5,
> > .prng = CE_ALG_PRNG,
> > + .trng = CE_ID_NOTSUPP,
> > };
> >
> > static const struct ce_variant ce_h6_variant = {
> > @@ -76,6 +78,7 @@ static const struct ce_variant ce_h6_variant = {
> > .cipher_t_dlen_in_bytes = true,
> > .hash_t_dlen_in_bits = true,
> > .prng_t_dlen_in_bytes = true,
> > + .trng_t_dlen_in_bytes = true,
> > .ce_clks = {
> > { "bus", 0, 200000000 },
> > { "mod", 300000000, 0 },
> > @@ -83,6 +86,7 @@ static const struct ce_variant ce_h6_variant = {
> > },
> > .esr = ESR_H6,
> > .prng = CE_ALG_PRNG_V2,
> > + .trng = CE_ALG_TRNG_V2,
> > };
> >
> > static const struct ce_variant ce_a64_variant = {
> > @@ -99,6 +103,7 @@ static const struct ce_variant ce_a64_variant = {
> > },
> > .esr = ESR_A64,
> > .prng = CE_ALG_PRNG,
> > + .trng = CE_ID_NOTSUPP,
> > };
> >
> > static const struct ce_variant ce_r40_variant = {
> > @@ -115,6 +120,7 @@ static const struct ce_variant ce_r40_variant = {
> > },
> > .esr = ESR_R40,
> > .prng = CE_ALG_PRNG,
> > + .trng = CE_ID_NOTSUPP,
> > };
> >
> > /*
> > @@ -579,6 +585,10 @@ static int sun8i_ce_dbgfs_read(struct seq_file *seq,
> > void *v) break;
> > }
> > }
> > +#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_TRNG
> > + seq_printf(seq, "HWRNG %lu %lu\n",
> > + ce->hwrng_stat_req, ce->hwrng_stat_bytes);
> > +#endif
> > return 0;
> > }
> >
> > @@ -928,6 +938,10 @@ static int sun8i_ce_probe(struct platform_device *pdev)
> > if (err < 0)
> > goto error_alg;
> >
> > +#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_TRNG
> > + sun8i_ce_hwrng_register(ce);
> > +#endif
> > +
> > v = readl(ce->base + CE_CTR);
> > v >>= CE_DIE_ID_SHIFT;
> > v &= CE_DIE_ID_MASK;
> > @@ -957,6 +971,10 @@ static int sun8i_ce_remove(struct platform_device
> > *pdev) {
> > struct sun8i_ce_dev *ce = platform_get_drvdata(pdev);
> >
> > +#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_TRNG
> > + sun8i_ce_hwrng_unregister(ce);
> > +#endif
> > +
> > sun8i_ce_unregister_algs(ce);
> >
> > #ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG
> > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> > b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c new file mode 100644
> > index 000000000000..5e4effe29ed3
> > --- /dev/null
> > +++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c
> > @@ -0,0 +1,123 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * sun8i-ce-trng.c - hardware cryptographic offloader for
> > + * Allwinner H3/A64/H5/H2+/H6/R40 SoC
> > + *
> > + * Copyright (C) 2015-2020 Corentin Labbe <clabbe@xxxxxxxxxxxx>
> > + *
> > + * This file handle the TRNG
> > + *
> > + * You could find a link for the datasheet in
> > Documentation/arm/sunxi/README + */
> > +#include "sun8i-ce.h"
> > +#include <linux/pm_runtime.h>
> > +#include <linux/hw_random.h>
> > +/*
> > + * Note that according to the algorithm ID, 2 versions of the TRNG exists,
> > + * The first present in H3/H5/R40/A64 and the second present in H6.
> > + * This file adds support for both, but only the second is working
> > + * reliabily according to rngtest.
> > + **/
> > +
> > +int sun8i_ce_trng_read(struct hwrng *rng, void *data, size_t max, bool
> > wait) +{
> > + struct sun8i_ce_dev *ce;
> > + dma_addr_t dma_dst;
> > + int err = 0;
> > + int flow = 3;
> > + unsigned int todo;
> > + struct sun8i_ce_flow *chan;
> > + struct ce_task *cet;
> > + u32 common;
> > + void *d;
> > +
> > + ce = container_of(rng, struct sun8i_ce_dev, trng);
> > +
> > + todo = max + 32;
> > + todo -= todo % 32;
> > +
> > + d = kzalloc(todo, GFP_KERNEL | GFP_DMA);
> > + if (!d)
> > + return -ENOMEM;
> > +
> > +#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_DEBUG
> > + ce->hwrng_stat_req++;
> > + ce->hwrng_stat_bytes += todo;
> > +#endif
> > +
> > + dma_dst = dma_map_single(ce->dev, d, todo, DMA_FROM_DEVICE);
> > + if (dma_mapping_error(ce->dev, dma_dst)) {
> > + dev_err(ce->dev, "Cannot DMA MAP DST\n");
> > + err = -EFAULT;
> > + goto err_dst;
> > + }
> > +
> > + err = pm_runtime_get_sync(ce->dev);
> > + if (err < 0)
> > + goto err_pm;
> > +
> > + mutex_lock(&ce->rnglock);
> > + chan = &ce->chanlist[flow];
> > +
> > + cet = &chan->tl[0];
> > + memset(cet, 0, sizeof(struct ce_task));
> > +
> > + cet->t_id = cpu_to_le32(flow);
> > + common = ce->variant->trng | CE_COMM_INT;
> > + cet->t_common_ctl = cpu_to_le32(common);
> > +
> > + /* recent CE (H6) need length in bytes, in word otherwise */
> > + if (ce->variant->trng_t_dlen_in_bytes)
> > + cet->t_dlen = cpu_to_le32(todo);
> > + else
> > + cet->t_dlen = cpu_to_le32(todo / 4);
> > +
> > + cet->t_sym_ctl = 0;
> > + cet->t_asym_ctl = 0;
> > +
> > + cet->t_dst[0].addr = cpu_to_le32(dma_dst);
> > + cet->t_dst[0].len = cpu_to_le32(todo / 4);
> > + ce->chanlist[flow].timeout = 2000;
> > +
> > + err = sun8i_ce_run_task(ce, 3, "TRNG");
> > + mutex_unlock(&ce->rnglock);
> > +
> > + pm_runtime_put(ce->dev);
> > +
> > +err_pm:
> > + dma_unmap_single(ce->dev, dma_dst, todo, DMA_FROM_DEVICE);
> > +
> > + if (!err) {
> > + memcpy(data, d, max);
> > + err = max;
> > + }
> > +
> > +err_dst:
> > + kfree(d);
>
> kzfree? I would assume d contains sensitive data, no?
>
Like sun8i-ce, yes.
I will fix that
Thanks