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 v5 2/3] EmbeddedPkg/AndroidBoot: boot android kernel from storage
Date: Tue, 8 Aug 2017 14:34:43 +0100 [thread overview]
Message-ID: <20170808133443.mjdz7c5fkkg3aazb@bivouac.eciton.net> (raw)
In-Reply-To: <1501682622-8223-2-git-send-email-jun.nie@linaro.org>
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.nie@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
next prev parent reply other threads:[~2017-08-08 13:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-02 14:03 [PATCH v5 1/3] EmbeddedPkg/AndroidFastboot: split android boot header Jun Nie
2017-08-02 14:03 ` [PATCH v5 2/3] EmbeddedPkg/AndroidBoot: boot android kernel from storage Jun Nie
2017-08-08 13:34 ` Leif Lindholm [this message]
2017-08-02 14:03 ` [PATCH v5 3/3] EmbeddedPkg: add Android boot build entry Jun Nie
2017-08-07 16:27 ` [PATCH v5 1/3] EmbeddedPkg/AndroidFastboot: split android boot header 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=20170808133443.mjdz7c5fkkg3aazb@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