On Sun, 29 Dec 2019 at 02:59, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/28/19 7:28 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 V6:
Handle review comments from Heinrich Schuchardt to handle the scenario of data buffer being passed to the virtio rng driver not being aligned on 4 bytes.
drivers/virtio/Kconfig | 6 +++ drivers/virtio/Makefile | 1 + drivers/virtio/virtio-uclass.c | 1 + drivers/virtio/virtio_rng.c | 92
++++++++++++++++++++++++++++++++++++++++++
include/virtio.h | 4 +- 5 files changed, 103 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..ffb6835 --- /dev/null +++ b/drivers/virtio/virtio_rng.c @@ -0,0 +1,92 @@ +// 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>
+#define BUFFER_SIZE 16UL
+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;size_t nbytes = len;unsigned char buf[BUFFER_SIZE] __aligned(4);unsigned char *ptr = data;struct virtio_sg sg;struct virtio_sg *sgs[1];struct virtio_rng_priv *priv = dev_get_priv(dev);if (!len)return 0;do {If you use
while(nbytes) {
Ok.
here you can go without the if above. This will save 4 bytes of code on ARMv8. In U-Boot we have tight code size constraints.
sg.addr = buf;sg.length = nbytes;We must avoid buffer overruns for len > 16:
sg.length = min(nbytes, BUFFER_SIZE);
Oops. Forgot to take care of this. Will incorporate this in the next version. Also, I had sent you a unicast a couple of days back asking for the code in virtqueue_get_buf which requires a 4 byte alignment for the data pointer. The virtio32_to_cpu calls in virtqueue_get_buf are being used to reference 'id' and 'len' members of structs. I don't see the 'addr' member being used in this function. But i do see the data pointer being used in virtqueue_add function, which actually casts this to a u64 value. Can you point me to the function which actually dereferences the data pointer as a u32 pointer. Thanks a lot for your in depth review.
-sughosh