On Thu, Jul 27, 2017 at 06:07:19PM +0800, Jun Nie wrote:
> Add an android kernel loader that could load kernel from storage
> device. This patch is from Haojian's code as below link. The minor
> change is that alternative dtb is searched in second loader binary
> of Android bootimage if dtb is not found after Linux kernel.
> https://patches.linaro.org/patch/94683/
> 
> This android boot image BDS add addtitional cmdline/dtb/ramfs
> support besides kernel that is introduced by Android boot header.

Getting there. A few more comments below.

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jun Nie <jun....@linaro.org>
> ---
>  ArmPkg/Include/Library/BdsLib.h                    |   3 +
>  ArmPkg/Library/BdsLib/BdsFilePath.c                |   3 -
>  .../Application/AndroidBoot/AndroidBootApp.c       | 127 +++++++
>  .../Application/AndroidBoot/AndroidBootApp.inf     |  64 ++++
>  .../Application/AndroidFastboot/AndroidBootImg.c   |  35 +-
>  .../AndroidFastboot/AndroidFastbootApp.h           |   1 +
>  .../AndroidFastboot/Arm/BootAndroidBootImg.c       |   2 +-
>  EmbeddedPkg/Include/Library/AndroidBootImgLib.h    |  67 ++++
>  EmbeddedPkg/Include/Protocol/AndroidBootImg.h      |  47 +++
>  .../Library/AndroidBootImgLib/AndroidBootImgLib.c  | 419 
> +++++++++++++++++++++
>  .../AndroidBootImgLib/AndroidBootImgLib.inf        |  48 +++
>  11 files changed, 782 insertions(+), 34 deletions(-)
>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
>  create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
>  create mode 100644 EmbeddedPkg/Include/Library/AndroidBootImgLib.h
>  create mode 100644 EmbeddedPkg/Include/Protocol/AndroidBootImg.h
>  create mode 100644 EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
>  create mode 100644 
> EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> 
> diff --git a/ArmPkg/Include/Library/BdsLib.h b/ArmPkg/Include/Library/BdsLib.h
> index c58f47e..4528c2e 100644
> --- a/ArmPkg/Include/Library/BdsLib.h
> +++ b/ArmPkg/Include/Library/BdsLib.h
> @@ -15,6 +15,9 @@
>  #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
>  **/
> diff --git a/ArmPkg/Library/BdsLib/BdsFilePath.c 
> b/ArmPkg/Library/BdsLib/BdsFilePath.c
> index f9d8c4c..41557bb 100644
> --- a/ArmPkg/Library/BdsLib/BdsFilePath.c
> +++ b/ArmPkg/Library/BdsLib/BdsFilePath.c
> @@ -24,9 +24,6 @@
>  #include <Protocol/Dhcp4.h>
>  #include <Protocol/Mtftp4.h>
>  
> -
> -#define IS_DEVICE_PATH_NODE(node,type,subtype) (((node)->Type == (type)) && 
> ((node)->SubType == (subtype)))
> -

Could you break these bits of moving macros and definitions into
common header files as a separate patch, preceding the rest of the changes?

