Message ID | 1577381108-6320-2-git-send-email-sughosh.ganu@linaro.org |
---|---|
State | Superseded, archived |
Headers | show |
Series | Add a random number generator uclass | expand |
On 12/26/19 6:25 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@linaro.org> > Reviewed-by: Patrice Chotard <patrice.chotard@st.com> > --- > Changes since V4: > * Use Sphinx syntax for Return value in the read function > * Return number of bytes read on a successful read, and a -ve value on > error. > > 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 | 33 +++++++++++++++++++++++++++++++++ > 7 files changed, 73 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..9a71e81 > --- /dev/null > +++ b/include/rng.h > @@ -0,0 +1,33 @@ > +// 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: number of bytes read if OK, -ve on error > + */ > +int dm_rng_read(struct udevice *dev, void *buffer, size_t size); > + > +/* 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: number of bytes read if OK, -ve on error > + */ > + int (*read)(struct udevice *dev, void *data, size_t max); With this interface definition the caller has to check the return value and to call read() again and again until all bytes needed are requested. The EFI_RNG_PROTOCOL will not be the only consumer of the hardware RNG so we will have to rewrite that logic in multiple places. In U-Boot we are very tight on the binary size. So I would really appreciate to avoid such code duplication by making it the drivers duty to return exactly the number of bytes requested. Best regards Heinrich > +}; > + > +#endi.f /* _RNG_H_ */ >
On Fri, 27 Dec 2019 at 12:39, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 12/26/19 6:25 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@linaro.org> > > Reviewed-by: Patrice Chotard <patrice.chotard@st.com> > > --- > > Changes since V4: > > * Use Sphinx syntax for Return value in the read function > > * Return number of bytes read on a successful read, and a -ve value on > > error. > > > > 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 | 33 +++++++++++++++++++++++++++++++++ > > 7 files changed, 73 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 > <snip> > > diff --git a/include/rng.h b/include/rng.h > > new file mode 100644 > > index 0000000..9a71e81 > > --- /dev/null > > +++ b/include/rng.h > > @@ -0,0 +1,33 @@ > > +// 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: number of bytes read if OK, -ve on error > > + */ > > +int dm_rng_read(struct udevice *dev, void *buffer, size_t size); > > + > > +/* 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: number of bytes read if OK, -ve on error > > + */ > > + int (*read)(struct udevice *dev, void *data, size_t max); > > With this interface definition the caller has to check the return value > and to call read() again and again until all bytes needed are requested. > The EFI_RNG_PROTOCOL will not be the only consumer of the hardware RNG > so we will have to rewrite that logic in multiple places. In U-Boot we > are very tight on the binary size. So I would really appreciate to avoid > such code duplication by making it the drivers duty to return exactly > the number of bytes requested. > Ok. I had tested the code with the loop inside the virtio driver yesterday, and with more than max-bytes requested, the system would freeze for a pretty longish bit before returning the data. I mistook this for a system hang, which is why I moved the loop out of the driver. Testing again, after putting the loop inside the driver, i see that this works. I will send an updated version with the loop inside the driver as you ask. The read function will return 0 on a successful read, and a -ve error on failure. -sughosh
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..9a71e81 --- /dev/null +++ b/include/rng.h @@ -0,0 +1,33 @@ +// 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: number of bytes read if OK, -ve on error + */ +int dm_rng_read(struct udevice *dev, void *buffer, size_t size); + +/* 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: number of bytes read if OK, -ve on error + */ + int (*read)(struct udevice *dev, void *data, size_t max); +}; + +#endif /* _RNG_H_ */