Re: [PATCH 3/4] Platforms/zx: Add boot manager lib and entries

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, Aug 09, 2017 at 10:12:38PM +0800, Jun Nie wrote:
> Add boot manager lib and entries, including Android and Grub.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
>  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c        | 105 ++++++
>  .../Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf      |  66 ++++
>  .../Library/PlatformBootManagerLib/PlatformBm.c    | 404 +++++++++++++++++++++
>  .../Library/PlatformBootManagerLib/PlatformBm.h    |  30 ++
>  .../PlatformBootManagerLib.inf                     |  91 +++++
>  Silicon/Sanchip/SanchipPkg.dec                     |  29 ++
>  6 files changed, 725 insertions(+)
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
>  create mode 100644 Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
>  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
>  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
>  create mode 100644 Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 Silicon/Sanchip/SanchipPkg.dec
> 
> diff --git a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
> new file mode 100644
> index 0000000..47d02bf
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.c
> @@ -0,0 +1,105 @@
> +/** @file
> +*
> +*  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +*  Copyright (c) 2017, Linaro Ltd.
> +*
> +*  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 <Library/BaseMemoryLib.h>
> +#include <Library/BdsLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Protocol/AndroidBootImg.h>
> +#include <Protocol/BlockIo.h>

Which aspect of BlockIo is used here?
On the whole, several of the above includes look unused - could you
prune them back a bit?

> +
> +#include <libfdt.h>
> +
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgAppendKernelArgs (
> +  IN CHAR16            *Args,
> +  IN UINTN              Size
> +  )
> +{
> +  UnicodeSPrint (
> +    Args + StrLen (Args), Size - StrLen (Args), L" efi=noruntime");

I am not sure what your intent is here, but I am entirely convinced
there is a better way to achieve it. Can you give some background,
please?

> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +AndroidBootImgUpdateDtb (
> +  IN  EFI_PHYSICAL_ADDRESS        OrigFdtBase,
> +  OUT EFI_PHYSICAL_ADDRESS       *NewFdtBase
> +  )
> +{
> +  UINTN             FdtSize, NumPages;
> +  INTN              err;
> +  EFI_STATUS        Status;
> +
> +  //
> +  // Store the FDT as Runtime Service Data to prevent the Kernel from
> +  // overwritting its data.
> +  //

This should really not be necessary and has never been needed
elsewhere. Have you seen this cause any change in behaviour in an
actual system?

> +  FdtSize = fdt_totalsize ((VOID *)(UINTN)OrigFdtBase);
> +  NumPages = EFI_SIZE_TO_PAGES (FdtSize) + 20;
> +  Status = gBS->AllocatePages (
> +                  AllocateAnyPages, EfiRuntimeServicesData,
> +                  NumPages, NewFdtBase);
> +  if (EFI_ERROR (Status)) {
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  CopyMem (
> +    (VOID*)(UINTN)*NewFdtBase,
> +    (VOID*)(UINTN)OrigFdtBase,
> +    FdtSize
> +    );
> +
> +  fdt_pack ((VOID*)(UINTN)*NewFdtBase);
> +  err = fdt_check_header ((VOID*)(UINTN)*NewFdtBase);

This does not feel like it belongs here. Check it when you first load
it, by all means, but there is no need to keep checking it did not get
destroyed by normal execution.

> +  if (err != 0) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: Device Tree header not valid (err:%d)\n", err));
> +    gBS->FreePages (*NewFdtBase, NumPages);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +ANDROID_BOOTIMG_PROTOCOL mAndroidBootImg = {
> +  AndroidBootImgAppendKernelArgs,
> +  AndroidBootImgUpdateDtb
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +Zx296718EvbEntryPoint (
> +  IN EFI_HANDLE         ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS           Status;
> +
> +  Status = gBS->InstallProtocolInterface (
> +                  &ImageHandle,
> +                  &gAndroidBootImgProtocolGuid,
> +                  EFI_NATIVE_INTERFACE,
> +                  &mAndroidBootImg
> +                  );
> +  return Status;
> +}
> diff --git a/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> new file mode 100644
> index 0000000..03982d1
> --- /dev/null
> +++ b/Platform/Sanchip/Zx296718Evb/Drivers/Zx296718EvbDxe/Zx296718EvbDxe.inf
> @@ -0,0 +1,66 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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]
> +  INF_VERSION                    = 0x00010005

0x00010019/1.25.

> +  BASE_NAME                      = Zx296718EvbDxe
> +  FILE_GUID                      = db154b2a-031f-4f3c-9315-c4697ff33e6c
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = Zx296718EvbEntryPoint
> +
> +[Sources.common]
> +  Zx296718EvbDxe.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

M (MdeM) before P (MdeP).

> +  Platform/Sanchip/Zx296718Evb/Zx296718Evb.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  BdsLib
> +  CacheMaintenanceLib
> +  DebugLib
> +  DxeServicesLib
> +  DxeServicesTableLib
> +  FdtLib
> +  IoLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +  TimerLib
> +  UefiBootServicesTableLib
> +  UefiRuntimeServicesTableLib
> +  UefiLib

L before R.

> +  UefiDriverEntryPoint

But D before L.

But also, how many of these LibraryClasses are actually used by this
driver?

> +
> +[Guids]
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiFileInfoGuid
> +  gEfiGlobalVariableGuid
> +  gFdtTableGuid
> +  gFdtVariableGuid

How many of these Guids are actually used by this driver?

> +
> +[Protocols]
> +  gAndroidBootImgProtocolGuid
> +  gEfiBlockIoProtocolGuid
> +  gEfiDevicePathFromTextProtocolGuid
> +  gEfiDevicePathToTextProtocolGuid
> +  gEfiDevicePathProtocolGuid
> +  gEfiSimpleFileSystemProtocolGuid

How many of these protocols are actually used by this driver?

> +
> +[Depex]
> +  TRUE
> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c

It looks to me like this implementation (perhaps with a few small
modifications) should be reusable across multiple implementations.

That does not need to happen for this to be merged, but it would be
good if you and Haojian could discuss between yourselves. (Also,
perhaps we should all sit down and discuss this at Linaro Connect next
month.)

> new file mode 100644
> index 0000000..41861e2
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -0,0 +1,404 @@
> +/** @file
> +  Implementation for PlatformBootManagerLib library class interfaces.
> +
> +  Copyright (C) 2015-2016, Red Hat, Inc.
> +  Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016-2017, Linaro Ltd. All rights reserved.<BR>
> +
> +  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 <Library/DevicePathLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootManagerLib.h>
> +#include <Library/UefiLib.h>
> +#include <Protocol/BlockIo.h>
> +#include <Protocol/DevicePath.h>
> +#include <Protocol/DevicePathFromText.h>
> +#include <Protocol/DevicePathToText.h>
> +#include <Protocol/LoadedImage.h>
> +#include <Guid/EventGroup.h>
> +#include <Guid/TtyTerm.h>

Can you verify all of the above are actually used by this file?

> +
> +#include "PlatformBm.h"
> +
> +#define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
> +
> +#define GRUB_FILE_NAME       L"\\EFI\\BOOT\\GRUBAA64.EFI"
> +
> +
> +#pragma pack (1)
> +typedef struct {
> +  VENDOR_DEVICE_PATH         SerialDxe;
> +  UART_DEVICE_PATH           Uart;
> +  VENDOR_DEFINED_DEVICE_PATH TermType;
> +  EFI_DEVICE_PATH_PROTOCOL   End;
> +} PLATFORM_SERIAL_CONSOLE;
> +#pragma pack ()
> +
> +#define SERIAL_DXE_FILE_GUID { \
> +          0xD3987D4B, 0x971A, 0x435F, \
> +          { 0x8C, 0xAF, 0x49, 0x67, 0xEB, 0x62, 0x72, 0x41 } \
> +          }
> +
> +STATIC PLATFORM_SERIAL_CONSOLE mSerialConsole = {
> +  //
> +  // VENDOR_DEVICE_PATH SerialDxe
> +  //
> +  {
> +    { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, DP_NODE_LEN (VENDOR_DEVICE_PATH) },
> +    SERIAL_DXE_FILE_GUID
> +  },
> +
> +  //
> +  // UART_DEVICE_PATH Uart
> +  //
> +  {
> +    { MESSAGING_DEVICE_PATH, MSG_UART_DP, DP_NODE_LEN (UART_DEVICE_PATH) },
> +    0,                                      // Reserved
> +    FixedPcdGet64 (PcdUartDefaultBaudRate), // BaudRate
> +    FixedPcdGet8 (PcdUartDefaultDataBits),  // DataBits
> +    FixedPcdGet8 (PcdUartDefaultParity),    // Parity
> +    FixedPcdGet8 (PcdUartDefaultStopBits)   // StopBits
> +  },
> +
> +  //
> +  // VENDOR_DEFINED_DEVICE_PATH TermType
> +  //
> +  {
> +    {
> +      MESSAGING_DEVICE_PATH, MSG_VENDOR_DP,
> +      DP_NODE_LEN (VENDOR_DEFINED_DEVICE_PATH)
> +    }
> +    //
> +    // Guid to be filled in dynamically
> +    //
> +  },
> +
> +  //
> +  // EFI_DEVICE_PATH_PROTOCOL End
> +  //
> +  {
> +    END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    DP_NODE_LEN (EFI_DEVICE_PATH_PROTOCOL)
> +  }
> +};
> +
> +/**
> +  Check if the handle satisfies a particular condition.
> +
> +  @param[in] Handle      The handle to check.
> +  @param[in] ReportText  A caller-allocated string passed in for reporting
> +                         purposes. It must never be NULL.
> +
> +  @retval TRUE   The condition is satisfied.
> +  @retval FALSE  Otherwise. This includes the case when the condition could not
> +                 be fully evaluated due to an error.
> +**/
> +typedef
> +BOOLEAN
> +(EFIAPI *FILTER_FUNCTION) (
> +  IN EFI_HANDLE   Handle,
> +  IN CONST CHAR16 *ReportText
> +  );
> +
> +
> +/**
> +  Process a handle.
> +
> +  @param[in] Handle      The handle to process.
> +  @param[in] ReportText  A caller-allocated string passed in for reporting
> +                         purposes. It must never be NULL.
> +**/
> +typedef
> +VOID
> +(EFIAPI *CALLBACK_FUNCTION)  (
> +  IN EFI_HANDLE   Handle,
> +  IN CONST CHAR16 *ReportText
> +  );
> +
> +STATIC
> +VOID
> +PlatformRegisterFvBootOption (
> +  EFI_GUID                         *FileGuid,
> +  CHAR16                           *Description,
> +  UINT32                           Attributes
> +  )
> +{
> +  EFI_STATUS                        Status;
> +  INTN                              OptionIndex;
> +  EFI_BOOT_MANAGER_LOAD_OPTION      NewOption;
> +  EFI_BOOT_MANAGER_LOAD_OPTION      *BootOptions;
> +  UINTN                             BootOptionCount;
> +  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;
> +  EFI_LOADED_IMAGE_PROTOCOL         *LoadedImage;
> +  EFI_DEVICE_PATH_PROTOCOL          *DevicePath;
> +
> +  Status = gBS->HandleProtocol (
> +                  gImageHandle,
> +                  &gEfiLoadedImageProtocolGuid,
> +                  (VOID **) &LoadedImage
> +                  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
> +  DevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
> +  ASSERT (DevicePath != NULL);
> +  DevicePath = AppendDevicePathNode (
> +                 DevicePath,
> +                 (EFI_DEVICE_PATH_PROTOCOL *) &FileNode
> +                 );
> +  ASSERT (DevicePath != NULL);
> +
> +  Status = EfiBootManagerInitializeLoadOption (
> +             &NewOption,
> +             LoadOptionNumberUnassigned,
> +             LoadOptionTypeBoot,
> +             Attributes,
> +             Description,
> +             DevicePath,
> +             NULL,
> +             0
> +             );
> +  ASSERT_EFI_ERROR (Status);
> +  FreePool (DevicePath);
> +
> +  BootOptions = EfiBootManagerGetLoadOptions (
> +                  &BootOptionCount, LoadOptionTypeBoot
> +                  );
> +
> +  OptionIndex = EfiBootManagerFindLoadOption (
> +                  &NewOption, BootOptions, BootOptionCount
> +                  );
> +
> +  if (OptionIndex == -1) {
> +    Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  EfiBootManagerFreeLoadOption (&NewOption);
> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +}
> +
> +
> +STATIC
> +VOID
> +PlatformRegisterBootGrub (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                           Status;
> +  CHAR16                              *BootPathStr;
> +  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
> +  EFI_DEVICE_PATH                     *DevicePath;
> +  EFI_DEVICE_PATH                     *FileDevicePath;
> +  FILEPATH_DEVICE_PATH                *FilePath;
> +  UINTN                                Size;
> +  EFI_BOOT_MANAGER_LOAD_OPTION         NewOption;
> +  EFI_BOOT_MANAGER_LOAD_OPTION        *BootOptions;
> +  UINTN                                BootOptionCount;
> +  INTN                                 OptionIndex;
> +
> +  //
> +  // Get PcdAndroidBootDevicePath
> +  //
> +  BootPathStr = (CHAR16 *)PcdGetPtr (PcdAndroidBootDevicePath);
> +  ASSERT (BootPathStr != NULL);
> +  Status = gBS->LocateProtocol (&gEfiDevicePathFromTextProtocolGuid, NULL,
> +                                (VOID **)&EfiDevicePathFromTextProtocol);
> +  ASSERT_EFI_ERROR(Status);
> +  DevicePath = (EFI_DEVICE_PATH *)EfiDevicePathFromTextProtocol->ConvertTextToDevicePath (BootPathStr);
> +  ASSERT (DevicePath != NULL);
> +
> +  Size = StrSize (GRUB_FILE_NAME);
> +  FileDevicePath = AllocatePool (Size + SIZE_OF_FILEPATH_DEVICE_PATH + END_DEVICE_PATH_LENGTH);
> +  if (FileDevicePath != NULL) {
> +    FilePath = (FILEPATH_DEVICE_PATH *) FileDevicePath;
> +    FilePath->Header.Type    = MEDIA_DEVICE_PATH;
> +    FilePath->Header.SubType = MEDIA_FILEPATH_DP;
> +    CopyMem (&FilePath->PathName, GRUB_FILE_NAME, Size);
> +    SetDevicePathNodeLength (&FilePath->Header, Size + SIZE_OF_FILEPATH_DEVICE_PATH);
> +    SetDevicePathEndNode (NextDevicePathNode (&FilePath->Header));
> +
> +    DevicePath = AppendDevicePath (DevicePath, FileDevicePath);
> +    FreePool (FileDevicePath);
> +  }
> +  Status = EfiBootManagerInitializeLoadOption (
> +             &NewOption,
> +             LoadOptionNumberUnassigned,
> +             LoadOptionTypeBoot,
> +             LOAD_OPTION_ACTIVE,
> +             L"Grub",
> +             DevicePath,
> +             NULL,
> +             0
> +             );
> +  ASSERT_EFI_ERROR (Status);
> +  FreePool (DevicePath);
> +
> +  BootOptions = EfiBootManagerGetLoadOptions (
> +                  &BootOptionCount, LoadOptionTypeBoot
> +                  );
> +
> +  OptionIndex = EfiBootManagerFindLoadOption (
> +                  &NewOption, BootOptions, BootOptionCount
> +                  );
> +
> +  if (OptionIndex == -1) {
> +    Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN);
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
> +  EfiBootManagerFreeLoadOption (&NewOption);
> +  EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +
> +}
> +
> +STATIC
> +VOID
> +PlatformRegisterOptionsAndKeys (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                           Status;
> +  EFI_INPUT_KEY                        Enter;
> +  EFI_INPUT_KEY                        Esc;
> +  EFI_BOOT_MANAGER_LOAD_OPTION         BootOption;
> +
> +  //
> +  // Register Boot. OptionNumber is 1.
> +  //
> +  PlatformRegisterBootGrub ();
> +
> +  //
> +  // Register Android Boot. OptionNumber is 2.
> +  //
> +  PlatformRegisterFvBootOption (
> +    PcdGetPtr (PcdAndroidBootFile), L"Android Boot", LOAD_OPTION_ACTIVE
> +    );
> +
> +  //
> +  // Register ENTER as CONTINUE key
> +  //
> +  Enter.ScanCode    = SCAN_NULL;
> +  Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
> +  Status = EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  //
> +  // Map ESC to Boot Manager Menu
> +  //
> +  Esc.ScanCode    = SCAN_ESC;
> +  Esc.UnicodeChar = CHAR_NULL;
> +  Status = EfiBootManagerGetBootManagerMenu (&BootOption);
> +  ASSERT_EFI_ERROR (Status);
> +  Status = EfiBootManagerAddKeyOptionVariable (
> +             NULL, (UINT16) BootOption.OptionNumber, 0, &Esc, NULL
> +             );
> +  ASSERT (Status == EFI_SUCCESS || Status == EFI_ALREADY_STARTED);
> +}
> +
> +
> +//
> +// BDS Platform Functions
> +//
> +/**
> +  Do the platform init, can be customized by OEM/IBV
> +  Possible things that can be done in PlatformBootManagerBeforeConsole:
> +  > Update console variable: 1. include hot-plug devices;
> +  >                          2. Clear ConIn and add SOL for AMT
> +  > Register new Driver#### or Boot####
> +  > Register new Key####: e.g.: F12
> +  > Signal ReadyToLock event
> +  > Authentication action: 1. connect Auth devices;
> +  >                        2. Identify auto logon user.
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerBeforeConsole (
> +  VOID
> +  )
> +{
> +  //
> +  // Signal EndOfDxe PI Event
> +  //
> +  EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);
> +
> +  //
> +  // Add the hardcoded serial console device path to ConIn, ConOut, ErrOut.
> +  //
> +  ASSERT (FixedPcdGet8 (PcdDefaultTerminalType) == 4);
> +  CopyGuid (&mSerialConsole.TermType.Guid, &gEfiTtyTermGuid);
> +
> +  EfiBootManagerUpdateConsoleVariable (ConIn,
> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConOut,
> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ErrOut,
> +    (EFI_DEVICE_PATH_PROTOCOL *)&mSerialConsole, NULL);
> +
> +  //
> +  // Register platform-specific boot options and keyboard shortcuts.
> +  //
> +  PlatformRegisterOptionsAndKeys ();
> +}
> +
> +/**
> +  Do the platform specific action after the console is ready
> +  Possible things that can be done in PlatformBootManagerAfterConsole:
> +  > Console post action:
> +    > Dynamically switch output mode from 100x31 to 80x25 for certain senarino
> +    > Signal console ready platform customized event
> +  > Run diagnostics like memory testing
> +  > Connect certain devices
> +  > Dispatch aditional option roms
> +  > Special boot
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerAfterConsole (
> +  VOID
> +  )
> +{
> +  Print (L"Press ESCAPE for boot options ");
> +
> +  //
> +  // Connect the rest of the devices.
> +  //
> +  EfiBootManagerConnectAll ();
> +
> +  //
> +  // Enumerate all possible boot options.
> +  //
> +  EfiBootManagerRefreshAllBootOption ();
> +
> +  //
> +  // Register UEFI Shell
> +  //
> +  PlatformRegisterFvBootOption (
> +    PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE
> +    );
> +}
> +
> +/**
> +  This function is called each second during the boot manager waits the
> +  timeout.
> +
> +  @param TimeoutRemain  The remaining timeout.
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerWaitCallback (
> +  UINT16          TimeoutRemain
> +  )
> +{
> +  Print (L".");
> +}
> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
> new file mode 100644
> index 0000000..de52a9c
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBm.h
> @@ -0,0 +1,30 @@
> +/** @file
> +  Head file for BDS Platform specific code
> +
> +  Copyright (C) 2015-2016, Red Hat, Inc.
> +  Copyright (c) 2004 - 2008, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016-2017, Linaro Ltd. All rights reserved.<BR>
> +
> +  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 _PLATFORM_BM_H_
> +#define _PLATFORM_BM_H_
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>

If all this holds is a set of include files for a single source file,
may they not just go directly into the .c file?

> +
> +#endif // _PLATFORM_BM_H_
> diff --git a/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> new file mode 100644
> index 0000000..b9c6aaa
> --- /dev/null
> +++ b/Silicon/Sanchip/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -0,0 +1,91 @@
> +## @file
> +#  Implementation for PlatformBootManagerLib library class interfaces.
> +#
> +#  Copyright (C) 2015-2016, Red Hat, Inc.
> +#  Copyright (c) 2014, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2017, Linaro Ltd. All rights reserved.<BR>
> +#
> +#  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]
> +  INF_VERSION                    = 0x00010005

0x00010019/1.25.

> +  BASE_NAME                      = PlatformBootManagerLib
> +  FILE_GUID                      = 9001ec12-8757-4638-b6d0-e0863dddddb0
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformBootManagerLib|DXE_DRIVER
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = ARM AARCH64
> +#
> +
> +[Sources]
> +  PlatformBm.c
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/Sanchip/SanchipPkg.dec

Are all of these actually used?

> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  DevicePathLib
> +  DxeServicesLib
> +  MemoryAllocationLib
> +  PcdLib
> +  PrintLib
> +  UefiBootManagerLib
> +  UefiBootServicesTableLib
> +  UefiLib

Are all of these actually used?

> +
> +[FeaturePcd]
> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdBootlogoOnlyEnable
> +  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport

Are all of these actually used?

> +
> +[FixedPcd]
> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile
> +  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> +  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits
> +  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType

Are all of these actually used?

> +
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> +  gSanchipTokenSpaceGuid.PcdAndroidBootFile

Are all of these actually used?

> +
> +[Guids]
> +  gEfiFileInfoGuid
> +  gEfiFileSystemInfoGuid
> +  gEfiFileSystemVolumeLabelInfoIdGuid
> +  gEfiEndOfDxeEventGroupGuid
> +  gEfiTtyTermGuid

Are all of these actually used?

> +
> +[Protocols]
> +  gEfiDevicePathFromTextProtocolGuid
> +  gEfiDevicePathProtocolGuid
> +  gEfiLoadedImageProtocolGuid
> +  gEfiOEMBadgingProtocolGuid
> +  gEfiPciRootBridgeIoProtocolGuid
> +  gEfiSimpleFileSystemProtocolGuid
> +  gEfiDevicePathToTextProtocolGuid

Are all of these actually used?

> +
> +[Pcd]
> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath
> diff --git a/Silicon/Sanchip/SanchipPkg.dec b/Silicon/Sanchip/SanchipPkg.dec
> new file mode 100644
> index 0000000..92d439d
> --- /dev/null
> +++ b/Silicon/Sanchip/SanchipPkg.dec
> @@ -0,0 +1,29 @@
> +#
> +#  Copyright (C) 2017 Sanechips Technology Co., Ltd.
> +#  Copyright (c) 2017, Linaro Ltd.
> +#
> +#  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              = 0x00010005

0x00010019/1.25.

> +  PACKAGE_NAME                   = SanchipPkg
> +  PACKAGE_GUID                   = f991248f-9e21-4e4f-b344-eaad28e42ec0
> +  PACKAGE_VERSION                = 0.1
> +
> +[Includes.common]
> +  Include                        # Root include for the package

This directory does not exist, causing compliation to fail.

> +
> +[Guids.common]
> +  gSanchipTokenSpaceGuid          =  { 0x9589ba86, 0x58e6, 0x426c, { 0xbb, 0x39, 0x21, 0xf5, 0x68, 0x78, 0xe4, 0x27 } }
> +
> +[PcdsFixedAtBuild.common]
> +  gSanchipTokenSpaceGuid.PcdAndroidBootFile|{ 0x36, 0x8b, 0x73, 0x3a, 0xc5, 0xb9, 0x63, 0x47, 0xab, 0xbd, 0x6c, 0xbd, 0x4b, 0x25, 0xf9, 0xff }|VOID*|0x00000003

What?

> +  gSanchipTokenSpaceGuid.PcdZxRtcClockBase|0|UINT64|0x00000002
> +  gSanchipTokenSpaceGuid.PcdZxRtcClockFreq|24000|UINT64|0x00000001
> -- 
> 1.9.1
>