[PATCH 01/17] android: boot: rename andr_img_hdr -> andr_boot_img_hdr_v0_v1_v2

Safae Ouajih souajih at baylibre.com
Tue Nov 29 12:07:19 CET 2022


On Mon, 28 Nov 2022 11:08:20 -0500
Sean Anderson <sean.anderson at seco.com> wrote:

> On 11/26/22 11:59, Safae Ouajih wrote:
> > [You don't often get email from souajih at baylibre.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > Android introduced boot header version 3 or 4.
> > The header structure change with version 3 and 4 to support
> > the new updates such as:
> > - Introducing Vendor boot image: with a vendor ramdisk
> > - Bootconfig feature (v4)
> > 
> > To maintain support for version v0, v1 and v2 while introducing
> > version 3 and 4, this struct name must change to refer to the header
> > structure before v3.
> > 
> > Signed-off-by: Safae Ouajih <souajih at baylibre.com>
> > ---
> >  boot/image-android.c      | 26 +++++++++++++-------------
> >  boot/image-fdt.c          |  2 +-
> >  cmd/abootimg.c            |  4 ++--
> >  drivers/fastboot/fb_mmc.c |  8 ++++----
> >  include/android_image.h   |  4 ++--
> >  include/image.h           | 18 +++++++++---------
> >  6 files changed, 31 insertions(+), 31 deletions(-)
> > 
> > diff --git a/boot/image-android.c b/boot/image-android.c
> > index 2628db374121..926d94ecbe8e 100644
> > --- a/boot/image-android.c
> > +++ b/boot/image-android.c
> > @@ -18,7 +18,7 @@
> > 
> >  static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
> > 
> > -static ulong android_image_get_kernel_addr(const struct andr_img_hdr *hdr)
> > +static ulong android_image_get_kernel_addr(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> 
> This line is too long. You will have to break it like
> 
> static ulong 
> android_image_get_kernel_addr(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> 
> But really, this is because your struct name is too long. Can we use
> something like andr_img_hdr_v0 instead? I would like to have only one
> version in the struct name because then there is no urge to update the
> name when e.g. v5 comes out with mostly the same format. And _boot adds
> nothing.
> 
> --Sean
> 

Hello Sean,

Thank you for your review.

Since vendor boot image is introduced, we need to differentiate it from the boot image. Thus,
I suggest to use :
        andr_boot_img_hdr_v0 and andr_boot_img_hdr_v3 for the boot image
        andr_vendor_img_hdr for the vendor boot image

--Safae

> >  {
> >         /*
> >          * All the Android tools that generate a boot.img use this
> > @@ -59,7 +59,7 @@ static ulong android_image_get_kernel_addr(const struct andr_img_hdr *hdr)
> >   * Return: Zero, os start address and length on success,
> >   *             otherwise on failure.
> >   */
> > -int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify,
> > +int android_image_get_kernel(const struct andr_boot_img_hdr_v0_v1_v2 *hdr, int verify,
> >                              ulong *os_data, ulong *os_len)
> >  {
> >         u32 kernel_addr = android_image_get_kernel_addr(hdr);
> > @@ -122,12 +122,12 @@ int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify,
> >         return 0;
> >  }
> > 
> > -int android_image_check_header(const struct andr_img_hdr *hdr)
> > +int android_image_check_header(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> >  {
> >         return memcmp(ANDR_BOOT_MAGIC, hdr->magic, ANDR_BOOT_MAGIC_SIZE);
> >  }
> > 
> > -ulong android_image_get_end(const struct andr_img_hdr *hdr)
> > +ulong android_image_get_end(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> >  {
> >         ulong end;
> > 
> > @@ -150,12 +150,12 @@ ulong android_image_get_end(const struct andr_img_hdr *hdr)
> >         return end;
> >  }
> > 
> > -ulong android_image_get_kload(const struct andr_img_hdr *hdr)
> > +ulong android_image_get_kload(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> >  {
> >         return android_image_get_kernel_addr(hdr);
> >  }
> > 
> > -ulong android_image_get_kcomp(const struct andr_img_hdr *hdr)
> > +ulong android_image_get_kcomp(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> >  {
> >         const void *p = (void *)((uintptr_t)hdr + hdr->page_size);
> > 
> > @@ -167,7 +167,7 @@ ulong android_image_get_kcomp(const struct andr_img_hdr *hdr)
> >                 return image_decomp_type(p, sizeof(u32));
> >  }
> > 
> > -int android_image_get_ramdisk(const struct andr_img_hdr *hdr,
> > +int android_image_get_ramdisk(const struct andr_boot_img_hdr_v0_v1_v2 *hdr,
> >                               ulong *rd_data, ulong *rd_len)
> >  {
> >         if (!hdr->ramdisk_size) {
> > @@ -186,7 +186,7 @@ int android_image_get_ramdisk(const struct andr_img_hdr *hdr,
> >         return 0;
> >  }
> > 
> > -int android_image_get_second(const struct andr_img_hdr *hdr,
> > +int android_image_get_second(const struct andr_boot_img_hdr_v0_v1_v2 *hdr,
> >                               ulong *second_data, ulong *second_len)
> >  {
> >         if (!hdr->second_size) {
> > @@ -226,7 +226,7 @@ int android_image_get_second(const struct andr_img_hdr *hdr,
> >   */
> >  bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, u32 *size)
> >  {
> > -       const struct andr_img_hdr *hdr;
> > +       const struct andr_boot_img_hdr_v0_v1_v2 *hdr;
> >         ulong dtbo_img_addr;
> >         bool ret = true;
> > 
> > @@ -275,7 +275,7 @@ exit:
> >   */
> >  static bool android_image_get_dtb_img_addr(ulong hdr_addr, ulong *addr)
> >  {
> > -       const struct andr_img_hdr *hdr;
> > +       const struct andr_boot_img_hdr_v0_v1_v2 *hdr;
> >         ulong dtb_img_addr;
> >         bool ret = true;
> > 
> > @@ -328,7 +328,7 @@ exit:
> >  bool android_image_get_dtb_by_index(ulong hdr_addr, u32 index, ulong *addr,
> >                                     u32 *size)
> >  {
> > -       const struct andr_img_hdr *hdr;
> > +       const struct andr_boot_img_hdr_v0_v1_v2 *hdr;
> >         bool res;
> >         ulong dtb_img_addr;     /* address of DTB part in boot image */
> >         u32 dtb_img_size;       /* size of DTB payload in boot image */
> > @@ -393,7 +393,7 @@ bool android_image_get_dtb_by_index(ulong hdr_addr, u32 index, ulong *addr,
> >   * returns:
> >   *     no returned results
> >   */
> > -void android_print_contents(const struct andr_img_hdr *hdr)
> > +void android_print_contents(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> >  {
> >         const char * const p = IMAGE_INDENT_STRING;
> >         /* os_version = ver << 11 | lvl */
> > @@ -485,7 +485,7 @@ static bool android_image_print_dtb_info(const struct fdt_header *fdt,
> >   */
> >  bool android_image_print_dtb_contents(ulong hdr_addr)
> >  {
> > -       const struct andr_img_hdr *hdr;
> > +       const struct andr_boot_img_hdr_v0_v1_v2 *hdr;
> >         bool res;
> >         ulong dtb_img_addr;     /* address of DTB part in boot image */
> >         u32 dtb_img_size;       /* size of DTB payload in boot image */
> > diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> > index b830a0ab4187..b7cc9bfd538f 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_img_hdr *hdr = buf;
> > +               struct andr_boot_img_hdr_v0_v1_v2 *hdr = buf;
> >                 ulong           fdt_data, fdt_len;
> >                 u32                     fdt_size, dtb_idx;
> >                 /*
> > diff --git a/cmd/abootimg.c b/cmd/abootimg.c
> > index f48a9dcb021d..de2aa3307d9a 100644
> > --- a/cmd/abootimg.c
> > +++ b/cmd/abootimg.c
> > @@ -18,7 +18,7 @@ static ulong _abootimg_addr = -1;
> > 
> >  static int abootimg_get_ver(int argc, char *const argv[])
> >  {
> > -       const struct andr_img_hdr *hdr;
> > +       const struct andr_boot_img_hdr_v0_v1_v2 *hdr;
> >         int res = CMD_RET_SUCCESS;
> > 
> >         if (argc > 1)
> > @@ -65,7 +65,7 @@ static int abootimg_get_recovery_dtbo(int argc, char *const argv[])
> > 
> >  static int abootimg_get_dtb_load_addr(int argc, char *const argv[])
> >  {
> > -       const struct andr_img_hdr *hdr;
> > +       const struct andr_boot_img_hdr_v0_v1_v2 *hdr;
> >         int res = CMD_RET_SUCCESS;
> > 
> >         if (argc > 1)
> > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
> > index 033c510bc096..7b2f964b4c72 100644
> > --- a/drivers/fastboot/fb_mmc.c
> > +++ b/drivers/fastboot/fb_mmc.c
> > @@ -287,7 +287,7 @@ static void fb_mmc_boot_ops(struct blk_desc *dev_desc, void *buffer,
> >   */
> >  static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
> >                                        struct disk_partition *info,
> > -                                      struct andr_img_hdr *hdr,
> > +                                      struct andr_boot_img_hdr_v0_v1_v2 *hdr,
> >                                        char *response)
> >  {
> >         ulong sector_size;              /* boot partition sector size */
> > @@ -296,7 +296,7 @@ static lbaint_t fb_mmc_get_boot_header(struct blk_desc *dev_desc,
> > 
> >         /* Calculate boot image sectors count */
> >         sector_size = info->blksz;
> > -       hdr_sectors = DIV_ROUND_UP(sizeof(struct andr_img_hdr), sector_size);
> > +       hdr_sectors = DIV_ROUND_UP(sizeof(struct andr_boot_img_hdr_v0_v1_v2), sector_size);
> >         if (hdr_sectors == 0) {
> >                 pr_err("invalid number of boot sectors: 0\n");
> >                 fastboot_fail("invalid number of boot sectors: 0", response);
> > @@ -338,7 +338,7 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
> >                                 char *response)
> >  {
> >         uintptr_t hdr_addr;                     /* boot image header address */
> > -       struct andr_img_hdr *hdr;               /* boot image header */
> > +       struct andr_boot_img_hdr_v0_v1_v2 *hdr;         /* boot image header */
> >         lbaint_t hdr_sectors;                   /* boot image header sectors */
> >         u8 *ramdisk_buffer;
> >         u32 ramdisk_sector_start;
> > @@ -361,7 +361,7 @@ static int fb_mmc_update_zimage(struct blk_desc *dev_desc,
> > 
> >         /* Put boot image header in fastboot buffer after downloaded zImage */
> >         hdr_addr = (uintptr_t)download_buffer + ALIGN(download_bytes, PAGE_SIZE);
> > -       hdr = (struct andr_img_hdr *)hdr_addr;
> > +       hdr = (struct andr_boot_img_hdr_v0_v1_v2 *)hdr_addr;
> > 
> >         /* Read boot image header */
> >         hdr_sectors = fb_mmc_get_boot_header(dev_desc, &info, hdr, response);
> > diff --git a/include/android_image.h b/include/android_image.h
> > index 54d25af0684a..bf9b8c6f0955 100644
> > --- a/include/android_image.h
> > +++ b/include/android_image.h
> > @@ -20,9 +20,9 @@
> >  #define ANDR_BOOT_ARGS_SIZE 512
> >  #define ANDR_BOOT_EXTRA_ARGS_SIZE 1024
> > 
> > -/* The bootloader expects the structure of andr_img_hdr with header
> > +/* The bootloader expects the structure of andr_boot_img_hdr_v0_v1_v2 with header
> >   * version 0 to be as follows: */
> > -struct andr_img_hdr {
> > +struct andr_boot_img_hdr_v0_v1_v2 {
> >      /* Must be ANDR_BOOT_MAGIC. */
> >      char magic[ANDR_BOOT_MAGIC_SIZE];
> > 
> > diff --git a/include/image.h b/include/image.h
> > index 6f21dafba8ce..931cf18c7e82 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -1723,21 +1723,21 @@ int fit_image_cipher_get_algo(const void *fit, int noffset, char **algo);
> > 
> >  struct cipher_algo *image_get_cipher_algo(const char *full_name);
> > 
> > -struct andr_img_hdr;
> > -int android_image_check_header(const struct andr_img_hdr *hdr);
> > -int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify,
> > +struct andr_boot_img_hdr_v0_v1_v2;
> > +int android_image_check_header(const struct andr_boot_img_hdr_v0_v1_v2 *hdr);
> > +int android_image_get_kernel(const struct andr_boot_img_hdr_v0_v1_v2 *hdr, int verify,
> >                              ulong *os_data, ulong *os_len);
> > -int android_image_get_ramdisk(const struct andr_img_hdr *hdr,
> > +int android_image_get_ramdisk(const struct andr_boot_img_hdr_v0_v1_v2 *hdr,
> >                               ulong *rd_data, ulong *rd_len);
> > -int android_image_get_second(const struct andr_img_hdr *hdr,
> > +int android_image_get_second(const struct andr_boot_img_hdr_v0_v1_v2 *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, u32 index, ulong *addr,
> >                                     u32 *size);
> > -ulong android_image_get_end(const struct andr_img_hdr *hdr);
> > -ulong android_image_get_kload(const struct andr_img_hdr *hdr);
> > -ulong android_image_get_kcomp(const struct andr_img_hdr *hdr);
> > -void android_print_contents(const struct andr_img_hdr *hdr);
> > +ulong android_image_get_end(const struct andr_boot_img_hdr_v0_v1_v2 *hdr);
> > +ulong android_image_get_kload(const struct andr_boot_img_hdr_v0_v1_v2 *hdr);
> > +ulong android_image_get_kcomp(const struct andr_boot_img_hdr_v0_v1_v2 *hdr);
> > +void android_print_contents(const struct andr_boot_img_hdr_v0_v1_v2 *hdr);
> >  bool android_image_print_dtb_contents(ulong hdr_addr);
> > 
> >  /**
> > --
> > 2.25.1
> > 


-- 
Safae Ouajih <souajih at baylibre.com>


More information about the U-Boot mailing list