public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib
@ 2018-11-23  8:44 Ard Biesheuvel
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 1/3] Platform/ARM: import ARM platform specific BdsLib header Ard Biesheuvel
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-23  8:44 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, thomas.abraham, nariman.poushin, lersek, philmd,
	Ard Biesheuvel

The deprecated BdsLib library class in ArmPkg is still depended upon, but
only a single implementation exists, which now resides in edk2-platforms.

This implementation has some issues in how it deals with Device Paths,
so let's fix those, but first move over the library interface declaration
and get rid of the parts that are no longer used. This will permit dropping
it from ArmPkg in EDK2.

Changes since v1:
- add Laszlo's ack to #1
- update #2 to remove everything we no longer need from BdsLib
- drop #3 which was bogus
- update #4 to ensure that we only duplicate the device path when we
  are about to return EFI_SUCCESS

Ard Biesheuvel (3):
  Platform/ARM: import ARM platform specific BdsLib header
  Platform/ARM/BdsLib: drop unused functions
  Platform/ARM/BdsLib: maintain alignment for DevicePaths

 Platform/ARM/ARM.dec                          |   3 +
 .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
 Platform/ARM/Include/Library/BdsLib.h         |  26 ++
 Platform/ARM/Library/BdsLib/BdsAppLoader.c    | 253 ----------------
 Platform/ARM/Library/BdsLib/BdsFilePath.c     |  95 +-----
 Platform/ARM/Library/BdsLib/BdsHelper.c       | 122 --------
 Platform/ARM/Library/BdsLib/BdsInternal.h     |  16 +-
 Platform/ARM/Library/BdsLib/BdsLib.inf        |   4 +-
 Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 ------------------
 9 files changed, 52 insertions(+), 741 deletions(-)
 create mode 100644 Platform/ARM/Include/Library/BdsLib.h
 delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
 delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c

