| Message ID | 20170404123010.11722-2-ard.biesheuvel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Juno cleanup -- EDK2 edition | expand |
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?). Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > > 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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. > 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 >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 >>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 > >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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
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. 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(-) -- 2.9.3 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel