[PATCH v2 4/7] efi_loader: Add a version of efi_binary_run() with more parameters

[Simon Glass]

Hi Ilias,

On Thu, 12 Dec 2024 at 00:49, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Thu, 12 Dec 2024 at 00:38, Simon Glass <sjg at chromium.org> wrote:
> >
> > This uses a few global variables at present. With the bootflow we have
> > the required parameters, so add a function which accepts these. Update
> > the existing function to call the new one with the globals.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  lib/efi_loader/efi_bootbin.c | 29 +++++++++++++++++++++++++----
> >  1 file changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c
> > index 8febd325f34..8ebd48547cb 100644
> > --- a/lib/efi_loader/efi_bootbin.c
> > +++ b/lib/efi_loader/efi_bootbin.c
> > @@ -209,18 +209,22 @@ out:
> >  }
> >
> >  /**
> > - * efi_binary_run() - run loaded UEFI image
> > + * efi_binary_run_() - run loaded UEFI image
> >   *
> >   * @image_ptr: memory address of the UEFI image
> >   * @size:      size of the UEFI image
> >   * @fdt:       device-tree
> > + * @device:    EFI device-path
> > + * @image:     EFI image-path
> >   *
> >   * Execute an EFI binary image loaded at @image.
> >   * @size may be zero if the binary is loaded with U-Boot load command.
> >   *
> >   * Return:     status code
> >   */
> > -efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt)
> > +efi_status_t efi_binary_run_(void *image_ptr, size_t size, void *fdt,
> > +                            struct efi_device_path *device,
> > +                            struct efi_device_path *image)
> >  {
>
> This needs to be static here. Not the next patch. But I don't think we
> need this at all (look below)
>
> >         efi_status_t ret;
> >
> > @@ -236,6 +240,23 @@ efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt)
> >         if (ret != EFI_SUCCESS)
> >                 return ret;
> >
> > -       return efi_run_image(image_ptr, size, bootefi_device_path,
> > -                            bootefi_image_path);
> > +       return efi_run_image(image_ptr, size, device, image);
> > +}
> > +
> > +/**
> > + * efi_binary_run() - run loaded UEFI image
> > + *
> > + * @image:     memory address of the UEFI image
> > + * @size:      size of the UEFI image
> > + * @fdt:       device-tree
> > + *
> > + * Execute an EFI binary image loaded at @image.
> > + * @size may be zero if the binary is loaded with U-Boot load command.
> > + *
> > + * Return:     status code
> > + */
> > +efi_status_t efi_binary_run(void *image, size_t size, void *fdt)
> > +{
> > +       return efi_binary_run_(image, size, fdt, bootefi_device_path,
> > +                              bootefi_image_path);
> >  }
>
> We don't need functions and wrappers for such a simple call. Just
> update all the callisites with the extra parameters and keep one
> function

If you look at the two call sites, you'll see that they would need to
pass bootefi_device_path and bootefi_image_path in. but these are
static variables in this file.

>
> Thanks
> /Ilias
> > --
> > 2.34.1
> >

Regards,
Simon