[PATCH v2 15/17] android: boot: support boot image header version 3 and 4

Safae Ouajih souajih at baylibre.com
Thu Feb 2 10:54:59 CET 2023


On 01/02/2023 09:44, Mattijs Korpershoek wrote:
> On Thu, Jan 26, 2023 at 17:55, Simon Glass <sjg at chromium.org> wrote:
>
>> Hi Safae,
>>
>> On Thu, 26 Jan 2023 at 09:05, Safae Ouajih <souajih at 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 <souajih at 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?


Hi Mattijs,

Thank you for the review,

I will update the documentation in a v3.

Best Regards,

Safae

>>> 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


More information about the U-Boot mailing list