[PATCH v4 1/8] dm: rng: Add random number generator(rng) uclass

[Heinrich Schuchardt]

On 12/17/19 12:51 PM, Sughosh Ganu wrote:
> Add a uclass for reading a random number seed from a random number
> generator device.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> Reviewed-by: Patrice Chotard <patrice.chotard at st.com>
> ---
> Changes since V3:
>   Handle review comments from Patrick Delaunay.
>
>   drivers/Kconfig          |  2 ++
>   drivers/Makefile         |  1 +
>   drivers/rng/Kconfig      |  7 +++++++
>   drivers/rng/Makefile     |  6 ++++++
>   drivers/rng/rng-uclass.c | 23 +++++++++++++++++++++++
>   include/dm/uclass-id.h   |  1 +
>   include/rng.h            | 32 ++++++++++++++++++++++++++++++++
>   7 files changed, 72 insertions(+)
>   create mode 100644 drivers/rng/Kconfig
>   create mode 100644 drivers/rng/Makefile
>   create mode 100644 drivers/rng/rng-uclass.c
>   create mode 100644 include/rng.h
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 9d99ce0..e34a227 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -90,6 +90,8 @@ source "drivers/remoteproc/Kconfig"
>
>   source "drivers/reset/Kconfig"
>
> +source "drivers/rng/Kconfig"
> +
>   source "drivers/rtc/Kconfig"
>
>   source "drivers/scsi/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index e977f19..6c619b1 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -115,4 +115,5 @@ obj-$(CONFIG_W1_EEPROM) += w1-eeprom/
>
>   obj-$(CONFIG_MACH_PIC32) += ddr/microchip/
>   obj-$(CONFIG_DM_HWSPINLOCK) += hwspinlock/
> +obj-$(CONFIG_DM_RNG) += rng/
>   endif
> diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> new file mode 100644
> index 0000000..dd44cc0
> --- /dev/null
> +++ b/drivers/rng/Kconfig
> @@ -0,0 +1,7 @@
> +config DM_RNG
> +	bool "Driver support for Random Number Generator devices"
> +	depends on DM
> +	help
> +	  Enable driver model for random number generator(rng) devices.
> +	  This interface is used to initialise the rng device and to
> +	  read the random seed from the device.
> diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> new file mode 100644
> index 0000000..311705b
> --- /dev/null
> +++ b/drivers/rng/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2019, Linaro Limited
> +#
> +
> +obj-$(CONFIG_DM_RNG) += rng-uclass.o
> diff --git a/drivers/rng/rng-uclass.c b/drivers/rng/rng-uclass.c
> new file mode 100644
> index 0000000..b6af3b8
> --- /dev/null
> +++ b/drivers/rng/rng-uclass.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <rng.h>
> +
> +int dm_rng_read(struct udevice *dev, void *buffer, size_t size)
> +{
> +	const struct dm_rng_ops *ops = device_get_ops(dev);
> +
> +	if (!ops->read)
> +		return -ENOSYS;
> +
> +	return ops->read(dev, buffer, size);
> +}
> +
> +UCLASS_DRIVER(rng) = {
> +	.name = "rng",
> +	.id = UCLASS_RNG,
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 0c563d8..192202d 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -86,6 +86,7 @@ enum uclass_id {
>   	UCLASS_REGULATOR,	/* Regulator device */
>   	UCLASS_REMOTEPROC,	/* Remote Processor device */
>   	UCLASS_RESET,		/* Reset controller device */
> +	UCLASS_RNG,		/* Random Number Generator */
>   	UCLASS_RTC,		/* Real time clock device */
>   	UCLASS_SCSI,		/* SCSI device */
>   	UCLASS_SERIAL,		/* Serial UART */
> diff --git a/include/rng.h b/include/rng.h
> new file mode 100644
> index 0000000..80891fc
> --- /dev/null
> +++ b/include/rng.h
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#if !defined _RNG_H_
> +#define _RNG_H_
> +
> +#include <dm.h>
> +
> +/**
> + * dm_rng_read() - read a random number seed from the rng device
> + * @buffer:	input buffer to put the read random seed into
> + * @size:	number of bytes of random seed read
> + * @return 0 if OK, -ve on error

Please, stick to the Spinx syntax (Return:). See
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

The Linux kernel code uses the return value to return the number of
bytes retrieved.

Please, provide as description here of what will happen if the RNG
device blocks.

> + *
> + */
> +int dm_rng_read(struct udevice *dev, void *buffer, size_t size);

We have different places where we retrieve the first RNG device and read
from it. Shouldn't we provide a single function in the uclass that
returns random bytes without a udevice parameter?

Linux uses an entropy pool where the hardware RNG is one of multiple
sources of randomness and the entropy runs through hash functions
(chacha20, sha1) before being consumed.

Shouldn't we call a hashing function in this uclass?

Shouldn't we further provide a function that can later be used to add
additional sources of entropy? E.g. a seed file, the MAC address, timing
of network accesses.

Please, take a look at Linux' drivers/char/random.c.

Best regards

Heinrich

> +
> +/* struct dm_rng_ops - Operations for the hwrng uclass */
> +struct dm_rng_ops {
> +	/**
> +	 * @read() - read a random number seed
> +	 *
> +	 * @data:	input buffer to read the random seed
> +	 * @max:	total number of bytes to read
> +	 * @return 0 if OK, -ve on error
> +	 */
> +	int (*read)(struct udevice *dev, void *data, size_t max);
> +};
> +
> +#endif /* _RNG_H_ */
>