[PATCH] virtio: rng: gracefully handle 0 byte returns

[Simon Glass]

Hi Andre,

On Tue, 7 Nov 2023 at 09:09, Andre Przywara <andre.przywara at arm.com> wrote:
>
> According to the virtio v1.x "entropy device" specification, a virtio-rng
> device is supposed to always return at least one byte of entropy.
> However the virtio v0.9 spec does not mention such a requirement.
>
> The Arm Fixed Virtual Platform (FVP) implementation of virtio-rng always
> returns 8 bytes less of entropy than requested. If 8 bytes or less are
> requested, it will return 0 bytes.
> This behaviour makes U-Boot's virtio_rng_read() implementation go into an
> endless loop, hanging the system.
>
> Work around this problem by always requesting 8 bytes more than needed,
> but only if a previous call to virtqueue_get_buf() returned 0 bytes.
>
> This should never trigger on a v1.x spec compliant implementation, but
> fixes the hang on the Arm FVP.
>
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> Reported-by: Peter Hoyes <peter.hoyes at arm.com>
> ---
>  drivers/virtio/virtio_rng.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Unrelated to this patch, but do you know the hardware architecture of
the ARM RNG? Is there one RNG unit per CPU or one for the whole SoC?

>
> diff --git a/drivers/virtio/virtio_rng.c b/drivers/virtio/virtio_rng.c
> index b85545c2ee5..786359a6e36 100644
> --- a/drivers/virtio/virtio_rng.c
> +++ b/drivers/virtio/virtio_rng.c
> @@ -20,7 +20,7 @@ struct virtio_rng_priv {
>  static int virtio_rng_read(struct udevice *dev, void *data, size_t len)
>  {
>         int ret;
> -       unsigned int rsize;
> +       unsigned int rsize = 1;
>         unsigned char buf[BUFFER_SIZE] __aligned(4);
>         unsigned char *ptr = data;
>         struct virtio_sg sg;
> @@ -29,7 +29,12 @@ static int virtio_rng_read(struct udevice *dev, void *data, size_t len)
>
>         while (len) {
>                 sg.addr = buf;
> -               sg.length = min(len, sizeof(buf));
> +               /*
> +                * Work around implementations which always return 8 bytes
> +                * less than requested, down to 0 bytes, which would
> +                * cause an endless loop otherwise.
> +                */
> +               sg.length = min(rsize ? len : len + 8, sizeof(buf));
>                 sgs[0] = &sg;
>
>                 ret = virtqueue_add(priv->rng_vq, sgs, 0, 1);
> --
> 2.25.1
>

Regards,
Simon