[PATCH v3 2/3] efi: qemu: arm64: Add efi_rng_protocol implementation for the platform

[Sughosh Ganu]

On Sat, 28 Dec 2019 at 20:54, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:

> On 12/28/19 4:03 PM, Sughosh Ganu wrote:
> >
> > On Sat, 28 Dec 2019 at 20:01, Heinrich Schuchardt <xypron.glpk at gmx.de
> > <mailto:xypron.glpk at gmx.de>> wrote:
> >
> >     On 12/27/19 3:26 PM, Sughosh Ganu wrote:
> >      > Add support for the EFI_RNG_PROTOCOL routines for the qemu arm64
> >      > platform. EFI_RNG_PROTOCOL is an uefi boottime service which is
> >      > invoked by the efi stub in the kernel for getting random seed for
> >      > kaslr.
> >      >
> >      > The routines are platform specific, and use the virtio-rng device
> on
> >      > the platform to get random data.
> >      >
> >      > The feature can be enabled through the following config
> >      > CONFIG_EFI_RNG_PROTOCOL
> >      >
> >      > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org
> >     <mailto:sughosh.ganu at linaro.org>>
> >      > ---
> >      > Changes since V2:
> >      > * Based on review comments from Heinrich Schuchardt, the rng
> drivers
> >      >    read all the bytes requested in the individual
> >      >    drivers. Corresponding changes made in getrng routine to
> >     remove the
> >      >    loop to read the bytes requested, since that would be handled
> >     in the
> >      >    drivers.
> >      >
> >      >   board/emulation/qemu-arm/qemu-arm.c | 41 +++++++++++++++++++
> >      >   include/efi_rng.h                   | 32 +++++++++++++++
> >      >   lib/efi_loader/Kconfig              |  8 ++++
> >      >   lib/efi_loader/Makefile             |  1 +
> >      >   lib/efi_loader/efi_rng.c            | 80
> >     +++++++++++++++++++++++++++++++++++++
> >      >   5 files changed, 162 insertions(+)
> >      >   create mode 100644 include/efi_rng.h
> >      >   create mode 100644 lib/efi_loader/efi_rng.c
> >      >
> >
> >
> > <snip>
> >
> >      > diff --git a/lib/efi_loader/efi_rng.c b/lib/efi_loader/efi_rng.c
> >      > new file mode 100644
> >      > index 0000000..eb91aa7
> >      > --- /dev/null
> >      > +++ b/lib/efi_loader/efi_rng.c
> >      > @@ -0,0 +1,80 @@
> >      > +// SPDX-License-Identifier: GPL-2.0+
> >      > +/*
> >      > + * Copyright (c) 2019, Linaro Limited
> >      > + */
> >      > +
> >      > +#include <common.h>
> >      > +#include <dm.h>
> >      > +#include <efi_loader.h>
> >      > +#include <efi_rng.h>
> >      > +#include <rng.h>
> >      > +
> >      > +DECLARE_GLOBAL_DATA_PTR;
> >      > +
> >      > +static efi_status_t EFIAPI rng_getinfo(struct efi_rng_protocol
> >     *this,
> >      > +                                    efi_uintn_t
> >     *rng_algorithm_list_size,
> >      > +                                    efi_guid_t
> *rng_algorithm_list)
> >      > +{
> >      > +     efi_guid_t rng_algo_guid = EFI_RNG_ALGORITHM_RAW;
> >      > +
> >      > +     EFI_ENTRY("%p, %p, %p", this, rng_algorithm_list_size,
> >      > +               rng_algorithm_list);
> >      > +
> >      > +     if (!this || !rng_algorithm_list_size)
> >      > +             return EFI_INVALID_PARAMETER;
> >      > +
> >      > +     if (!rng_algorithm_list ||
> >      > +         *rng_algorithm_list_size < sizeof(*rng_algorithm_list))
> {
> >      > +             *rng_algorithm_list_size =
> sizeof(*rng_algorithm_list);
> >      > +             return EFI_BUFFER_TOO_SMALL;
> >      > +     }
> >      > +
> >      > +     /*
> >      > +      * For now, use EFI_RNG_ALGORITHM_RAW as the default
> >      > +      * algorithm. If a new algorithm gets added in the
> >      > +      * future through a Kconfig, rng_algo_guid will be set
> >      > +      * based on that Kconfig option
> >      > +      */
> >      > +     *rng_algorithm_list_size = sizeof(*rng_algorithm_list);
> >      > +     guidcpy(rng_algorithm_list, &rng_algo_guid);
> >      > +
> >      > +     return EFI_EXIT(EFI_SUCCESS);
> >      > +}
> >      > +
> >      > +static efi_status_t EFIAPI getrng(struct efi_rng_protocol *this,
> >      > +                               efi_guid_t *rng_algorithm,
> >      > +                               efi_uintn_t rng_value_length,
> >      > +                               uint8_t *rng_value)
> >      > +{
> >      > +     int ret;
> >      > +     struct udevice *dev;
> >      > +     const efi_guid_t rng_raw_guid = EFI_RNG_ALGORITHM_RAW;
> >      > +
> >      > +     EFI_ENTRY("%p, %p, %zu, %p", this, rng_algorithm,
> >     rng_value_length,
> >      > +               rng_value);
> >      > +
> >      > +     if (!this || !rng_value || !rng_value_length)
> >      > +             return EFI_INVALID_PARAMETER;
> >      > +
> >      > +     if (rng_algorithm) {
> >      > +             if (guidcmp(rng_algorithm, &rng_raw_guid))
> >      > +                     return EFI_UNSUPPORTED;
> >      > +     }
> >      > +
> >      > +     ret = platform_get_rng_device(&dev);
> >
> >     This does not compile for sandbox_defconfig.
> >
> >     You could replace this by:
> >
> >     ret = uclass_get_device(UCLASS_RNG, 0, &dev);
> >
> >
> > Like I had stated in one of my earlier mail, I would prefer having a
> > platform specific routine for fetching the rng device. For example, on
> > qemu, where the rng device is the child of a virtio-pci device, the
> > above logic would not get the rng device without having previously
> > scanned the virtio devices. Instead, i will add a weak function
> > platform_get_rng_device, which uses uclass_get_device. Any platform
> > which has a different topology, can then define it's own
> > platform_get_rng_device function.
> >
> > -sughosh
>
> For patches series the expectation is that the code compiles when
> stopping after any patch in the series.
>

I do check the build of individual commits before sending out the patches,
although I might have missed that while sending some version. Did you
encounter any build error on a commit. Btw, the error you got for sandbox
was expected, since the patch series only adds support for the qemu arm64
platform. I will send a follow-up series which enables this for sandbox as
well.

-sughosh