[PATCH v4 0/8] Add a random number generator uclass

[PATCH v4 0/8] Add a random number generator uclass

[Sughosh Ganu]

Add a random number generator(rng) uclass to facilitate adding drivers
for rng devices. I plan to add an implementation of the
EFI_RNG_PROTOCOL, which would get the random number from the rng
uclass -- the protocol would be used by the efi stub for getting a
random number for the kaslr feature.

The patch series also adds a driver for the rng device found on the
stm32mp1 and qemu platforms. A dummy rng driver for sandbox has also
been added, along with the unit test for the rng uclass.

Changes since V3:
* Handle review comments from Patrick Delaunay

Changes since V2:
* Add a driver for the virtio-rng device on qemu platform

Changes since V1:
* Add a SPDX header in rng.h
* Change the UCLASS_DRIVER name from hwrng to rng, consistent with the
  rest of the naming convention
* Handle review comment from Patrice Chotard


Sughosh Ganu (8):
  dm: rng: Add random number generator(rng) uclass
  clk: stm32mp1: Add a clock entry for RNG1 device
  stm32mp1: rng: Add a driver for random number generator(rng) device
  configs: stm32mp15: Enable random number generator(rng) device
  sandbox: rng: Add a random number generator(rng) driver
  configs: sandbox: Enable random number generator(rng) device
  test: rng: Add basic test for random number generator(rng) uclass
  virtio: rng: Add a random number generator(rng) driver

 arch/sandbox/dts/test.dts           |   4 +
 configs/sandbox64_defconfig         |   2 +
 configs/sandbox_defconfig           |   2 +
 configs/stm32mp15_basic_defconfig   |   2 +
 configs/stm32mp15_optee_defconfig   |   2 +
 configs/stm32mp15_trusted_defconfig |   2 +
 drivers/Kconfig                     |   2 +
 drivers/Makefile                    |   1 +
 drivers/clk/clk_stm32mp1.c          |   1 +
 drivers/rng/Kconfig                 |  21 +++++
 drivers/rng/Makefile                |   8 ++
 drivers/rng/rng-uclass.c            |  23 ++++++
 drivers/rng/sandbox_rng.c           |  39 +++++++++
 drivers/rng/stm32mp1_rng.c          | 157 ++++++++++++++++++++++++++++++++++++
 drivers/virtio/Kconfig              |   6 ++
 drivers/virtio/Makefile             |   1 +
 drivers/virtio/virtio-uclass.c      |   1 +
 drivers/virtio/virtio_rng.c         |  72 +++++++++++++++++
 include/dm/uclass-id.h              |   1 +
 include/rng.h                       |  32 ++++++++
 include/virtio.h                    |   4 +-
 test/dm/Makefile                    |   1 +
 test/dm/rng.c                       |  28 +++++++
 23 files changed, 411 insertions(+), 1 deletion(-)
 create mode 100644 drivers/rng/Kconfig
 create mode 100644 drivers/rng/Makefile
 create mode 100644 drivers/rng/rng-uclass.c
 create mode 100644 drivers/rng/sandbox_rng.c
 create mode 100644 drivers/rng/stm32mp1_rng.c
 create mode 100644 drivers/virtio/virtio_rng.c
 create mode 100644 include/rng.h
 create mode 100644 test/dm/rng.c

-- 
2.7.4

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

[Sughosh Ganu]

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 V3:
 Handle review comments from Patrick Delaunay.

 drivers/Kconfig          |  2 ++
 drivers/Makefile         |  1 +
 drivers/rng/Kconfig      |  7 +++++++
 drivers/rng/Makefile     |  6 ++++++
 drivers/rng/rng-uclass.c | 23 +++++++++++++++++++++++
 include/dm/uclass-id.h   |  1 +
 include/rng.h            | 32 ++++++++++++++++++++++++++++++++
 7 files changed, 72 insertions(+)
 create mode 100644 drivers/rng/Kconfig
 create mode 100644 drivers/rng/Makefile
 create mode 100644 drivers/rng/rng-uclass.c
 create mode 100644 include/rng.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9d99ce0..e34a227 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -90,6 +90,8 @@ source "drivers/remoteproc/Kconfig"
 
 source "drivers/reset/Kconfig"
 
