On Thu, Jan 26, 2023 at 17:55, Simon Glass <s...@chromium.org> wrote:
> Hi Safae,
>
> On Thu, 26 Jan 2023 at 09:05, Safae Ouajih <soua...@baylibre.com> wrote:
>>
>> This enables the support for boot image header version 3 and 4
>> using abootimg command.
>>
>> In order to use version 3 or 4:
>>
>> 1- Vendor boot image address should be given to abootimg cmd.
>>
>> abootimg addr $1 $vendor_boot_load_addr
>>
>> 2- "ramdisk_addr_r" env variable (ramdisk address) should be set to host
>> the ramdisk : generic ramdisk + vendor ramdisk
>>
>> "struct andr_boot_img_hdr_v0*" is replaced by "void *" in
>> some functions since v3 and v4 are now supported as well.
>>
>> Signed-off-by: Safae Ouajih <soua...@baylibre.com>
>> ---
>> boot/bootm.c | 29 ++++++++++++++++++++++++-----
>> boot/image-android.c | 16 ++++++++++------
>> boot/image-board.c | 14 +++++++++++---
>> boot/image-fdt.c | 2 +-
>> cmd/abootimg.c | 24 ++++++++++++++++++++++--
>> include/image.h | 14 ++++++++------
>> 6 files changed, 76 insertions(+), 23 deletions(-)
Since this introduces the new formats (v3 and v4), can we also update
the doc/android/boot-image.rst files which lists all the boot formats
for android?
>>
>> diff --git a/boot/bootm.c b/boot/bootm.c
>> index a58e6f391e..3e130c175c 100644
>> --- a/boot/bootm.c
>> +++ b/boot/bootm.c
>> @@ -113,6 +113,10 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int
>> flag, int argc,
>> char *const argv[])
>> {
>> const void *os_hdr;
>> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
>> + const void *vendor_boot_img;
>> + const void *boot_img;
>> +#endif
>> bool ep_found = false;
>> int ret;
>>
>> @@ -181,12 +185,17 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int
>> flag, int argc,
>> #endif
>> #ifdef CONFIG_ANDROID_BOOT_IMAGE
>> case IMAGE_FORMAT_ANDROID:
>> + boot_img = os_hdr;
>> + vendor_boot_img = NULL;
>> + if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
>> + boot_img = (void *)get_abootimg_addr();
>> + vendor_boot_img = (void *)get_avendor_bootimg_addr();
>
> map_sysmem() so it owrks on sandbox
>
>> + }
>> images.os.type = IH_TYPE_KERNEL;
>> - images.os.comp = android_image_get_kcomp(os_hdr, NULL);
>> + images.os.comp = android_image_get_kcomp(boot_img,
>> vendor_boot_img);
>> images.os.os = IH_OS_LINUX;
>> -
>> - images.os.end = android_image_get_end(os_hdr, NULL);
>> - images.os.load = android_image_get_kload(os_hdr, NULL);
>> + images.os.end = android_image_get_end(boot_img,
>> vendor_boot_img);
>> + images.os.load = android_image_get_kload(boot_img,
>> vendor_boot_img);
>> images.ep = images.os.load;
>> ep_found = true;
>> break;
>> @@ -889,6 +898,10 @@ static const void *boot_get_kernel(struct cmd_tbl
>> *cmdtp, int flag, int argc,
>> int os_noffset;
>> #endif
>>
>> +#ifdef CONFIG_ANDROID_BOOT_IMAGE
>> + const void *boot_img;
>> + const void *vendor_boot_img;
>> +#endif
>> img_addr = genimg_get_kernel_addr_fit(argc < 1 ? NULL : argv[0],
>> &fit_uname_config,
>> &fit_uname_kernel);
>> @@ -964,8 +977,14 @@ static const void *boot_get_kernel(struct cmd_tbl
>> *cmdtp, int flag, int argc,
>> #endif
>> #ifdef CONFIG_ANDROID_BOOT_IMAGE
>> case IMAGE_FORMAT_ANDROID:
>> + boot_img = buf;
>> + vendor_boot_img = NULL;
>> + if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
>> + boot_img = (void *)get_abootimg_addr();
>> + vendor_boot_img = (void *)get_avendor_bootimg_addr();
>
> and here
>
>> + }
>> printf("## Booting Android Image at 0x%08lx ...\n",
>> img_addr);
>> - if (android_image_get_kernel(buf, NULL, images->verify,
>> + if (android_image_get_kernel(boot_img, vendor_boot_img,
>> images->verify,
>> os_data, os_len))
>> return NULL;
>> break;
>> diff --git a/boot/image-android.c b/boot/image-android.c
>> index edeeaaaee0..dd06c09279 100644
>> --- a/boot/image-android.c
>> +++ b/boot/image-android.c
>> @@ -201,7 +201,7 @@ static ulong android_image_get_kernel_addr(struct
>> andr_image_data *img_data)
>> * Return: Zero, os start address and length on success,
>> * otherwise on failure.
>> */
>> -int android_image_get_kernel(const struct andr_boot_img_hdr_v0 *hdr,
>> +int android_image_get_kernel(const void *hdr,
>> const void *vendor_boot_img, int verify,
>> ulong *os_data, ulong *os_len)
>> {
>> @@ -286,7 +286,7 @@ bool is_android_vendor_boot_image_header(const void
>> *vendor_boot_img)
>> return !memcmp(VENDOR_BOOT_MAGIC, vendor_boot_img,
>> ANDR_VENDOR_BOOT_MAGIC_SIZE);
>> }
>>
>> -bool is_android_boot_image_header(const struct andr_boot_img_hdr_v0 *hdr)
>> +bool is_android_boot_image_header(const void *hdr)
>> {
>> return !memcmp(ANDR_BOOT_MAGIC, hdr, ANDR_BOOT_MAGIC_SIZE);
>> }
>> @@ -305,7 +305,7 @@ ulong android_image_get_end(const struct
>> andr_boot_img_hdr_v0 *hdr,
>> return img_data.boot_img_total_size;
>> }
>>
>> -ulong android_image_get_kload(const struct andr_boot_img_hdr_v0 *hdr,
>> +ulong android_image_get_kload(const void *hdr,
>> const void *vendor_boot_img)
>> {
>> struct andr_image_data img_data;
>> @@ -316,7 +316,7 @@ ulong android_image_get_kload(const struct
>> andr_boot_img_hdr_v0 *hdr,
>> return android_image_get_kernel_addr(&img_data);
>> }
>>
>> -ulong android_image_get_kcomp(const struct andr_boot_img_hdr_v0 *hdr,
>> +ulong android_image_get_kcomp(const void *hdr,
>> const void *vendor_boot_img)
>> {
>> struct andr_image_data img_data;
>> @@ -364,14 +364,18 @@ int android_image_get_ramdisk(const void *hdr, const
>> void *vendor_boot_img,
>> return 0;
>> }
>>
>> -int android_image_get_second(const struct andr_boot_img_hdr_v0 *hdr,
>> - ulong *second_data, ulong *second_len)
>> +int android_image_get_second(const void *hdr, ulong *second_data, ulong
>> *second_len)
>> {
>> struct andr_image_data img_data;
>>
>> if (!android_image_get_data(hdr, NULL, &img_data))
>> return -EINVAL;
>>
>> + if (img_data.header_version > 2) {
>> + printf("Second stage bootloader is only supported for boot
>> image version <= 2\n");
>> + return -1;
>
> Use proper error number. This is -EPERM which seems wrong. Perhaps -ENOTSUPP ?
>
>> + }
>> +
>> if (!img_data.second_size) {
>> *second_data = *second_len = 0;
>> return -1;
>> diff --git a/boot/image-board.c b/boot/image-board.c
>> index b6c9bbe8f5..a20b57900e 100644
>> --- a/boot/image-board.c
>> +++ b/boot/image-board.c
>> @@ -426,10 +426,18 @@ static int select_ramdisk(struct bootm_headers
>> *images, const char *select, u8 a
>> break;
>> case IMAGE_FORMAT_ANDROID:
>> if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) {
>> - void *ptr = map_sysmem(images->os.start, 0);
>> int ret;
>> - ret = android_image_get_ramdisk(ptr, NULL, rd_datap,
>> rd_lenp);
>> - unmap_sysmem(ptr);
>> + if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) {
>> + ret = android_image_get_ramdisk((void
>> *)get_abootimg_addr(),
>
> map_sysmem() again. Or perhaps update that function to take an address?
>
>> + (void
>> *)get_avendor_bootimg_addr(),
>> + rd_datap,
>> rd_lenp);
>> + } else {
>> + void *ptr = map_sysmem(images->os.start, 0);
>> +
>> + ret = android_image_get_ramdisk(ptr, NULL,
>> rd_datap, rd_lenp);
>> + unmap_sysmem(ptr);
>> + }
>> +
>> if (ret)
>> return ret;
>> done = true;
>> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
>> index f270d00a5f..5f9cf7b4e6 100644
>> --- a/boot/image-fdt.c
>> +++ b/boot/image-fdt.c
>> @@ -529,7 +529,7 @@ int boot_get_fdt(int flag, int argc, char *const argv[],
>> uint8_t arch,
>> }
>> #ifdef CONFIG_ANDROID_BOOT_IMAGE
>> } else if (genimg_get_format(buf) == IMAGE_FORMAT_ANDROID) {
>> - struct andr_boot_img_hdr_v0 *hdr = buf;
>> + void *hdr = buf;
>> ulong fdt_data, fdt_len;
>> u32 fdt_size, dtb_idx;
>> /*
>> diff --git a/cmd/abootimg.c b/cmd/abootimg.c
>> index 4d6cf0fa3e..ef7130da0b 100644
>> --- a/cmd/abootimg.c
>> +++ b/cmd/abootimg.c
>> @@ -17,6 +17,16 @@
>> static ulong _abootimg_addr = -1;
>> static ulong _avendor_bootimg_addr = -1;
>>
>> +ulong get_abootimg_addr(void)
>> +{
>> + return (_abootimg_addr == -1 ? image_load_addr : _abootimg_addr);
>> +}
>> +
>> +ulong get_avendor_bootimg_addr(void)
>> +{
>> + return _avendor_bootimg_addr;
>> +}
>> +
>> static int abootimg_get_ver(int argc, char *const argv[])
>> {
>> const struct andr_boot_img_hdr_v0 *hdr;
>> @@ -70,12 +80,21 @@ static int abootimg_get_dtb_load_addr(int argc, char
>> *const argv[])
>> return CMD_RET_USAGE;
>> struct andr_image_data img_data = {0};
>> const struct andr_boot_img_hdr_v0 *hdr;
>> + const struct andr_vendor_img_hdr *vhdr;
>>
>> hdr = map_sysmem(abootimg_addr(), sizeof(*hdr));
>> - if (!android_image_get_data(hdr, NULL, &img_data)) {
>> + if (get_avendor_bootimg_addr() != -1)
>> + vhdr = map_sysmem(get_avendor_bootimg_addr(), sizeof(*vhdr));
>> +
>> + if (!android_image_get_data(hdr, vhdr, &img_data)) {
>> + if (get_avendor_bootimg_addr() != -1)
>> + unmap_sysmem(vhdr);
>> unmap_sysmem(hdr);
>> return CMD_RET_FAILURE;
>> }
>> +
>> + if (get_avendor_bootimg_addr() != -1)
>> + unmap_sysmem(vhdr);
>> unmap_sysmem(hdr);
>>
>> if (img_data.header_version < 2) {
>> @@ -119,7 +138,8 @@ static int abootimg_get_dtb_by_index(int argc, char
>> *const argv[])
>> return CMD_RET_FAILURE;
>> }
>>
>> - if (!android_image_get_dtb_by_index(abootimg_addr(), 0, num,
>> + if (!android_image_get_dtb_by_index(abootimg_addr(),
>> + get_avendor_bootimg_addr(), num,
>> &addr, &size)) {
>> return CMD_RET_FAILURE;
>> }
>> diff --git a/include/image.h b/include/image.h
>> index ac94ecaa14..5bd4bf5d57 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -1739,29 +1739,31 @@ bool android_image_get_data(const void *boot_hdr,
>> const void *vendor_boot_hdr,
>> struct andr_image_data *data);
>>
>> struct andr_boot_img_hdr_v0;
>> -int android_image_get_kernel(const struct andr_boot_img_hdr_v0 *hdr,
>> +int android_image_get_kernel(const void *hdr,
>> const void *vendor_boot_img, int verify,
>> ulong *os_data, ulong *os_len);
>> int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
>> ulong *rd_data, ulong *rd_len);
>> -int android_image_get_second(const struct andr_boot_img_hdr_v0 *hdr,
>> - ulong *second_data, ulong *second_len);
>> +int android_image_get_second(const void *hdr, ulong *second_data, ulong
>> *second_len);
>> bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, u32 *size);
>> bool android_image_get_dtb_by_index(ulong hdr_addr, ulong vendor_boot_img,
>> u32 index, ulong *addr, u32 *size);
>> ulong android_image_get_end(const struct andr_boot_img_hdr_v0 *hdr,
>> const void *vendor_boot_img);
>> -ulong android_image_get_kload(const struct andr_boot_img_hdr_v0 *hdr,
>> +ulong android_image_get_kload(const void *hdr,
>> const void *vendor_boot_img);
>> -ulong android_image_get_kcomp(const struct andr_boot_img_hdr_v0 *hdr,
>> +ulong android_image_get_kcomp(const void *hdr,
>> const void *vendor_boot_img);
>> void android_print_contents(const struct andr_boot_img_hdr_v0 *hdr);
>> #ifdef CONFIG_CMD_ABOOTIMG
>> bool android_image_print_dtb_contents(ulong hdr_addr);
>> #endif
>> -bool is_android_boot_image_header(const struct andr_boot_img_hdr_v0 *hdr);
>> +bool is_android_boot_image_header(const void *hdr);
>> bool is_android_vendor_boot_image_header(const void *vendor_boot_img);
>>
>> +ulong get_abootimg_addr(void);
>> +ulong get_avendor_bootimg_addr(void);
>
> again please add comments to all of these.
>
>> +
>> /**
>> * board_fit_config_name_match() - Check for a matching board name
>> *
>> --
>> 2.34.1
>>
>
> Regards,
> Simon
