[PATCH edk2-platforms v5 0/2] add platform boot options

[PATCH edk2-platforms v5 0/2] add platform boot options

[Haojian Zhuang]

Changelog:
v5:
  * Avoid to merge device path and grub's file path in driver.
    Merge them directly in DSC file.
  * Avoid duplicated code to create boot options.
  * Use goto to handle error condition.
v4:
  * Add BootCount parameter in the interface.
v3:
  * Move in initilization of boot entry.
  * Update the name of interface in platform boot manager protocol.
v2:
  * Update with platform boot manager protocol.

Haojian Zhuang (2):
  Platform/HiKey960: register predefined boot options
  Platform/HiKey: create 4 boot options

 Platform/Hisilicon/HiKey/HiKey.dec                 |   8 +-
 Platform/Hisilicon/HiKey/HiKey.dsc                 |   7 +
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c       | 171 ++++++++++++++++++++
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf     |  11 ++
 .../{HiKey/HiKey.dec => HiKey960/HiKey960.dec}     |  17 +-
 Platform/Hisilicon/HiKey960/HiKey960.dsc           |   6 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 172 +++++++++++++++++++++
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  11 ++
 8 files changed, 391 insertions(+), 12 deletions(-)
 copy Platform/Hisilicon/{HiKey/HiKey.dec => HiKey960/HiKey960.dec} (56%)

-- 
2.7.4

[PATCH edk2-platforms v5 1/2] Platform/HiKey960: register predefined boot options

[Haojian Zhuang]

Create 4 boot options on HiKey960 platform. They're "Boot from SD",
"Grub", "Android Boot" and "Android Fastboot".

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Platform/Hisilicon/HiKey960/HiKey960.dec           |  35 +++++
 Platform/Hisilicon/HiKey960/HiKey960.dsc           |   6 +
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 172 +++++++++++++++++++++
 .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  11 ++
 4 files changed, 224 insertions(+)
 create mode 100644 Platform/Hisilicon/HiKey960/HiKey960.dec

diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dec b/Platform/Hisilicon/HiKey960/HiKey960.dec
new file mode 100644
index 000000000000..922d7199c5a5
--- /dev/null
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dec
@@ -0,0 +1,35 @@
+#
+#  Copyright (c) 2018, Linaro 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.
+#
+
+[Defines]
+  DEC_SPECIFICATION              = 0x00010019
+  PACKAGE_NAME                   = HiKey960
+  PACKAGE_GUID                   = 1892b5b5-d18d-47a3-8fab-e3ae6b4226b0
+  PACKAGE_VERSION                = 0.1
+
+################################################################################
+#
+# Include Section - list of Include Paths that are provided by this package.
+#                   Comments are used for Keywords and Module Types.
+#
+# Supported Module Types:
+#  BASE SEC PEI_CORE PEIM DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER DXE_SAL_DRIVER UEFI_DRIVER UEFI_APPLICATION
+#
+################################################################################
+[Guids.common]
+  gHiKey960TokenSpaceGuid        = { 0x99a14446, 0xaad7, 0xe460, {0xb4, 0xe5, 0x1f, 0x79, 0xaa, 0xa4, 0x93, 0xfd } }
+
+[PcdsFixedAtBuild.common]
+  gHiKey960TokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x00000001
+  gHiKey960TokenSpaceGuid.PcdAndroidBootFile|{ 0x36, 0x8b, 0x73, 0x3a, 0xc5, 0xb9, 0x63, 0x47, 0xab, 0xbd, 0x6c, 0xbd, 0x4b, 0x25, 0xf9, 0xff }|VOID*|0x00000002
+  gHiKey960TokenSpaceGuid.PcdAndroidFastbootFile|{ 0x2a, 0x50, 0x88, 0x95, 0x70, 0x53, 0xe3, 0x11, 0x86, 0x31, 0xd7, 0xc5, 0x95, 0x13, 0x64, 0xc8 }|VOID*|0x00000003
+  gHiKey960TokenSpaceGuid.PcdSdBootDevicePath|L""|VOID*|0x00000004
diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 859ab84f8415..4097fbfd77f5 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -132,6 +132,12 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbVendorId|0x18d1
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xd00d
 