-- 
2.17.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 edk2-platforms 1/3] Platform/ARM: import ARM platform specific BdsLib header
  2018-11-23  8:44 [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Ard Biesheuvel
@ 2018-11-23  8:44 ` Ard Biesheuvel
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions Ard Biesheuvel
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-23  8:44 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, thomas.abraham, nariman.poushin, lersek, philmd,
	Ard Biesheuvel

The BdsLib library has been moved into Platform/ARM a while ago,
but the BdsLib.h header was left behind, and so all users in
Platform/ARM are still relying on it to be available in ArmPkg.

So let's add a copy to Platform/ARM and wire it up, so we can
drop it from ArmPkg going forward. Note that the BdsLib
implementation included ArmLib.h from ArmPkg without using
anything it provides, so drop that false dependency as well.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Laszlo Ersek <lersek@redhat.com>
---
 Platform/ARM/ARM.dec                                   |   3 +
 Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
 Platform/ARM/Include/Library/BdsLib.h                  | 212 ++++++++++++++++++++
 Platform/ARM/Library/BdsLib/BdsInternal.h              |   1 -
 Platform/ARM/Library/BdsLib/BdsLib.inf                 |   2 +-
 5 files changed, 217 insertions(+), 3 deletions(-)

diff --git a/Platform/ARM/ARM.dec b/Platform/ARM/ARM.dec
index f9bf3294a0ca..6a6eeb6559fd 100644
--- a/Platform/ARM/ARM.dec
+++ b/Platform/ARM/ARM.dec
@@ -21,5 +21,8 @@
 [Includes]
   Include                        # Root include for the package
 
+[LibraryClasses]
+  BdsLib|Include/Library/BdsLib.h
+
 [Guids]
   gArmBootMonFsFileInfoGuid   = { 0x41e26b9c, 0xada6, 0x45b3, { 0x80, 0x8e, 0x23, 0x57, 0xa3, 0x5b, 0x60, 0xd6 } }
diff --git a/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf b/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf
index f9a5aee3596e..d4aef4bfce92 100644
--- a/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf
+++ b/Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.inf
@@ -28,10 +28,10 @@
   ShellSetFdt.c
 
 [Packages]
-  ArmPkg/ArmPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
+  Platform/ARM/ARM.dec
   Platform/ARM/Drivers/FdtPlatformDxe/FdtPlatformDxe.dec
   ShellPkg/ShellPkg.dec
 
diff --git a/Platform/ARM/Include/Library/BdsLib.h b/Platform/ARM/Include/Library/BdsLib.h
new file mode 100644
index 000000000000..4528c2e8739b
--- /dev/null
+++ b/Platform/ARM/Include/Library/BdsLib.h
@@ -0,0 +1,212 @@
+/** @file
+*
+*  Copyright (c) 2013-2015, ARM Limited. All rights reserved.
+*
+*  This program and the accompanying materials
+*  are licensed and made available under the terms and conditions of the BSD License
+*  which accompanies this distribution.  The full text of the license may be found at
+*  http://opensource.org/licenses/bsd-license.php
+*
+*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+*
+**/
+
+#ifndef __BDS_ENTRY_H__
+#define __BDS_ENTRY_H__
+
+#define IS_DEVICE_PATH_NODE(node,type,subtype)    \
+        (((node)->Type == (type)) && ((node)->SubType == (subtype)))
+
+/**
+  This is defined by the UEFI specs, don't change it
+**/
+typedef struct {
+  UINT16                      LoadOptionIndex;
+  EFI_LOAD_OPTION             *LoadOption;
+  UINTN                       LoadOptionSize;
+
+  UINT32                      Attributes;
+  UINT16                      FilePathListLength;
+  CHAR16                      *Description;
+  EFI_DEVICE_PATH_PROTOCOL    *FilePathList;
+
+  VOID*                       OptionalData;
+  UINTN                       OptionalDataSize;
+} BDS_LOAD_OPTION;
+
+/**
+  Connect a Device Path and return the handle of the driver that support this DevicePath
+
+  @param  DevicePath            Device Path of the File to connect
+  @param  Handle                Handle of the driver that support this DevicePath
+  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match the driver DevicePath
+
+  @retval EFI_SUCCESS           A driver that matches the Device Path has been found
+  @retval EFI_NOT_FOUND         No handles match the search.
+  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
+
+**/
+EFI_STATUS
+BdsConnectDevicePath (
+  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
+  OUT EFI_HANDLE                *Handle,
+  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
+  );
+
+/**
+  Connect all DXE drivers
+
+  @retval EFI_SUCCESS           All drivers have been connected
+  @retval EFI_NOT_FOUND         No handles match the search.
+  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the matching results.
+
+**/
+EFI_STATUS
+BdsConnectAllDrivers (
+  VOID
+  );
+
+/**
+  Return the value of a global variable defined by its VariableName.
+  The variable must be defined with the VendorGuid gEfiGlobalVariableGuid.
+
+  @param  VariableName          A Null-terminated string that is the name of the vendor's
+                                variable.
+  @param  DefaultValue          Value returned by the function if the variable does not exist
+  @param  DataSize              On input, the size in bytes of the return Data buffer.
+                                On output the size of data returned in Data.
+  @param  Value                 Value read from the UEFI Variable or copy of the default value
+                                if the UEFI Variable does not exist
+
+  @retval EFI_SUCCESS           All drivers have been connected
+  @retval EFI_NOT_FOUND         No handles match the search.
+  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the matching results.
+
+**/
+EFI_STATUS
+GetGlobalEnvironmentVariable (
+  IN     CONST CHAR16*   VariableName,
+  IN     VOID*           DefaultValue,
+  IN OUT UINTN*          Size,
+  OUT    VOID**          Value
+  );
+
+/**
+  Return the value of the variable defined by its VariableName and VendorGuid
+
+  @param  VariableName          A Null-terminated string that is the name of the vendor's
+                                variable.
+  @param  VendorGuid            A unique identifier for the vendor.
+  @param  DefaultValue          Value returned by the function if the variable does not exist
+  @param  DataSize              On input, the size in bytes of the return Data buffer.
+                                On output the size of data returned in Data.
+  @param  Value                 Value read from the UEFI Variable or copy of the default value
+                                if the UEFI Variable does not exist
+
+  @retval EFI_SUCCESS           All drivers have been connected
+  @retval EFI_NOT_FOUND         No handles match the search.
+  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the matching results.
+
+**/
+EFI_STATUS
+GetEnvironmentVariable (
+  IN     CONST CHAR16*   VariableName,
+  IN     EFI_GUID*       VendorGuid,
+  IN     VOID*           DefaultValue,
+  IN OUT UINTN*          Size,
+  OUT    VOID**          Value
+  );
+
+EFI_STATUS
+BootOptionFromLoadOptionIndex (
+  IN  UINT16            LoadOptionIndex,
+  OUT BDS_LOAD_OPTION** BdsLoadOption
+  );
+
+EFI_STATUS
+BootOptionFromLoadOptionVariable (
+  IN  CHAR16*           BootVariableName,
+  OUT BDS_LOAD_OPTION** BdsLoadOption
+  );
+
+EFI_STATUS
+BootOptionToLoadOptionVariable (
+  IN BDS_LOAD_OPTION*   BdsLoadOption
+  );
+
+UINT16
+BootOptionAllocateBootIndex (
+  VOID
+  );
+
+/**
+  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.
+
+**/
+EFI_STATUS
+BdsStartEfiApplication (
+  IN EFI_HANDLE                  ParentImageHandle,
+  IN EFI_DEVICE_PATH_PROTOCOL    *DevicePath,
+  IN UINTN                       LoadOptionsSize,
+  IN VOID*                       LoadOptions
+  );
+
+EFI_STATUS
+BdsLoadImage (
+  IN     EFI_DEVICE_PATH       *DevicePath,
+  IN     EFI_ALLOCATE_TYPE     Type,
+  IN OUT EFI_PHYSICAL_ADDRESS* Image,
+  OUT    UINTN                 *FileSize
+  );
+
+/**
+ * Call BS.ExitBootServices with the appropriate Memory Map information
+ */
+EFI_STATUS
+ShutdownUefiBootServices (
+  VOID
+  );
+
+/**
+  Locate an EFI application in a the Firmware Volumes by its name
+
+  @param  EfiAppGuid            Guid of the EFI Application into the Firmware Volume
+  @param  DevicePath            EFI Device Path of the EFI application
+
+  @return EFI_SUCCESS           The function completed successfully.
+  @return EFI_NOT_FOUND         The protocol could not be located.
+  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.
+
+**/
+EFI_STATUS
+LocateEfiApplicationInFvByName (
+  IN  CONST CHAR16*             EfiAppName,
+  OUT EFI_DEVICE_PATH           **DevicePath
+  );
+
+/**
+  Locate an EFI application in a the Firmware Volumes by its GUID
+
+  @param  EfiAppGuid            Guid of the EFI Application into the Firmware Volume
+  @param  DevicePath            EFI Device Path of the EFI application
+
+  @return EFI_SUCCESS           The function completed successfully.
+  @return EFI_NOT_FOUND         The protocol could not be located.
+  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.
+
+**/
+EFI_STATUS
+LocateEfiApplicationInFvByGuid (
+  IN  CONST EFI_GUID            *EfiAppGuid,
+  OUT EFI_DEVICE_PATH           **DevicePath
+  );
+
+#endif
diff --git a/Platform/ARM/Library/BdsLib/BdsInternal.h b/Platform/ARM/Library/BdsLib/BdsInternal.h
index f70aae603d69..bb4566e3a6c4 100644
--- a/Platform/ARM/Library/BdsLib/BdsInternal.h
+++ b/Platform/ARM/Library/BdsLib/BdsInternal.h
@@ -16,7 +16,6 @@
 #define __BDS_INTERNAL_H__
 
 #include <PiDxe.h>
-#include <Library/ArmLib.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DxeServicesTableLib.h>
diff --git a/Platform/ARM/Library/BdsLib/BdsLib.inf b/Platform/ARM/Library/BdsLib/BdsLib.inf
index 96c1d6e7e200..637ef5a08128 100644
--- a/Platform/ARM/Library/BdsLib/BdsLib.inf
+++ b/Platform/ARM/Library/BdsLib/BdsLib.inf
@@ -27,10 +27,10 @@
   BdsLoadOption.c
 
 [Packages]
-  ArmPkg/ArmPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
+  Platform/ARM/ARM.dec
 
 [LibraryClasses]
   ArmLib
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions
  2018-11-23  8:44 [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Ard Biesheuvel
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 1/3] Platform/ARM: import ARM platform specific BdsLib header Ard Biesheuvel
@ 2018-11-23  8:44 ` Ard Biesheuvel
  2018-11-23 12:39   ` Laszlo Ersek
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths Ard Biesheuvel
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-23  8:44 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, thomas.abraham, nariman.poushin, lersek, philmd,
	Ard Biesheuvel

Clean up BdsLib (which is deprecated and should not be used for future
development) by removing all the pieces that are not being used at the
moment.

After this patch, only BdsLoadImage() remains, and the pieces it relies
upon. This function is used by FdtPlatformDxe to load device tree
binaries from device paths.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/Include/Library/BdsLib.h       | 186 -------------
 Platform/ARM/Library/BdsLib/BdsAppLoader.c  | 253 ------------------
 Platform/ARM/Library/BdsLib/BdsFilePath.c   |  83 +-----
 Platform/ARM/Library/BdsLib/BdsHelper.c     | 122 ---------
 Platform/ARM/Library/BdsLib/BdsInternal.h   |  15 +-
 Platform/ARM/Library/BdsLib/BdsLib.inf      |   2 -
 Platform/ARM/Library/BdsLib/BdsLoadOption.c | 272 --------------------
 7 files changed, 13 insertions(+), 920 deletions(-)

diff --git a/Platform/ARM/Include/Library/BdsLib.h b/Platform/ARM/Include/Library/BdsLib.h
index 4528c2e8739b..eab868439207 100644
--- a/Platform/ARM/Include/Library/BdsLib.h
+++ b/Platform/ARM/Include/Library/BdsLib.h
@@ -15,150 +15,6 @@
 #ifndef __BDS_ENTRY_H__
 #define __BDS_ENTRY_H__
 
-#define IS_DEVICE_PATH_NODE(node,type,subtype)    \
-        (((node)->Type == (type)) && ((node)->SubType == (subtype)))
-
-/**
-  This is defined by the UEFI specs, don't change it
-**/
-typedef struct {
-  UINT16                      LoadOptionIndex;
-  EFI_LOAD_OPTION             *LoadOption;
-  UINTN                       LoadOptionSize;
-
-  UINT32                      Attributes;
-  UINT16                      FilePathListLength;
-  CHAR16                      *Description;
-  EFI_DEVICE_PATH_PROTOCOL    *FilePathList;
-
-  VOID*                       OptionalData;
-  UINTN                       OptionalDataSize;
-} BDS_LOAD_OPTION;
-
-/**
-  Connect a Device Path and return the handle of the driver that support this DevicePath
-
-  @param  DevicePath            Device Path of the File to connect
-  @param  Handle                Handle of the driver that support this DevicePath
-  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match the driver DevicePath
-
-  @retval EFI_SUCCESS           A driver that matches the Device Path has been found
-  @retval EFI_NOT_FOUND         No handles match the search.
-  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
-
-**/
-EFI_STATUS
-BdsConnectDevicePath (
-  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
-  OUT EFI_HANDLE                *Handle,
-  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
-  );
-
-/**
-  Connect all DXE drivers
-
-  @retval EFI_SUCCESS           All drivers have been connected
-  @retval EFI_NOT_FOUND         No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the matching results.
-
-**/
-EFI_STATUS
-BdsConnectAllDrivers (
-  VOID
-  );
-
-/**
-  Return the value of a global variable defined by its VariableName.
-  The variable must be defined with the VendorGuid gEfiGlobalVariableGuid.
-
-  @param  VariableName          A Null-terminated string that is the name of the vendor's
-                                variable.
-  @param  DefaultValue          Value returned by the function if the variable does not exist
-  @param  DataSize              On input, the size in bytes of the return Data buffer.
-                                On output the size of data returned in Data.
-  @param  Value                 Value read from the UEFI Variable or copy of the default value
-                                if the UEFI Variable does not exist
-
-  @retval EFI_SUCCESS           All drivers have been connected
-  @retval EFI_NOT_FOUND         No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the matching results.
-
-**/
-EFI_STATUS
-GetGlobalEnvironmentVariable (
-  IN     CONST CHAR16*   VariableName,
-  IN     VOID*           DefaultValue,
-  IN OUT UINTN*          Size,
-  OUT    VOID**          Value
-  );
-
-/**
-  Return the value of the variable defined by its VariableName and VendorGuid
-
-  @param  VariableName          A Null-terminated string that is the name of the vendor's
-                                variable.
-  @param  VendorGuid            A unique identifier for the vendor.
-  @param  DefaultValue          Value returned by the function if the variable does not exist
-  @param  DataSize              On input, the size in bytes of the return Data buffer.
-                                On output the size of data returned in Data.
-  @param  Value                 Value read from the UEFI Variable or copy of the default value
-                                if the UEFI Variable does not exist
-
-  @retval EFI_SUCCESS           All drivers have been connected
-  @retval EFI_NOT_FOUND         No handles match the search.
-  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the matching results.
-
-**/
-EFI_STATUS
-GetEnvironmentVariable (
-  IN     CONST CHAR16*   VariableName,
-  IN     EFI_GUID*       VendorGuid,
-  IN     VOID*           DefaultValue,
-  IN OUT UINTN*          Size,
-  OUT    VOID**          Value
-  );
-
-EFI_STATUS
-BootOptionFromLoadOptionIndex (
-  IN  UINT16            LoadOptionIndex,
-  OUT BDS_LOAD_OPTION** BdsLoadOption
-  );
-
-EFI_STATUS
-BootOptionFromLoadOptionVariable (
-  IN  CHAR16*           BootVariableName,
-  OUT BDS_LOAD_OPTION** BdsLoadOption
-  );
-
-EFI_STATUS
-BootOptionToLoadOptionVariable (
-  IN BDS_LOAD_OPTION*   BdsLoadOption
-  );
-
-UINT16
-BootOptionAllocateBootIndex (
-  VOID
-  );
-
-/**
-  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.
-
-**/
-EFI_STATUS
-BdsStartEfiApplication (
-  IN EFI_HANDLE                  ParentImageHandle,
-  IN EFI_DEVICE_PATH_PROTOCOL    *DevicePath,
-  IN UINTN                       LoadOptionsSize,
-  IN VOID*                       LoadOptions
-  );
-
 EFI_STATUS
 BdsLoadImage (
   IN     EFI_DEVICE_PATH       *DevicePath,
@@ -167,46 +23,4 @@ BdsLoadImage (
   OUT    UINTN                 *FileSize
   );
 
-/**
- * Call BS.ExitBootServices with the appropriate Memory Map information
- */
-EFI_STATUS
-ShutdownUefiBootServices (
-  VOID
-  );
-
-/**
-  Locate an EFI application in a the Firmware Volumes by its name
-
-  @param  EfiAppGuid            Guid of the EFI Application into the Firmware Volume
-  @param  DevicePath            EFI Device Path of the EFI application
-
-  @return EFI_SUCCESS           The function completed successfully.
-  @return EFI_NOT_FOUND         The protocol could not be located.
-  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.
-
-**/
-EFI_STATUS
-LocateEfiApplicationInFvByName (
-  IN  CONST CHAR16*             EfiAppName,
-  OUT EFI_DEVICE_PATH           **DevicePath
-  );
-
-/**
-  Locate an EFI application in a the Firmware Volumes by its GUID
-
-  @param  EfiAppGuid            Guid of the EFI Application into the Firmware Volume
-  @param  DevicePath            EFI Device Path of the EFI application
-
-  @return EFI_SUCCESS           The function completed successfully.
-  @return EFI_NOT_FOUND         The protocol could not be located.
-  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.
-
-**/
-EFI_STATUS
-LocateEfiApplicationInFvByGuid (
-  IN  CONST EFI_GUID            *EfiAppGuid,
-  OUT EFI_DEVICE_PATH           **DevicePath
-  );
-
 #endif
diff --git a/Platform/ARM/Library/BdsLib/BdsAppLoader.c b/Platform/ARM/Library/BdsLib/BdsAppLoader.c
deleted file mode 100644
index 1f208f8dd796..000000000000
--- a/Platform/ARM/Library/BdsLib/BdsAppLoader.c
+++ /dev/null
@@ -1,253 +0,0 @@
-/** @file
-*
-*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
-*
-*  This program and the accompanying materials
-*  are licensed and made available under the terms and conditions of the BSD License
-*  which accompanies this distribution.  The full text of the license may be found at
-*  http://opensource.org/licenses/bsd-license.php
-*
-*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-*
-**/
-
-#include "BdsInternal.h"
-
-/**
-  Locate an EFI application in a the Firmware Volumes by its Name
-
-  @param  EfiAppGuid            Guid of the EFI Application into the Firmware Volume
-  @param  DevicePath            EFI Device Path of the EFI application
-
-  @return EFI_SUCCESS           The function completed successfully.
-  @return EFI_NOT_FOUND         The protocol could not be located.
-  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.
-
-**/
-EFI_STATUS
-LocateEfiApplicationInFvByName (
-  IN  CONST CHAR16*             EfiAppName,
-  OUT EFI_DEVICE_PATH           **DevicePath
-  )
-{
-  VOID                          *Key;
-  EFI_STATUS                    Status, FileStatus;
-  EFI_GUID                      NameGuid;
-  EFI_FV_FILETYPE               FileType;
-  EFI_FV_FILE_ATTRIBUTES        Attributes;
-  UINTN                         Size;
-  UINTN                         UiStringLen;
-  CHAR16                        *UiSection;
-  UINT32                        Authentication;
-  EFI_DEVICE_PATH               *FvDevicePath;
-  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH    FileDevicePath;
-  EFI_HANDLE                    *HandleBuffer;
-  UINTN                         NumberOfHandles;
-  UINTN                         Index;
-  EFI_FIRMWARE_VOLUME2_PROTOCOL *FvInstance;
-
-  ASSERT (DevicePath != NULL);
-
-  // Length of FilePath
-  UiStringLen = StrLen (EfiAppName);
-
-  // Locate all the Firmware Volume protocols.
-  Status = gBS->LocateHandleBuffer (
-                   ByProtocol,
-                   &gEfiFirmwareVolume2ProtocolGuid,
-                   NULL,
-                   &NumberOfHandles,
-                   &HandleBuffer
-                   );
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-
-  *DevicePath = NULL;
-
-  // Looking for FV with ACPI storage file
-  for (Index = 0; Index < NumberOfHandles; Index++) {
-    //
-    // Get the protocol on this handle
-    // This should not fail because of LocateHandleBuffer
-    //
-    Status = gBS->HandleProtocol (
-                     HandleBuffer[Index],
-                     &gEfiFirmwareVolume2ProtocolGuid,
-                     (VOID**) &FvInstance
-                     );
-    if (EFI_ERROR (Status)) {
-      goto FREE_HANDLE_BUFFER;
-    }
-
-    // Allocate Key
-    Key = AllocatePool (FvInstance->KeySize);
-    ASSERT (Key != NULL);
-    ZeroMem (Key, FvInstance->KeySize);
-
-    do {
-      // Search in all files
-      FileType = EFI_FV_FILETYPE_ALL;
-
-      Status = FvInstance->GetNextFile (FvInstance, Key, &FileType, &NameGuid, &Attributes, &Size);
-      if (!EFI_ERROR (Status)) {
-        UiSection = NULL;
-        FileStatus = FvInstance->ReadSection (
-                      FvInstance,
-                      &NameGuid,
-                      EFI_SECTION_USER_INTERFACE,
-                      0,
-                      (VOID **)&UiSection,
-                      &Size,
-                      &Authentication
-                      );
-        if (!EFI_ERROR (FileStatus)) {
-          if (StrnCmp (EfiAppName, UiSection, UiStringLen) == 0) {
-            //
-            // We found a UiString match.
-            //
-            Status = gBS->HandleProtocol (HandleBuffer[Index], &gEfiDevicePathProtocolGuid, (VOID **)&FvDevicePath);
-
-            // Generate the Device Path for the file
-            EfiInitializeFwVolDevicepathNode (&FileDevicePath, &NameGuid);
-            *DevicePath = AppendDevicePathNode (FvDevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&FileDevicePath);
-            ASSERT (*DevicePath != NULL);
-
-            FreePool (Key);
-            FreePool (UiSection);
-            FreePool (HandleBuffer);
-            return FileStatus;
-          }
-          FreePool (UiSection);
-        }
-      }
-    } while (!EFI_ERROR (Status));
-
-    FreePool (Key);
-  }
-
-FREE_HANDLE_BUFFER:
-  FreePool (HandleBuffer);
-  return EFI_NOT_FOUND;
-}
-
-/**
-  Locate an EFI application in a the Firmware Volumes by its GUID
-
-  @param  EfiAppGuid            Guid of the EFI Application into the Firmware Volume
-  @param  DevicePath            EFI Device Path of the EFI application
-
-  @return EFI_SUCCESS           The function completed successfully.
-  @return EFI_NOT_FOUND         The protocol could not be located.
-  @return EFI_OUT_OF_RESOURCES  There are not enough resources to find the protocol.
-
-**/
-EFI_STATUS
-LocateEfiApplicationInFvByGuid (
-  IN  CONST EFI_GUID            *EfiAppGuid,
-  OUT EFI_DEVICE_PATH           **DevicePath
-  )
-{
-  EFI_STATUS                    Status;
-  EFI_DEVICE_PATH               *FvDevicePath;
-  EFI_HANDLE                    *HandleBuffer;
-  UINTN                         NumberOfHandles;
-  UINTN                         Index;
-  EFI_FIRMWARE_VOLUME2_PROTOCOL *FvInstance;
-  EFI_FV_FILE_ATTRIBUTES        Attributes;
-  UINT32                        AuthenticationStatus;
-  EFI_FV_FILETYPE               Type;
-  UINTN                         Size;
-  CHAR16                        *UiSection;
-  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FvFileDevicePath;
-
-  ASSERT (DevicePath != NULL);
-
-  // Locate all the Firmware Volume protocols.
-  Status = gBS->LocateHandleBuffer (
-                   ByProtocol,
-                   &gEfiFirmwareVolume2ProtocolGuid,
-                   NULL,
-                   &NumberOfHandles,
-                   &HandleBuffer
-                   );
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-
-  *DevicePath = NULL;
-
-  // Looking for FV with ACPI storage file
-  for (Index = 0; Index < NumberOfHandles; Index++) {
-    //
-    // Get the protocol on this handle
-    // This should not fail because of LocateHandleBuffer
-    //
-    Status = gBS->HandleProtocol (
-                     HandleBuffer[Index],
-                     &gEfiFirmwareVolume2ProtocolGuid,
-                     (VOID**) &FvInstance
-                     );
-    if (EFI_ERROR (Status)) {
-      goto FREE_HANDLE_BUFFER;
-    }
-
-    Status = FvInstance->ReadFile (
-                  FvInstance,
-                  EfiAppGuid,
-                  NULL,
-                  &Size,
-                  &Type,
-                  &Attributes,
-                  &AuthenticationStatus
-                  );
-    if (EFI_ERROR (Status)) {
-      //
-      // Skip if no EFI application file in the FV
-      //
-      continue;
-    } else {
-      UiSection = NULL;
-      Status = FvInstance->ReadSection (
-                    FvInstance,
-                    EfiAppGuid,
-                    EFI_SECTION_USER_INTERFACE,
-                    0,
-                    (VOID **)&UiSection,
-                    &Size,
-                    &AuthenticationStatus
-                    );
-      if (!EFI_ERROR (Status)) {
-        //
-        // Create the EFI Device Path for the application using the Filename of the application
-        //
-        *DevicePath = FileDevicePath (HandleBuffer[Index], UiSection);
-      } else {
-        Status = gBS->HandleProtocol (HandleBuffer[Index], &gEfiDevicePathProtocolGuid, (VOID**)&FvDevicePath);
-        ASSERT_EFI_ERROR (Status);
-
-        //
-        // Create the EFI Device Path for the application using the EFI GUID of the application
-        //
-        EfiInitializeFwVolDevicepathNode (&FvFileDevicePath, EfiAppGuid);
-
-        *DevicePath = AppendDevicePathNode (FvDevicePath, (EFI_DEVICE_PATH_PROTOCOL *)&FvFileDevicePath);
-        ASSERT (*DevicePath != NULL);
-      }
-      break;
-    }
-  }
-
-FREE_HANDLE_BUFFER:
-  //
-  // Free any allocated buffers
-  //
-  FreePool (HandleBuffer);
-
-  if (*DevicePath == NULL) {
-    return EFI_NOT_FOUND;
-  } else {
-    return EFI_SUCCESS;
-  }
-}
diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c b/Platform/ARM/Library/BdsLib/BdsFilePath.c
index 7a4a5052a786..62f796e5526d 100644
--- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
+++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
@@ -24,6 +24,9 @@
 #include <Protocol/Dhcp4.h>
 #include <Protocol/Mtftp4.h>
 
+#define IS_DEVICE_PATH_NODE(node,type,subtype)    \
+        (((node)->Type == (type)) && ((node)->SubType == (subtype)))
+
 #define MAX_TFTP_FILE_SIZE    0x01000000
 
 /* Type and defines to set up the DHCP4 options */
@@ -427,28 +430,6 @@ BdsConnectAndUpdateDevicePath (
   return Status;
 }
 
-/**
-  Connect a Device Path and return the handle of the driver that support this DevicePath
-
-  @param  DevicePath            Device Path of the File to connect
-  @param  Handle                Handle of the driver that support this DevicePath
-  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match the driver DevicePath
-
-  @retval EFI_SUCCESS           A driver that matches the Device Path has been found
-  @retval EFI_NOT_FOUND         No handles match the search.
-  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
-
-**/
-EFI_STATUS
-BdsConnectDevicePath (
-  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
-  OUT EFI_HANDLE                *Handle,
-  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
-  )
-{
-  return BdsConnectAndUpdateDevicePath (&DevicePath, Handle, RemainingDevicePath);
-}
-
 BOOLEAN
 BdsFileSystemSupport (
   IN EFI_DEVICE_PATH *DevicePath,
@@ -1353,61 +1334,3 @@ BdsLoadImage (
 {
   return BdsLoadImageAndUpdateDevicePath (&DevicePath, Type, Image, FileSize);
 }
-
-/**
-  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.
-
-**/
-EFI_STATUS
-BdsStartEfiApplication (
-  IN EFI_HANDLE                  ParentImageHandle,
-  IN EFI_DEVICE_PATH_PROTOCOL    *DevicePath,
-  IN UINTN                       LoadOptionsSize,
-  IN VOID*                       LoadOptions
-  )
-{
-  EFI_STATUS                   Status;
-  EFI_HANDLE                   ImageHandle;
-  EFI_PHYSICAL_ADDRESS         BinaryBuffer;
-  UINTN                        BinarySize;
-  EFI_LOADED_IMAGE_PROTOCOL*   LoadedImage;
-
-  // Find the nearest supported file loader
-  Status = BdsLoadImageAndUpdateDevicePath (&DevicePath, AllocateAnyPages, &BinaryBuffer, &BinarySize);
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-
-  // Load the image from the Buffer with Boot Services function
-  Status = gBS->LoadImage (TRUE, ParentImageHandle, DevicePath, (VOID*)(UINTN)BinaryBuffer, BinarySize, &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;
-}
diff --git a/Platform/ARM/Library/BdsLib/BdsHelper.c b/Platform/ARM/Library/BdsLib/BdsHelper.c
index b10fe2074d53..de40fb5cf639 100644
--- a/Platform/ARM/Library/BdsLib/BdsHelper.c
+++ b/Platform/ARM/Library/BdsLib/BdsHelper.c
@@ -14,62 +14,6 @@
 
 #include "BdsInternal.h"
 
-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;
-}
-
 /**
   Connect all DXE drivers
 
@@ -115,69 +59,3 @@ BdsConnectAllDrivers (
 
   return EFI_SUCCESS;
 }
-
-EFI_STATUS
-GetGlobalEnvironmentVariable (
-  IN     CONST CHAR16*   VariableName,
-  IN     VOID*           DefaultValue,
-  IN OUT UINTN*          Size,
-  OUT    VOID**          Value
-  )
-{
-  return GetEnvironmentVariable (VariableName, &gEfiGlobalVariableGuid,
-           DefaultValue, Size, Value);
-}
-
-EFI_STATUS
-GetEnvironmentVariable (
-  IN     CONST CHAR16*   VariableName,
-  IN     EFI_GUID*       VendorGuid,
-  IN     VOID*           DefaultValue,
-  IN OUT UINTN*          Size,
-  OUT    VOID**          Value
-  )
-{
-  EFI_STATUS  Status;
-  UINTN       VariableSize;
-
-  // Try to get the variable size.
-  *Value = NULL;
-  VariableSize = 0;
-  Status = gRT->GetVariable ((CHAR16 *) VariableName, VendorGuid, NULL, &VariableSize, *Value);
-  if (Status == EFI_NOT_FOUND) {
-    if ((DefaultValue != NULL) && (Size != NULL) && (*Size != 0)) {
-      // If the environment variable does not exist yet then set it with the default value
-      Status = gRT->SetVariable (
-                    (CHAR16*)VariableName,
-                    VendorGuid,
-                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
-                    *Size,
-                    DefaultValue
-                    );
-      *Value = AllocateCopyPool (*Size, DefaultValue);
-    } else {
-      return EFI_NOT_FOUND;
-    }
-  } else if (Status == EFI_BUFFER_TOO_SMALL) {
-    // Get the environment variable value
-    *Value = AllocatePool (VariableSize);
-    if (*Value == NULL) {
-      return EFI_OUT_OF_RESOURCES;
-    }
-
-    Status = gRT->GetVariable ((CHAR16 *)VariableName, VendorGuid, NULL, &VariableSize, *Value);
-    if (EFI_ERROR (Status)) {
-      FreePool(*Value);
-      return EFI_INVALID_PARAMETER;
-    }
-
-    if (Size) {
-      *Size = VariableSize;
-    }
-  } else {
-    *Value = AllocateCopyPool (*Size, DefaultValue);
-    return Status;
-  }
-
-  return EFI_SUCCESS;
-}
diff --git a/Platform/ARM/Library/BdsLib/BdsInternal.h b/Platform/ARM/Library/BdsLib/BdsInternal.h
index bb4566e3a6c4..850618c624c5 100644
--- a/Platform/ARM/Library/BdsLib/BdsInternal.h
+++ b/Platform/ARM/Library/BdsLib/BdsInternal.h
@@ -99,12 +99,17 @@ typedef struct {
   UINT64  LastReportedNbOfBytes;
 } BDS_TFTP_CONTEXT;
 
+/**
+  Connect all DXE drivers
+
+  @retval EFI_SUCCESS           All drivers have been connected
+  @retval EFI_NOT_FOUND         No handles match the search.
+  @retval EFI_OUT_OF_RESOURCES  There is not resource pool memory to store the matching results.
+
+**/
 EFI_STATUS
-BdsLoadImage (
-  IN     EFI_DEVICE_PATH       *DevicePath,
-  IN     EFI_ALLOCATE_TYPE     Type,
-  IN OUT EFI_PHYSICAL_ADDRESS* Image,
-  OUT    UINTN                 *FileSize
+BdsConnectAllDrivers (
+  VOID
   );
 
 #endif
diff --git a/Platform/ARM/Library/BdsLib/BdsLib.inf b/Platform/ARM/Library/BdsLib/BdsLib.inf
index 637ef5a08128..7441fe539be1 100644
--- a/Platform/ARM/Library/BdsLib/BdsLib.inf
+++ b/Platform/ARM/Library/BdsLib/BdsLib.inf
@@ -22,9 +22,7 @@
 
 [Sources.common]
   BdsFilePath.c
-  BdsAppLoader.c
   BdsHelper.c
-  BdsLoadOption.c
 
 [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
diff --git a/Platform/ARM/Library/BdsLib/BdsLoadOption.c b/Platform/ARM/Library/BdsLib/BdsLoadOption.c
deleted file mode 100644
index 766a9890fc09..000000000000
--- a/Platform/ARM/Library/BdsLib/BdsLoadOption.c
+++ /dev/null
@@ -1,272 +0,0 @@
-/** @file
-*
-*  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
-*
-*  This program and the accompanying materials
-*  are licensed and made available under the terms and conditions of the BSD License
-*  which accompanies this distribution.  The full text of the license may be found at
-*  http://opensource.org/licenses/bsd-license.php
-*
-*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-*
-**/
-
-#include "BdsInternal.h"
-
-EFI_STATUS
-BootOptionParseLoadOption (
-  IN     EFI_LOAD_OPTION *EfiLoadOption,
-  IN     UINTN           EfiLoadOptionSize,
-  IN OUT BDS_LOAD_OPTION **BdsLoadOption
-  )
-{
-  BDS_LOAD_OPTION *LoadOption;
-  UINTN           DescriptionLength;
-  UINTN           EfiLoadOptionPtr;
-
-  if (EfiLoadOption == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  if (EfiLoadOptionSize < sizeof(UINT32) + sizeof(UINT16) + sizeof(CHAR16) + sizeof(EFI_DEVICE_PATH_PROTOCOL)) {
-    return EFI_BAD_BUFFER_SIZE;
-  }
-
-  if (*BdsLoadOption == NULL) {
-    LoadOption = (BDS_LOAD_OPTION*)AllocateZeroPool (sizeof(BDS_LOAD_OPTION));
-    if (LoadOption == NULL) {
-      return EFI_OUT_OF_RESOURCES;
-    }
-  } else {
-    LoadOption = *BdsLoadOption;
-  }
-
-  EfiLoadOptionPtr           = (UINTN)EfiLoadOption;
-  LoadOption->LoadOption     = EfiLoadOption;
-  LoadOption->LoadOptionSize = EfiLoadOptionSize;
-
-  LoadOption->Attributes         = *(UINT32*)EfiLoadOptionPtr;
-  LoadOption->FilePathListLength = *(UINT16*)(EfiLoadOptionPtr + sizeof(UINT32));
-  LoadOption->Description        = (CHAR16*)(EfiLoadOptionPtr + sizeof(UINT32) + sizeof(UINT16));
-  DescriptionLength              = StrSize (LoadOption->Description);
-  LoadOption->FilePathList       = (EFI_DEVICE_PATH_PROTOCOL*)(EfiLoadOptionPtr + sizeof(UINT32) + sizeof(UINT16) + DescriptionLength);
-
-  // If ((End of EfiLoadOptiony - Start of EfiLoadOption) == EfiLoadOptionSize) then No Optional Data
-  if ((UINTN)((UINTN)LoadOption->FilePathList + LoadOption->FilePathListLength - EfiLoadOptionPtr) == EfiLoadOptionSize) {
-    LoadOption->OptionalData     = NULL;
-    LoadOption->OptionalDataSize = 0;
-  } else {
-    LoadOption->OptionalData     = (VOID*)((UINTN)(LoadOption->FilePathList) + LoadOption->FilePathListLength);
-    LoadOption->OptionalDataSize = EfiLoadOptionSize - ((UINTN)LoadOption->OptionalData - EfiLoadOptionPtr);
-  }
-
-  if (*BdsLoadOption == NULL) {
-    *BdsLoadOption = LoadOption;
-  }
-
-  return EFI_SUCCESS;
-}
-
-EFI_STATUS
-BootOptionFromLoadOptionVariable (
-  IN  CHAR16*           BootVariableName,
-  OUT BDS_LOAD_OPTION** BdsLoadOption
-  )
-{
-  EFI_STATUS            Status;
-  EFI_LOAD_OPTION       *EfiLoadOption;
-  UINTN                 EfiLoadOptionSize;
-
-  Status = GetGlobalEnvironmentVariable (BootVariableName, NULL, &EfiLoadOptionSize, (VOID**)&EfiLoadOption);
-  if (!EFI_ERROR(Status)) {
-    *BdsLoadOption = NULL;
-    Status = BootOptionParseLoadOption (EfiLoadOption, EfiLoadOptionSize, BdsLoadOption);
-  }
-
-  return Status;
-}
-
-EFI_STATUS
-BootOptionFromLoadOptionIndex (
-  IN  UINT16            LoadOptionIndex,
-  OUT BDS_LOAD_OPTION **BdsLoadOption
-  )
-{
-  CHAR16        BootVariableName[9];
-  EFI_STATUS    Status;
-
-  UnicodeSPrint (BootVariableName, 9 * sizeof(CHAR16), L"Boot%04X", LoadOptionIndex);
-
-  Status = BootOptionFromLoadOptionVariable (BootVariableName, BdsLoadOption);
-  if (!EFI_ERROR(Status)) {
-    (*BdsLoadOption)->LoadOptionIndex = LoadOptionIndex;
-  }
-
-  return Status;
-}
-
-EFI_STATUS
-BootOptionToLoadOptionVariable (
-  IN BDS_LOAD_OPTION*   BdsLoadOption
-  )
-{
-  EFI_STATUS                    Status;
-  UINTN                         DescriptionSize;
-  //UINT16                        FilePathListLength;
-  EFI_DEVICE_PATH_PROTOCOL*     DevicePathNode;
-  UINTN                         NodeLength;
-  UINT8*                        EfiLoadOptionPtr;
-  VOID*                         OldLoadOption;
-  CHAR16                        BootVariableName[9];
-  UINTN                         BootOrderSize;
-  UINT16*                       BootOrder;
-
-  // If we are overwriting an existent Boot Option then we have to free previously allocated memory
-  if (BdsLoadOption->LoadOptionSize > 0) {
-    OldLoadOption = BdsLoadOption->LoadOption;
-  } else {
-    OldLoadOption = NULL;
-
-    // If this function is called at the creation of the Boot Device entry (not at the update) the
-    // BootOption->LoadOptionSize must be zero then we get a new BootIndex for this entry
-    BdsLoadOption->LoadOptionIndex = BootOptionAllocateBootIndex ();
-
-    //TODO: Add to the the Boot Entry List
-  }
-
-  DescriptionSize = StrSize(BdsLoadOption->Description);
-
-  // Ensure the FilePathListLength information is correct
-  ASSERT (GetDevicePathSize (BdsLoadOption->FilePathList) == BdsLoadOption->FilePathListLength);
-
-  // Allocate the memory for the EFI Load Option
-  BdsLoadOption->LoadOptionSize = sizeof(UINT32) + sizeof(UINT16) + DescriptionSize + BdsLoadOption->FilePathListLength + BdsLoadOption->OptionalDataSize;
-
-  BdsLoadOption->LoadOption = (EFI_LOAD_OPTION *)AllocateZeroPool (BdsLoadOption->LoadOptionSize);
-  if (BdsLoadOption->LoadOption == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  EfiLoadOptionPtr = (UINT8 *) BdsLoadOption->LoadOption;
-
-  //
-  // Populate the EFI Load Option and BDS Boot Option structures
-  //
-
-  // Attributes fields
-  *(UINT32*)EfiLoadOptionPtr = BdsLoadOption->Attributes;
-  EfiLoadOptionPtr += sizeof(UINT32);
-
-  // FilePath List fields
-  *(UINT16*)EfiLoadOptionPtr = BdsLoadOption->FilePathListLength;
-  EfiLoadOptionPtr += sizeof(UINT16);
-
-  // Boot description fields
-  CopyMem (EfiLoadOptionPtr, BdsLoadOption->Description, DescriptionSize);
-  EfiLoadOptionPtr += DescriptionSize;
-
-  // File path fields
-  DevicePathNode = BdsLoadOption->FilePathList;
-  while (!IsDevicePathEndType (DevicePathNode)) {
-    NodeLength = DevicePathNodeLength(DevicePathNode);
-    CopyMem (EfiLoadOptionPtr, DevicePathNode, NodeLength);
-    EfiLoadOptionPtr += NodeLength;
-    DevicePathNode = NextDevicePathNode (DevicePathNode);
-  }
-
-  // Set the End Device Path Type
-  SetDevicePathEndNode (EfiLoadOptionPtr);
-  EfiLoadOptionPtr += sizeof(EFI_DEVICE_PATH);
-
-  // Fill the Optional Data
-  if (BdsLoadOption->OptionalDataSize > 0) {
-    CopyMem (EfiLoadOptionPtr, BdsLoadOption->OptionalData, BdsLoadOption->OptionalDataSize);
-  }
-
-  // Case where the fields have been updated
-  if (OldLoadOption) {
-    // Now, the old data has been copied to the new allocated packed structure, we need to update the pointers of BdsLoadOption
-    BootOptionParseLoadOption (BdsLoadOption->LoadOption, BdsLoadOption->LoadOptionSize, &BdsLoadOption);
-    // Free the old packed structure
-    FreePool (OldLoadOption);
-  }
-
-  // Create/Update Boot#### environment variable
-  UnicodeSPrint (BootVariableName, 9 * sizeof(CHAR16), L"Boot%04X", BdsLoadOption->LoadOptionIndex);
-  Status = gRT->SetVariable (
-      BootVariableName,
-      &gEfiGlobalVariableGuid,
-      EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
-      BdsLoadOption->LoadOptionSize,
-      BdsLoadOption->LoadOption
-      );
-
-  // When it is a new entry we must add the entry to the BootOrder
-  if (OldLoadOption == NULL) {
-    // Add the new Boot Index to the list
-    Status = GetGlobalEnvironmentVariable (L"BootOrder", NULL, &BootOrderSize, (VOID**)&BootOrder);
-    if (!EFI_ERROR(Status)) {
-      BootOrder = ReallocatePool (BootOrderSize, BootOrderSize + sizeof(UINT16), BootOrder);
-      // Add the new index at the end
-      BootOrder[BootOrderSize / sizeof(UINT16)] = BdsLoadOption->LoadOptionIndex;
-      BootOrderSize += sizeof(UINT16);
-    } else {
-      // BootOrder does not exist. Create it
-      BootOrderSize = sizeof(UINT16);
-      BootOrder = &(BdsLoadOption->LoadOptionIndex);
-    }
-
-    // Update (or Create) the BootOrder environment variable
-    gRT->SetVariable (
-        L"BootOrder",
-        &gEfiGlobalVariableGuid,
-        EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
-        BootOrderSize,
-        BootOrder
-        );
-    DEBUG((EFI_D_ERROR,"Create %s\n",BootVariableName));
-
-    // Free memory allocated by GetGlobalEnvironmentVariable
-    if (!EFI_ERROR(Status)) {
-      FreePool (BootOrder);
-    }
-  } else {
-    DEBUG((EFI_D_ERROR,"Update %s\n",BootVariableName));
-  }
-
-  return EFI_SUCCESS;
-}
-
-UINT16
-BootOptionAllocateBootIndex (
-  VOID
-  )
-{
-  EFI_STATUS        Status;
-  UINTN             Index;
-  UINT32            BootIndex;
-  UINT16            *BootOrder;
-  UINTN             BootOrderSize;
-  BOOLEAN           Found;
-
-  // Get the Boot Option Order from the environment variable
-  Status = GetGlobalEnvironmentVariable (L"BootOrder", NULL, &BootOrderSize, (VOID**)&BootOrder);
-  if (!EFI_ERROR(Status)) {
-    for (BootIndex = 0; BootIndex <= 0xFFFF; BootIndex++) {
-      Found = FALSE;
-      for (Index = 0; Index < BootOrderSize / sizeof (UINT16); Index++) {
-        if (BootOrder[Index] == BootIndex) {
-          Found = TRUE;
-          break;
-        }
-      }
-      if (!Found) {
-        return BootIndex;
-      }
-    }
-    FreePool (BootOrder);
-  }
-  // Return the first index
-  return 0;
-}
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths
  2018-11-23  8:44 [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Ard Biesheuvel
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 1/3] Platform/ARM: import ARM platform specific BdsLib header Ard Biesheuvel
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions Ard Biesheuvel
@ 2018-11-23  8:44 ` Ard Biesheuvel
  2018-11-23 12:46   ` Laszlo Ersek
  2018-11-23 14:15 ` [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Thomas Abraham
  2018-11-26 15:06 ` Leif Lindholm
  4 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-23  8:44 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, thomas.abraham, nariman.poushin, lersek, philmd,
	Ard Biesheuvel

DevicePath node types may have any size, and so it is up to the
code that manipulates them to ensure that dereferencing them only
occurs when the pointer is aligned explicitly.

Since BdsConnectAndUpdateDevicePath() has only two callers, one of
which itself, we can simply duplicate the device path (similar to
how DxeCore's CoreConnectController () does it), and free the pool
allocation again on the way out. (Note that the allocation only
occurs when the non-recursive path is taken and the function
returns EFI_SUCCESS)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/ARM/Library/BdsLib/BdsFilePath.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c b/Platform/ARM/Library/BdsLib/BdsFilePath.c
index 62f796e5526d..ad66b2f82718 100644
--- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
+++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
@@ -423,8 +423,8 @@ BdsConnectAndUpdateDevicePath (
     }
   }
 
-  if (RemainingDevicePath) {
-    *RemainingDevicePath = Remaining;
+  if (!EFI_ERROR (Status) && RemainingDevicePath != NULL) {
+    *RemainingDevicePath = DuplicateDevicePath (Remaining);
   }
 
   return Status;
@@ -1314,14 +1314,18 @@ BdsLoadImageAndUpdateDevicePath (
   }
 
   FileLoader = FileLoaders;
+  Status = EFI_UNSUPPORTED;
   while (FileLoader->Support != NULL) {
     if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {
-      return FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath, Type, Image, FileSize);
+      Status = FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath,
+                             Type, Image, FileSize);
+      break;
     }
     FileLoader++;
   }
 
-  return EFI_UNSUPPORTED;
+  FreePool (RemainingDevicePath);
+  return Status;
 }
 
 EFI_STATUS
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions Ard Biesheuvel
@ 2018-11-23 12:39   ` Laszlo Ersek
  0 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-11-23 12:39 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: nariman.poushin

On 11/23/18 09:44, Ard Biesheuvel wrote:
> Clean up BdsLib (which is deprecated and should not be used for future
> development) by removing all the pieces that are not being used at the
> moment.
> 
> After this patch, only BdsLoadImage() remains, and the pieces it relies
> upon. This function is used by FdtPlatformDxe to load device tree
> binaries from device paths.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/ARM/Include/Library/BdsLib.h       | 186 -------------
>  Platform/ARM/Library/BdsLib/BdsAppLoader.c  | 253 ------------------
>  Platform/ARM/Library/BdsLib/BdsFilePath.c   |  83 +-----
>  Platform/ARM/Library/BdsLib/BdsHelper.c     | 122 ---------
>  Platform/ARM/Library/BdsLib/BdsInternal.h   |  15 +-
>  Platform/ARM/Library/BdsLib/BdsLib.inf      |   2 -
>  Platform/ARM/Library/BdsLib/BdsLoadOption.c | 272 --------------------
>  7 files changed, 13 insertions(+), 920 deletions(-)

Heh, this diffstat is a bit beyond my willingness to review in detail :)
I do see it only removes lines, so if it compiles, it must be good.
Also, BdsConnectDevicePath() in particular is removed:

> -/**
> -  Connect a Device Path and return the handle of the driver that support this DevicePath
> -
> -  @param  DevicePath            Device Path of the File to connect
> -  @param  Handle                Handle of the driver that support this DevicePath
> -  @param  RemainingDevicePath   Remaining DevicePath nodes that do not match the driver DevicePath
> -
> -  @retval EFI_SUCCESS           A driver that matches the Device Path has been found
> -  @retval EFI_NOT_FOUND         No handles match the search.
> -  @retval EFI_INVALID_PARAMETER DevicePath or Handle is NULL
> -
> -**/
> -EFI_STATUS
> -BdsConnectDevicePath (
> -  IN  EFI_DEVICE_PATH_PROTOCOL* DevicePath,
> -  OUT EFI_HANDLE                *Handle,
> -  OUT EFI_DEVICE_PATH_PROTOCOL  **RemainingDevicePath
> -  )
> -{
> -  return BdsConnectAndUpdateDevicePath (&DevicePath, Handle, RemainingDevicePath);
> -}

Hence:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths Ard Biesheuvel
@ 2018-11-23 12:46   ` Laszlo Ersek
  0 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2018-11-23 12:46 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel; +Cc: nariman.poushin

On 11/23/18 09:44, Ard Biesheuvel wrote:
> DevicePath node types may have any size, and so it is up to the
> code that manipulates them to ensure that dereferencing them only
> occurs when the pointer is aligned explicitly.
> 
> Since BdsConnectAndUpdateDevicePath() has only two callers, one of
> which itself, we can simply duplicate the device path (similar to
> how DxeCore's CoreConnectController () does it), and free the pool
> allocation again on the way out. (Note that the allocation only
> occurs when the non-recursive path is taken and the function
> returns EFI_SUCCESS)
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/ARM/Library/BdsLib/BdsFilePath.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/Platform/ARM/Library/BdsLib/BdsFilePath.c b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> index 62f796e5526d..ad66b2f82718 100644
> --- a/Platform/ARM/Library/BdsLib/BdsFilePath.c
> +++ b/Platform/ARM/Library/BdsLib/BdsFilePath.c
> @@ -423,8 +423,8 @@ BdsConnectAndUpdateDevicePath (
>      }
>    }
>  
> -  if (RemainingDevicePath) {
> -    *RemainingDevicePath = Remaining;
> +  if (!EFI_ERROR (Status) && RemainingDevicePath != NULL) {
> +    *RemainingDevicePath = DuplicateDevicePath (Remaining);
>    }
>  
>    return Status;
> @@ -1314,14 +1314,18 @@ BdsLoadImageAndUpdateDevicePath (
>    }
>  
>    FileLoader = FileLoaders;
> +  Status = EFI_UNSUPPORTED;
>    while (FileLoader->Support != NULL) {
>      if (FileLoader->Support (*DevicePath, Handle, RemainingDevicePath)) {
> -      return FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath, Type, Image, FileSize);
> +      Status = FileLoader->LoadImage (DevicePath, Handle, RemainingDevicePath,
> +                             Type, Image, FileSize);
> +      break;
>      }
>      FileLoader++;
>    }
>  
> -  return EFI_UNSUPPORTED;
> +  FreePool (RemainingDevicePath);
> +  return Status;
>  }
>  
>  EFI_STATUS
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib
  2018-11-23  8:44 [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2018-11-23  8:44 ` [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths Ard Biesheuvel
@ 2018-11-23 14:15 ` Thomas Abraham
  2018-11-23 15:17   ` Ard Biesheuvel
  2018-11-26 15:06 ` Leif Lindholm
  4 siblings, 1 reply; 10+ messages in thread
From: Thomas Abraham @ 2018-11-23 14:15 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, lersek, Nariman Poushin

Hi Ard,
On Fri, Nov 23, 2018 at 2:16 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> The deprecated BdsLib library class in ArmPkg is still depended upon, but
> only a single implementation exists, which now resides in edk2-platforms.
>
> This implementation has some issues in how it deals with Device Paths,
> so let's fix those, but first move over the library interface declaration
> and get rid of the parts that are no longer used. This will permit dropping
> it from ArmPkg in EDK2.
>
> Changes since v1:
> - add Laszlo's ack to #1
> - update #2 to remove everything we no longer need from BdsLib
> - drop #3 which was bogus
> - update #4 to ensure that we only duplicate the device path when we
>   are about to return EFI_SUCCESS
>
> Ard Biesheuvel (3):
>   Platform/ARM: import ARM platform specific BdsLib header
>   Platform/ARM/BdsLib: drop unused functions
>   Platform/ARM/BdsLib: maintain alignment for DevicePaths
>
>  Platform/ARM/ARM.dec                          |   3 +
>  .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
>  Platform/ARM/Include/Library/BdsLib.h         |  26 ++
>  Platform/ARM/Library/BdsLib/BdsAppLoader.c    | 253 ----------------
>  Platform/ARM/Library/BdsLib/BdsFilePath.c     |  95 +-----
>  Platform/ARM/Library/BdsLib/BdsHelper.c       | 122 --------
>  Platform/ARM/Library/BdsLib/BdsInternal.h     |  16 +-
>  Platform/ARM/Library/BdsLib/BdsLib.inf        |   4 +-
>  Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 ------------------
>  9 files changed, 52 insertions(+), 741 deletions(-)
>  create mode 100644 Platform/ARM/Include/Library/BdsLib.h
>  delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
>  delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c

Tested this patch series with the following two patch series applied
on the Juno board.
 - [PATCH v2 0/5] ArmPlatformPkg, ArmVirtPkg: discover NOR flash banks from DTB
 - [PATCH edk2-platforms 0/3] drop GUIDs from NOR flash bank descriptors

Boot on Juno board works fine.

For this and the other two patch series
Tested-by: Thomas Abraham <thomas.abraham@arm.com>

>
> --
> 2.17.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib
  2018-11-23 14:15 ` [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Thomas Abraham
@ 2018-11-23 15:17   ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-23 15:17 UTC (permalink / raw)
  To: Thomas Panakamattam Abraham
  Cc: edk2-devel@lists.01.org, Laszlo Ersek, Nariman Poushin

On Fri, 23 Nov 2018 at 15:16, Thomas Abraham <thomas.abraham@arm.com> wrote:
>
> Hi Ard,
> On Fri, Nov 23, 2018 at 2:16 PM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > The deprecated BdsLib library class in ArmPkg is still depended upon, but
> > only a single implementation exists, which now resides in edk2-platforms.
> >
> > This implementation has some issues in how it deals with Device Paths,
> > so let's fix those, but first move over the library interface declaration
> > and get rid of the parts that are no longer used. This will permit dropping
> > it from ArmPkg in EDK2.
> >
> > Changes since v1:
> > - add Laszlo's ack to #1
> > - update #2 to remove everything we no longer need from BdsLib
> > - drop #3 which was bogus
> > - update #4 to ensure that we only duplicate the device path when we
> >   are about to return EFI_SUCCESS
> >
> > Ard Biesheuvel (3):
> >   Platform/ARM: import ARM platform specific BdsLib header
> >   Platform/ARM/BdsLib: drop unused functions
> >   Platform/ARM/BdsLib: maintain alignment for DevicePaths
> >
> >  Platform/ARM/ARM.dec                          |   3 +
> >  .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
> >  Platform/ARM/Include/Library/BdsLib.h         |  26 ++
> >  Platform/ARM/Library/BdsLib/BdsAppLoader.c    | 253 ----------------
> >  Platform/ARM/Library/BdsLib/BdsFilePath.c     |  95 +-----
> >  Platform/ARM/Library/BdsLib/BdsHelper.c       | 122 --------
> >  Platform/ARM/Library/BdsLib/BdsInternal.h     |  16 +-
> >  Platform/ARM/Library/BdsLib/BdsLib.inf        |   4 +-
> >  Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 ------------------
> >  9 files changed, 52 insertions(+), 741 deletions(-)
> >  create mode 100644 Platform/ARM/Include/Library/BdsLib.h
> >  delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
> >  delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c
>
> Tested this patch series with the following two patch series applied
> on the Juno board.
>  - [PATCH v2 0/5] ArmPlatformPkg, ArmVirtPkg: discover NOR flash banks from DTB
>  - [PATCH edk2-platforms 0/3] drop GUIDs from NOR flash bank descriptors
>
> Boot on Juno board works fine.
>
> For this and the other two patch series
> Tested-by: Thomas Abraham <thomas.abraham@arm.com>
>

Thanks Thomas!


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib
  2018-11-23  8:44 [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2018-11-23 14:15 ` [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Thomas Abraham
@ 2018-11-26 15:06 ` Leif Lindholm
  2018-11-26 16:53   ` Ard Biesheuvel
  4 siblings, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2018-11-26 15:06 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel, thomas.abraham, nariman.poushin, lersek, philmd

On Fri, Nov 23, 2018 at 09:44:03AM +0100, Ard Biesheuvel wrote:
> The deprecated BdsLib library class in ArmPkg is still depended upon, but
> only a single implementation exists, which now resides in edk2-platforms.
> 
> This implementation has some issues in how it deals with Device Paths,
> so let's fix those, but first move over the library interface declaration
> and get rid of the parts that are no longer used. This will permit dropping
> it from ArmPkg in EDK2.

For the series:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> Changes since v1:
> - add Laszlo's ack to #1
> - update #2 to remove everything we no longer need from BdsLib
> - drop #3 which was bogus
> - update #4 to ensure that we only duplicate the device path when we
>   are about to return EFI_SUCCESS
> 
> Ard Biesheuvel (3):
>   Platform/ARM: import ARM platform specific BdsLib header
>   Platform/ARM/BdsLib: drop unused functions
>   Platform/ARM/BdsLib: maintain alignment for DevicePaths
> 
>  Platform/ARM/ARM.dec                          |   3 +
>  .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
>  Platform/ARM/Include/Library/BdsLib.h         |  26 ++
>  Platform/ARM/Library/BdsLib/BdsAppLoader.c    | 253 ----------------
>  Platform/ARM/Library/BdsLib/BdsFilePath.c     |  95 +-----
>  Platform/ARM/Library/BdsLib/BdsHelper.c       | 122 --------
>  Platform/ARM/Library/BdsLib/BdsInternal.h     |  16 +-
>  Platform/ARM/Library/BdsLib/BdsLib.inf        |   4 +-
>  Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 ------------------
>  9 files changed, 52 insertions(+), 741 deletions(-)
>  create mode 100644 Platform/ARM/Include/Library/BdsLib.h
>  delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
>  delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c
> 
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib
  2018-11-26 15:06 ` Leif Lindholm
@ 2018-11-26 16:53   ` Ard Biesheuvel
  0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-26 16:53 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: edk2-devel@lists.01.org, Thomas Panakamattam Abraham,
	Nariman Poushin, Laszlo Ersek, Philippe Mathieu-Daudé

On Mon, 26 Nov 2018 at 16:06, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Nov 23, 2018 at 09:44:03AM +0100, Ard Biesheuvel wrote:
> > The deprecated BdsLib library class in ArmPkg is still depended upon, but
> > only a single implementation exists, which now resides in edk2-platforms.
> >
> > This implementation has some issues in how it deals with Device Paths,
> > so let's fix those, but first move over the library interface declaration
> > and get rid of the parts that are no longer used. This will permit dropping
> > it from ArmPkg in EDK2.
>
> For the series:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks. This series (and the Platform/Comcast patch) pushed as
397bbafdbff3..f98fb46d3a3d


> > Changes since v1:
> > - add Laszlo's ack to #1
> > - update #2 to remove everything we no longer need from BdsLib
> > - drop #3 which was bogus
> > - update #4 to ensure that we only duplicate the device path when we
> >   are about to return EFI_SUCCESS
> >
> > Ard Biesheuvel (3):
> >   Platform/ARM: import ARM platform specific BdsLib header
> >   Platform/ARM/BdsLib: drop unused functions
> >   Platform/ARM/BdsLib: maintain alignment for DevicePaths
> >
> >  Platform/ARM/ARM.dec                          |   3 +
> >  .../Drivers/FdtPlatformDxe/FdtPlatformDxe.inf |   2 +-
> >  Platform/ARM/Include/Library/BdsLib.h         |  26 ++
> >  Platform/ARM/Library/BdsLib/BdsAppLoader.c    | 253 ----------------
> >  Platform/ARM/Library/BdsLib/BdsFilePath.c     |  95 +-----
> >  Platform/ARM/Library/BdsLib/BdsHelper.c       | 122 --------
> >  Platform/ARM/Library/BdsLib/BdsInternal.h     |  16 +-
> >  Platform/ARM/Library/BdsLib/BdsLib.inf        |   4 +-
> >  Platform/ARM/Library/BdsLib/BdsLoadOption.c   | 272 ------------------
> >  9 files changed, 52 insertions(+), 741 deletions(-)
> >  create mode 100644 Platform/ARM/Include/Library/BdsLib.h
> >  delete mode 100644 Platform/ARM/Library/BdsLib/BdsAppLoader.c
> >  delete mode 100644 Platform/ARM/Library/BdsLib/BdsLoadOption.c
> >
> > --
> > 2.17.1
> >


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-11-26 16:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-23  8:44 [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Ard Biesheuvel
2018-11-23  8:44 ` [PATCH v2 edk2-platforms 1/3] Platform/ARM: import ARM platform specific BdsLib header Ard Biesheuvel
2018-11-23  8:44 ` [PATCH v2 edk2-platforms 2/3] Platform/ARM/BdsLib: drop unused functions Ard Biesheuvel
2018-11-23 12:39   ` Laszlo Ersek
2018-11-23  8:44 ` [PATCH v2 edk2-platforms 3/3] Platform/ARM/BdsLib: maintain alignment for DevicePaths Ard Biesheuvel
2018-11-23 12:46   ` Laszlo Ersek
2018-11-23 14:15 ` [PATCH v2 edk2-platforms 0/3] Platform/ARM: fix DevicePath mishandling in BdsLib Thomas Abraham
2018-11-23 15:17   ` Ard Biesheuvel
2018-11-26 15:06 ` Leif Lindholm
2018-11-26 16:53   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox