From: Leif Lindholm <leif.lindholm@linaro.org>
To: Jun Nie <jun.nie@linaro.org>
Cc: haojian.zhuang@linaro.org, ard.biesheuvel@linaro.org,
edk2-devel@lists.01.org, linaro-uefi@lists.linaro.org,
shawn.guo@linaro.org, jason.liu@linaro.org
Subject: Re: [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage
Date: Thu, 27 Jul 2017 15:09:00 +0100 [thread overview]
Message-ID: <20170727140859.GL1501@bivouac.eciton.net> (raw)
In-Reply-To: <1501150040-32613-1-git-send-email-jun.nie@linaro.org>
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.nie@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
>
next prev parent reply other threads:[~2017-07-27 14:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-27 10:07 [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage Jun Nie
2017-07-27 10:07 ` [PATCH v3 2/2] EmbeddedPkg: add Android boot device path and guid Jun Nie
2017-07-27 14:10 ` Leif Lindholm
2017-07-27 14:09 ` Leif Lindholm [this message]
2017-07-28 9:47 ` [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage Jun Nie
2017-07-28 13:06 ` Leif Lindholm
2017-07-28 14:18 ` Jun Nie
2017-07-28 14:52 ` Leif Lindholm
2017-07-30 14:22 ` Jun Nie
2017-07-30 17:10 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170727140859.GL1501@bivouac.eciton.net \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox