Re: [RFT PATCH] EmbeddedPkg/AndroidFastbootApp: remove dependency on deprecated BdsLib

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Apr 18, 2017 at 07:08:57PM +0100, Ard Biesheuvel wrote:
> On 10 April 2017 at 17:55, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > One of the last remaining modules with a dependency on the deprecated
> > BdsLib implementation from ArmPkg is the Android fastboot application.
> >
> > Its only dependency on BdsLib is BdsStartEfiApplication(), which is
> > used in the most peculiar way: the fastboot app loads the kernel image
> > into memory, and creates a MemoryMapped() device path for it. It then
> > proceeds and calls BdsStartEfiApplication(), which explicitly loads the
> > contents of the devicepath into memory, creating a second in-memory copy
> > of the kernel image, after which it invokes gBS->LoadImage() with a
> > buffer address and size (while it is perfectly capable of loading from
> > a devicepath directly)
> >
> > Since we know the device path is fully qualified and connected, and does
> > not require any of the additional processing that BdsStartEfiApplication()
> > does when dereferencing a device path, we should be able to pass this
> > devicepath into LoadImage() directly.
> >
> > So create a simplified local clone of BdsStartEfiApplication(), and drop
> > the dependency on BdsLib.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Should we just merge this and see what happens?

With no other comments so far - sure:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> > ---
> >  EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf   |  2 +-
> >  EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c | 57 +++++++++++++++++++-
> >  2 files changed, 57 insertions(+), 2 deletions(-)
> >
> > diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf
> > index 3e115171ce01..8d2b23c6f8b8 100644
> > --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf
> > +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.inf
> > @@ -30,7 +30,6 @@ [Sources.ARM, Sources.AARCH64]
> >  [LibraryClasses]
> >    BaseLib
> >    BaseMemoryLib
> > -  BdsLib
> >    DebugLib
> >    DevicePathLib
> >    DxeServicesTableLib
> > @@ -45,6 +44,7 @@ [LibraryClasses]
> >  [Protocols]
> >    gAndroidFastbootTransportProtocolGuid
> >    gAndroidFastbootPlatformProtocolGuid
> > +  gEfiLoadedImageProtocolGuid
> >    gEfiSimpleTextOutProtocolGuid
> >    gEfiSimpleTextInProtocolGuid
> >
> > diff --git a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
> > index 46a7ceb3a41c..f446cce2855a 100644
> > --- a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
> > +++ b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
> > @@ -15,6 +15,7 @@
> >  #include "AndroidFastbootApp.h"
> >
> >  #include <Protocol/DevicePath.h>
> > +#include <Protocol/LoadedImage.h>
> >
> >  #include <Library/BdsLib.h>
> >  #include <Library/DevicePathLib.h>
> > @@ -50,6 +51,60 @@ STATIC CONST MEMORY_DEVICE_PATH MemoryDevicePathTemplate =
> >    } // End
> >  };
> >
> > +
> > +/**
> > +  Start an EFI Application from a Device Path
> > +
> > +  @param  ParentImageHandle     Handle of the calling image
> > +  @param  DevicePath            Location of the EFI Application
> > +
> > +  @retval EFI_SUCCESS           All drivers have been connected
> > +  @retval EFI_NOT_FOUND         The Linux kernel Device Path has not been found
> > +  @retval EFI_OUT_OF_RESOURCES  There is not enough resource memory to store the matching results.
> > +
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +StartEfiApplication (
> > +  IN EFI_HANDLE                  ParentImageHandle,
> > +  IN EFI_DEVICE_PATH_PROTOCOL    *DevicePath,
> > +  IN UINTN                       LoadOptionsSize,
> > +  IN VOID*                       LoadOptions
> > +  )
> > +{
> > +  EFI_STATUS                   Status;
> > +  EFI_HANDLE                   ImageHandle;
> > +  EFI_LOADED_IMAGE_PROTOCOL*   LoadedImage;
> > +
> > +  // Load the image from the device path with Boot Services function
> > +  Status = gBS->LoadImage (TRUE, ParentImageHandle, DevicePath, NULL, 0,
> > +                  &ImageHandle);
> > +  if (EFI_ERROR (Status)) {
> > +    return Status;
> > +  }
> > +
> > +  // Passed LoadOptions to the EFI Application
> > +  if (LoadOptionsSize != 0) {
> > +    Status = gBS->HandleProtocol (ImageHandle, &gEfiLoadedImageProtocolGuid,
> > +                    (VOID **) &LoadedImage);
> > +    if (EFI_ERROR (Status)) {
> > +      return Status;
> > +    }
> > +
> > +    LoadedImage->LoadOptionsSize  = LoadOptionsSize;
> > +    LoadedImage->LoadOptions      = LoadOptions;
> > +  }
> > +
> > +  // Before calling the image, enable the Watchdog Timer for  the 5 Minute period
> > +  gBS->SetWatchdogTimer (5 * 60, 0x0000, 0x00, NULL);
> > +  // Start the image
> > +  Status = gBS->StartImage (ImageHandle, NULL, NULL);
> > +  // Clear the Watchdog Timer after the image returns
> > +  gBS->SetWatchdogTimer (0x0000, 0x0000, 0x0000, NULL);
> > +
> > +  return Status;
> > +}
> > +
> >  EFI_STATUS
> >  BootAndroidBootImg (
> >    IN UINTN    BufferSize,
> > @@ -100,7 +155,7 @@ BootAndroidBootImg (
> >      LoadOptions = NewLoadOptions;
> >    }
> >
> > -  Status = BdsStartEfiApplication (gImageHandle,
> > +  Status = StartEfiApplication (gImageHandle,
> >               (EFI_DEVICE_PATH_PROTOCOL *) &KernelDevicePath,
> >               StrSize (LoadOptions),
> >               LoadOptions);
> > --
> > 2.9.3
> >