Re: [PATCH v3 1/6] ArmPlatformPkg/ArmShellCmdRunAxf: remove BdsLib dependency

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

On Wed, Apr 05, 2017 at 02:11:44PM +0100, Ard Biesheuvel wrote:
> On 5 April 2017 at 14:09, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Tue, Apr 04, 2017 at 01:30:05PM +0100, Ard Biesheuvel wrote:
> >> Remove ArmShellCmdRunAxf's dependency on the deprecated BdsLib by
> >> cloning the ShutdownUefiBootServices() routine into a local source
> >> file; this is the only BdsLib feature 'runaxf' depends on.
> >
> > I was going to go through these individually and give a R-b for the
> > whole series at the end if I didn't come across anything, but I'd just
> > like to add for this one:
> >
> > This is abslutely the right thing to do - BdsLib only had this
> > functionality due to the LinuxLoader, and it could be deleted now (but
> > I guess we're planning to get rid of the whole thing?).
> >
> 
> I'd like to avoid doing any 'improvements' to BdsLib while it's on its
> way you -- once AndroidFastBootApp is the last remaining user, we can
> fold whatever it uses into it instead.

Agreed - thanks!

/
    Leif

> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >
> 
> Thanks
> 
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf |  1 -
> >>  ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c              | 58 +++++++++++++++++++-
> >>  2 files changed, 57 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
> >> index 9a34f666612a..7d15f6934608 100644
> >> --- a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
> >> +++ b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
> >> @@ -43,7 +43,6 @@ [Packages]
> >>  [LibraryClasses]
> >>    ArmLib
> >>    BaseLib
> >> -  BdsLib
> >>    DebugLib
> >>    HiiLib
> >>    ShellLib
> >> diff --git a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c
> >> index 2abfb6cc1053..9f4fc780307d 100644
> >> --- a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c
> >> +++ b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c
> >> @@ -21,7 +21,6 @@
> >>  #include <Library/DevicePathLib.h>
> >>  #include <Library/BaseLib.h>
> >>  #include <Library/BaseMemoryLib.h>
> >> -#include <Library/BdsLib.h>
> >>  #include <Library/MemoryAllocationLib.h>
> >>  #include <Library/DebugLib.h>
> >>
> >> @@ -35,6 +34,63 @@
> >>  typedef VOID (*ELF_ENTRYPOINT)(UINTN arg0, UINTN arg1,
> >>                                 UINTN arg2, UINTN arg3);
> >>
> >> +STATIC
> >> +EFI_STATUS
> >> +ShutdownUefiBootServices (
> >> +  VOID
> >> +  )
> >> +{
> >> +  EFI_STATUS              Status;
> >> +  UINTN                   MemoryMapSize;
> >> +  EFI_MEMORY_DESCRIPTOR   *MemoryMap;
> >> +  UINTN                   MapKey;
> >> +  UINTN                   DescriptorSize;
> >> +  UINT32                  DescriptorVersion;
> >> +  UINTN                   Pages;
> >> +
> >> +  MemoryMap = NULL;
> >> +  MemoryMapSize = 0;
> >> +  Pages = 0;
> >> +
> >> +  do {
> >> +    Status = gBS->GetMemoryMap (
> >> +                    &MemoryMapSize,
> >> +                    MemoryMap,
> >> +                    &MapKey,
> >> +                    &DescriptorSize,
> >> +                    &DescriptorVersion
> >> +                    );
> >> +    if (Status == EFI_BUFFER_TOO_SMALL) {
> >> +
> >> +      Pages = EFI_SIZE_TO_PAGES (MemoryMapSize) + 1;
> >> +      MemoryMap = AllocatePages (Pages);
> >> +
> >> +      //
> >> +      // Get System MemoryMap
> >> +      //
> >> +      Status = gBS->GetMemoryMap (
> >> +                      &MemoryMapSize,
> >> +                      MemoryMap,
> >> +                      &MapKey,
> >> +                      &DescriptorSize,
> >> +                      &DescriptorVersion
> >> +                      );
> >> +    }
> >> +
> >> +    // Don't do anything between the GetMemoryMap() and ExitBootServices()
> >> +    if (!EFI_ERROR(Status)) {
> >> +      Status = gBS->ExitBootServices (gImageHandle, MapKey);
> >> +      if (EFI_ERROR(Status)) {
> >> +        FreePages (MemoryMap, Pages);
> >> +        MemoryMap = NULL;
> >> +        MemoryMapSize = 0;
> >> +      }
> >> +    }
> >> +  } while (EFI_ERROR(Status));
> >> +
> >> +  return Status;
> >> +}
> >> +
> >>
> >>  STATIC
> >>  EFI_STATUS
> >> --
> >> 2.9.3
> >>