+  #
+  # Android Loader
+  #
+  gHiKey960TokenSpaceGuid.PcdAndroidBootDevicePath|L"VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00003BFF0000000000)/UFS(0x0,0x3)/HD(7,GPT,D3340696-9B95-4C64-8DF6-E6D4548FBA41,0x12100,0x4000)/\\EFI\\BOOT\\GRUBAA64.EFI"
+  gHiKey960TokenSpaceGuid.PcdSdBootDevicePath|L"VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00F037FF0000000000)/SD(0x0)"
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
index 473d61ed384e..413423e75569 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
+++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c
@@ -30,10 +30,16 @@
 #include <Library/PrintLib.h>
 #include <Library/SerialPortLib.h>
 #include <Library/TimerLib.h>
+#include <Library/UefiBootManagerLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 
+#include <Protocol/DevicePath.h>
+#include <Protocol/DevicePathFromText.h>
 #include <Protocol/EmbeddedGpio.h>
+#include <Protocol/LoadedImage.h>
+#include <Protocol/PlatformBootManager.h>
 #include <Protocol/PlatformVirtualKeyboard.h>
 
 #define ADC_ADCIN0                       0
@@ -86,6 +92,10 @@
 
 #define DETECT_SW_FASTBOOT               68        // GPIO8_4
 
+#define HIKEY960_BOOT_OPTION_NUM         4
+
+#define GRUB_FILE_NAME                   L"\\EFI\\BOOT\\GRUBAA64.EFI"
+
 typedef struct {
   UINT64        Magic;
   UINT64        Data;
@@ -359,6 +369,158 @@ OnEndOfDxe (
   }
 }
 
