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

2017-04-19 Thread Ard Biesheuvel 
On 18 April 2017 at 20:21, Leif Lindholm  wrote:
> On Tue, Apr 18, 2017 at 07:08:57PM +0100, Ard Biesheuvel wrote:
>> On 10 April 2017 at 17:55, Ard Biesheuvel  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 
>>
>> Should we just merge this and see what happens?
>
> With no other comments so far - sure:
> Reviewed-by: Leif Lindholm 
>

OK, pushed.

Thanks,
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

2017-04-18 Thread Leif Lindholm 
On Tue, Apr 18, 2017 at 07:08:57PM +0100, Ard Biesheuvel wrote:
> On 10 April 2017 at 17:55, Ard Biesheuvel  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 
> 
> Should we just merge this and see what happens?

With no other comments so far - sure:
Reviewed-by: Leif Lindholm 

> > ---
> >  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 
> > +#include 
> >
> >  #include 
> >  #include 
> > @@ -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  DevicePathLocation 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,
> > +  );
> > +  if (EFI_ERROR (Status)) {
> > +return Status;
> > +  }
> > +
> > +  // Passed LoadOptions to the EFI Application
> > +  if (LoadOptionsSize != 0) {
> > +Status = gBS->HandleProtocol (ImageHandle, 
> > ,
> > +(VOID **) );
> > +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, 0x, 0x00, NULL);
> > +  // Start the image
> > +  Status = gBS->StartImage (ImageHandle, NULL, NULL);
> > +  // Clear the Watchdog Timer after the image returns
> > +  gBS->SetWatchdogTimer (0x, 0x, 0x, NULL);
> > +

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

2017-04-18 Thread Ard Biesheuvel 
On 10 April 2017 at 17:55, Ard Biesheuvel  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 

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 
> +#include 
>
>  #include 
>  #include 
> @@ -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  DevicePathLocation 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,
> +  );
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  // Passed LoadOptions to the EFI Application
> +  if (LoadOptionsSize != 0) {
> +Status = gBS->HandleProtocol (ImageHandle, ,
> +(VOID **) );
> +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, 0x, 0x00, NULL);
> +  // Start the image
> +  Status = gBS->StartImage (ImageHandle, NULL, NULL);
> +  // Clear the Watchdog Timer after the image returns
> +  gBS->SetWatchdogTimer (0x, 0x, 0x, NULL);
> +
> +  return Status;
> +}
> +
>  EFI_STATUS
>  BootAndroidBootImg (
>IN UINTNBufferSize,
> @@ -100,7 +155,7 @@ BootAndroidBootImg (
>  LoadOptions = NewLoadOptions;
>}
>
> -  Status = BdsStartEfiApplication (gImageHandle,
> +  Status = StartEfiApplication (gImageHandle,
>   (EFI_DEVICE_PATH_PROTOCOL *) ,
>   StrSize (LoadOptions),
>   LoadOptions);
> --
> 2.9.3

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

2017-04-10 Thread 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 
---
 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 
+#include 
 
 #include 
 #include 
@@ -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  DevicePathLocation 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,
+  );
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  // Passed LoadOptions to the EFI Application
+  if (LoadOptionsSize != 0) {
+Status = gBS->HandleProtocol (ImageHandle, ,
+(VOID **) );
+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, 0x, 0x00, NULL);
+  // Start the image
+  Status = gBS->StartImage (ImageHandle, NULL, NULL);
+  // Clear the Watchdog Timer after the image returns
+  gBS->SetWatchdogTimer (0x, 0x, 0x, NULL);
+
+  return Status;
+}
+
 EFI_STATUS
 BootAndroidBootImg (
   IN UINTNBufferSize,
@@ -100,7 +155,7 @@ BootAndroidBootImg (
 LoadOptions = NewLoadOptions;
   }
 
-  Status = BdsStartEfiApplication (gImageHandle,
+  Status = StartEfiApplication (gImageHandle,
  (EFI_DEVICE_PATH_PROTOCOL *) ,
  StrSize (LoadOptions),
  LoadOptions);
-- 
2.9.3

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel