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

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

On 11/26/22 11:59, Safae Ouajih wrote:

...

[You don't often get email from souajih@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@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