>  /* Type and defines to set up the DHCP4 options */
>  
>  typedef struct {
> diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c 
> b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
> new file mode 100644
> index 0000000..2de1d8a
> --- /dev/null
> +++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c
> @@ -0,0 +1,127 @@
> +/** @file
> +
> +  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2017, Linaro. 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 <Library/AndroidBootImgLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/BdsLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +#include <Protocol/BlockIo.h>
> +#include <Protocol/DevicePathFromText.h>
> +
> +EFI_STATUS
> +EFIAPI
> +AndroidBootAppEntryPoint (
> +  IN EFI_HANDLE                            ImageHandle,
> +  IN EFI_SYSTEM_TABLE                      *SystemTable
> +  )
> +{
> +  EFI_STATUS                          Status;
> +  CHAR16                              *BootPathStr;
> +  EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL  *EfiDevicePathFromTextProtocol;
> +  EFI_DEVICE_PATH                     *DevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL            *Node, *NextNode;
> +  EFI_BLOCK_IO_PROTOCOL               *BlockIo;
> +  UINT32                              MediaId, BlockSize;
> +  VOID                                *Buffer;
> +  EFI_HANDLE                          Handle;
> +  UINTN                               Size;
> +
> +  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);
> +
> +  /* Find DevicePath node of Partition */
> +  NextNode = DevicePath;
> +  while (1) {

Should this not be while (NextNode != NULL), with some check that the
node was found before progressing?

> +    Node = NextNode;
> +    if (IS_DEVICE_PATH_NODE (Node, MEDIA_DEVICE_PATH, MEDIA_HARDDRIVE_DP)) {
> +      break;
> +    }
> +    NextNode = NextDevicePathNode (Node);
> +  }
> +
> +  Status = gBS->LocateDevicePath (&gEfiDevicePathProtocolGuid,
> +                                  &DevicePath, &Handle);

And should this not use &Node rather than &DevicePath?

> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = gBS->OpenProtocol (
> +                  Handle,
> +                  &gEfiBlockIoProtocolGuid,
> +                  (VOID **) &BlockIo,
> +                  gImageHandle,
> +                  NULL,
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "Failed to get BlockIo: %r\n", Status));
> +    return Status;
> +  }
> +
> +  MediaId = BlockIo->Media->MediaId;
> +  BlockSize = BlockIo->Media->BlockSize;
> +  Buffer = AllocatePages (EFI_SIZE_TO_PAGES 
> (sizeof(ANDROID_BOOTIMG_HEADER)));
> +  if (Buffer == NULL) {
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +  /* Load header of boot.img */
> +  Status = BlockIo->ReadBlocks (
> +                      BlockIo,
> +                      MediaId,
> +                      0,
> +                      BlockSize,
> +                      Buffer
> +                      );
> +  Status = AbootimgGetImgSize (Buffer, &Size);

AndroidBootImgGetImageSize.

(The "img" would normally be expected to be expanded to "Image", but
it appears "boot.img" is basically the official name for this format.)

> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "Failed to get Abootimg Size: %r\n", Status));
> +    return Status;
> +  }
> +  Size = ALIGN_VALUE (Size, BlockSize);
> +  FreePages (Buffer, EFI_SIZE_TO_PAGES (sizeof(ANDROID_BOOTIMG_HEADER)));
> +
> +  /* Both PartitionStart and PartitionSize are counted as block size. */
> +  Buffer = AllocatePages (EFI_SIZE_TO_PAGES (Size));
> +  if (Buffer == NULL) {
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +
> +  /* Load header of boot.img */
> +  Status = BlockIo->ReadBlocks (
> +                      BlockIo,
> +                      MediaId,
> +                      0,
> +                      Size,
> +                      Buffer
> +                      );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "Failed to read blocks: %r\n", Status));
> +    goto EXIT;
> +  }
> +
> +  Status = AbootimgBoot (Buffer, Size);

AndroidBootImgBoot.

> +
> +EXIT:
> +  return Status;
> +}
> diff --git a/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf 
> b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
> new file mode 100644
> index 0000000..f1ee0bd
> --- /dev/null
> +++ b/EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf
> @@ -0,0 +1,64 @@
> +#/** @file
> +#
> +#  Copyright (c) 2013-2015, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2017, Linaro. 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]
> +  INF_VERSION                    = 0x00010019
> +  BASE_NAME                      = AndroidBootApp
> +  FILE_GUID                      = 3a738b36-b9c5-4763-abbd-6cbd4b25f9ff
> +  MODULE_TYPE                    = UEFI_APPLICATION
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = AndroidBootAppEntryPoint
> +
> +[Sources.common]
> +  AndroidBootApp.c
> +
> +[LibraryClasses]
> +  AndroidBootImgLib
> +  BaseLib
> +  BaseMemoryLib
> +  BdsLib
> +  DebugLib
> +  DevicePathLib
> +  DxeServicesTableLib
> +  FdtLib
> +  MemoryAllocationLib
> +  PcdLib
> +  PrintLib
> +  UefiApplicationEntryPoint
> +  UefiBootServicesTableLib
> +  UefiLib
> +  UefiRuntimeServicesTableLib
> +
> +[Protocols]
> +  gAndroidFastbootPlatformProtocolGuid
> +  gEfiBlockIoProtocolGuid
> +  gEfiDevicePathFromTextProtocolGuid
> +  gEfiSimpleTextOutProtocolGuid
> +  gEfiSimpleTextInProtocolGuid
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[Packages.ARM, Packages.AARCH64]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +
> +[Guids]
> +  gFdtTableGuid
> +
> +[Pcd]
> +  gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c 
> b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> index f3e770b..2f7f093 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidBootImg.c
> @@ -14,32 +14,6 @@
>  
>  #include "AndroidFastbootApp.h"
>  
> -#define BOOT_MAGIC        "ANDROID!"
> -#define BOOT_MAGIC_LENGTH sizeof (BOOT_MAGIC) - 1
> -
> -// Check Val (unsigned) is a power of 2 (has only one bit set)
> -#define IS_POWER_OF_2(Val) (Val != 0 && ((Val & (Val - 1)) == 0))
> -
> -// No documentation for this really - sizes of fields has been determined
> -// empirically.
> -#pragma pack(1)
> -typedef struct {
> -  CHAR8   BootMagic[BOOT_MAGIC_LENGTH];
> -  UINT32  KernelSize;
> -  UINT32  KernelAddress;
> -  UINT32  RamdiskSize;
> -  UINT32  RamdiskAddress;
> -  UINT32  SecondStageBootloaderSize;
> -  UINT32  SecondStageBootloaderAddress;
> -  UINT32  KernelTaggsAddress;
> -  UINT32  PageSize;
> -  UINT32  Reserved[2];
> -  CHAR8   ProductName[16];
> -  CHAR8   KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
> -  UINT32  Id[32];
> -} ANDROID_BOOTIMG_HEADER;
> -#pragma pack()
> -

(This bit also in separate commit, with other things moving to common headers?)

