Re: [PATCH v4 8/8] virtio: rng: Add a random number generator(rng) driver
On Wed, 25 Dec 2019 at 07:00, Heinrich Schuchardt
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
> >> ---
> >> 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 000..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
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +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
Re: [PATCH v4 8/8] virtio: rng: Add a random number generator(rng) driver
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
---
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 000..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
+#include
+#include
+#include
+#include
+#include
+
+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/
Re: [PATCH v4 8/8] virtio: rng: Add a random number generator(rng) driver
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
---
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 000..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
+#include
+#include
+#include
+#include
+#include
+
+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 8/8] virtio: rng: Add a random number generator(rng) driver
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
---
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 000..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
+#include
+#include
+#include
+#include
+#include
+
+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_BLOCK2 /* 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