+source "drivers/rng/Kconfig"
+
 source "drivers/rtc/Kconfig"
 
 source "drivers/scsi/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index e977f19..6c619b1 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -115,4 +115,5 @@ obj-$(CONFIG_W1_EEPROM) += w1-eeprom/
 
 obj-$(CONFIG_MACH_PIC32) += ddr/microchip/
 obj-$(CONFIG_DM_HWSPINLOCK) += hwspinlock/
+obj-$(CONFIG_DM_RNG) += rng/
 endif
diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
new file mode 100644
index 0000000..dd44cc0
--- /dev/null
+++ b/drivers/rng/Kconfig
@@ -0,0 +1,7 @@
+config DM_RNG
+	bool "Driver support for Random Number Generator devices"
+	depends on DM
+	help
+	  Enable driver model for random number generator(rng) devices.
+	  This interface is used to initialise the rng device and to
+	  read the random seed from the device.
diff --git a/drivers/rng/Makefile b/drivers/rng/Makefile
new file mode 100644
index 0000000..311705b
--- /dev/null
+++ b/drivers/rng/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) 2019, Linaro Limited
+#
+
+obj-$(CONFIG_DM_RNG) += rng-uclass.o
diff --git a/drivers/rng/rng-uclass.c b/drivers/rng/rng-uclass.c
new file mode 100644
index 0000000..b6af3b8
--- /dev/null
+++ b/drivers/rng/rng-uclass.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <rng.h>
+
+int dm_rng_read(struct udevice *dev, void *buffer, size_t size)
+{
+	const struct dm_rng_ops *ops = device_get_ops(dev);
+
+	if (!ops->read)
+		return -ENOSYS;
+
+	return ops->read(dev, buffer, size);
+}
+
+UCLASS_DRIVER(rng) = {
+	.name = "rng",
+	.id = UCLASS_RNG,
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0c563d8..192202d 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -86,6 +86,7 @@ enum uclass_id {
 	UCLASS_REGULATOR,	/* Regulator device */
 	UCLASS_REMOTEPROC,	/* Remote Processor device */
 	UCLASS_RESET,		/* Reset controller device */
+	UCLASS_RNG,		/* Random Number Generator */
 	UCLASS_RTC,		/* Real time clock device */
 	UCLASS_SCSI,		/* SCSI device */
 	UCLASS_SERIAL,		/* Serial UART */
diff --git a/include/rng.h b/include/rng.h
new file mode 100644
index 0000000..80891fc
--- /dev/null
+++ b/include/rng.h
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#if !defined _RNG_H_
+#define _RNG_H_
+
+#include <dm.h>
+
+/**
+ * dm_rng_read() - read a random number seed from the rng device
+ * @buffer:	input buffer to put the read random seed into
+ * @size:	number of bytes of random seed read
+ * @return 0 if OK, -ve on error
+ *
+ */
+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 0 if OK, -ve on error
+	 */
+	int (*read)(struct udevice *dev, void *data, size_t max);
+};
+
+#endif /* _RNG_H_ */
-- 
2.7.4

[PATCH v4 2/8] clk: stm32mp1: Add a clock entry for RNG1 device

[Sughosh Ganu]

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

[PATCH v4 3/8] stm32mp1: rng: Add a driver for random number generator(rng) device

[Sughosh Ganu]

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 V3:
 Return 0 for a successful read.

 drivers/rng/Kconfig        |   7 ++
 drivers/rng/Makefile       |   1 +
 drivers/rng/stm32mp1_rng.c | 157 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 165 insertions(+)
 create mode 100644 drivers/rng/stm32mp1_rng.c

diff --git a/drivers/rng/Kconfig b/drivers/rng/Kconfig
index dd44cc0..5fc11db 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..6ef8f16
--- /dev/null
+++ b/drivers/rng/stm32mp1_rng.c
@@ -0,0 +1,157 @@
+// 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;
+	u32 sr, count, reg;
+	size_t increment;
+	struct stm32_rng_platdata *pdata = dev_get_platdata(dev);
+
+	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, &reg, min(len, sizeof(u32)));
+			increment = min(len, sizeof(u32));
+			data += increment;
+			len -= increment;
+			count--;
+		}
+	}
+
+	return 0;
+}
+
+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

[PATCH v4 4/8] configs: stm32mp15: Enable random number generator(rng) device

[Sughosh Ganu]

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 358c2cd..5ca40e0 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 a065d3e..b161cd1 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 632f11f..ddf244d 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

[PATCH v4 5/8] sandbox: rng: Add a random number generator(rng) driver

[Sughosh Ganu]

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 V3:
 Handle review comments from Patrick Delaunay.

 arch/sandbox/dts/test.dts |  4 ++++
 drivers/rng/Kconfig       |  7 +++++++
 drivers/rng/Makefile      |  1 +
 drivers/rng/sandbox_rng.c | 39 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 51 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 5fc11db..3a1d3f0 100644
--- a/drivers/rng/Kconfig
+++ b/drivers/rng/Kconfig
@@ -6,6 +6,13 @@ 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
+       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..8262e0d
--- /dev/null
+++ b/drivers/rng/sandbox_rng.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019, Linaro Limited
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <rng.h>
+
+static unsigned long random = 0xdeadbeef;
+
+static int sandbox_rng_read(struct udevice *dev, void *data, size_t len)
+{
+	if (len != sizeof(random))
+		return -EINVAL;
+
+	random ^= ~0UL;
+	*(unsigned long *)data = random;
+
+	return 0;
+}
+
+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

[PATCH v4 6/8] configs: sandbox: Enable random number generator(rng) device

[Sughosh Ganu]

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

[PATCH v4 7/8] test: rng: Add basic test for random number generator(rng) uclass

[Sughosh Ganu]

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 V3:
 Handle review comments from Patrick Delaunay.

 test/dm/Makefile |  1 +
 test/dm/rng.c    | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 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..d533cf6
--- /dev/null
+++ b/test/dm/rng.c
@@ -0,0 +1,28 @@
+// 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_assertok(dm_rng_read(dev, &rand1, sizeof(rand1)));
+	ut_assertok(dm_rng_read(dev, &rand2, sizeof(rand2)));
+	ut_assert(rand1 != rand2);
+	ut_asserteq(0x21524110, rand1);
+	ut_asserteq(0xdeadbeef, rand2);
+
+	return 0;
+}
+DM_TEST(dm_test_rng_read, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
-- 
2.7.4

[PATCH v4 8/8] virtio: rng: Add a random number generator(rng) driver

[Sughosh Ganu]

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>
---
 drivers/virtio/Kconfig         |  6 ++++
 drivers/virtio/Makefile        |  1 +
 drivers/virtio/virtio-uclass.c |  1 +
 drivers/virtio/virtio_rng.c    | 72 ++++++++++++++++++++++++++++++++++++++++++
 include/virtio.h               |  4 ++-
 5 files changed, 83 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..19a0cc1
--- /dev/null
+++ b/drivers/virtio/virtio_rng.c
@@ -0,0 +1,72 @@
+// 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)
+{
+	struct virtio_sg sg = { data, len };
+	struct virtio_sg *sgs[] = { &sg };
+	struct virtio_rng_priv *priv = dev_get_priv(dev);
+	unsigned int rsize;
+	int ret;
+
+	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)
+		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

[PATCH v4 8/8] virtio: rng: Add a random number generator(rng) driver

[Heinrich Schuchardt]

On 12/17/19 12:52 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>
> ---
>   drivers/virtio/Kconfig         |  6 ++++
>   drivers/virtio/Makefile        |  1 +
>   drivers/virtio/virtio-uclass.c |  1 +
>   drivers/virtio/virtio_rng.c    | 72 ++++++++++++++++++++++++++++++++++++++++++
>   include/virtio.h               |  4 ++-
>   5 files changed, 83 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..19a0cc1
> --- /dev/null
> +++ b/drivers/virtio/virtio_rng.c
> @@ -0,0 +1,72 @@
> +// 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)
> +{
> +	struct virtio_sg sg = { data, len };
> +	struct virtio_sg *sgs[] = { &sg };
> +	struct virtio_rng_priv *priv = dev_get_priv(dev);
> +	unsigned int rsize;
> +	int ret;
> +
> +	ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
> +	if (ret)

Please, add a debug statement for each error path, e.g.

		debug("%s: virtqueue_add() failed\n", __func__)

> +		return ret;
> +
> +	virtqueue_kick(priv->rng_vq);
> +
> +	while (!virtqueue_get_buf(priv->rng_vq, &rsize))
> +		;
> +
> +	return rsize;

The return value is inconsistent with sandbox_rng_read() and
stm32_rng_read(). Both return 0 on success.

With "return 0;" the driver works fine.

> +}
> +
> +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)

Add a debug statement here, please, e.g.

		debug("%s: virtio_find_vqs() failed\n", __func__)

Best regards

Heinrich

> +		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 */
>
>

[PATCH v4 5/8] sandbox: rng: Add a random number generator(rng) driver

[Heinrich Schuchardt]

On 12/17/19 12:52 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 V3:
>   Handle review comments from Patrick Delaunay.
>
>   arch/sandbox/dts/test.dts |  4 ++++
>   drivers/rng/Kconfig       |  7 +++++++
>   drivers/rng/Makefile      |  1 +
>   drivers/rng/sandbox_rng.c | 39 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 51 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 5fc11db..3a1d3f0 100644
> --- a/drivers/rng/Kconfig
> +++ b/drivers/rng/Kconfig
> @@ -6,6 +6,13 @@ 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
> +       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..8262e0d
> --- /dev/null
> +++ b/drivers/rng/sandbox_rng.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <rng.h>
> +
> +static unsigned long random = 0xdeadbeef;
> +
> +static int sandbox_rng_read(struct udevice *dev, void *data, size_t len)
> +{
> +	if (len != sizeof(random))
> +		return -EINVAL;

This will drop on our feet once we use the RNG class for anything real.

Just imagine an EFI_RNG_PROTOCOL test failing on the sandbox because we
cannot read 128 bit.

Can't we use a PRNG here and reinitialize it when len == ~0UL?

Functions rand() and srand() are good enough for the job.

Best regards

Heinrich

> +
> +	random ^= ~0UL;
> +	*(unsigned long *)data = random;
> +
> +	return 0;
> +}
> +
> +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,
> +};
>

[PATCH v4 5/8] sandbox: rng: Add a random number generator(rng) driver

[Heinrich Schuchardt]

Resending with Cc Tom, Simon, Ilias

On 12/17/19 12:52 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 V3:
>   Handle review comments from Patrick Delaunay.
>
>   arch/sandbox/dts/test.dts |  4 ++++
>   drivers/rng/Kconfig       |  7 +++++++
>   drivers/rng/Makefile      |  1 +
>   drivers/rng/sandbox_rng.c | 39 +++++++++++++++++++++++++++++++++++++++
>   4 files changed, 51 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 5fc11db..3a1d3f0 100644
> --- a/drivers/rng/Kconfig
> +++ b/drivers/rng/Kconfig
> @@ -6,6 +6,13 @@ 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
> +       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..8262e0d
> --- /dev/null
> +++ b/drivers/rng/sandbox_rng.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <rng.h>
> +
> +static unsigned long random = 0xdeadbeef;
> +
> +static int sandbox_rng_read(struct udevice *dev, void *data, size_t len)
> +{
> +	if (len != sizeof(random))
> +		return -EINVAL;

This will drop on our feet once we use the RNG class for anything real.

Just imagine an EFI_RNG_PROTOCOL test failing on the sandbox because we
cannot read 128 bit.

Can't we use a PRNG here and reinitialize it when len == ~0UL?

Functions rand() and srand() are good enough for the job.

Best regards

Heinrich

> +
> +	random ^= ~0UL;
> +	*(unsigned long *)data = random;
> +
> +	return 0;
> +}
> +
> +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,
> +};
>

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

[Heinrich Schuchardt]

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

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

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

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

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

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

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

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

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

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

Best regards

Heinrich

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

[PATCH v4 8/8] virtio: rng: Add a random number generator(rng) driver

[Heinrich Schuchardt]

On 12/24/19 10:39 PM, Heinrich Schuchardt wrote:
> On 12/17/19 12:52 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>
>> ---
>>   drivers/virtio/Kconfig         |  6 ++++
>>   drivers/virtio/Makefile        |  1 +
>>   drivers/virtio/virtio-uclass.c |  1 +
>>   drivers/virtio/virtio_rng.c    | 72
>> ++++++++++++++++++++++++++++++++++++++++++
>>   include/virtio.h               |  4 ++-
>>   5 files changed, 83 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..19a0cc1
>> --- /dev/null
>> +++ b/drivers/virtio/virtio_rng.c
>> @@ -0,0 +1,72 @@
>> +// 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)
>> +{
>> +    struct virtio_sg sg = { data, len };
>> +    struct virtio_sg *sgs[] = { &sg };
>> +    struct virtio_rng_priv *priv = dev_get_priv(dev);
>> +    unsigned int rsize;
>> +    int ret;
>> +
>> +    ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
>> +    if (ret)
>
> Please, add a debug statement for each error path, e.g.
>
>          debug("%s: virtqueue_add() failed\n", __func__)
>
>> +        return ret;
>> +
>> +    virtqueue_kick(priv->rng_vq);
>> +
>> +    while (!virtqueue_get_buf(priv->rng_vq, &rsize))
>> +        ;
>> +
>> +    return rsize;
>
> The return value is inconsistent with sandbox_rng_read() and
> stm32_rng_read(). Both return 0 on success.
>
> With "return 0;" the driver works fine.

I retested with
-device virtio-rng-pci,disable-legacy=on,max-bytes=16,period=1000

In this case virtqueue_get_buf() only returns a maximum of 16 bytes. All
other bytes of data remain unchanged. If the next call occurs within
1000 milli-seconds virtqueue_get_buf() blocks until the period is over.

So either you have to change the definition of the return parameter in
patch 1/8 (and change the other drivers accordingly) or you have to add
a loop to this driver that loops until len random bytes have been retrieved.

I would prefer to add a loop in the driver instead of adding a loop in
every consumer.

Best regards

Heinrich

>
>> +}
>> +
>> +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)
>
> Add a debug statement here, please, e.g.
>
>          debug("%s: virtio_find_vqs() failed\n", __func__)
>
> Best regards
>
> Heinrich
>
>> +        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 */
>>
>>
>
>

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

[Sughosh Ganu]

On Wed, 25 Dec 2019 at 06:41, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

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


Ok.


>
>
> The Linux kernel code uses the return value to return the number of
> bytes retrieved.
>
> Please, provide as description here of what will happen if the RNG
> device blocks.
>

I guess that would be device specific behavior. For example, in the
stm32mp1 rng driver, readl_poll_timeout is used to check for random data
availability, failing which, we return an error.


> > + *
> > + */
> > +int dm_rng_read(struct udevice *dev, void *buffer, size_t size);
>
> We have different places where we retrieve the first RNG device and read
> from it. Shouldn't we provide a single function in the uclass that
> returns random bytes without a udevice parameter?
>

Even in scenarios where there is no hardware rng device, i think we can
have a dummy driver, with it's read function using srand kind on function.


> Linux uses an entropy pool where the hardware RNG is one of multiple
> sources of randomness and the entropy runs through hash functions
> (chacha20, sha1) before being consumed.
>
> Shouldn't we call a hashing function in this uclass?
>

Should these functions be  part of the uclass, or should these be called
separately by the caller, in case the caller needs to run the random data
through an additional hashing function. I think that would be better than
mixing rng uclass with hashing.


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

I am not sure if we need to add this functionality in the bootloader. In
case you think this is needed, I think this can be taken up as a separate
task.

-sughosh

[PATCH v4 5/8] sandbox: rng: Add a random number generator(rng) driver

[Sughosh Ganu]

On Wed, 25 Dec 2019 at 03:44, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> Resending with Cc Tom, Simon, Ilias
>
> On 12/17/19 12:52 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 V3:
> >   Handle review comments from Patrick Delaunay.
> >
> >   arch/sandbox/dts/test.dts |  4 ++++
> >   drivers/rng/Kconfig       |  7 +++++++
> >   drivers/rng/Makefile      |  1 +
> >   drivers/rng/sandbox_rng.c | 39 +++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 51 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 5fc11db..3a1d3f0 100644
> > --- a/drivers/rng/Kconfig
> > +++ b/drivers/rng/Kconfig
> > @@ -6,6 +6,13 @@ 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
> > +       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..8262e0d
> > --- /dev/null
> > +++ b/drivers/rng/sandbox_rng.c
> > @@ -0,0 +1,39 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019, Linaro Limited
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <rng.h>
> > +
> > +static unsigned long random = 0xdeadbeef;
> > +
> > +static int sandbox_rng_read(struct udevice *dev, void *data, size_t len)
> > +{
> > +     if (len != sizeof(random))
> > +             return -EINVAL;
>
> This will drop on our feet once we use the RNG class for anything real.
>
> Just imagine an EFI_RNG_PROTOCOL test failing on the sandbox because we
> cannot read 128 bit.
>
> Can't we use a PRNG here and reinitialize it when len == ~0UL?
>
> Functions rand() and srand() are good enough for the job.
>

Have sent a V5 using srand and rand functions for returning random bytes.
Please take a look. Thanks.

-sughosh

[PATCH v4 8/8] virtio: rng: Add a random number generator(rng) driver

[Sughosh Ganu]

On Wed, 25 Dec 2019 at 07:00, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> On 12/24/19 10:39 PM, Heinrich Schuchardt wrote:
> > On 12/17/19 12:52 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>
> >> ---
> >>   drivers/virtio/Kconfig         |  6 ++++
> >>   drivers/virtio/Makefile        |  1 +
> >>   drivers/virtio/virtio-uclass.c |  1 +
> >>   drivers/virtio/virtio_rng.c    | 72
> >> ++++++++++++++++++++++++++++++++++++++++++
> >>   include/virtio.h               |  4 ++-
> >>   5 files changed, 83 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..19a0cc1
> >> --- /dev/null
> >> +++ b/drivers/virtio/virtio_rng.c
> >> @@ -0,0 +1,72 @@
> >> +// 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)
> >> +{
> >> +    struct virtio_sg sg = { data, len };
> >> +    struct virtio_sg *sgs[] = { &sg };
> >> +    struct virtio_rng_priv *priv = dev_get_priv(dev);
> >> +    unsigned int rsize;
> >> +    int ret;
> >> +
> >> +    ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
> >> +    if (ret)
> >
> > Please, add a debug statement for each error path, e.g.
> >
> >          debug("%s: virtqueue_add() failed\n", __func__)
> >
> >> +        return ret;
> >> +
> >> +    virtqueue_kick(priv->rng_vq);
> >> +
> >> +    while (!virtqueue_get_buf(priv->rng_vq, &rsize))
> >> +        ;
> >> +
> >> +    return rsize;
> >
> > The return value is inconsistent with sandbox_rng_read() and
> > stm32_rng_read(). Both return 0 on success.
> >
> > With "return 0;" the driver works fine.
>
> I retested with
> -device virtio-rng-pci,disable-legacy=on,max-bytes=16,period=1000
>
> In this case virtqueue_get_buf() only returns a maximum of 16 bytes. All
> other bytes of data remain unchanged. If the next call occurs within
> 1000 milli-seconds virtqueue_get_buf() blocks until the period is over.
>
> So either you have to change the definition of the return parameter in
> patch 1/8 (and change the other drivers accordingly) or you have to add
> a loop to this driver that loops until len random bytes have been
> retrieved.
>
> I would prefer to add a loop in the driver instead of adding a loop in
> every consumer.
>

So I tried putting a loop in the virtio rng driver, but that does not work.
I was able to get it working with the loop in the consumer function. So I
have changed the return value of the read function in all the drivers to
return the number of bytes read, instead of returning 0. I think you will
have to change your logic in the rng command patch that you had sent.

-sughosh