>  // Find the kernel and ramdisk in an Android boot.img.
>  // return EFI_INVALID_PARAMTER if the boot.img is invalid (i.e. doesn't have 
> the
>  //  right magic value),
> @@ -64,7 +38,8 @@ ParseAndroidBootImg (
>  
>    Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
>  
> -  if (AsciiStrnCmp (Header->BootMagic, BOOT_MAGIC, BOOT_MAGIC_LENGTH) != 0) {
> +  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
> +                    ANDROID_BOOT_MAGIC_LENGTH) != 0) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
> @@ -72,7 +47,7 @@ ParseAndroidBootImg (
>      return EFI_NOT_FOUND;
>    }
>  
> -  ASSERT (IS_POWER_OF_2 (Header->PageSize));
> +  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
>  
>    *KernelSize = Header->KernelSize;
>    *Kernel = BootImgBytePtr + Header->PageSize;
> @@ -84,8 +59,8 @@ ParseAndroidBootImg (
>                   + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
>    }
>  
> -  AsciiStrnCpyS (KernelArgs, BOOTIMG_KERNEL_ARGS_SIZE, Header->KernelArgs,
> -    BOOTIMG_KERNEL_ARGS_SIZE);
> +  AsciiStrnCpyS (KernelArgs, ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, 
> Header->KernelArgs,
> +    ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
>  
>    return EFI_SUCCESS;
>  }
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h 
> b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
> index f62660f..e4c5aa3 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.h
> @@ -15,6 +15,7 @@
>  #ifndef __ANDROID_FASTBOOT_APP_H__
>  #define __ANDROID_FASTBOOT_APP_H__
>  
> +#include <Library/AndroidBootImgLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c 
> b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
> index f446cce..1d9024b 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
> +++ b/EmbeddedPkg/Application/AndroidFastboot/Arm/BootAndroidBootImg.c
> @@ -112,7 +112,7 @@ BootAndroidBootImg (
>    )
>  {
>    EFI_STATUS                          Status;
> -  CHAR8                               KernelArgs[BOOTIMG_KERNEL_ARGS_SIZE];
> +  CHAR8                               
> KernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
>    VOID                               *Kernel;
>    UINTN                               KernelSize;
>    VOID                               *Ramdisk;
> diff --git a/EmbeddedPkg/Include/Library/AndroidBootImgLib.h 
> b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
> new file mode 100644
> index 0000000..3c825eb
> --- /dev/null
> +++ b/EmbeddedPkg/Include/Library/AndroidBootImgLib.h
> @@ -0,0 +1,67 @@
> +/** @file
> +
> +  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2017, Linaro.
> +
> +  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 __ABOOTIMG_H__
> +#define __ABOOTIMG_H__
> +
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Uefi/UefiSpec.h>
> +
> +#define ANDROID_BOOTIMG_KERNEL_ARGS_SIZE  512
> +
> +#define ANDROID_BOOT_MAGIC                "ANDROID!"
> +#define ANDROID_BOOT_MAGIC_LENGTH         (sizeof (ANDROID_BOOT_MAGIC) - 1)
> +
> +/* 
> https://android.googlesource.com/platform/system/core/+/master/mkbootimg/bootimg.h
>  */
> +typedef struct {
> +  UINT8   BootMagic[ANDROID_BOOT_MAGIC_LENGTH];
> +  UINT32  KernelSize;
> +  UINT32  KernelAddress;
> +  UINT32  RamdiskSize;
> +  UINT32  RamdiskAddress;
> +  UINT32  SecondStageBootloaderSize;
> +  UINT32  SecondStageBootloaderAddress;
> +  UINT32  KernelTaggsAddress;
> +  UINT32  PageSize;
> +  UINT32  Reserved[2];
> +  CHAR8   ProductName[16];
> +  CHAR8   KernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];
> +  UINT32  Id[32];
> +} ANDROID_BOOTIMG_HEADER;
> +
> +/* Check Val (unsigned) is a power of 2 (has only one bit set) */
> +#define IS_POWER_OF_2(Val)       ((Val) != 0 && (((Val) & ((Val) - 1)) == 0))
> +/* Android boot image page size is not specified, but it should be power of 2
> + * and larger than boot header */
> +#define IS_VALID_ANDROID_PAGE_SIZE(Val)   \
> +             (IS_POWER_OF_2(Val) && (Val > sizeof(ANDROID_BOOTIMG_HEADER)))
> +
> +EFI_STATUS
> +AbootimgGetImgSize (
> +  IN  VOID    *BootImg,
> +  OUT UINTN   *ImgSize
> +  );
> +
> +EFI_STATUS
> +AbootimgBoot (
> +  IN VOID                   *Buffer,
> +  IN UINTN                   BufferSize
> +  );
> +
> +#endif /* __ABOOTIMG_H__ */
> diff --git a/EmbeddedPkg/Include/Protocol/AndroidBootImg.h 
> b/EmbeddedPkg/Include/Protocol/AndroidBootImg.h
> new file mode 100644
> index 0000000..6bee5cf
> --- /dev/null
> +++ b/EmbeddedPkg/Include/Protocol/AndroidBootImg.h
> @@ -0,0 +1,47 @@
> +/** @file
> +
> +  Copyright (c) 2017, Linaro. 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 __ABOOTIMG_PROTOCOL_H__
> +#define __ABOOTIMG_PROTOCOL_H__
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _ABOOTIMG_PROTOCOL    ABOOTIMG_PROTOCOL;

ANDROID_BOOTIMG_PROTOCOL

> +
> +//
> +// Function Prototypes
> +//
> +typedef
> +EFI_STATUS
> +(EFIAPI *ABOOTIMG_APPEND_KERNEL_ARGS) (

ANDROID_BOOTIMG

> +  IN CHAR16            *Args,
> +  IN UINTN              Size
> +  );
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *ABOOTIMG_UPDATE_DTB) (

ANDROID_BOOTIMG

> +  IN  EFI_PHYSICAL_ADDRESS    OrigDtbBase;
> +  OUT EFI_PHYSICAL_ADDRESS   *NewDtbBase;
> +  );
> +
> +struct _ABOOTIMG_PROTOCOL {
> +  ABOOTIMG_APPEND_KERNEL_ARGS        AppendArgs;
> +  ABOOTIMG_UPDATE_DTB                UpdateDtb;
> +};
> +
> +extern EFI_GUID gAndroidBootImgProtocolGuid;
> +
> +#endif /* __ABOOTIMG_PROTOCOL_H__ */
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c 
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> new file mode 100644
> index 0000000..72c6322
> --- /dev/null
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -0,0 +1,419 @@
> +/** @file
> +
> +  Copyright (c) 2013-2014, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2017, Linaro. 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 <libfdt.h>
> +#include <Library/AndroidBootImgLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <Protocol/AndroidBootImg.h>
> +#include <Protocol/LoadedImage.h>
> +
> +#include <libfdt.h>
> +
> +#define FDT_ADDITIONAL_ENTRIES_SIZE 0x400
> +
> +typedef struct {
> +  MEMMAP_DEVICE_PATH                      Node1;
> +  EFI_DEVICE_PATH_PROTOCOL                End;
> +} MEMORY_DEVICE_PATH;
> +
> +STATIC ABOOTIMG_PROTOCOL                 *mAbootimg;

mAndroidBootImg.

> +
> +STATIC CONST MEMORY_DEVICE_PATH MemoryDevicePathTemplate =

Should also have an 'm'-prefix.

> +{
> +  {
> +    {
> +      HARDWARE_DEVICE_PATH,
> +      HW_MEMMAP_DP,
> +      {
> +        (UINT8)(sizeof (MEMMAP_DEVICE_PATH)),
> +        (UINT8)((sizeof (MEMMAP_DEVICE_PATH)) >> 8),
> +      },
> +    }, // Header
> +    0, // StartingAddress (set at runtime)
> +    0  // EndingAddress   (set at runtime)
> +  }, // Node1
> +  {
> +    END_DEVICE_PATH_TYPE,
> +    END_ENTIRE_DEVICE_PATH_SUBTYPE,
> +    { sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }
> +  } // End
> +};
> +
> +EFI_STATUS
> +AbootimgGetImgSize (

AndroidBootImgGetImageSize.

> +  IN  VOID    *BootImg,
> +  OUT UINTN   *ImgSize
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +
> +  Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
> +
> +  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
> +                    ANDROID_BOOT_MAGIC_LENGTH) != 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  /* The page size is not specified, but it should be power of 2 at least */
> +  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
> +
> +  /* Get real size of abootimg */
> +  *ImgSize = ALIGN_VALUE (Header->KernelSize, Header->PageSize) +
> +             ALIGN_VALUE (Header->RamdiskSize, Header->PageSize) +
> +             ALIGN_VALUE (Header->SecondStageBootloaderSize, 
> Header->PageSize) +
> +             Header->PageSize;
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +AbootimgGetKernelInfo (

AndroidBootImgGetKernelInfo.

> +  IN  VOID    *BootImg,
> +  OUT VOID   **Kernel,
> +  OUT UINTN   *KernelSize
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +
> +  Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
> +
> +  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
> +                    ANDROID_BOOT_MAGIC_LENGTH) != 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Header->KernelSize == 0) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
> +
> +  *KernelSize = Header->KernelSize;
> +  *Kernel = BootImg + Header->PageSize;
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +AbootimgGetRamdiskInfo (
> +  IN  VOID    *BootImg,
> +  OUT VOID   **Ramdisk,
> +  OUT UINTN   *RamdiskSize
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +  UINT8                    *BootImgBytePtr;
> +
> +  // Cast to UINT8 so we can do pointer arithmetic
> +  BootImgBytePtr = (UINT8 *) BootImg;
> +
> +  Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
> +
> +  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
> +                    ANDROID_BOOT_MAGIC_LENGTH) != 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
> +
> +  *RamdiskSize = Header->RamdiskSize;
> +
> +  if (Header->RamdiskSize != 0) {
> +    *Ramdisk = (VOID *) (BootImgBytePtr
> +                 + Header->PageSize
> +                 + ALIGN_VALUE (Header->KernelSize, Header->PageSize));
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +AbootimgGetSecondBootLoaderInfo (

AndroidBootImg...

> +  IN  VOID    *BootImg,
> +  OUT VOID   **Second,
> +  OUT UINTN   *SecondSize
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +  UINT8                    *BootImgBytePtr;
> +
> +  // Cast to UINT8 so we can do pointer arithmetic
> +  BootImgBytePtr = (UINT8 *) BootImg;
> +
> +  Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
> +
> +  if (AsciiStrnCmp ((CONST CHAR8 *)Header->BootMagic, ANDROID_BOOT_MAGIC,
> +                    ANDROID_BOOT_MAGIC_LENGTH) != 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
> +
> +  *SecondSize = Header->SecondStageBootloaderSize;
> +
> +  if (Header->SecondStageBootloaderSize != 0) {
> +    *Second = (VOID *) (BootImgBytePtr
> +                 + Header->PageSize
> +                 + ALIGN_VALUE (Header->KernelSize, Header->PageSize)
> +                 + ALIGN_VALUE (Header->RamdiskSize, Header->PageSize));
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +AbootimgGetKernelArgs (

AndroidBootImg...

> +  IN  VOID    *BootImg,
> +  OUT CHAR8   *KernelArgs
> +  )
> +{
> +  ANDROID_BOOTIMG_HEADER   *Header;
> +
> +  Header = (ANDROID_BOOTIMG_HEADER *) BootImg;
> +  AsciiStrnCpyS (KernelArgs, ANDROID_BOOTIMG_KERNEL_ARGS_SIZE, 
> Header->KernelArgs,
> +    ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +AbootimgGetFdt (

AndroidBootImg...

> +  IN  VOID                  *BootImg,
> +  IN  VOID                  **FdtBase
> +  )
> +{
> +  UINTN                      SecondLoaderSize;
> +  EFI_STATUS                 Status;
> +
> +  /* Check whether FDT is located in second boot loader as some vendor do so,

It would be more correct to say "second boot loader region" than
"second boot loader".

> +   * because second loader is never used as far as I know. */
> +  Status = AbootimgGetSecondBootLoaderInfo (
> +          BootImg,
> +          FdtBase,
> +          &SecondLoaderSize
> +          );
> +  return Status;
> +}
> +
> +EFI_STATUS
> +AbootimgUpdateArgsFdt (

AndroidBootImgUpdateKernelArgs
(The arguments always come through Fdt, so I do not feel that needs to
be explicitly pointed out.)

General comment: this function needs to be broken down into several
smaller helper functions:
- extract kernel arguments from boot.img
- extract ramdisk information from boot.img
- locate FDT
- update FDT

> +  IN  VOID                  *BootImg,
> +  OUT VOID                  *KernelArgs
> +  )
> +{
> +  VOID                      *Ramdisk;

RamdiskData?

> +  UINT64                     Ramdisk64, RamdiskEnd64;

RamdiskStart, RamDiskEnd?

> +  UINTN                      RamdiskSize;
> +  CHAR8                      ImgKernelArgs[ANDROID_BOOTIMG_KERNEL_ARGS_SIZE];

ImageKernelArgs
or
BootImgKernelArgs

> +  INTN                       Err, NewFdtSize, chosen_node;

ChosenNode

> +  EFI_STATUS                 Status;
> +  EFI_PHYSICAL_ADDRESS       FdtBase, UpdatedFdtBase, NewFdtBase;
> +  struct fdt_property       *prop;

*Property.

> +  int                        len;

INTN Len;

> +
> +  Status = gBS->LocateProtocol (&gAndroidBootImgProtocolGuid, NULL,
> +                                (VOID **) &mAbootimg);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = AbootimgGetKernelArgs (BootImg, ImgKernelArgs);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +  // Get kernel arguments from Android boot image
> +  AsciiStrToUnicodeStrS (ImgKernelArgs, KernelArgs,
> +                         ANDROID_BOOTIMG_KERNEL_ARGS_SIZE >> 1);
> +  // Append platform kernel arguments
> +  if(mAbootimg->AppendArgs) {
> +    Status = mAbootimg->AppendArgs (KernelArgs,
> +                                    ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
> +    if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
> +  }
> +
> +  Status = EfiGetSystemConfigurationTable (&gFdtTableGuid, (VOID 
> **)&FdtBase);
> +  if (!EFI_ERROR (Status)) {

Should this not be
  if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND)
?

> +    return Status;
> +  }
> +
> +  Status = AbootimgGetFdt (BootImg, (VOID **)&FdtBase);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +  Err = fdt_check_header ((VOID*)(UINTN)FdtBase);
> +  if (Err != 0) {
> +    DEBUG ((DEBUG_ERROR, "ERROR: Device Tree header not valid (Err:%d)\n",
> +           Err));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = AbootimgGetRamdiskInfo (
> +            BootImg,
> +            &Ramdisk,
> +            &RamdiskSize
> +            );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  NewFdtSize = (UINTN)fdt_totalsize ((VOID*)(UINTN)(FdtBase))
> +               + FDT_ADDITIONAL_ENTRIES_SIZE;
> +  Status = gBS->AllocatePages (AllocateAnyPages, EfiBootServicesData,
> +                  EFI_SIZE_TO_PAGES (NewFdtSize), &UpdatedFdtBase);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_WARN, "Warning: Failed to reallocate FDT, err %d.\n",
> +           Status));
> +    return Status;
> +  }
> +
> +  // Load the Original FDT tree into the new region
> +  Err = fdt_open_into((VOID*)FdtBase, (VOID*)UpdatedFdtBase, NewFdtSize);
> +  if (Err) {
> +    DEBUG ((EFI_D_ERROR, "fdt_open_into(): %a\n", fdt_strerror (Err)));
> +    Status = EFI_INVALID_PARAMETER;
> +    goto Fdt_Exit;
> +  }
> +
> +  Ramdisk64 = cpu_to_fdt64((UINT64)Ramdisk);
> +  RamdiskEnd64 = cpu_to_fdt64((UINT64)(Ramdisk + RamdiskSize));
> +
> +  chosen_node = fdt_subnode_offset ((const void *)UpdatedFdtBase, 0, 
> "chosen");
> +  if (chosen_node < 0) {
> +    chosen_node = fdt_add_subnode((void *)UpdatedFdtBase, 0, "chosen");
> +      if (chosen_node < 0) {
> +        DEBUG ((EFI_D_ERROR, "Failed to find chosen node in fdt!\n"));
> +        goto Fdt_Exit;
> +    }
> +  }
> +  prop = fdt_get_property_w((void *)UpdatedFdtBase, chosen_node,
> +                            "linux,initrd-start", &len);
> +  if (NULL == prop && len == -FDT_ERR_NOTFOUND) {
> +    fdt_appendprop ((void *)UpdatedFdtBase, chosen_node,
> +                    "linux,initrd-start", &Ramdisk64, sizeof (UINT64));
> +  } else if (prop != NULL) {
> +    fdt_setprop_u64((void *)UpdatedFdtBase, chosen_node,
> +                    "linux,initrd-start", (uint64_t)Ramdisk64);
> +  } else {
> +    DEBUG ((EFI_D_ERROR, "Failed to append fdt prop initrd-start\n",
> +           fdt_strerror (Err)));
> +    Status = EFI_INVALID_PARAMETER;
> +    goto Fdt_Exit;
> +  }
> +
> +  prop = fdt_get_property_w((void *)UpdatedFdtBase, chosen_node,
> +                            "linux,initrd-end", &len);
> +  if (NULL == prop && len == -FDT_ERR_NOTFOUND) {
> +    fdt_appendprop ((void *)UpdatedFdtBase, chosen_node,
> +                    "linux,initrd-end", &RamdiskEnd64, sizeof (UINT64));
> +  } else if (prop != NULL) {
> +    fdt_setprop_u64((void *)UpdatedFdtBase, chosen_node,
> +                    "linux,initrd-end", (uint64_t)RamdiskEnd64);
> +  } else {
> +    DEBUG ((EFI_D_ERROR, "Failed to append fdt prop initrd-end\n",
> +           fdt_strerror (Err)));
> +    Status = EFI_INVALID_PARAMETER;
> +    goto Fdt_Exit;
> +  }
> +
> +  if ( mAbootimg->UpdateDtb) {
> +    Status = mAbootimg->UpdateDtb (UpdatedFdtBase, &NewFdtBase);
> +    if (EFI_ERROR (Status)) {
> +      goto Fdt_Exit;
> +    }
> +  }
> +
> +  //
> +  // Sanity checks on the new FDT blob.
> +  //
> +  Err = fdt_check_header ((VOID*)(UINTN)NewFdtBase);

I don't think this test is needed. The state of the FDT is completely
under our control at this point. The only thing it could uncover would
be a stray pointer, or a bug in libfdt.

> +  if (Err != 0) {
> +    Print (L"ERROR: Device Tree header not valid (err:%d)\n", Err);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = gBS->InstallConfigurationTable (
> +                  &gFdtTableGuid,
> +                  (VOID *)(UINTN)NewFdtBase
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    goto Fdt_Exit;
> +  }
> +  return Status;

This is preference only, but I think
  if (!EFI_ERROR (Status)) {
    return EFI_SUCCESS;
  }
would be more clear.

> +
> +Fdt_Exit:
> +  gBS->FreePages (UpdatedFdtBase, EFI_SIZE_TO_PAGES (NewFdtSize));
> +  return Status;
> +}
> +
> +EFI_STATUS
> +AbootimgBoot (

AndroidBootImgBoot

> +  IN VOID                            *Buffer,
> +  IN UINTN                            BufferSize
> +  )
> +{
> +  EFI_STATUS                          Status;
> +  VOID                               *Kernel;
> +  UINTN                               KernelSize;
> +  MEMORY_DEVICE_PATH                  KernelDevicePath;
> +  EFI_HANDLE                          ImageHandle;
> +  VOID                               *NewKernelArg;
> +  EFI_LOADED_IMAGE_PROTOCOL          *ImageInfo;
> +
> +  Status = AbootimgGetKernelInfo (
> +            Buffer,
> +            &Kernel,
> +            &KernelSize
> +            );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  NewKernelArg = AllocateZeroPool (ANDROID_BOOTIMG_KERNEL_ARGS_SIZE);
> +  if (NewKernelArg == NULL) {
> +    DEBUG ((DEBUG_ERROR, "Fail to allocate memory\n"));
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Status = AbootimgUpdateArgsFdt (Buffer, NewKernelArg);
> +  if (EFI_ERROR (Status)) {
> +    FreePool (NewKernelArg);
> +    return EFI_INVALID_PARAMETER;

return Status?

/
    Leif

> +  }
> +
> +  KernelDevicePath = MemoryDevicePathTemplate;
> +
> +  KernelDevicePath.Node1.StartingAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) 
> Kernel;
> +  KernelDevicePath.Node1.EndingAddress   = (EFI_PHYSICAL_ADDRESS)(UINTN) 
> Kernel
> +                                           + KernelSize;
> +
> +  Status = gBS->LoadImage (TRUE, gImageHandle,
> +                           (EFI_DEVICE_PATH *)&KernelDevicePath,
> +                           (VOID*)(UINTN)Kernel, KernelSize, &ImageHandle);
> +
> +  // Set kernel arguments
> +  Status = gBS->HandleProtocol (ImageHandle, &gEfiLoadedImageProtocolGuid,
> +                                (VOID **) &ImageInfo);
> +  ImageInfo->LoadOptions = NewKernelArg;
> +  ImageInfo->LoadOptionsSize = StrLen (NewKernelArg) * sizeof (CHAR16);
> +
> +  // Before calling the image, enable the Watchdog Timer for  the 5 Minute 
> period
> +  gBS->SetWatchdogTimer (5 * 60, 0x10000, 0, NULL);
> +  // Start the image
> +  Status = gBS->StartImage (ImageHandle, NULL, NULL);
> +  // Clear the Watchdog Timer if the image returns
> +  gBS->SetWatchdogTimer (0, 0x10000, 0, NULL);
> +  return EFI_SUCCESS;
> +}
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf 
> b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> new file mode 100644
> index 0000000..c92bac0
> --- /dev/null
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.inf
> @@ -0,0 +1,48 @@
> +#/** @file
> +#
> +#  Copyright (c) 2013-2015, ARM Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2017, Linaro. 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]
> +  INF_VERSION                    = 0x00010019
> +  BASE_NAME                      = AndroidBootImgLib
> +  FILE_GUID                      = ed3b8739-6fa7-4cb1-8aeb-2496f8fcaefa
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = AndroidBootImgLib
> +
> +#
> +# The following information is for reference only and not required by the 
> build tools.
> +#
> +#  VALID_ARCHITECTURES           = ARM AARCH64
> +#
> +
> +[Sources]
> +  AndroidBootImgLib.c
> +
> +[LibraryClasses]
> +  DebugLib
> +  FdtLib
> +  PrintLib
> +  UefiBootServicesTableLib
> +  UefiLib
> +
> +[Packages]
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[Protocols]
> +  gAndroidBootImgProtocolGuid
> +
> +[Guids]
> +  gFdtTableGuid
> -- 
> 1.9.1
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to