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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 5 April 2017 at 14:11, Ard Biesheuvel <ard.biesheuvel@linaro.org> 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

way *out*

> -- once AndroidFastBootApp is the last remaining user, we can
> fold whatever it uses into it instead.
>
>
>> 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
>>>