+STATIC
+EFI_STATUS
+CreatePlatformBootOptionFromPath (
+  IN     CHAR16                          *PathStr,
+  IN     CHAR16                          *Description,
+  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION    *BootOption
+  )
+{
+  EFI_STATUS                   Status;
+  EFI_DEVICE_PATH              *DevicePath;
+
+  DevicePath = (EFI_DEVICE_PATH *)ConvertTextToDevicePath (PathStr);
+  ASSERT (DevicePath != NULL);
+  Status = EfiBootManagerInitializeLoadOption (
+             BootOption,
+             LoadOptionNumberUnassigned,
+             LoadOptionTypeBoot,
+             LOAD_OPTION_ACTIVE,
+             Description,
+             DevicePath,
+             NULL,
+             0
+             );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  FreePool (DevicePath);
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+CreatePlatformBootOptionFromGuid (
+  IN     EFI_GUID                        *FileGuid,
+  IN     CHAR16                          *Description,
+  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION    *BootOption
+  )
+{
+  EFI_STATUS                             Status;
+  EFI_DEVICE_PATH                        *DevicePath;
+  EFI_DEVICE_PATH                        *TempDevicePath;
+  EFI_LOADED_IMAGE_PROTOCOL              *LoadedImage;
+  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH      FileNode;
+
+  Status = gBS->HandleProtocol (
+                  gImageHandle,
+                  &gEfiLoadedImageProtocolGuid,
+                  (VOID **) &LoadedImage
+                  );
+  ASSERT_EFI_ERROR (Status);
+  EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
+  TempDevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
+  ASSERT (TempDevicePath != NULL);
+  DevicePath = AppendDevicePathNode (
+                 TempDevicePath,
+                 (EFI_DEVICE_PATH_PROTOCOL *) &FileNode
+                 );
+  ASSERT (DevicePath != NULL);
+  Status = EfiBootManagerInitializeLoadOption (
+             BootOption,
+             LoadOptionNumberUnassigned,
+             LoadOptionTypeBoot,
+             LOAD_OPTION_ACTIVE,
+             Description,
+             DevicePath,
+             NULL,
+             0
+             );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  FreePool (DevicePath);
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+GetPlatformBootOptionsAndKeys (
+  OUT UINTN                              *BootCount,
+  OUT EFI_BOOT_MANAGER_LOAD_OPTION       **BootOptions,
+  OUT EFI_INPUT_KEY                      **BootKeys
+  )
+{
+  EFI_GUID                               *FileGuid;
+  CHAR16                                 *PathStr;
+  EFI_STATUS                             Status;
+  UINTN                                  Size;
+
+  Size = sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * HIKEY960_BOOT_OPTION_NUM;
+  *BootOptions = (EFI_BOOT_MANAGER_LOAD_OPTION *)AllocateZeroPool (Size);
+  if (*BootOptions == NULL) {
+    DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootOptions\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+  Size = sizeof (EFI_INPUT_KEY) * HIKEY960_BOOT_OPTION_NUM;
+  *BootKeys = (EFI_INPUT_KEY *)AllocateZeroPool (Size);
+  if (*BootKeys == NULL) {
+    DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootKeys\n"));
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Error;
+  }
+
+  PathStr = (CHAR16 *)PcdGetPtr (PcdSdBootDevicePath);
+  ASSERT (PathStr != NULL);
+  Status = CreatePlatformBootOptionFromPath (
+             PathStr,
+             L"Boot from SD",
+             &(*BootOptions)[0]
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  PathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (PathStr != NULL);
+  Status = CreatePlatformBootOptionFromPath (
+             PathStr,
+             L"Grub",
+             &(*BootOptions)[1]
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  FileGuid = PcdGetPtr (PcdAndroidBootFile);
+  ASSERT (FileGuid != NULL);
+  Status = CreatePlatformBootOptionFromGuid (
+             FileGuid,
+             L"Android Boot",
+             &(*BootOptions)[2]
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  FileGuid = PcdGetPtr (PcdAndroidFastbootFile);
+  ASSERT (FileGuid != NULL);
+  Status = CreatePlatformBootOptionFromGuid (
+             FileGuid,
+             L"Android Fastboot",
+             &(*BootOptions)[3]
+             );
+  ASSERT_EFI_ERROR (Status);
+  (*BootKeys)[3].ScanCode = SCAN_NULL;
+  (*BootKeys)[3].UnicodeChar = 'f';
+
+  *BootCount = 4;
+
+  return EFI_SUCCESS;
+Error:
+  FreePool (*BootOptions);
+  return Status;
+}
+
+PLATFORM_BOOT_MANAGER_PROTOCOL mPlatformBootManager = {
+  GetPlatformBootOptionsAndKeys
+};
+
 EFI_STATUS
 EFIAPI
 VirtualKeyboardRegister (
@@ -487,5 +649,15 @@ HiKey960EntryPoint (
                   EFI_NATIVE_INTERFACE,
                   &mVirtualKeyboard
                   );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gPlatformBootManagerProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mPlatformBootManager
+                  );
   return Status;
 }
diff --git a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
index cc517656b340..5adedd79d3e8 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
+++ b/Platform/Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf
@@ -25,6 +25,7 @@ [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  Platform/Hisilicon/HiKey960/HiKey960.dec
 
 [LibraryClasses]
   BaseMemoryLib
@@ -37,15 +38,25 @@ [LibraryClasses]
   PrintLib
   SerialPortLib
   TimerLib
+  UefiBootManagerLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
   UefiLib
   UefiRuntimeServicesTableLib
 
 [Protocols]
+  gEfiDevicePathFromTextProtocolGuid
+  gEfiLoadedImageProtocolGuid
   gEmbeddedGpioProtocolGuid
+  gPlatformBootManagerProtocolGuid
   gPlatformVirtualKeyboardProtocolGuid
 
+[Pcd]
+  gHiKey960TokenSpaceGuid.PcdAndroidBootDevicePath
+  gHiKey960TokenSpaceGuid.PcdAndroidBootFile
+  gHiKey960TokenSpaceGuid.PcdAndroidFastbootFile
+  gHiKey960TokenSpaceGuid.PcdSdBootDevicePath
+
 [Guids]
   gEfiEndOfDxeEventGroupGuid
   gEfiFileInfoGuid
-- 
2.7.4

[PATCH edk2-platforms v5 2/2] Platform/HiKey: create 4 boot options

[Haojian Zhuang]

Create 4 predefined boot options for HiKey. They're
"Boot from SD", "Grub", "Android Boot" and "Android Fastboot".

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Platform/Hisilicon/HiKey/HiKey.dec             |   8 +-
 Platform/Hisilicon/HiKey/HiKey.dsc             |   7 +
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c   | 171 +++++++++++++++++++++++++
 Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf |  11 ++
 4 files changed, 194 insertions(+), 3 deletions(-)

diff --git a/Platform/Hisilicon/HiKey/HiKey.dec b/Platform/Hisilicon/HiKey/HiKey.dec
index 537138eb45a1..bb94c5cab13f 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dec
+++ b/Platform/Hisilicon/HiKey/HiKey.dec
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
+#  Copyright (c) 2014-2018, Linaro Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -32,5 +32,7 @@ [Guids.common]
   gHiKeyTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
 
 [PcdsFixedAtBuild.common]
-  gHiKeyTokenSpaceGuid.PcdAndroidFastbootNvmDevicePath|L""|VOID*|0x00000001
-  gHiKeyTokenSpaceGuid.PcdArmFastbootFlashLimit|L""|VOID*|0x00000002
+  gHiKeyTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x00000001
+  gHiKeyTokenSpaceGuid.PcdAndroidBootFile|{ 0x36, 0x8b, 0x73, 0x3a, 0xc5, 0xb9, 0x63, 0x47, 0xab, 0xbd, 0x6c, 0xbd, 0x4b, 0x25, 0xf9, 0xff }|VOID*|0x00000002
+  gHiKeyTokenSpaceGuid.PcdAndroidFastbootFile|{ 0x2a, 0x50, 0x88, 0x95, 0x70, 0x53, 0xe3, 0x11, 0x86, 0x31, 0xd7, 0xc5, 0x95, 0x13, 0x64, 0xc8 }|VOID*|0x00000003
+  gHiKeyTokenSpaceGuid.PcdSdBootDevicePath|L""|VOID*|0x00000004
diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc b/Platform/Hisilicon/HiKey/HiKey.dsc
index 83dd68a820b1..9c1fc2e1b40d 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dsc
+++ b/Platform/Hisilicon/HiKey/HiKey.dsc
@@ -138,6 +138,13 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbVendorId|0x18d1
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xd00d
 
+  #
+  # Android Loader
+  #
+  gHiKeyTokenSpaceGuid.PcdAndroidBootDevicePath|L"VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00D023F70000000000)/eMMC(0x0)/Ctrl(0x0)/HD(6,GPT,5C0F213C-17E1-4149-88C8-8B50FB4EC70E,0x7000,0x20000)/\\EFI\\BOOT\\GRUBAA64.EFI"
+  gHiKeyTokenSpaceGuid.PcdSdBootDevicePath|L"VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,00E023F70000000000)/SD(0x0)"
+
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c
index 65e800116b76..e20538bd77bf 100644
--- a/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c
+++ b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c
@@ -18,12 +18,18 @@
 #include <Library/DevicePathLib.h>
 #include <Library/IoLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
 #include <Library/PrintLib.h>
+#include <Library/UefiBootManagerLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
 
+#include <Protocol/DevicePath.h>
+#include <Protocol/DevicePathFromText.h>
 #include <Protocol/EmbeddedGpio.h>
+#include <Protocol/LoadedImage.h>
+#include <Protocol/PlatformBootManager.h>
 #include <Protocol/PlatformVirtualKeyboard.h>
 
 #include <Hi6220.h>
@@ -42,6 +48,9 @@
 #define ADB_REBOOT_BOOTLOADER            0x77665500
 #define ADB_REBOOT_NONE                  0x77665501
 
+#define HIKEY_BOOT_OPTION_NUM            4
+
+#define GRUB_FILE_NAME                   L"\\EFI\\BOOT\\GRUBAA64.EFI"
 
 typedef struct {
   UINT64        Magic;
@@ -121,6 +130,158 @@ HiKeyInitPeripherals (
   return EFI_SUCCESS;
 }
 
+STATIC
+EFI_STATUS
+CreatePlatformBootOptionFromPath (
+  IN     CHAR16                          *PathStr,
+  IN     CHAR16                          *Description,
+  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION    *BootOption
+  )
+{
+  EFI_STATUS                   Status;
+  EFI_DEVICE_PATH              *DevicePath;
+
+  DevicePath = (EFI_DEVICE_PATH *)ConvertTextToDevicePath (PathStr);
+  ASSERT (DevicePath != NULL);
+  Status = EfiBootManagerInitializeLoadOption (
+             BootOption,
+             LoadOptionNumberUnassigned,
+             LoadOptionTypeBoot,
+             LOAD_OPTION_ACTIVE,
+             Description,
+             DevicePath,
+             NULL,
+             0
+             );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  FreePool (DevicePath);
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+CreatePlatformBootOptionFromGuid (
+  IN     EFI_GUID                        *FileGuid,
+  IN     CHAR16                          *Description,
+  IN OUT EFI_BOOT_MANAGER_LOAD_OPTION    *BootOption
+  )
+{
+  EFI_STATUS                             Status;
+  EFI_DEVICE_PATH                        *DevicePath;
+  EFI_DEVICE_PATH                        *TempDevicePath;
+  EFI_LOADED_IMAGE_PROTOCOL              *LoadedImage;
+  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH      FileNode;
+
+  Status = gBS->HandleProtocol (
+                  gImageHandle,
+                  &gEfiLoadedImageProtocolGuid,
+                  (VOID **) &LoadedImage
+                  );
+  ASSERT_EFI_ERROR (Status);
+  EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
+  TempDevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
+  ASSERT (TempDevicePath != NULL);
+  DevicePath = AppendDevicePathNode (
+                 TempDevicePath,
+                 (EFI_DEVICE_PATH_PROTOCOL *) &FileNode
+                 );
+  ASSERT (DevicePath != NULL);
+  Status = EfiBootManagerInitializeLoadOption (
+             BootOption,
+             LoadOptionNumberUnassigned,
+             LoadOptionTypeBoot,
+             LOAD_OPTION_ACTIVE,
+             Description,
+             DevicePath,
+             NULL,
+             0
+             );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  FreePool (DevicePath);
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+GetPlatformBootOptionsAndKeys (
+  OUT UINTN                              *BootCount,
+  OUT EFI_BOOT_MANAGER_LOAD_OPTION       **BootOptions,
+  OUT EFI_INPUT_KEY                      **BootKeys
+  )
+{
+  EFI_GUID                               *FileGuid;
+  CHAR16                                 *PathStr;
+  EFI_STATUS                             Status;
+  UINTN                                  Size;
+
+  Size = sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * HIKEY_BOOT_OPTION_NUM;
+  *BootOptions = (EFI_BOOT_MANAGER_LOAD_OPTION *)AllocateZeroPool (Size);
+  if (*BootOptions == NULL) {
+    DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootOptions\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+  Size = sizeof (EFI_INPUT_KEY) * HIKEY_BOOT_OPTION_NUM;
+  *BootKeys = (EFI_INPUT_KEY *)AllocateZeroPool (Size);
+  if (*BootKeys == NULL) {
+    DEBUG ((DEBUG_ERROR, "Failed to allocate memory for BootKeys\n"));
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Error;
+  }
+
+  PathStr = (CHAR16 *)PcdGetPtr (PcdSdBootDevicePath);
+  ASSERT (PathStr != NULL);
+  Status = CreatePlatformBootOptionFromPath (
+             PathStr,
+             L"Boot from SD",
+             &(*BootOptions)[0]
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  PathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
+  ASSERT (PathStr != NULL);
+  Status = CreatePlatformBootOptionFromPath (
+             PathStr,
+             L"Grub",
+             &(*BootOptions)[1]
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  FileGuid = PcdGetPtr (PcdAndroidBootFile);
+  ASSERT (FileGuid != NULL);
+  Status = CreatePlatformBootOptionFromGuid (
+             FileGuid,
+             L"Android Boot",
+             &(*BootOptions)[2]
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  FileGuid = PcdGetPtr (PcdAndroidFastbootFile);
+  ASSERT (FileGuid != NULL);
+  Status = CreatePlatformBootOptionFromGuid (
+             FileGuid,
+             L"Android Fastboot",
+             &(*BootOptions)[3]
+             );
+  ASSERT_EFI_ERROR (Status);
+  (*BootKeys)[3].ScanCode = SCAN_NULL;
+  (*BootKeys)[3].UnicodeChar = 'f';
+
+  *BootCount = 4;
+
+  return EFI_SUCCESS;
+Error:
+  FreePool (*BootOptions);
+  return Status;
+}
+
+PLATFORM_BOOT_MANAGER_PROTOCOL mPlatformBootManager = {
+  GetPlatformBootOptionsAndKeys
+};
+
 EFI_STATUS
 EFIAPI
 VirtualKeyboardRegister (
@@ -225,5 +386,15 @@ HiKeyEntryPoint (
                   EFI_NATIVE_INTERFACE,
                   &mVirtualKeyboard
                   );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = gBS->InstallProtocolInterface (
+                  &ImageHandle,
+                  &gPlatformBootManagerProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mPlatformBootManager
+                  );
   return Status;
 }
diff --git a/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
index 702fdb1eebf0..e97afab8785a 100644
--- a/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
+++ b/Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf
@@ -27,6 +27,7 @@ [Packages]
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  Platform/Hisilicon/HiKey/HiKey.dec
 
 [LibraryClasses]
   BaseMemoryLib
@@ -40,15 +41,25 @@ [LibraryClasses]
   PrintLib
   SerialPortLib
   TimerLib
+  UefiBootManagerLib
   UefiBootServicesTableLib
   UefiRuntimeServicesTableLib
   UefiLib
   UefiDriverEntryPoint
 
 [Protocols]
+  gEfiDevicePathFromTextProtocolGuid
+  gEfiLoadedImageProtocolGuid
   gEmbeddedGpioProtocolGuid
+  gPlatformBootManagerProtocolGuid
   gPlatformVirtualKeyboardProtocolGuid
 
+[Pcd]
+  gHiKeyTokenSpaceGuid.PcdAndroidBootDevicePath
+  gHiKeyTokenSpaceGuid.PcdAndroidBootFile
+  gHiKeyTokenSpaceGuid.PcdAndroidFastbootFile
+  gHiKeyTokenSpaceGuid.PcdSdBootDevicePath
+
 [Guids]
   gEfiEndOfDxeEventGroupGuid
   gEfiFileInfoGuid
-- 
2.7.4

Re: [PATCH edk2-platforms v5 0/2] add platform boot options

[Laszlo Ersek]

Hello Haojian,

On 04/25/18 06:59, Haojian Zhuang wrote:
> Changelog:
> v5:
>   * Avoid to merge device path and grub's file path in driver.
>     Merge them directly in DSC file.
>   * Avoid duplicated code to create boot options.
>   * Use goto to handle error condition.
> v4:
>   * Add BootCount parameter in the interface.
> v3:
>   * Move in initilization of boot entry.
>   * Update the name of interface in platform boot manager protocol.
> v2:
>   * Update with platform boot manager protocol.
> 
> Haojian Zhuang (2):
>   Platform/HiKey960: register predefined boot options
>   Platform/HiKey: create 4 boot options
> 
>  Platform/Hisilicon/HiKey/HiKey.dec                 |   8 +-
>  Platform/Hisilicon/HiKey/HiKey.dsc                 |   7 +
>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c       | 171 ++++++++++++++++++++
>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf     |  11 ++
>  .../{HiKey/HiKey.dec => HiKey960/HiKey960.dec}     |  17 +-
>  Platform/Hisilicon/HiKey960/HiKey960.dsc           |   6 +
>  .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 172 +++++++++++++++++++++
>  .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  11 ++
>  8 files changed, 391 insertions(+), 12 deletions(-)
>  copy Platform/Hisilicon/{HiKey/HiKey.dec => HiKey960/HiKey960.dec} (56%)
> 

(1) in the future, please CC the blurb -- patch 0/xxx -- to everyone who
are CC'd on any of the patches. What I usually do is:

- format all the patches
- grep them for '^Cc:'
- sort the resultant list uniquely
- paste the output into the blurb message

This way, if a person is CC'd at least on one of the patches in the
series, they will receive a copy of the cover letter too.


(2) As of commit b581a4934d8f ("ARM/JunoPkg: Add HDLCD platform
library", 2018-04-23), I cannot find HiKeyDxe or HiKey960Dxe in the
edk2-platforms tree. That's not a problem, it just means that I cannot
verify the resource management / error handling in the
HiKey960EntryPoint() and HiKeyEntryPoint() functions. For this reason, I
cannot give R-b, just A-b. I hope that will suffice -- please do not
repost because of this.


(3) The GRUB_FILE_NAME macro is now superfluous in both of the patches.
Again, it's not necessary for me to review the series just because of
such an update. Perhaps Leif or Ard can remove the macro defs for you
when they push the patches.


(4) In both patches, in both of the CreatePlatformBootOptionFromPath()
and CreatePlatformBootOptionFromGuid() helper functions, the
"DevicePath" variable is leaked when
EfiBootManagerInitializeLoadOption() fails.

"DevicePath" should be freed unconditionally at that point, in both
patches and in both helper functions (4 instances in total). Simply
eliminate the following:

  if (EFI_ERROR (Status)) {
    return Status;
  }

and you will be left with:

  Status = EfiBootManagerInitializeLoadOption (
             ...
             );
  FreePool (DevicePath);
  return Status;

I believe I need not separately review this update either.


With (3) and (4) addressed, for the series:

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

Thanks
Laszlo

Re: [PATCH edk2-platforms v5 0/2] add platform boot options

[Haojian Zhuang]

On 26 April 2018 at 00:10, Laszlo Ersek <lersek@redhat.com> wrote:
> Hello Haojian,
>
> On 04/25/18 06:59, Haojian Zhuang wrote:
>> Changelog:
>> v5:
>>   * Avoid to merge device path and grub's file path in driver.
>>     Merge them directly in DSC file.
>>   * Avoid duplicated code to create boot options.
>>   * Use goto to handle error condition.
>> v4:
>>   * Add BootCount parameter in the interface.
>> v3:
>>   * Move in initilization of boot entry.
>>   * Update the name of interface in platform boot manager protocol.
>> v2:
>>   * Update with platform boot manager protocol.
>>
>> Haojian Zhuang (2):
>>   Platform/HiKey960: register predefined boot options
>>   Platform/HiKey: create 4 boot options
>>
>>  Platform/Hisilicon/HiKey/HiKey.dec                 |   8 +-
>>  Platform/Hisilicon/HiKey/HiKey.dsc                 |   7 +
>>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.c       | 171 ++++++++++++++++++++
>>  Platform/Hisilicon/HiKey/HiKeyDxe/HiKeyDxe.inf     |  11 ++
>>  .../{HiKey/HiKey.dec => HiKey960/HiKey960.dec}     |  17 +-
>>  Platform/Hisilicon/HiKey960/HiKey960.dsc           |   6 +
>>  .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.c   | 172 +++++++++++++++++++++
>>  .../Hisilicon/HiKey960/HiKey960Dxe/HiKey960Dxe.inf |  11 ++
>>  8 files changed, 391 insertions(+), 12 deletions(-)
>>  copy Platform/Hisilicon/{HiKey/HiKey.dec => HiKey960/HiKey960.dec} (56%)
>>
>
> (1) in the future, please CC the blurb -- patch 0/xxx -- to everyone who
> are CC'd on any of the patches. What I usually do is:
>
> - format all the patches
> - grep them for '^Cc:'
> - sort the resultant list uniquely
> - paste the output into the blurb message
>
> This way, if a person is CC'd at least on one of the patches in the
> series, they will receive a copy of the cover letter too.
>
OK

>
> (2) As of commit b581a4934d8f ("ARM/JunoPkg: Add HDLCD platform
> library", 2018-04-23), I cannot find HiKeyDxe or HiKey960Dxe in the
> edk2-platforms tree. That's not a problem, it just means that I cannot
> verify the resource management / error handling in the
> HiKey960EntryPoint() and HiKeyEntryPoint() functions. For this reason, I
> cannot give R-b, just A-b. I hope that will suffice -- please do not
> repost because of this.
>

Yes, it's depend on another patch series on virtual keyboard.

>
> (3) The GRUB_FILE_NAME macro is now superfluous in both of the patches.
> Again, it's not necessary for me to review the series just because of
> such an update. Perhaps Leif or Ard can remove the macro defs for you
> when they push the patches.
>

OK. I'll remove it.

>
> (4) In both patches, in both of the CreatePlatformBootOptionFromPath()
> and CreatePlatformBootOptionFromGuid() helper functions, the
> "DevicePath" variable is leaked when
> EfiBootManagerInitializeLoadOption() fails.
>
> "DevicePath" should be freed unconditionally at that point, in both
> patches and in both helper functions (4 instances in total). Simply
> eliminate the following:
>
>   if (EFI_ERROR (Status)) {
>     return Status;
>   }
>
> and you will be left with:
>
>   Status = EfiBootManagerInitializeLoadOption (
>              ...
>              );
>   FreePool (DevicePath);
>   return Status;
>
> I believe I need not separately review this update either.
>

For this one, I have a different opinion.

EFI_STATUS
EFIAPI
EfiBootManagerInitializeLoadOption (
  ...
  )
{
  if ((Option == NULL) || (Description == NULL) || (FilePath == NULL)) {
    return EFI_INVALID_PARAMETER;
  }

  if (((OptionalData != NULL) && (OptionalDataSize == 0)) ||
      ((OptionalData == NULL) && (OptionalDataSize != 0))) {
    return EFI_INVALID_PARAMETER;
  }
  if ((UINT32) OptionType >= LoadOptionTypeMax) {
    return EFI_INVALID_PARAMETER;
  }

  ZeroMem (Option, sizeof (EFI_BOOT_MANAGER_LOAD_OPTION));
  Option->OptionNumber       = OptionNumber;
  Option->OptionType         = OptionType;
  Option->Attributes         = Attributes;
  Option->Description        = AllocateCopyPool (StrSize
(Description), Description);
  Option->FilePath           = DuplicateDevicePath (FilePath);
  if (OptionalData != NULL) {
    Option->OptionalData     = AllocateCopyPool (OptionalDataSize,
OptionalData);
    Option->OptionalDataSize = OptionalDataSize;
  }

  return EFI_SUCCESS;
}

We can find that no memory is allocated to "DevicePath" if it returns
with error.

So we shouldn't free memory on "DevicePath" if "Status" is error code.

Other issues will be fixed in v6.

>
> With (3) and (4) addressed, for the series:
>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
>

Thanks a lot.

Best Regards
Haojian

Re: [PATCH edk2-platforms v5 0/2] add platform boot options

[Laszlo Ersek]

On 04/26/18 05:12, Haojian Zhuang wrote:
> On 26 April 2018 at 00:10, Laszlo Ersek <lersek@redhat.com> wrote:

>> (4) In both patches, in both of the CreatePlatformBootOptionFromPath()
>> and CreatePlatformBootOptionFromGuid() helper functions, the
>> "DevicePath" variable is leaked when
>> EfiBootManagerInitializeLoadOption() fails.
>>
>> "DevicePath" should be freed unconditionally at that point, in both
>> patches and in both helper functions (4 instances in total). Simply
>> eliminate the following:
>>
>>   if (EFI_ERROR (Status)) {
>>     return Status;
>>   }
>>
>> and you will be left with:
>>
>>   Status = EfiBootManagerInitializeLoadOption (
>>              ...
>>              );
>>   FreePool (DevicePath);
>>   return Status;
>>
>> I believe I need not separately review this update either.
>>
> 
> For this one, I have a different opinion.
> 
> EFI_STATUS
> EFIAPI
> EfiBootManagerInitializeLoadOption (
>   ...
>   )
> {
>   if ((Option == NULL) || (Description == NULL) || (FilePath == NULL)) {
>     return EFI_INVALID_PARAMETER;
>   }
> 
>   if (((OptionalData != NULL) && (OptionalDataSize == 0)) ||
>       ((OptionalData == NULL) && (OptionalDataSize != 0))) {
>     return EFI_INVALID_PARAMETER;
>   }
>   if ((UINT32) OptionType >= LoadOptionTypeMax) {
>     return EFI_INVALID_PARAMETER;
>   }
> 
>   ZeroMem (Option, sizeof (EFI_BOOT_MANAGER_LOAD_OPTION));
>   Option->OptionNumber       = OptionNumber;
>   Option->OptionType         = OptionType;
>   Option->Attributes         = Attributes;
>   Option->Description        = AllocateCopyPool (StrSize
> (Description), Description);
>   Option->FilePath           = DuplicateDevicePath (FilePath);
>   if (OptionalData != NULL) {
>     Option->OptionalData     = AllocateCopyPool (OptionalDataSize,
> OptionalData);
>     Option->OptionalDataSize = OptionalDataSize;
>   }
> 
>   return EFI_SUCCESS;
> }
> 
> We can find that no memory is allocated to "DevicePath" if it returns
> with error.
> 
> So we shouldn't free memory on "DevicePath" if "Status" is error code.

I disagree. We have:

  CreatePlatformBootOptionFromPath()
    DevicePath = ConvertTextToDevicePath(...)              #1
    EfiBootManagerInitializeLoadOption(... DevicePath ...)
      DuplicateDevicePath(FilePath=DevicePath)             #2

The ConvertTextToDevicePath() function returns the binary representation
of the device path in a dynamically allocated area. That is dynamic
object #1, tracked by the "DevicePath" variable. Then,
EfiBootManagerInitializeLoadOption() creates a deep copy, by calling the
DuplicateDevicePath() function. That creates dynamic object #2, tracked
by Option->FilePath.

- If EfiBootManagerInitializeLoadOption() returns with failure, then
dynamic object #2 does not exist, and we no longer need dynamic object
#1, so we have to free dynamic object #1.

- If EfiBootManagerInitializeLoadOption() returns with success, then
dynamic object #2 exists, and is correctly tracked by Option->FilePath.
We no longer need dynamic object #1, so we have to free it.

In other words, regardless of the return status of
EfiBootManagerInitializeLoadOption(), we must release dynamic object #1.
We need dynamic object #1 only temporarily, so we can call
EfiBootManagerInitializeLoadOption(), and let it make a copy.

The same applies to CreatePlatformBootOptionFromGuid(), except replace
ConvertTextToDevicePath() with AppendDevicePathNode():

  CreatePlatformBootOptionFromGuid()
    DevicePath = AppendDevicePathNode(...)                  #1
    EfiBootManagerInitializeLoadOption (... DevicePath ...)
      DuplicateDevicePath(FilePath=DevicePath)              #2

Again, AppendDevicePathNode() produces dynamic object #1, similarly to
ConvertTextToDevicePath().

Thanks
Laszlo