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

[Heinrich Schuchardt]

On 12/27/19 12:29 PM, Sughosh Ganu wrote:
>
> On Fri, 27 Dec 2019 at 12:42, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at 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 at linaro.org
>     <mailto:sughosh.ganu at 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