On Wed, Aug 02, 2017 at 10:03:41PM +0800, Jun Nie wrote: > Add an android kernel loader that could load kernel from storage > device. > This android boot image BDS add addtitional cmdline/dtb/ramfs > support besides kernel that is introduced by Android boot header. > > This patch is derived from Haojian's code as below link. > https://patches.linaro.org/patch/94683/ > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jun Nie <jun....@linaro.org>
I have one style-related comment below. Other than that, my final remaining concern for this series is that this _still_ breaks bisect. This application still cannot be built until what is currently 3/3 has been applied. My view is that there is no separate need for 3/3 and it can simply be squashed into this patch. > --- > .../Application/AndroidBoot/AndroidBootApp.c | 140 ++++++ > .../Application/AndroidBoot/AndroidBootApp.inf | 64 +++ > EmbeddedPkg/Include/Library/AndroidBootImgLib.h | 13 + > EmbeddedPkg/Include/Protocol/AndroidBootImg.h | 47 ++ > .../Library/AndroidBootImgLib/AndroidBootImgLib.c | 471 > +++++++++++++++++++++ > .../AndroidBootImgLib/AndroidBootImgLib.inf | 48 +++ > 6 files changed, 783 insertions(+) > create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.c > create mode 100644 EmbeddedPkg/Application/AndroidBoot/AndroidBootApp.inf > 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/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > new file mode 100644 > index 0000000..da5b5d2 > --- /dev/null > +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c > @@ -0,0 +1,471 @@ > +/** @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 ANDROID_BOOTIMG_PROTOCOL *mAndroidBootImg; > + > +STATIC CONST MEMORY_DEVICE_PATH mMemoryDevicePathTemplate = > +{ > + { > + { > + 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 > +AndroidBootImgGetImgSize ( > + 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 > +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 > +AndroidBootImgGetRamdiskInfo ( > + 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 > +AndroidBootImgGetSecondBootLoaderInfo ( > + 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; Can you not replace this bit of casting... > + > + 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 With a simple (UINTN)BootImg here? If not, could you flip the input parameter to be a UINTN BootImgBase? > + + Header->PageSize > + + ALIGN_VALUE (Header->KernelSize, Header->PageSize) > + + ALIGN_VALUE (Header->RamdiskSize, Header->PageSize)); > + } > + return EFI_SUCCESS; > +} / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel