* [RFT PATCH] EmbeddedPkg/AndroidFastbootApp: remove dependency on deprecated BdsLib
@ 2017-04-10 16:55 Ard Biesheuvel
2017-04-18 18:08 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2017-04-10 16:55 UTC (permalink / raw)
To: edk2-devel, leif.lindholm; +Cc: ryan.harkin, haojian.zhuang, Ard Biesheuvel
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>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFT PATCH] EmbeddedPkg/AndroidFastbootApp: remove dependency on deprecated BdsLib
2017-04-10 16:55 [RFT PATCH] EmbeddedPkg/AndroidFastbootApp: remove dependency on deprecated BdsLib Ard Biesheuvel
@ 2017-04-18 18:08 ` Ard Biesheuvel
2017-04-18 19:21 ` Leif Lindholm
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2017-04-18 18:08 UTC (permalink / raw)
To: edk2-devel@lists.01.org, Leif Lindholm
Cc: Ryan Harkin, Haojian Zhuang, Ard Biesheuvel
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?
> ---
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFT PATCH] EmbeddedPkg/AndroidFastbootApp: remove dependency on deprecated BdsLib
2017-04-18 18:08 ` Ard Biesheuvel
@ 2017-04-18 19:21 ` Leif Lindholm
2017-04-19 7:18 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2017-04-18 19:21 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Ryan Harkin, Haojian Zhuang
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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFT PATCH] EmbeddedPkg/AndroidFastbootApp: remove dependency on deprecated BdsLib
2017-04-18 19:21 ` Leif Lindholm
@ 2017-04-19 7:18 ` Ard Biesheuvel
0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2017-04-19 7:18 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Ryan Harkin, Haojian Zhuang
On 18 April 2017 at 20:21, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> 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>
>
OK, pushed.
Thanks,
Ard.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-19 7:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-10 16:55 [RFT PATCH] EmbeddedPkg/AndroidFastbootApp: remove dependency on deprecated BdsLib Ard Biesheuvel
2017-04-18 18:08 ` Ard Biesheuvel
2017-04-18 19:21 ` Leif Lindholm
2017-04-19 7:18 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox