Re: [PATCH] crypto: sun4i-ss: support the Security System PRNG

[Maxime Ripard <maxime.ripard@free-electrons.com>]

[-- Attachment #1: Type: text/plain, Size: 6894 bytes --]

Hi,

On Tue, Jun 20, 2017 at 10:58:19AM +0200, Corentin Labbe wrote:
> The Security System have a PRNG, this patch add support for it via
> crypto_rng.

This might be a dumb question, but is the CRYPTO_RNG code really
supposed to be used with PRNG?

> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  drivers/crypto/Kconfig                  |  8 +++++
>  drivers/crypto/sunxi-ss/Makefile        |  1 +
>  drivers/crypto/sunxi-ss/sun4i-ss-core.c | 30 ++++++++++++++++++
>  drivers/crypto/sunxi-ss/sun4i-ss-prng.c | 56 +++++++++++++++++++++++++++++++++
>  drivers/crypto/sunxi-ss/sun4i-ss.h      |  9 ++++++
>  5 files changed, 104 insertions(+)
>  create mode 100644 drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index ab82536d64e2..bde0b102eb70 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -618,6 +618,14 @@ config CRYPTO_DEV_SUN4I_SS
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called sun4i-ss.
>  
> +config CRYPTO_DEV_SUN4I_SS_PRNG
> +	bool "Support for Allwinner Security System PRNG"
> +	depends on CRYPTO_DEV_SUN4I_SS
> +	select CRYPTO_RNG
> +	help
> +	  Select this option if you to provides kernel-side support for
> +	  the Pseudo-Random Number Generator found in the Security System.
> +
>  config CRYPTO_DEV_ROCKCHIP
>  	tristate "Rockchip's Cryptographic Engine driver"
>  	depends on OF && ARCH_ROCKCHIP
> diff --git a/drivers/crypto/sunxi-ss/Makefile b/drivers/crypto/sunxi-ss/Makefile
> index 8f4c7a273141..ccb893219079 100644
> --- a/drivers/crypto/sunxi-ss/Makefile
> +++ b/drivers/crypto/sunxi-ss/Makefile
> @@ -1,2 +1,3 @@
>  obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sun4i-ss.o
>  sun4i-ss-y += sun4i-ss-core.o sun4i-ss-hash.o sun4i-ss-cipher.o
> +sun4i-ss-$(CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG) += sun4i-ss-prng.o
> diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
> index 02ad8256e900..d6bb2991c000 100644
> --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
> +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
> @@ -213,6 +213,23 @@ static struct sun4i_ss_alg_template ss_algs[] = {
>  		}
>  	}
>  },
> +#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG
> +{
> +	.type = CRYPTO_ALG_TYPE_RNG,
> +	.alg.rng = {
> +		.base = {
> +			.cra_name		= "stdrng",
> +			.cra_driver_name	= "sun4i_ss_rng",
> +			.cra_priority		= 300,
> +			.cra_ctxsize		= 0,
> +			.cra_module		= THIS_MODULE,
> +		},
> +		.generate               = sun4i_ss_prng_generate,
> +		.seed                   = sun4i_ss_prng_seed,
> +		.seedsize               = SS_SEED_LEN,
> +	}
> +},
> +#endif
>  };
>  
>  static int sun4i_ss_probe(struct platform_device *pdev)
> @@ -355,6 +372,13 @@ static int sun4i_ss_probe(struct platform_device *pdev)
>  				goto error_alg;
>  			}
>  			break;
> +		case CRYPTO_ALG_TYPE_RNG:
> +			err = crypto_register_rng(&ss_algs[i].alg.rng);
> +			if (err) {
> +				dev_err(ss->dev, "Fail to register %s\n",
> +					ss_algs[i].alg.rng.base.cra_name);
> +			}
> +			break;
>  		}
>  	}
>  	platform_set_drvdata(pdev, ss);
> @@ -369,6 +393,9 @@ static int sun4i_ss_probe(struct platform_device *pdev)
>  		case CRYPTO_ALG_TYPE_AHASH:
>  			crypto_unregister_ahash(&ss_algs[i].alg.hash);
>  			break;
> +		case CRYPTO_ALG_TYPE_RNG:
> +			crypto_unregister_rng(&ss_algs[i].alg.rng);
> +			break;
>  		}
>  	}
>  	if (ss->reset)
> @@ -393,6 +420,9 @@ static int sun4i_ss_remove(struct platform_device *pdev)
>  		case CRYPTO_ALG_TYPE_AHASH:
>  			crypto_unregister_ahash(&ss_algs[i].alg.hash);
>  			break;
> +		case CRYPTO_ALG_TYPE_RNG:
> +			crypto_unregister_rng(&ss_algs[i].alg.rng);
> +			break;
>  		}
>  	}
>  
> diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> new file mode 100644
> index 000000000000..3941587def6b
> --- /dev/null
> +++ b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> @@ -0,0 +1,56 @@
> +#include "sun4i-ss.h"
> +
> +int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> +		       unsigned int slen)
> +{
> +	struct sun4i_ss_alg_template *algt;
> +	struct rng_alg *alg = crypto_rng_alg(tfm);
> +
> +	algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
> +	memcpy(algt->ss->seed, seed, slen);
> +
> +	return 0;
> +}
> +
> +int sun4i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src,
> +			   unsigned int slen, u8 *dst, unsigned int dlen)
> +{
> +	struct sun4i_ss_alg_template *algt;
> +	struct rng_alg *alg = crypto_rng_alg(tfm);
> +	int i;
> +	u32 v;
> +	u32 *data = (u32 *)dst;
> +	const u32 mode = SS_OP_PRNG | SS_PRNG_CONTINUE | SS_ENABLED;
> +	size_t len;
> +	struct sun4i_ss_ctx *ss;
> +	unsigned int todo = (dlen / 4) * 4;
> +
> +	algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
> +	ss = algt->ss;
> +
> +	spin_lock(&ss->slock);
> +
> +	writel(mode, ss->base + SS_CTL);
> +
> +	while (todo > 0) {
> +		/* write the seed */
> +		for (i = 0; i < SS_SEED_LEN / 4; i++)
> +			writel(ss->seed[i], ss->base + SS_KEY0 + i * 4);
> +
> +		/* Read the random data */
> +		len = min_t(size_t, SS_DATA_LEN, todo);
> +		readsl(ss->base + SS_TXFIFO, data, len / 4);
> +		data += len / 4;
> +		todo -= len;
> +
> +		/* Update the seed */
> +		for (i = 0; i < SS_SEED_LEN / 4; i++) {
> +			v = readl(ss->base + SS_KEY0 + i * 4);
> +			ss->seed[i] = v;
> +		}
> +	}
> +
> +	writel(0, ss->base + SS_CTL);
> +	spin_unlock(&ss->slock);
> +	return dlen;
> +}
> diff --git a/drivers/crypto/sunxi-ss/sun4i-ss.h b/drivers/crypto/sunxi-ss/sun4i-ss.h
> index a0e1efc1cb2a..293632b1cf27 100644
> --- a/drivers/crypto/sunxi-ss/sun4i-ss.h
> +++ b/drivers/crypto/sunxi-ss/sun4i-ss.h
> @@ -32,6 +32,7 @@
>  #include <crypto/aes.h>
>  #include <crypto/des.h>
>  #include <crypto/internal/rng.h>
> +#include <crypto/rng.h>
>  
>  #define SS_CTL            0x00
>  #define SS_KEY0           0x04
> @@ -127,6 +128,9 @@
>  #define SS_RXFIFO_EMP_INT_ENABLE	(1 << 2)
>  #define SS_TXFIFO_AVA_INT_ENABLE	(1 << 0)
>  
> +#define SS_SEED_LEN (192 / 8)
> +#define SS_DATA_LEN (160 / 8)
> +
>  struct sun4i_ss_ctx {
>  	void __iomem *base;
>  	int irq;
> @@ -136,6 +140,7 @@ struct sun4i_ss_ctx {
>  	struct device *dev;
>  	struct resource *res;
>  	spinlock_t slock; /* control the use of the device */
> +	u32 seed[SS_SEED_LEN / 4];

Shouldn't you define SS_SEED_LEN in bits, and then use either
BITS_PER_BYTE and BITS_PER_LONG so that it's obvious what you're doing
?

And you could also make that variable defined based on the option,
otherwise you'll always allocate that array, even if you're not using
it.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]