* [PATCH v5 1/8] dm: rng: Add random number generator(rng) uclass
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-27 7:04 ` Heinrich Schuchardt
2019-12-26 17:25 ` [PATCH v5 2/8] clk: stm32mp1: Add a clock entry for RNG1 device Sughosh Ganu
` (6 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
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);
+};
+
+#endif /* _RNG_H_ */
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 1/8] dm: rng: Add random number generator(rng) uclass
2019-12-26 17:25 ` [PATCH v5 1/8] dm: rng: Add random number generator(rng) uclass Sughosh Ganu
@ 2019-12-27 7:04 ` Heinrich Schuchardt
2019-12-27 11:38 ` Sughosh Ganu
0 siblings, 1 reply; 22+ messages in thread
From: Heinrich Schuchardt @ 2019-12-27 7:04 UTC (permalink / raw)
To: u-boot
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_ */
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 1/8] dm: rng: Add random number generator(rng) uclass
2019-12-27 7:04 ` Heinrich Schuchardt
@ 2019-12-27 11:38 ` Sughosh Ganu
0 siblings, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-27 11:38 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/8] clk: stm32mp1: Add a clock entry for RNG1 device
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 1/8] dm: rng: Add random number generator(rng) uclass Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device Sughosh Ganu
` (5 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
Add an entry for allowing clock enablement for the random number
generator peripheral, RNG1.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
Acked-by: Patrick Delaunay <patrick.delaunay@st.com>
---
drivers/clk/clk_stm32mp1.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/clk_stm32mp1.c b/drivers/clk/clk_stm32mp1.c
index 3718970..da66bde 100644
--- a/drivers/clk/clk_stm32mp1.c
+++ b/drivers/clk/clk_stm32mp1.c
@@ -563,6 +563,7 @@ static const struct stm32mp1_clk_gate stm32mp1_clk_gate[] = {
STM32MP1_CLK_SET_CLR(RCC_MP_AHB4ENSETR, 10, GPIOK, _UNKNOWN_SEL),
STM32MP1_CLK_SET_CLR(RCC_MP_AHB5ENSETR, 0, GPIOZ, _UNKNOWN_SEL),
+ STM32MP1_CLK_SET_CLR(RCC_MP_AHB5ENSETR, 6, RNG1_K, _UNKNOWN_SEL),
STM32MP1_CLK_SET_CLR(RCC_MP_AHB6ENSETR, 7, ETHCK_K, _ETH_SEL),
STM32MP1_CLK_SET_CLR(RCC_MP_AHB6ENSETR, 8, ETHTX, _UNKNOWN_SEL),
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 1/8] dm: rng: Add random number generator(rng) uclass Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 2/8] clk: stm32mp1: Add a clock entry for RNG1 device Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-27 7:51 ` Heinrich Schuchardt
2019-12-26 17:25 ` [PATCH v5 4/8] configs: stm32mp15: Enable " Sughosh Ganu
` (4 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
Add a driver for the rng device found on stm32mp1 platforms. The
driver provides a routine for reading the random number seed from the
hardware device.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
Acked-by: Patrick Delaunay <patrick.delaunay@st.com>
---
Changes since V4:
* Return number of bytes read on a successful read, and a -ve value on
error.
drivers/rng/Kconfig | 7 ++
drivers/rng/Makefile | 1 +
drivers/rng/stm32mp1_rng.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+)
create mode 100644 drivers/rng/stm32mp1_rng.c
diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
index dd44cc0..c9c4751 100644
--- a/drivers/rng/Kconfig
+++ b/drivers/rng/Kconfig
@@ -5,3 +5,10 @@ config DM_RNG
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.
+
+config RNG_STM32MP1
+ bool "Enable random number generator for STM32MP1"
+ depends on ARCH_STM32MP && DM_RNG
+ default n
+ help
+ Enable STM32MP1 rng driver.
diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
index 311705b..699beb3 100644
--- a/drivers/rng/Makefile
+++ b/drivers/rng/Makefile
@@ -4,3 +4,4 @@
#
obj-$(CONFIG_DM_RNG) += rng-uclass.o
+obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
diff --git a/drivers/rng/stm32mp1_rng.c b/drivers/rng/stm32mp1_rng.c
new file mode 100644
index 0000000..56ad848
--- /dev/null
+++ b/drivers/rng/stm32mp1_rng.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <reset.h>
+#include <rng.h>
+
+#include <asm/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+
+#define RNG_CR 0x00
+#define RNG_CR_RNGEN BIT(2)
+#define RNG_CR_CED BIT(5)
+
+#define RNG_SR 0x04
+#define RNG_SR_SEIS BIT(6)
+#define RNG_SR_CEIS BIT(5)
+#define RNG_SR_SECS BIT(2)
+#define RNG_SR_DRDY BIT(0)
+
+#define RNG_DR 0x08
+
+struct stm32_rng_platdata {
+ fdt_addr_t base;
+ struct clk clk;
+ struct reset_ctl rst;
+};
+
+static int stm32_rng_read(struct udevice *dev, void *data, size_t len)
+{
+ int retval = 0, i, nbytes;
+ u32 sr, count, reg;
+ size_t increment;
+ struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+
+ /*
+ * Only INT_MAX number of bytes can be returned. If more
+ * bytes need to be read, the caller must do it in a loop.
+ */
+ if (len > INT_MAX)
+ len = INT_MAX;
+
+ nbytes = len;
+ while (len > 0) {
+ retval = readl_poll_timeout(pdata->base + RNG_SR, sr,
+ sr & RNG_SR_DRDY, 10000);
+ if (retval)
+ return retval;
+
+ if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
+ /* As per SoC TRM */
+ clrbits_le32(pdata->base + RNG_SR, RNG_SR_SEIS);
+ for (i = 0; i < 12; i++)
+ readl(pdata->base + RNG_DR);
+ if (readl(pdata->base + RNG_SR) & RNG_SR_SEIS) {
+ printf("RNG Noise");
+ return -EIO;
+ }
+ /* start again */
+ continue;
+ }
+
+ count = 4;
+ while (len && count) {
+ reg = readl(pdata->base + RNG_DR);
+ memcpy(data, ®, min(len, sizeof(u32)));
+ increment = min(len, sizeof(u32));
+ data += increment;
+ len -= increment;
+ count--;
+ }
+ }
+
+ return nbytes;
+}
+
+static int stm32_rng_init(struct stm32_rng_platdata *pdata)
+{
+ int err;
+
+ err = clk_enable(&pdata->clk);
+ if (err)
+ return err;
+
+ /* Disable CED */
+ writel(RNG_CR_RNGEN | RNG_CR_CED, pdata->base + RNG_CR);
+
+ /* clear error indicators */
+ writel(0, pdata->base + RNG_SR);
+
+ return 0;
+}
+
+static int stm32_rng_cleanup(struct stm32_rng_platdata *pdata)
+{
+
+ writel(0, pdata->base + RNG_CR);
+
+ return clk_disable(&pdata->clk);
+}
+
+static int stm32_rng_probe(struct udevice *dev)
+{
+ struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+
+ reset_assert(&pdata->rst);
+ udelay(20);
+ reset_deassert(&pdata->rst);
+
+ return stm32_rng_init(pdata);
+}
+
+static int stm32_rng_remove(struct udevice *dev)
+{
+ struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+
+ return stm32_rng_cleanup(pdata);
+}
+
+static int stm32_rng_ofdata_to_platdata(struct udevice *dev)
+{
+ struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+ int err;
+
+ pdata->base = dev_read_addr(dev);
+ if (!pdata->base)
+ return -ENOMEM;
+
+ err = clk_get_by_index(dev, 0, &pdata->clk);
+ if (err)
+ return err;
+
+ err = reset_get_by_index(dev, 0, &pdata->rst);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static const struct dm_rng_ops stm32_rng_ops = {
+ .read = stm32_rng_read,
+};
+
+static const struct udevice_id stm32_rng_match[] = {
+ {
+ .compatible = "st,stm32-rng",
+ },
+ {},
+};
+
+U_BOOT_DRIVER(stm32_rng) = {
+ .name = "stm32-rng",
+ .id = UCLASS_RNG,
+ .of_match = stm32_rng_match,
+ .ops = &stm32_rng_ops,
+ .probe = stm32_rng_probe,
+ .remove = stm32_rng_remove,
+ .platdata_auto_alloc_size = sizeof(struct stm32_rng_platdata),
+ .ofdata_to_platdata = stm32_rng_ofdata_to_platdata,
+};
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device
2019-12-26 17:25 ` [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device Sughosh Ganu
@ 2019-12-27 7:51 ` Heinrich Schuchardt
2019-12-27 11:19 ` Sughosh Ganu
0 siblings, 1 reply; 22+ messages in thread
From: Heinrich Schuchardt @ 2019-12-27 7:51 UTC (permalink / raw)
To: u-boot
On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> Add a driver for the rng device found on stm32mp1 platforms. The
> driver provides a routine for reading the random number seed from the
> hardware device.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
> Acked-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
> Changes since V4:
> * Return number of bytes read on a successful read, and a -ve value on
> error.
>
> drivers/rng/Kconfig | 7 ++
> drivers/rng/Makefile | 1 +
> drivers/rng/stm32mp1_rng.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 173 insertions(+)
> create mode 100644 drivers/rng/stm32mp1_rng.c
>
> diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> index dd44cc0..c9c4751 100644
> --- a/drivers/rng/Kconfig
> +++ b/drivers/rng/Kconfig
> @@ -5,3 +5,10 @@ config DM_RNG
> 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.
> +
> +config RNG_STM32MP1
> + bool "Enable random number generator for STM32MP1"
> + depends on ARCH_STM32MP && DM_RNG
> + default n
> + help
> + Enable STM32MP1 rng driver.
> diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> index 311705b..699beb3 100644
> --- a/drivers/rng/Makefile
> +++ b/drivers/rng/Makefile
> @@ -4,3 +4,4 @@
> #
>
> obj-$(CONFIG_DM_RNG) += rng-uclass.o
> +obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> diff --git a/drivers/rng/stm32mp1_rng.c b/drivers/rng/stm32mp1_rng.c
> new file mode 100644
> index 0000000..56ad848
> --- /dev/null
> +++ b/drivers/rng/stm32mp1_rng.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <reset.h>
> +#include <rng.h>
> +
> +#include <asm/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +
> +#define RNG_CR 0x00
> +#define RNG_CR_RNGEN BIT(2)
> +#define RNG_CR_CED BIT(5)
> +
> +#define RNG_SR 0x04
> +#define RNG_SR_SEIS BIT(6)
> +#define RNG_SR_CEIS BIT(5)
> +#define RNG_SR_SECS BIT(2)
> +#define RNG_SR_DRDY BIT(0)
> +
> +#define RNG_DR 0x08
> +
> +struct stm32_rng_platdata {
> + fdt_addr_t base;
> + struct clk clk;
> + struct reset_ctl rst;
> +};
> +
> +static int stm32_rng_read(struct udevice *dev, void *data, size_t len)
> +{
> + int retval = 0, i, nbytes;
> + u32 sr, count, reg;
> + size_t increment;
> + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> +
> + /*
> + * Only INT_MAX number of bytes can be returned. If more
> + * bytes need to be read, the caller must do it in a loop.
> + */
> + if (len > INT_MAX)
> + len = INT_MAX;
> +
> + nbytes = len;
> + while (len > 0) {
> + retval = readl_poll_timeout(pdata->base + RNG_SR, sr,
> + sr & RNG_SR_DRDY, 10000);
> + if (retval)
> + return retval;
> +
> + if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
> + /* As per SoC TRM */
> + clrbits_le32(pdata->base + RNG_SR, RNG_SR_SEIS);
> + for (i = 0; i < 12; i++)
> + readl(pdata->base + RNG_DR);
> + if (readl(pdata->base + RNG_SR) & RNG_SR_SEIS) {
> + printf("RNG Noise");
> + return -EIO;
The SEIS bit indicates a seed error. According to the RM0090 Reference
manual for the STM32F429/439 you should clear the SEIS bit and set the
RNGEN bit to restart the RNG.
https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf,
page 768.
So why do you return an error code here? What do you expect the caller
to do?
Should we check the CEIS flag which indicates a clock error?
> + }
> + /* start again */
> + continue;
> + }
> +
It took me some time to understand what this loop does. Please, add a
comment indicating that we copy up to 16 random bytes from the RNG.
Best regards
Heinrich
> + count = 4;
> + while (len && count) {
> + reg = readl(pdata->base + RNG_DR);
> + memcpy(data, ®, min(len, sizeof(u32)));
> + increment = min(len, sizeof(u32));
> + data += increment;
> + len -= increment;
> + count--;
> + }
> + }
> +
> + return nbytes;
> +}
> +
> +static int stm32_rng_init(struct stm32_rng_platdata *pdata)
> +{
> + int err;
> +
> + err = clk_enable(&pdata->clk);
> + if (err)
> + return err;
> +
> + /* Disable CED */
> + writel(RNG_CR_RNGEN | RNG_CR_CED, pdata->base + RNG_CR);
> +
> + /* clear error indicators */
> + writel(0, pdata->base + RNG_SR);
> +
> + return 0;
> +}
> +
> +static int stm32_rng_cleanup(struct stm32_rng_platdata *pdata)
> +{
> +
> + writel(0, pdata->base + RNG_CR);
> +
> + return clk_disable(&pdata->clk);
> +}
> +
> +static int stm32_rng_probe(struct udevice *dev)
> +{
> + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> +
> + reset_assert(&pdata->rst);
> + udelay(20);
> + reset_deassert(&pdata->rst);
> +
> + return stm32_rng_init(pdata);
> +}
> +
> +static int stm32_rng_remove(struct udevice *dev)
> +{
> + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> +
> + return stm32_rng_cleanup(pdata);
> +}
> +
> +static int stm32_rng_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> + int err;
> +
> + pdata->base = dev_read_addr(dev);
> + if (!pdata->base)
> + return -ENOMEM;
> +
> + err = clk_get_by_index(dev, 0, &pdata->clk);
> + if (err)
> + return err;
> +
> + err = reset_get_by_index(dev, 0, &pdata->rst);
> + if (err)
> + return err;
> +
> + return 0;
> +}
> +
> +static const struct dm_rng_ops stm32_rng_ops = {
> + .read = stm32_rng_read,
> +};
> +
> +static const struct udevice_id stm32_rng_match[] = {
> + {
> + .compatible = "st,stm32-rng",
> + },
> + {},
> +};
> +
> +U_BOOT_DRIVER(stm32_rng) = {
> + .name = "stm32-rng",
> + .id = UCLASS_RNG,
> + .of_match = stm32_rng_match,
> + .ops = &stm32_rng_ops,
> + .probe = stm32_rng_probe,
> + .remove = stm32_rng_remove,
> + .platdata_auto_alloc_size = sizeof(struct stm32_rng_platdata),
> + .ofdata_to_platdata = stm32_rng_ofdata_to_platdata,
> +};
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device
2019-12-27 7:51 ` Heinrich Schuchardt
@ 2019-12-27 11:19 ` Sughosh Ganu
2019-12-27 12:42 ` Heinrich Schuchardt
0 siblings, 1 reply; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-27 11:19 UTC (permalink / raw)
To: u-boot
On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> > Add a driver for the rng device found on stm32mp1 platforms. The
> > driver provides a routine for reading the random number seed from the
> > hardware device.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
> > Acked-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> > Changes since V4:
> > * Return number of bytes read on a successful read, and a -ve value on
> > error.
> >
> > drivers/rng/Kconfig | 7 ++
> > drivers/rng/Makefile | 1 +
> > drivers/rng/stm32mp1_rng.c | 165
> +++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 173 insertions(+)
> > create mode 100644 drivers/rng/stm32mp1_rng.c
> >
> > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> > index dd44cc0..c9c4751 100644
> > --- a/drivers/rng/Kconfig
> > +++ b/drivers/rng/Kconfig
> > @@ -5,3 +5,10 @@ config DM_RNG
> > 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.
> > +
> > +config RNG_STM32MP1
> > + bool "Enable random number generator for STM32MP1"
> > + depends on ARCH_STM32MP && DM_RNG
> > + default n
> > + help
> > + Enable STM32MP1 rng driver.
> > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > index 311705b..699beb3 100644
> > --- a/drivers/rng/Makefile
> > +++ b/drivers/rng/Makefile
> > @@ -4,3 +4,4 @@
> > #
> >
> > obj-$(CONFIG_DM_RNG) += rng-uclass.o
> > +obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > diff --git a/drivers/rng/stm32mp1_rng.c b/drivers/rng/stm32mp1_rng.c
> > new file mode 100644
> > index 0000000..56ad848
> > --- /dev/null
> > +++ b/drivers/rng/stm32mp1_rng.c
> > @@ -0,0 +1,165 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <reset.h>
> > +#include <rng.h>
> > +
> > +#include <asm/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/kernel.h>
> > +
> > +#define RNG_CR 0x00
> > +#define RNG_CR_RNGEN BIT(2)
> > +#define RNG_CR_CED BIT(5)
> > +
> > +#define RNG_SR 0x04
> > +#define RNG_SR_SEIS BIT(6)
> > +#define RNG_SR_CEIS BIT(5)
> > +#define RNG_SR_SECS BIT(2)
> > +#define RNG_SR_DRDY BIT(0)
> > +
> > +#define RNG_DR 0x08
> > +
> > +struct stm32_rng_platdata {
> > + fdt_addr_t base;
> > + struct clk clk;
> > + struct reset_ctl rst;
> > +};
> > +
> > +static int stm32_rng_read(struct udevice *dev, void *data, size_t len)
> > +{
> > + int retval = 0, i, nbytes;
> > + u32 sr, count, reg;
> > + size_t increment;
> > + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> > +
> > + /*
> > + * Only INT_MAX number of bytes can be returned. If more
> > + * bytes need to be read, the caller must do it in a loop.
> > + */
> > + if (len > INT_MAX)
> > + len = INT_MAX;
> > +
> > + nbytes = len;
> > + while (len > 0) {
> > + retval = readl_poll_timeout(pdata->base + RNG_SR, sr,
> > + sr & RNG_SR_DRDY, 10000);
> > + if (retval)
> > + return retval;
> > +
> > + if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
> > + /* As per SoC TRM */
> > + clrbits_le32(pdata->base + RNG_SR, RNG_SR_SEIS);
> > + for (i = 0; i < 12; i++)
> > + readl(pdata->base + RNG_DR);
> > + if (readl(pdata->base + RNG_SR) & RNG_SR_SEIS) {
> > + printf("RNG Noise");
> > + return -EIO;
>
> The SEIS bit indicates a seed error. According to the RM0090 Reference
> manual for the STM32F429/439 you should clear the SEIS bit and set the
> RNGEN bit to restart the RNG.
>
>
> https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf
> ,
> page 768.
>
> So why do you return an error code here? What do you expect the caller
> to do?
>
I am referring to the stm32mp157 trm, Noise source error detection, pg 1939.
>
> Should we check the CEIS flag which indicates a clock error?
>
The stm32mp1 clk driver in tf-a sets the rng peripheral source to lsi_ck,
which is 32KHz. This is configured after tests done by ST engineers to
provide better entropy. However, with this clock source, the CEIS bit is
set, since Frng is less than (Fhclk/32). This is not an issue though, since
the trm clearly states that the clock error has no impact on the generated
random numbers, and that the application can still read the RNG_DR register.
>
> > + }
> > + /* start again */
> > + continue;
> > + }
> > +
>
> It took me some time to understand what this loop does. Please, add a
> comment indicating that we copy up to 16 random bytes from the RNG.
>
Ok. Will do.
-sughosh
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device
2019-12-27 11:19 ` Sughosh Ganu
@ 2019-12-27 12:42 ` Heinrich Schuchardt
2019-12-27 12:50 ` Heinrich Schuchardt
2019-12-27 12:52 ` Sughosh Ganu
0 siblings, 2 replies; 22+ messages in thread
From: Heinrich Schuchardt @ 2019-12-27 12:42 UTC (permalink / raw)
To: u-boot
On 12/27/19 12:19 PM, Sughosh Ganu wrote:
>
> On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
> On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> > Add a driver for the rng device found on stm32mp1 platforms. The
> > driver provides a routine for reading the random number seed from the
> > hardware device.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
> <mailto:sughosh.ganu@linaro.org>>
> > Reviewed-by: Patrice Chotard <patrice.chotard@st.com
> <mailto:patrice.chotard@st.com>>
> > Acked-by: Patrick Delaunay <patrick.delaunay@st.com
> <mailto:patrick.delaunay@st.com>>
> > ---
> > Changes since V4:
> > * Return number of bytes read on a successful read, and a -ve
> value on
> > error.
> >
> > drivers/rng/Kconfig | 7 ++
> > drivers/rng/Makefile | 1 +
> > drivers/rng/stm32mp1_rng.c | 165
> +++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 173 insertions(+)
> > create mode 100644 drivers/rng/stm32mp1_rng.c
> >
> > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> > index dd44cc0..c9c4751 100644
> > --- a/drivers/rng/Kconfig
> > +++ b/drivers/rng/Kconfig
> > @@ -5,3 +5,10 @@ config DM_RNG
> > 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.
> > +
> > +config RNG_STM32MP1
> > + bool "Enable random number generator for STM32MP1"
> > + depends on ARCH_STM32MP && DM_RNG
> > + default n
> > + help
> > + Enable STM32MP1 rng driver.
> > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > index 311705b..699beb3 100644
> > --- a/drivers/rng/Makefile
> > +++ b/drivers/rng/Makefile
> > @@ -4,3 +4,4 @@
> > #
> >
> > obj-$(CONFIG_DM_RNG) += rng-uclass.o
> > +obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > diff --git a/drivers/rng/stm32mp1_rng.c b/drivers/rng/stm32mp1_rng.c
> > new file mode 100644
> > index 0000000..56ad848
> > --- /dev/null
> > +++ b/drivers/rng/stm32mp1_rng.c
> > @@ -0,0 +1,165 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <clk.h>
> > +#include <dm.h>
> > +#include <reset.h>
> > +#include <rng.h>
> > +
> > +#include <asm/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/kernel.h>
> > +
> > +#define RNG_CR 0x00
> > +#define RNG_CR_RNGEN BIT(2)
> > +#define RNG_CR_CED BIT(5)
> > +
> > +#define RNG_SR 0x04
> > +#define RNG_SR_SEIS BIT(6)
> > +#define RNG_SR_CEIS BIT(5)
> > +#define RNG_SR_SECS BIT(2)
> > +#define RNG_SR_DRDY BIT(0)
> > +
> > +#define RNG_DR 0x08
> > +
> > +struct stm32_rng_platdata {
> > + fdt_addr_t base;
> > + struct clk clk;
> > + struct reset_ctl rst;
> > +};
> > +
> > +static int stm32_rng_read(struct udevice *dev, void *data,
> size_t len)
> > +{
> > + int retval = 0, i, nbytes;
> > + u32 sr, count, reg;
> > + size_t increment;
> > + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> > +
> > + /*
> > + * Only INT_MAX number of bytes can be returned. If more
> > + * bytes need to be read, the caller must do it in a loop.
> > + */
> > + if (len > INT_MAX)
> > + len = INT_MAX;
> > +
> > + nbytes = len;
> > + while (len > 0) {
> > + retval = readl_poll_timeout(pdata->base + RNG_SR, sr,
> > + sr & RNG_SR_DRDY, 10000);
> > + if (retval)
> > + return retval;
> > +
> > + if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
> > + /* As per SoC TRM */
> > + clrbits_le32(pdata->base + RNG_SR,
> RNG_SR_SEIS);
> > + for (i = 0; i < 12; i++)
> > + readl(pdata->base + RNG_DR);
> > + if (readl(pdata->base + RNG_SR) &
> RNG_SR_SEIS) {
> > + printf("RNG Noise");
> > + return -EIO;
>
> The SEIS bit indicates a seed error. According to the RM0090 Reference
> manual for the STM32F429/439 you should clear the SEIS bit and set the
> RNGEN bit to restart the RNG.
>
> https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf,
> page 768.
>
> So why do you return an error code here? What do you expect the caller
> to do?
>
>
> I am referring to the stm32mp157 trm, Noise source error detection, pg 1939.
Could you, please, provide a link to the document for stm32mp157.
compatible = "st,stm32-rng" is provided in U-Boot by:
* stm32f429.dtsi included by
stm32429i-eval.dts, arch/arm/dts/stm32f429-disco.dts, and
stm32f469-disco.dts
* as well as by stm32mp157c.dtsi included by
stm32mp157a-avenger96.dts, stm32mp157a-dk1.dts, and
stm32mp157c-ed1.dts.
So we have to consider both SoCs here.
Best regards
Heinrich
>
>
> Should we check the CEIS flag which indicates a clock error?
>
>
> The stm32mp1 clk driver in tf-a sets the rng peripheral source to
> lsi_ck, which is 32KHz. This is configured after tests done by ST
> engineers to provide better entropy. However, with this clock source,
> the CEIS bit is set, since Frng is less than (Fhclk/32). This is not an
> issue though, since the trm clearly states that the clock error has no
> impact on the generated random numbers, and that the application can
> still read the RNG_DR register.
>
>
> > + }
> > + /* start again */
> > + continue;
> > + }
> > +
>
> It took me some time to understand what this loop does. Please, add a
> comment indicating that we copy up to 16 random bytes from the RNG.
>
>
> Ok. Will do.
>
> -sughosh
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device
2019-12-27 12:42 ` Heinrich Schuchardt
@ 2019-12-27 12:50 ` Heinrich Schuchardt
2019-12-27 12:58 ` Sughosh Ganu
2019-12-27 12:52 ` Sughosh Ganu
1 sibling, 1 reply; 22+ messages in thread
From: Heinrich Schuchardt @ 2019-12-27 12:50 UTC (permalink / raw)
To: u-boot
On 12/27/19 1:42 PM, Heinrich Schuchardt wrote:
> On 12/27/19 12:19 PM, Sughosh Ganu wrote:
>>
>> On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt <xypron.glpk@gmx.de
>> <mailto:xypron.glpk@gmx.de>> wrote:
>>
>> On 12/26/19 6:25 PM, Sughosh Ganu wrote:
>> > Add a driver for the rng device found on stm32mp1 platforms. The
>> > driver provides a routine for reading the random number seed
>> from the
>> > hardware device.
>> >
>> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
>> <mailto:sughosh.ganu@linaro.org>>
>> > Reviewed-by: Patrice Chotard <patrice.chotard@st.com
>> <mailto:patrice.chotard@st.com>>
>> > Acked-by: Patrick Delaunay <patrick.delaunay@st.com
>> <mailto:patrick.delaunay@st.com>>
>> > ---
>> > Changes since V4:
>> > * Return number of bytes read on a successful read, and a -ve
>> value on
>> > error.
>> >
>> > drivers/rng/Kconfig | 7 ++
>> > drivers/rng/Makefile | 1 +
>> > drivers/rng/stm32mp1_rng.c | 165
>> +++++++++++++++++++++++++++++++++++++++++++++
>> > 3 files changed, 173 insertions(+)
>> > create mode 100644 drivers/rng/stm32mp1_rng.c
>> >
>> > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
>> > index dd44cc0..c9c4751 100644
>> > --- a/drivers/rng/Kconfig
>> > +++ b/drivers/rng/Kconfig
>> > @@ -5,3 +5,10 @@ config DM_RNG
>> > 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.
>> > +
>> > +config RNG_STM32MP1
>> > + bool "Enable random number generator for STM32MP1"
>> > + depends on ARCH_STM32MP && DM_RNG
>> > + default n
>> > + help
>> > + Enable STM32MP1 rng driver.
>> > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
>> > index 311705b..699beb3 100644
>> > --- a/drivers/rng/Makefile
>> > +++ b/drivers/rng/Makefile
>> > @@ -4,3 +4,4 @@
>> > #
>> >
>> > obj-$(CONFIG_DM_RNG) += rng-uclass.o
>> > +obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
>> > diff --git a/drivers/rng/stm32mp1_rng.c
>> b/drivers/rng/stm32mp1_rng.c
>> > new file mode 100644
>> > index 0000000..56ad848
>> > --- /dev/null
>> > +++ b/drivers/rng/stm32mp1_rng.c
>> > @@ -0,0 +1,165 @@
>> > +// SPDX-License-Identifier: GPL-2.0-or-later
>> > +/*
>> > + * Copyright (c) 2019, Linaro Limited
>> > + */
>> > +
>> > +#include <common.h>
>> > +#include <clk.h>
>> > +#include <dm.h>
>> > +#include <reset.h>
>> > +#include <rng.h>
>> > +
>> > +#include <asm/io.h>
>> > +#include <linux/iopoll.h>
>> > +#include <linux/kernel.h>
>> > +
>> > +#define RNG_CR 0x00
>> > +#define RNG_CR_RNGEN BIT(2)
>> > +#define RNG_CR_CED BIT(5)
>> > +
>> > +#define RNG_SR 0x04
>> > +#define RNG_SR_SEIS BIT(6)
>> > +#define RNG_SR_CEIS BIT(5)
>> > +#define RNG_SR_SECS BIT(2)
>> > +#define RNG_SR_DRDY BIT(0)
>> > +
>> > +#define RNG_DR 0x08
>> > +
>> > +struct stm32_rng_platdata {
>> > + fdt_addr_t base;
>> > + struct clk clk;
>> > + struct reset_ctl rst;
>> > +};
>> > +
>> > +static int stm32_rng_read(struct udevice *dev, void *data,
>> size_t len)
>> > +{
>> > + int retval = 0, i, nbytes;
>> > + u32 sr, count, reg;
>> > + size_t increment;
>> > + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
>> > +
>> > + /*
>> > + * Only INT_MAX number of bytes can be returned. If more
>> > + * bytes need to be read, the caller must do it in a loop.
>> > + */
>> > + if (len > INT_MAX)
>> > + len = INT_MAX;
>> > +
>> > + nbytes = len;
>> > + while (len > 0) {
>> > + retval = readl_poll_timeout(pdata->base + RNG_SR,
>> sr,
>> > + sr & RNG_SR_DRDY,
>> 10000);
>> > + if (retval)
>> > + return retval;
>> > +
>> > + if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
>> > + /* As per SoC TRM */
>> > + clrbits_le32(pdata->base + RNG_SR,
>> RNG_SR_SEIS);
>> > + for (i = 0; i < 12; i++)
>> > + readl(pdata->base + RNG_DR);
>> > + if (readl(pdata->base + RNG_SR) &
>> RNG_SR_SEIS) {
>> > + printf("RNG Noise");
>> > + return -EIO;
>>
>> The SEIS bit indicates a seed error. According to the RM0090
>> Reference
>> manual for the STM32F429/439 you should clear the SEIS bit and set
>> the
>> RNGEN bit to restart the RNG.
>>
>>
>> https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf,
>>
>> page 768.
>>
>> So why do you return an error code here? What do you expect the
>> caller
>> to do?
>>
>>
>> I am referring to the stm32mp157 trm, Noise source error detection, pg
>> 1939.
>
> Could you, please, provide a link to the document for stm32mp157.
RM0436 Reference manual STM32MP157 advanced Arm®-based 32-bit MPUs
https://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/51/ba/9e/5e/78/5b/4b/dd/DM00327659/files/DM00327659.pdf/jcr:content/translations/en.DM00327659.pdf
chapter 37.3.7 - "Error management" requires the following error handling:
1. Clear the SEIS bit by writing it to “0”.
2. Read out 12 words from the RNG_DR register, and discard each of them
in order to clean the pipeline.
3. Confirm that SEIS is still cleared. Random number generation is back
to normal
So the same error handling can be used for both SoCs.
Best regards
Heinrich
>
> compatible = "st,stm32-rng" is provided in U-Boot by:
>
> * stm32f429.dtsi included by
> stm32429i-eval.dts, arch/arm/dts/stm32f429-disco.dts, and
> stm32f469-disco.dts
>
> * as well as by stm32mp157c.dtsi included by
> stm32mp157a-avenger96.dts, stm32mp157a-dk1.dts, and
> stm32mp157c-ed1.dts.
>
> So we have to consider both SoCs here.
>
> Best regards
>
> Heinrich
>
>>
>>
>> Should we check the CEIS flag which indicates a clock error?
>>
>>
>> The stm32mp1 clk driver in tf-a sets the rng peripheral source to
>> lsi_ck, which is 32KHz. This is configured after tests done by ST
>> engineers to provide better entropy. However, with this clock source,
>> the CEIS bit is set, since Frng is less than (Fhclk/32). This is not an
>> issue though, since the trm clearly states that the clock error has no
>> impact on the generated random numbers, and that the application can
>> still read the RNG_DR register.
>>
>>
>> > + }
>> > + /* start again */
>> > + continue;
>> > + }
>> > +
>>
>> It took me some time to understand what this loop does. Please, add a
>> comment indicating that we copy up to 16 random bytes from the RNG.
>>
>>
>> Ok. Will do.
>>
>> -sughosh
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device
2019-12-27 12:50 ` Heinrich Schuchardt
@ 2019-12-27 12:58 ` Sughosh Ganu
0 siblings, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-27 12:58 UTC (permalink / raw)
To: u-boot
On Fri, 27 Dec 2019 at 18:25, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 12/27/19 1:42 PM, Heinrich Schuchardt wrote:
> > On 12/27/19 12:19 PM, Sughosh Ganu wrote:
> >>
> >> On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt <xypron.glpk@gmx.de
> >> <mailto:xypron.glpk@gmx.de>> wrote:
> >>
> >> On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> >> > Add a driver for the rng device found on stm32mp1 platforms. The
> >> > driver provides a routine for reading the random number seed
> >> from the
> >> > hardware device.
> >> >
> >> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
> >> <mailto:sughosh.ganu@linaro.org>>
> >> > Reviewed-by: Patrice Chotard <patrice.chotard@st.com
> >> <mailto:patrice.chotard@st.com>>
> >> > Acked-by: Patrick Delaunay <patrick.delaunay@st.com
> >> <mailto:patrick.delaunay@st.com>>
> >> > ---
> >> > Changes since V4:
> >> > * Return number of bytes read on a successful read, and a -ve
> >> value on
> >> > error.
> >> >
> >> > drivers/rng/Kconfig | 7 ++
> >> > drivers/rng/Makefile | 1 +
> >> > drivers/rng/stm32mp1_rng.c | 165
> >> +++++++++++++++++++++++++++++++++++++++++++++
> >> > 3 files changed, 173 insertions(+)
> >> > create mode 100644 drivers/rng/stm32mp1_rng.c
> >> >
> >> > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> >> > index dd44cc0..c9c4751 100644
> >> > --- a/drivers/rng/Kconfig
> >> > +++ b/drivers/rng/Kconfig
> >> > @@ -5,3 +5,10 @@ config DM_RNG
> >> > 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.
> >> > +
> >> > +config RNG_STM32MP1
> >> > + bool "Enable random number generator for STM32MP1"
> >> > + depends on ARCH_STM32MP && DM_RNG
> >> > + default n
> >> > + help
> >> > + Enable STM32MP1 rng driver.
> >> > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> >> > index 311705b..699beb3 100644
> >> > --- a/drivers/rng/Makefile
> >> > +++ b/drivers/rng/Makefile
> >> > @@ -4,3 +4,4 @@
> >> > #
> >> >
> >> > obj-$(CONFIG_DM_RNG) += rng-uclass.o
> >> > +obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> >> > diff --git a/drivers/rng/stm32mp1_rng.c
> >> b/drivers/rng/stm32mp1_rng.c
> >> > new file mode 100644
> >> > index 0000000..56ad848
> >> > --- /dev/null
> >> > +++ b/drivers/rng/stm32mp1_rng.c
> >> > @@ -0,0 +1,165 @@
> >> > +// SPDX-License-Identifier: GPL-2.0-or-later
> >> > +/*
> >> > + * Copyright (c) 2019, Linaro Limited
> >> > + */
> >> > +
> >> > +#include <common.h>
> >> > +#include <clk.h>
> >> > +#include <dm.h>
> >> > +#include <reset.h>
> >> > +#include <rng.h>
> >> > +
> >> > +#include <asm/io.h>
> >> > +#include <linux/iopoll.h>
> >> > +#include <linux/kernel.h>
> >> > +
> >> > +#define RNG_CR 0x00
> >> > +#define RNG_CR_RNGEN BIT(2)
> >> > +#define RNG_CR_CED BIT(5)
> >> > +
> >> > +#define RNG_SR 0x04
> >> > +#define RNG_SR_SEIS BIT(6)
> >> > +#define RNG_SR_CEIS BIT(5)
> >> > +#define RNG_SR_SECS BIT(2)
> >> > +#define RNG_SR_DRDY BIT(0)
> >> > +
> >> > +#define RNG_DR 0x08
> >> > +
> >> > +struct stm32_rng_platdata {
> >> > + fdt_addr_t base;
> >> > + struct clk clk;
> >> > + struct reset_ctl rst;
> >> > +};
> >> > +
> >> > +static int stm32_rng_read(struct udevice *dev, void *data,
> >> size_t len)
> >> > +{
> >> > + int retval = 0, i, nbytes;
> >> > + u32 sr, count, reg;
> >> > + size_t increment;
> >> > + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> >> > +
> >> > + /*
> >> > + * Only INT_MAX number of bytes can be returned. If more
> >> > + * bytes need to be read, the caller must do it in a loop.
> >> > + */
> >> > + if (len > INT_MAX)
> >> > + len = INT_MAX;
> >> > +
> >> > + nbytes = len;
> >> > + while (len > 0) {
> >> > + retval = readl_poll_timeout(pdata->base + RNG_SR,
> >> sr,
> >> > + sr & RNG_SR_DRDY,
> >> 10000);
> >> > + if (retval)
> >> > + return retval;
> >> > +
> >> > + if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
> >> > + /* As per SoC TRM */
> >> > + clrbits_le32(pdata->base + RNG_SR,
> >> RNG_SR_SEIS);
> >> > + for (i = 0; i < 12; i++)
> >> > + readl(pdata->base + RNG_DR);
> >> > + if (readl(pdata->base + RNG_SR) &
> >> RNG_SR_SEIS) {
> >> > + printf("RNG Noise");
> >> > + return -EIO;
> >>
> >> The SEIS bit indicates a seed error. According to the RM0090
> >> Reference
> >> manual for the STM32F429/439 you should clear the SEIS bit and set
> >> the
> >> RNGEN bit to restart the RNG.
> >>
> >>
> >>
> https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf,
>
> >>
> >> page 768.
> >>
> >> So why do you return an error code here? What do you expect the
> >> caller
> >> to do?
> >>
> >>
> >> I am referring to the stm32mp157 trm, Noise source error detection, pg
> >> 1939.
> >
> > Could you, please, provide a link to the document for stm32mp157.
>
> RM0436 Reference manual STM32MP157 advanced Arm®-based 32-bit MPUs
>
>
> https://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/51/ba/9e/5e/78/5b/4b/dd/DM00327659/files/DM00327659.pdf/jcr:content/translations/en.DM00327659.pdf
>
> chapter 37.3.7 - "Error management" requires the following error handling:
>
> 1. Clear the SEIS bit by writing it to “0”.
> 2. Read out 12 words from the RNG_DR register, and discard each of them
> in order to clean the pipeline.
> 3. Confirm that SEIS is still cleared. Random number generation is back
> to normal
>
> So the same error handling can be used for both SoCs.
>
Which is what the current logic is doing. In case, the SEIS bit is still
set after reading the 12 words, the driver returns an error, else it
continues.
-sughosh
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device
2019-12-27 12:42 ` Heinrich Schuchardt
2019-12-27 12:50 ` Heinrich Schuchardt
@ 2019-12-27 12:52 ` Sughosh Ganu
1 sibling, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-27 12:52 UTC (permalink / raw)
To: u-boot
On Fri, 27 Dec 2019 at 18:12, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 12/27/19 12:19 PM, Sughosh Ganu wrote:
> >
> > On Fri, 27 Dec 2019 at 13:21, Heinrich Schuchardt <xypron.glpk@gmx.de
> > <mailto:xypron.glpk@gmx.de>> wrote:
> >
> > On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> > > Add a driver for the rng device found on stm32mp1 platforms. The
> > > driver provides a routine for reading the random number seed from
> the
> > > hardware device.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
> > <mailto:sughosh.ganu@linaro.org>>
> > > Reviewed-by: Patrice Chotard <patrice.chotard@st.com
> > <mailto:patrice.chotard@st.com>>
> > > Acked-by: Patrick Delaunay <patrick.delaunay@st.com
> > <mailto:patrick.delaunay@st.com>>
> > > ---
> > > Changes since V4:
> > > * Return number of bytes read on a successful read, and a -ve
> > value on
> > > error.
> > >
> > > drivers/rng/Kconfig | 7 ++
> > > drivers/rng/Makefile | 1 +
> > > drivers/rng/stm32mp1_rng.c | 165
> > +++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 173 insertions(+)
> > > create mode 100644 drivers/rng/stm32mp1_rng.c
> > >
> > > diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> > > index dd44cc0..c9c4751 100644
> > > --- a/drivers/rng/Kconfig
> > > +++ b/drivers/rng/Kconfig
> > > @@ -5,3 +5,10 @@ config DM_RNG
> > > 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.
> > > +
> > > +config RNG_STM32MP1
> > > + bool "Enable random number generator for STM32MP1"
> > > + depends on ARCH_STM32MP && DM_RNG
> > > + default n
> > > + help
> > > + Enable STM32MP1 rng driver.
> > > diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> > > index 311705b..699beb3 100644
> > > --- a/drivers/rng/Makefile
> > > +++ b/drivers/rng/Makefile
> > > @@ -4,3 +4,4 @@
> > > #
> > >
> > > obj-$(CONFIG_DM_RNG) += rng-uclass.o
> > > +obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> > > diff --git a/drivers/rng/stm32mp1_rng.c
> b/drivers/rng/stm32mp1_rng.c
> > > new file mode 100644
> > > index 0000000..56ad848
> > > --- /dev/null
> > > +++ b/drivers/rng/stm32mp1_rng.c
> > > @@ -0,0 +1,165 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * Copyright (c) 2019, Linaro Limited
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <clk.h>
> > > +#include <dm.h>
> > > +#include <reset.h>
> > > +#include <rng.h>
> > > +
> > > +#include <asm/io.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/kernel.h>
> > > +
> > > +#define RNG_CR 0x00
> > > +#define RNG_CR_RNGEN BIT(2)
> > > +#define RNG_CR_CED BIT(5)
> > > +
> > > +#define RNG_SR 0x04
> > > +#define RNG_SR_SEIS BIT(6)
> > > +#define RNG_SR_CEIS BIT(5)
> > > +#define RNG_SR_SECS BIT(2)
> > > +#define RNG_SR_DRDY BIT(0)
> > > +
> > > +#define RNG_DR 0x08
> > > +
> > > +struct stm32_rng_platdata {
> > > + fdt_addr_t base;
> > > + struct clk clk;
> > > + struct reset_ctl rst;
> > > +};
> > > +
> > > +static int stm32_rng_read(struct udevice *dev, void *data,
> > size_t len)
> > > +{
> > > + int retval = 0, i, nbytes;
> > > + u32 sr, count, reg;
> > > + size_t increment;
> > > + struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
> > > +
> > > + /*
> > > + * Only INT_MAX number of bytes can be returned. If more
> > > + * bytes need to be read, the caller must do it in a loop.
> > > + */
> > > + if (len > INT_MAX)
> > > + len = INT_MAX;
> > > +
> > > + nbytes = len;
> > > + while (len > 0) {
> > > + retval = readl_poll_timeout(pdata->base + RNG_SR,
> sr,
> > > + sr & RNG_SR_DRDY,
> 10000);
> > > + if (retval)
> > > + return retval;
> > > +
> > > + if (sr & (RNG_SR_SEIS | RNG_SR_SECS)) {
> > > + /* As per SoC TRM */
> > > + clrbits_le32(pdata->base + RNG_SR,
> > RNG_SR_SEIS);
> > > + for (i = 0; i < 12; i++)
> > > + readl(pdata->base + RNG_DR);
> > > + if (readl(pdata->base + RNG_SR) &
> > RNG_SR_SEIS) {
> > > + printf("RNG Noise");
> > > + return -EIO;
> >
> > The SEIS bit indicates a seed error. According to the RM0090
> Reference
> > manual for the STM32F429/439 you should clear the SEIS bit and set
> the
> > RNGEN bit to restart the RNG.
> >
> >
> https://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf
> ,
> > page 768.
> >
> > So why do you return an error code here? What do you expect the
> caller
> > to do?
> >
> >
> > I am referring to the stm32mp157 trm, Noise source error detection, pg
> 1939.
>
> Could you, please, provide a link to the document for stm32mp157.
>
I was required to create a login to wiki.st.com to get access to the
document. Don't think there is a link for the document available.
-sughosh
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 4/8] configs: stm32mp15: Enable random number generator(rng) device
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
` (2 preceding siblings ...)
2019-12-26 17:25 ` [PATCH v5 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 5/8] sandbox: rng: Add a random number generator(rng) driver Sughosh Ganu
` (3 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
Enable support for the rng device on the stm32mp15 configs.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
Acked-by: Patrick Delaunay <patrick.delaunay@st.com>
---
configs/stm32mp15_basic_defconfig | 2 ++
configs/stm32mp15_optee_defconfig | 2 ++
configs/stm32mp15_trusted_defconfig | 2 ++
3 files changed, 6 insertions(+)
diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index 713a7e6..c85369c 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -143,3 +143,5 @@ CONFIG_VIDEO_STM32_DSI=y
CONFIG_VIDEO_STM32_MAX_XRES=1280
CONFIG_VIDEO_STM32_MAX_YRES=800
CONFIG_FDT_FIXUP_PARTITIONS=y
+CONFIG_DM_RNG=y
+CONFIG_RNG_STM32MP1=y
diff --git a/configs/stm32mp15_optee_defconfig b/configs/stm32mp15_optee_defconfig
index f9161fd..c192d8d 100644
--- a/configs/stm32mp15_optee_defconfig
+++ b/configs/stm32mp15_optee_defconfig
@@ -127,3 +127,5 @@ CONFIG_VIDEO_STM32_DSI=y
CONFIG_VIDEO_STM32_MAX_XRES=1280
CONFIG_VIDEO_STM32_MAX_YRES=800
CONFIG_FDT_FIXUP_PARTITIONS=y
+CONFIG_DM_RNG=y
+CONFIG_RNG_STM32MP1=y
diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig
index a5ea528..a846962 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -126,3 +126,5 @@ CONFIG_VIDEO_STM32_DSI=y
CONFIG_VIDEO_STM32_MAX_XRES=1280
CONFIG_VIDEO_STM32_MAX_YRES=800
CONFIG_FDT_FIXUP_PARTITIONS=y
+CONFIG_DM_RNG=y
+CONFIG_RNG_STM32MP1=y
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 5/8] sandbox: rng: Add a random number generator(rng) driver
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
` (3 preceding siblings ...)
2019-12-26 17:25 ` [PATCH v5 4/8] configs: stm32mp15: Enable " Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-27 6:54 ` Heinrich Schuchardt
2019-12-26 17:25 ` [PATCH v5 6/8] configs: sandbox: Enable random number generator(rng) device Sughosh Ganu
` (2 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
Add a sandbox driver for random number generation. Mostly aimed at
providing a unit test for rng uclass.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
---
Changes since V4:
* Return number of bytes read on a successful read, and a -ve value on
error.
* Modify the logic of sandbox_rng_read function to use rand and srand
functions to return random data, based on feedback from Heinrich
Schuchardt.
arch/sandbox/dts/test.dts | 4 ++++
drivers/rng/Kconfig | 8 +++++++
drivers/rng/Makefile | 1 +
drivers/rng/sandbox_rng.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 69 insertions(+)
create mode 100644 drivers/rng/sandbox_rng.c
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fdb08f2..2c85540 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -599,6 +599,10 @@
reset-names = "other", "test";
};
+ rng at 0 {
+ compatible = "sandbox,sandbox-rng";
+ };
+
rproc_1: rproc at 1 {
compatible = "sandbox,test-processor";
remoteproc-name = "remoteproc-test-dev1";
diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
index c9c4751..35a3bd1 100644
--- a/drivers/rng/Kconfig
+++ b/drivers/rng/Kconfig
@@ -6,6 +6,14 @@ config DM_RNG
This interface is used to initialise the rng device and to
read the random seed from the device.
+config RNG_SANDBOX
+ bool "Sandbox random number generator"
+ depends on SANDBOX && DM_RNG
+ select CONFIG_LIB_RAND
+ help
+ Enable random number generator for sandbox. This is an
+ emulation of a rng device.
+
config RNG_STM32MP1
bool "Enable random number generator for STM32MP1"
depends on ARCH_STM32MP && DM_RNG
diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
index 699beb3..3517005 100644
--- a/drivers/rng/Makefile
+++ b/drivers/rng/Makefile
@@ -4,4 +4,5 @@
#
obj-$(CONFIG_DM_RNG) += rng-uclass.o
+obj-$(CONFIG_RNG_SANDBOX) += sandbox_rng.o
obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
diff --git a/drivers/rng/sandbox_rng.c b/drivers/rng/sandbox_rng.c
new file mode 100644
index 0000000..5de1799
--- /dev/null
+++ b/drivers/rng/sandbox_rng.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <rng.h>
+
+#include <linux/string.h>
+
+static int sandbox_rng_read(struct udevice *dev, void *data, size_t len)
+{
+ int nbytes;
+ unsigned int i, seed;
+ unsigned int *buf = data;
+ size_t nrem, nloops;
+
+ /*
+ * Only INT_MAX number of bytes can be returned. If more
+ * bytes need to be read, the caller must do it in a loop.
+ */
+ if (len > INT_MAX)
+ len = INT_MAX;
+
+ nbytes = len;
+ nloops = len / sizeof(unsigned int);
+ seed = get_timer(0) ^ rand();
+ srand(seed);
+
+ for (i = 0, nrem = len; i < nloops; i++, nrem -= sizeof(unsigned int))
+ *buf++ = rand();
+
+ seed = rand();
+ memcpy(buf, &seed, nrem);
+
+ return nbytes;
+}
+
+static const struct dm_rng_ops sandbox_rng_ops = {
+ .read = sandbox_rng_read,
+};
+
+static const struct udevice_id sandbox_rng_match[] = {
+ {
+ .compatible = "sandbox,sandbox-rng",
+ },
+ {},
+};
+
+U_BOOT_DRIVER(sandbox_rng) = {
+ .name = "sandbox-rng",
+ .id = UCLASS_RNG,
+ .of_match = sandbox_rng_match,
+ .ops = &sandbox_rng_ops,
+};
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 5/8] sandbox: rng: Add a random number generator(rng) driver
2019-12-26 17:25 ` [PATCH v5 5/8] sandbox: rng: Add a random number generator(rng) driver Sughosh Ganu
@ 2019-12-27 6:54 ` Heinrich Schuchardt
2019-12-27 11:40 ` Sughosh Ganu
0 siblings, 1 reply; 22+ messages in thread
From: Heinrich Schuchardt @ 2019-12-27 6:54 UTC (permalink / raw)
To: u-boot
On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> Add a sandbox driver for random number generation. Mostly aimed at
> providing a unit test for rng uclass.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
> ---
> Changes since V4:
> * Return number of bytes read on a successful read, and a -ve value on
> error.
> * Modify the logic of sandbox_rng_read function to use rand and srand
> functions to return random data, based on feedback from Heinrich
> Schuchardt.
>
> arch/sandbox/dts/test.dts | 4 ++++
> drivers/rng/Kconfig | 8 +++++++
> drivers/rng/Makefile | 1 +
> drivers/rng/sandbox_rng.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 69 insertions(+)
> create mode 100644 drivers/rng/sandbox_rng.c
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index fdb08f2..2c85540 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -599,6 +599,10 @@
> reset-names = "other", "test";
> };
>
> + rng at 0 {
> + compatible = "sandbox,sandbox-rng";
> + };
> +
> rproc_1: rproc at 1 {
> compatible = "sandbox,test-processor";
> remoteproc-name = "remoteproc-test-dev1";
> diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
> index c9c4751..35a3bd1 100644
> --- a/drivers/rng/Kconfig
> +++ b/drivers/rng/Kconfig
> @@ -6,6 +6,14 @@ config DM_RNG
> This interface is used to initialise the rng device and to
> read the random seed from the device.
>
> +config RNG_SANDBOX
> + bool "Sandbox random number generator"
> + depends on SANDBOX && DM_RNG
> + select CONFIG_LIB_RAND
> + help
> + Enable random number generator for sandbox. This is an
> + emulation of a rng device.
> +
> config RNG_STM32MP1
> bool "Enable random number generator for STM32MP1"
> depends on ARCH_STM32MP && DM_RNG
> diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
> index 699beb3..3517005 100644
> --- a/drivers/rng/Makefile
> +++ b/drivers/rng/Makefile
> @@ -4,4 +4,5 @@
> #
>
> obj-$(CONFIG_DM_RNG) += rng-uclass.o
> +obj-$(CONFIG_RNG_SANDBOX) += sandbox_rng.o
> obj-$(CONFIG_RNG_STM32MP1) += stm32mp1_rng.o
> diff --git a/drivers/rng/sandbox_rng.c b/drivers/rng/sandbox_rng.c
> new file mode 100644
> index 0000000..5de1799
> --- /dev/null
> +++ b/drivers/rng/sandbox_rng.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <rng.h>
> +
> +#include <linux/string.h>
> +
> +static int sandbox_rng_read(struct udevice *dev, void *data, size_t len)
> +{
> + int nbytes;
> + unsigned int i, seed;
> + unsigned int *buf = data;
You assume here that data is 4 byte aligned which may not hold true and
hence may lead to a crash on ARM.
Best regards
Heinrich
> + size_t nrem, nloops;
> +
> + /*
> + * Only INT_MAX number of bytes can be returned. If more
> + * bytes need to be read, the caller must do it in a loop.
> + */
> + if (len > INT_MAX)
> + len = INT_MAX;
> +
> + nbytes = len;
> + nloops = len / sizeof(unsigned int);
> + seed = get_timer(0) ^ rand();
> + srand(seed);
> +
> + for (i = 0, nrem = len; i < nloops; i++, nrem -= sizeof(unsigned int))
> + *buf++ = rand();
> +
> + seed = rand();
> + memcpy(buf, &seed, nrem);
> +
> + return nbytes;
> +}
> +
> +static const struct dm_rng_ops sandbox_rng_ops = {
> + .read = sandbox_rng_read,
> +};
> +
> +static const struct udevice_id sandbox_rng_match[] = {
> + {
> + .compatible = "sandbox,sandbox-rng",
> + },
> + {},
> +};
> +
> +U_BOOT_DRIVER(sandbox_rng) = {
> + .name = "sandbox-rng",
> + .id = UCLASS_RNG,
> + .of_match = sandbox_rng_match,
> + .ops = &sandbox_rng_ops,
> +};
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 5/8] sandbox: rng: Add a random number generator(rng) driver
2019-12-27 6:54 ` Heinrich Schuchardt
@ 2019-12-27 11:40 ` Sughosh Ganu
0 siblings, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-27 11:40 UTC (permalink / raw)
To: u-boot
On Fri, 27 Dec 2019 at 12:29, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> > Add a sandbox driver for random number generation. Mostly aimed at
> > providing a unit test for rng uclass.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
> > ---
> > Changes since V4:
> > * Return number of bytes read on a successful read, and a -ve value on
> > error.
> > * Modify the logic of sandbox_rng_read function to use rand and srand
> > functions to return random data, based on feedback from Heinrich
> > Schuchardt.
> >
> > arch/sandbox/dts/test.dts | 4 ++++
> > drivers/rng/Kconfig | 8 +++++++
> > drivers/rng/Makefile | 1 +
> > drivers/rng/sandbox_rng.c | 56
> +++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 69 insertions(+)
> > create mode 100644 drivers/rng/sandbox_rng.c
> >
>
<snip>
> diff --git a/drivers/rng/sandbox_rng.c b/drivers/rng/sandbox_rng.c
> > new file mode 100644
> > index 0000000..5de1799
> > --- /dev/null
> > +++ b/drivers/rng/sandbox_rng.c
> > @@ -0,0 +1,56 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <rng.h>
> > +
> > +#include <linux/string.h>
> > +
> > +static int sandbox_rng_read(struct udevice *dev, void *data, size_t len)
> > +{
> > + int nbytes;
> > + unsigned int i, seed;
> > + unsigned int *buf = data;
>
> You assume here that data is 4 byte aligned which may not hold true and
> hence may lead to a crash on ARM.
>
Will fix.
-sughosh
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 6/8] configs: sandbox: Enable random number generator(rng) device
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
` (4 preceding siblings ...)
2019-12-26 17:25 ` [PATCH v5 5/8] sandbox: rng: Add a random number generator(rng) driver Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 7/8] test: rng: Add basic test for random number generator(rng) uclass Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 8/8] virtio: rng: Add a random number generator(rng) driver Sughosh Ganu
7 siblings, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
Enable support for random number generator on sandbox configs. This is
aimed primarily at adding unit test support for rng uclass.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
---
configs/sandbox64_defconfig | 2 ++
configs/sandbox_defconfig | 2 ++
2 files changed, 4 insertions(+)
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index cc536ff..a21d832 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -158,6 +158,8 @@ CONFIG_REGULATOR_RK8XX=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
CONFIG_REGULATOR_TPS65090=y
+CONFIG_DM_RNG=y
+CONFIG_RNG_SANDBOX=y
CONFIG_DM_PWM=y
CONFIG_PWM_SANDBOX=y
CONFIG_RAM=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 64245f7..9bdc0f5 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -179,6 +179,8 @@ CONFIG_REGULATOR_RK8XX=y
CONFIG_REGULATOR_S5M8767=y
CONFIG_DM_REGULATOR_SANDBOX=y
CONFIG_REGULATOR_TPS65090=y
+CONFIG_DM_RNG=y
+CONFIG_RNG_SANDBOX=y
CONFIG_DM_PWM=y
CONFIG_PWM_SANDBOX=y
CONFIG_RAM=y
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 7/8] test: rng: Add basic test for random number generator(rng) uclass
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
` (5 preceding siblings ...)
2019-12-26 17:25 ` [PATCH v5 6/8] configs: sandbox: Enable random number generator(rng) device Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-26 17:25 ` [PATCH v5 8/8] virtio: rng: Add a random number generator(rng) driver Sughosh Ganu
7 siblings, 0 replies; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
Add a unit test for testing the rng uclass functionality using the
sandbox rng driver.
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Patrice Chotard <patrice.chotard@st.com>
---
Changes since V4:
* Change the test checking logic based on changes made in the sandbox
rng driver, which now returns number of bytes read.
test/dm/Makefile | 1 +
test/dm/rng.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
create mode 100644 test/dm/rng.c
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 0c2fd5c..f61bf65 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -65,4 +65,5 @@ obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
obj-$(CONFIG_DMA) += dma.o
obj-$(CONFIG_DM_MDIO) += mdio.o
obj-$(CONFIG_DM_MDIO_MUX) += mdio_mux.o
+obj-$(CONFIG_DM_RNG) += rng.o
endif
diff --git a/test/dm/rng.c b/test/dm/rng.c
new file mode 100644
index 0000000..02c6c44
--- /dev/null
+++ b/test/dm/rng.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <rng.h>
+#include <dm/test.h>
+#include <test/ut.h>
+
+/* Basic test of the rng uclass */
+static int dm_test_rng_read(struct unit_test_state *uts)
+{
+ unsigned long rand1 = 0, rand2 = 0;
+ struct udevice *dev;
+
+ ut_assertok(uclass_get_device(UCLASS_RNG, 0, &dev));
+ ut_assertnonnull(dev);
+ ut_asserteq(sizeof(rand1), dm_rng_read(dev, &rand1, sizeof(rand1)));
+ ut_asserteq(sizeof(rand2), dm_rng_read(dev, &rand2, sizeof(rand2)));
+ ut_assert(rand1 != rand2);
+
+ return 0;
+}
+DM_TEST(dm_test_rng_read, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 8/8] virtio: rng: Add a random number generator(rng) driver
2019-12-26 17:25 [PATCH v5 0/8] Add a random number generator uclass Sughosh Ganu
` (6 preceding siblings ...)
2019-12-26 17:25 ` [PATCH v5 7/8] test: rng: Add basic test for random number generator(rng) uclass Sughosh Ganu
@ 2019-12-26 17:25 ` Sughosh Ganu
2019-12-27 7:12 ` Heinrich Schuchardt
7 siblings, 1 reply; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-26 17:25 UTC (permalink / raw)
To: u-boot
Add a driver for the virtio-rng device on the qemu platform. The
device uses pci as a transport medium. The driver can be enabled with
the following configs
CONFIG_VIRTIO
CONFIG_DM_RNG
CONFIG_VIRTIO_PCI
CONFIG_VIRTIO_RNG
Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
Changes since V4:
* Return number of bytes read on a successful read, and a -ve value on
error.
drivers/virtio/Kconfig | 6 +++
drivers/virtio/Makefile | 1 +
drivers/virtio/virtio-uclass.c | 1 +
drivers/virtio/virtio_rng.c | 85 ++++++++++++++++++++++++++++++++++++++++++
include/virtio.h | 4 +-
5 files changed, 96 insertions(+), 1 deletion(-)
create mode 100644 drivers/virtio/virtio_rng.c
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index a9d5fd0..2e3dd3b 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -59,4 +59,10 @@ config VIRTIO_BLK
This is the virtual block driver for virtio. It can be used with
QEMU based targets.
+config VIRTIO_RNG
+ bool "virtio rng driver"
+ depends on VIRTIO
+ help
+ This is the virtual random number generator driver. It can be used
+ with Qemu based targets.
endmenu
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 4579044..dc88809 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci_legacy.o virtio_pci_modern.o
obj-$(CONFIG_VIRTIO_SANDBOX) += virtio_sandbox.o
obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
obj-$(CONFIG_VIRTIO_BLK) += virtio_blk.o
+obj-$(CONFIG_VIRTIO_RNG) += virtio_rng.o
diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c
index 34397d7..436faa4 100644
--- a/drivers/virtio/virtio-uclass.c
+++ b/drivers/virtio/virtio-uclass.c
@@ -24,6 +24,7 @@
static const char *const virtio_drv_name[VIRTIO_ID_MAX_NUM] = {
[VIRTIO_ID_NET] = VIRTIO_NET_DRV_NAME,
[VIRTIO_ID_BLOCK] = VIRTIO_BLK_DRV_NAME,
+ [VIRTIO_ID_RNG] = VIRTIO_RNG_DRV_NAME,
};
int virtio_get_config(struct udevice *vdev, unsigned int offset,
diff --git a/drivers/virtio/virtio_rng.c b/drivers/virtio/virtio_rng.c
new file mode 100644
index 0000000..fda8d04
--- /dev/null
+++ b/drivers/virtio/virtio_rng.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <rng.h>
+#include <virtio_types.h>
+#include <virtio.h>
+#include <virtio_ring.h>
+
+struct virtio_rng_priv {
+ struct virtqueue *rng_vq;
+};
+
+static int virtio_rng_read(struct udevice *dev, void *data, size_t len)
+{
+ int ret;
+ unsigned int rsize = 0;
+ struct virtio_sg sg;
+ struct virtio_sg *sgs[1];
+ struct virtio_rng_priv *priv = dev_get_priv(dev);
+
+ /*
+ * Only INT_MAX number of bytes can be returned. If more
+ * bytes need to be read, the caller must do it in a loop.
+ */
+ if (len > INT_MAX)
+ len = INT_MAX;
+
+ sg.addr = data;
+ sg.length = len;
+ sgs[0] = &sg;
+
+ ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
+ if (ret)
+ return ret;
+
+ virtqueue_kick(priv->rng_vq);
+
+ while (!virtqueue_get_buf(priv->rng_vq, &rsize))
+ ;
+
+ return rsize;
+}
+
+static int virtio_rng_bind(struct udevice *dev)
+{
+ struct virtio_dev_priv *uc_priv = dev_get_uclass_priv(dev->parent);
+
+ /* Indicate what driver features we support */
+ virtio_driver_features_init(uc_priv, NULL, 0, NULL, 0);
+
+ return 0;
+}
+
+static int virtio_rng_probe(struct udevice *dev)
+{
+ struct virtio_rng_priv *priv = dev_get_priv(dev);
+ int ret;
+
+ ret = virtio_find_vqs(dev, 1, &priv->rng_vq);
+ if (ret < 0) {
+ debug("%s: virtio_find_vqs failed\n", __func__);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct dm_rng_ops virtio_rng_ops = {
+ .read = virtio_rng_read,
+};
+
+U_BOOT_DRIVER(virtio_rng) = {
+ .name = VIRTIO_RNG_DRV_NAME,
+ .id = UCLASS_RNG,
+ .bind = virtio_rng_bind,
+ .probe = virtio_rng_probe,
+ .remove = virtio_reset,
+ .ops = &virtio_rng_ops,
+ .priv_auto_alloc_size = sizeof(struct virtio_rng_priv),
+ .flags = DM_FLAG_ACTIVE_DMA,
+};
diff --git a/include/virtio.h b/include/virtio.h
index 654fdf1..561dcc3 100644
--- a/include/virtio.h
+++ b/include/virtio.h
@@ -22,10 +22,12 @@
#define VIRTIO_ID_NET 1 /* virtio net */
#define VIRTIO_ID_BLOCK 2 /* virtio block */
-#define VIRTIO_ID_MAX_NUM 3
+#define VIRTIO_ID_RNG 4 /* virtio rng */
+#define VIRTIO_ID_MAX_NUM 5
#define VIRTIO_NET_DRV_NAME "virtio-net"
#define VIRTIO_BLK_DRV_NAME "virtio-blk"
+#define VIRTIO_RNG_DRV_NAME "virtio-rng"
/* Status byte for guest to report progress, and synchronize features */
--
2.7.4
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 8/8] virtio: rng: Add a random number generator(rng) driver
2019-12-26 17:25 ` [PATCH v5 8/8] virtio: rng: Add a random number generator(rng) driver Sughosh Ganu
@ 2019-12-27 7:12 ` Heinrich Schuchardt
2019-12-27 11:29 ` Sughosh Ganu
0 siblings, 1 reply; 22+ messages in thread
From: Heinrich Schuchardt @ 2019-12-27 7:12 UTC (permalink / raw)
To: u-boot
On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> Add a driver for the virtio-rng device on the qemu platform. The
> device uses pci as a transport medium. The driver can be enabled with
> the following configs
>
> CONFIG_VIRTIO
> CONFIG_DM_RNG
> CONFIG_VIRTIO_PCI
> CONFIG_VIRTIO_RNG
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since V4:
> * Return number of bytes read on a successful read, and a -ve value on
> error.
>
> drivers/virtio/Kconfig | 6 +++
> drivers/virtio/Makefile | 1 +
> drivers/virtio/virtio-uclass.c | 1 +
> drivers/virtio/virtio_rng.c | 85 ++++++++++++++++++++++++++++++++++++++++++
> include/virtio.h | 4 +-
> 5 files changed, 96 insertions(+), 1 deletion(-)
> create mode 100644 drivers/virtio/virtio_rng.c
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index a9d5fd0..2e3dd3b 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -59,4 +59,10 @@ config VIRTIO_BLK
> This is the virtual block driver for virtio. It can be used with
> QEMU based targets.
>
> +config VIRTIO_RNG
> + bool "virtio rng driver"
> + depends on VIRTIO
> + help
> + This is the virtual random number generator driver. It can be used
> + with Qemu based targets.
> endmenu
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index 4579044..dc88809 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIRTIO_PCI) += virtio_pci_legacy.o virtio_pci_modern.o
> obj-$(CONFIG_VIRTIO_SANDBOX) += virtio_sandbox.o
> obj-$(CONFIG_VIRTIO_NET) += virtio_net.o
> obj-$(CONFIG_VIRTIO_BLK) += virtio_blk.o
> +obj-$(CONFIG_VIRTIO_RNG) += virtio_rng.o
> diff --git a/drivers/virtio/virtio-uclass.c b/drivers/virtio/virtio-uclass.c
> index 34397d7..436faa4 100644
> --- a/drivers/virtio/virtio-uclass.c
> +++ b/drivers/virtio/virtio-uclass.c
> @@ -24,6 +24,7 @@
> static const char *const virtio_drv_name[VIRTIO_ID_MAX_NUM] = {
> [VIRTIO_ID_NET] = VIRTIO_NET_DRV_NAME,
> [VIRTIO_ID_BLOCK] = VIRTIO_BLK_DRV_NAME,
> + [VIRTIO_ID_RNG] = VIRTIO_RNG_DRV_NAME,
> };
>
> int virtio_get_config(struct udevice *vdev, unsigned int offset,
> diff --git a/drivers/virtio/virtio_rng.c b/drivers/virtio/virtio_rng.c
> new file mode 100644
> index 0000000..fda8d04
> --- /dev/null
> +++ b/drivers/virtio/virtio_rng.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <rng.h>
> +#include <virtio_types.h>
> +#include <virtio.h>
> +#include <virtio_ring.h>
> +
> +struct virtio_rng_priv {
> + struct virtqueue *rng_vq;
> +};
> +
> +static int virtio_rng_read(struct udevice *dev, void *data, size_t len)
> +{
> + int ret;
> + unsigned int rsize = 0;
> + struct virtio_sg sg;
> + struct virtio_sg *sgs[1];
> + struct virtio_rng_priv *priv = dev_get_priv(dev);
> +
> + /*
> + * Only INT_MAX number of bytes can be returned. If more
> + * bytes need to be read, the caller must do it in a loop.
> + */
> + if (len > INT_MAX)
> + len = INT_MAX;
> +
> + sg.addr = data;
> + sg.length = len;
> + sgs[0] = &sg;
> +
> + ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
> + if (ret)
> + return ret;
> +
> + virtqueue_kick(priv->rng_vq);
> +
> + while (!virtqueue_get_buf(priv->rng_vq, &rsize))
This code assumes that data is 4 byte aligned and may lead to a crash
due to missing alignment on ARM in function le32_to_cpu().
Best regards
Heinrich
> + ;
> +
> + return rsize;
> +}
> +
> +static int virtio_rng_bind(struct udevice *dev)
> +{
> + struct virtio_dev_priv *uc_priv = dev_get_uclass_priv(dev->parent);
> +
> + /* Indicate what driver features we support */
> + virtio_driver_features_init(uc_priv, NULL, 0, NULL, 0);
> +
> + return 0;
> +}
> +
> +static int virtio_rng_probe(struct udevice *dev)
> +{
> + struct virtio_rng_priv *priv = dev_get_priv(dev);
> + int ret;
> +
> + ret = virtio_find_vqs(dev, 1, &priv->rng_vq);
> + if (ret < 0) {
> + debug("%s: virtio_find_vqs failed\n", __func__);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct dm_rng_ops virtio_rng_ops = {
> + .read = virtio_rng_read,
> +};
> +
> +U_BOOT_DRIVER(virtio_rng) = {
> + .name = VIRTIO_RNG_DRV_NAME,
> + .id = UCLASS_RNG,
> + .bind = virtio_rng_bind,
> + .probe = virtio_rng_probe,
> + .remove = virtio_reset,
> + .ops = &virtio_rng_ops,
> + .priv_auto_alloc_size = sizeof(struct virtio_rng_priv),
> + .flags = DM_FLAG_ACTIVE_DMA,
> +};
> diff --git a/include/virtio.h b/include/virtio.h
> index 654fdf1..561dcc3 100644
> --- a/include/virtio.h
> +++ b/include/virtio.h
> @@ -22,10 +22,12 @@
>
> #define VIRTIO_ID_NET 1 /* virtio net */
> #define VIRTIO_ID_BLOCK 2 /* virtio block */
> -#define VIRTIO_ID_MAX_NUM 3
> +#define VIRTIO_ID_RNG 4 /* virtio rng */
> +#define VIRTIO_ID_MAX_NUM 5
>
> #define VIRTIO_NET_DRV_NAME "virtio-net"
> #define VIRTIO_BLK_DRV_NAME "virtio-blk"
> +#define VIRTIO_RNG_DRV_NAME "virtio-rng"
>
> /* Status byte for guest to report progress, and synchronize features */
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 8/8] virtio: rng: Add a random number generator(rng) driver
2019-12-27 7:12 ` Heinrich Schuchardt
@ 2019-12-27 11:29 ` Sughosh Ganu
2019-12-27 12:27 ` Heinrich Schuchardt
0 siblings, 1 reply; 22+ messages in thread
From: Sughosh Ganu @ 2019-12-27 11:29 UTC (permalink / raw)
To: u-boot
On Fri, 27 Dec 2019 at 12:42, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:
> On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> > Add a driver for the virtio-rng device on the qemu platform. The
> > device uses pci as a transport medium. The driver can be enabled with
> > the following configs
> >
> > CONFIG_VIRTIO
> > CONFIG_DM_RNG
> > CONFIG_VIRTIO_PCI
> > CONFIG_VIRTIO_RNG
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > Changes since V4:
> > * Return number of bytes read on a successful read, and a -ve value on
> > error.
> >
> > drivers/virtio/Kconfig | 6 +++
> > drivers/virtio/Makefile | 1 +
> > drivers/virtio/virtio-uclass.c | 1 +
> > drivers/virtio/virtio_rng.c | 85
> ++++++++++++++++++++++++++++++++++++++++++
> > include/virtio.h | 4 +-
> > 5 files changed, 96 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/virtio/virtio_rng.c
> >
>
<snip>
> > diff --git a/drivers/virtio/virtio_rng.c b/drivers/virtio/virtio_rng.c
> > new file mode 100644
> > index 0000000..fda8d04
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_rng.c
> > @@ -0,0 +1,85 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2019, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <rng.h>
> > +#include <virtio_types.h>
> > +#include <virtio.h>
> > +#include <virtio_ring.h>
> > +
> > +struct virtio_rng_priv {
> > + struct virtqueue *rng_vq;
> > +};
> > +
> > +static int virtio_rng_read(struct udevice *dev, void *data, size_t len)
> > +{
> > + int ret;
> > + unsigned int rsize = 0;
> > + struct virtio_sg sg;
> > + struct virtio_sg *sgs[1];
> > + struct virtio_rng_priv *priv = dev_get_priv(dev);
> > +
> > + /*
> > + * Only INT_MAX number of bytes can be returned. If more
> > + * bytes need to be read, the caller must do it in a loop.
> > + */
> > + if (len > INT_MAX)
> > + len = INT_MAX;
> > +
> > + sg.addr = data;
> > + sg.length = len;
> > + sgs[0] = &sg;
> > +
> > + ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
> > + if (ret)
> > + return ret;
> > +
> > + virtqueue_kick(priv->rng_vq);
> > +
> > + while (!virtqueue_get_buf(priv->rng_vq, &rsize))
>
> This code assumes that data is 4 byte aligned and may lead to a crash
> due to missing alignment on ARM in function le32_to_cpu().
>
Are you referring to the virtqueue_get_buf function. Not sure I understand.
In any case, I will be sending an updated version which moves the loop into
the driver, as you had commented on my other patch.
-sughosh
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v5 8/8] virtio: rng: Add a random number generator(rng) driver
2019-12-27 11:29 ` Sughosh Ganu
@ 2019-12-27 12:27 ` Heinrich Schuchardt
0 siblings, 0 replies; 22+ messages in thread
From: Heinrich Schuchardt @ 2019-12-27 12:27 UTC (permalink / raw)
To: u-boot
On 12/27/19 12:29 PM, Sughosh Ganu wrote:
>
> On Fri, 27 Dec 2019 at 12:42, Heinrich Schuchardt <xypron.glpk@gmx.de
> <mailto:xypron.glpk@gmx.de>> wrote:
>
> On 12/26/19 6:25 PM, Sughosh Ganu wrote:
> > Add a driver for the virtio-rng device on the qemu platform. The
> > device uses pci as a transport medium. The driver can be enabled with
> > the following configs
> >
> > CONFIG_VIRTIO
> > CONFIG_DM_RNG
> > CONFIG_VIRTIO_PCI
> > CONFIG_VIRTIO_RNG
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org
> <mailto:sughosh.ganu@linaro.org>>
> > ---
> > Changes since V4:
> > * Return number of bytes read on a successful read, and a -ve
> value on
> > error.
> >
> > drivers/virtio/Kconfig | 6 +++
> > drivers/virtio/Makefile | 1 +
> > drivers/virtio/virtio-uclass.c | 1 +
> > drivers/virtio/virtio_rng.c | 85
> ++++++++++++++++++++++++++++++++++++++++++
> > include/virtio.h | 4 +-
> > 5 files changed, 96 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/virtio/virtio_rng.c
> >
>
>
> <snip>
>
> > diff --git a/drivers/virtio/virtio_rng.c
> b/drivers/virtio/virtio_rng.c
> > new file mode 100644
> > index 0000000..fda8d04
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_rng.c
> > @@ -0,0 +1,85 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2019, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <rng.h>
> > +#include <virtio_types.h>
> > +#include <virtio.h>
> > +#include <virtio_ring.h>
> > +
> > +struct virtio_rng_priv {
> > + struct virtqueue *rng_vq;
> > +};
> > +
> > +static int virtio_rng_read(struct udevice *dev, void *data,
> size_t len)
> > +{
> > + int ret;
> > + unsigned int rsize = 0;
> > + struct virtio_sg sg;
> > + struct virtio_sg *sgs[1];
> > + struct virtio_rng_priv *priv = dev_get_priv(dev);
> > +
> > + /*
> > + * Only INT_MAX number of bytes can be returned. If more
> > + * bytes need to be read, the caller must do it in a loop.
> > + */
> > + if (len > INT_MAX)
> > + len = INT_MAX;
> > +
> > + sg.addr = data;
> > + sg.length = len;
> > + sgs[0] = &sg;
> > +
> > + ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
> > + if (ret)
> > + return ret;
> > +
> > + virtqueue_kick(priv->rng_vq);
> > +
> > + while (!virtqueue_get_buf(priv->rng_vq, &rsize))
>
> This code assumes that data is 4 byte aligned and may lead to a crash
> due to missing alignment on ARM in function le32_to_cpu().
>
>
> Are you referring to the virtqueue_get_buf function. Not sure I
> understand. In any case, I will be sending an updated version which
> moves the loop into the driver, as you had commented on my other patch.
virtqueue_get_buf() calls virtio32_to_cpu() which calls le32_to_cpu()
which assumes that the destination is properly aligned.
Best regards
Heinrich
^ permalink raw reply [flat|nested] 22+ messages in thread