Re: [PATCH v1 1/2] boot: android: Add Android 13+ bootflow support to bootmeth.

Hi Valentin,

On 2026-08-18T17:53:00, Valentin Liu <[email protected]> wrote:
> boot: android: Add Android 13+ bootflow support to bootmeth.

Please drop the trailing period and keep the subject under 60 characters.

>
> The devices launching Android 13+ were using a new partition
> named init_boot to store generic ramdisk.

>
> In the new bootflow, kernel still be stored in boot image,
> however, the First Stage files in ramdisk were moved to
> init_boot image. We should load it to memory and verify it
> so that the kernel can execute init program to continue booting.
>
> Currently, we have supported loading the init_boot image by
> abootimg command, but we still need bring this ability to
> bootmeth, so that booting Android 13+ will be more easily.

Please use present/imperative tense throughout: 'were using' -> 'use',
'kernel still be stored' -> 'the kernel is still stored', 'were moved'
-> 'are moved', 'we still need bring' -> 'we still need to bring',
'more easily' -> 'easier'.

>
> In the new bootflow, kernel still be stored in boot image,
> however, the First Stage files in ramdisk were moved to
> init_boot image. We should load it to memory and verify it
> so that the kernel can execute init program to continue booting.
>
> Currently, we have supported loading the init_boot image by
> abootimg command, but we still need bring this ability to
> bootmeth, so that booting Android 13+ will be more easily.
> Bootmeth will be able to recognize the new partition layout,
> and boot Android normally.
>
> Link: https://source.android.com/docs/core/architecture/partitions/generic-boot
> Signed-off-by: Valentin Liu <[email protected]>
>
> boot/bootmeth_android.c          | 67 ++++++++++++++++++++++++++++++++++++++++
>  boot/image-android.c             | 16 ++++++++++
>  cmd/abootimg.c                   |  5 +++
>  doc/develop/bootstd/overview.rst |  3 ++
>  include/android_image.h          |  1 +
>  include/image.h                  | 35 +++++++++++++++++++++
>  6 files changed, 127 insertions(+)

Please can you look at how to add a test for this addition?

> diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
> @@ -113,6 +115,51 @@ static int scan_boot_part(struct udevice *blk, struct android_priv *priv)
> +static int scan_init_boot_part(struct udevice *blk, struct android_priv *priv)
> +{
> +     struct blk_desc *desc = dev_get_uclass_plat(blk);
> +     struct disk_partition partition;
> +     char partname[PART_NAME_LEN];
> +     ulong num_blks, bufsz;
> +     char *buf;
> +     int ret;
> +
> +     if (priv->slot)
> +             sprintf(partname, INIT_BOOT_PART_NAME "_%s", priv->slot);
> +     else
> +             sprintf(partname, INIT_BOOT_PART_NAME);

This is a near-duplicate of scan_boot_part() and
scan_vendor_boot_part(). Please factor the common logic (build
partname, read the header block, check magic, extract size) into a
helper rather than adding a third copy.

> diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
> @@ -291,6 +338,17 @@ static int android_read_bootflow(struct udevice *dev, struct bootflow *bflow)
> +     if (priv->header_version >= 4) {
> +             ret = scan_init_boot_part(bflow->blk, priv);
> +             if (ret < 0) {
> +                     /*
> +                      * Android 12 devices do not have the init_boot partition.
> +                      * Some devices upgraded to Android 13 or later from
> +                      * earlier Android versions may also not have one.
> +                      */
> +                     log_debug("scan init_boot failed: err=%d\n", ret);
> +             }
> +     }

priv is allocated with plain malloc() above, so it is not zeroed. On
failure here priv->init_boot_img_size is left uninitialised, then
boot_android_normal() and (in patch 2) run_avb_verification() read it
back as 'priv->init_boot_img_size > 0'. Please use calloc()/memset(),
or explicitly set priv->init_boot_img_size = 0 before the call and on
the failure path.

> diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
> @@ -556,6 +614,7 @@ static int boot_android_normal(struct bootflow *bflow)
>       ulong loadaddr = env_get_hex("loadaddr", 0);
> +     ulong iloadaddr = env_get_hex("init_boot_comp_addr_r", 0);
>       ulong vloadaddr = env_get_hex("vendor_boot_comp_addr_r", 0);

If init_boot_comp_addr_r is unset, env_get_hex() returns 0 and you
silently load init_boot at address 0 and call
set_ainit_bootimg_addr(0). Please check that iloadaddr is non-zero and
error out with a clear message before using it - the vendor_boot path
has the same weakness, but let's not extend the pattern.

> diff --git a/boot/image-android.c b/boot/image-android.c
> @@ -326,6 +326,22 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr,
> +bool android_image_get_data_v4(const void *boot_hdr, const void *vendor_boot_hdr,
> +                            const void *init_boot_hdr, struct andr_image_data *data)
> +{
> +     if (!android_image_get_data(boot_hdr, vendor_boot_hdr, data))
> +             return false;
> +
> +     if (!is_android_boot_image_header(init_boot_hdr)) {
> +             printf("Incorrect init boot image header\n");
> +             return false;
> +     }
> +
> +     android_boot_image_v3_v4_parse_hdr(init_boot_hdr, data);
> +
> +     return true;
> +}

I can't find any caller of android_image_get_data_v4(). Please either
wire it up to whatever consumes init_boot_img_total_size, or drop it
(and the new struct field, and the header declaration) until it is
needed.

> diff --git a/include/image.h b/include/image.h
> @@ -2167,6 +2184,17 @@ bool android_image_print_dtb_contents(ulong hdr_addr);
> +/**
> + * is_android_init_boot_image_header() - Check the magic of init boot image
> + *
> + * This checks the header of Android init boot image and verifies the
> + * magic is "ANDROID!" (same with the boot image)
> + *
> + * @init_boot_img: Pointer to boot image
> + * Return: non-zero if the magic is correct, zero otherwise
> + */
> +bool is_android_init_boot_image_header(const void *init_boot_img);

Declared but never defined or called - scan_init_boot_part() uses
is_android_boot_image_header() directly, which is correct since the
magic is identical. Please drop the declaration.

> diff --git a/include/image.h b/include/image.h
> @@ -2199,6 +2227,13 @@ void set_abootimg_addr(ulong addr);
> +/**
> + * set_ainit_bootimg_addr() - Set Android init boot image address
> + *
> + * Return: no returned results
> + */
> +void set_ainit_bootimg_addr(ulong addr);

Missing @addr: description, and a void function does not need a
Return: line - please drop it.

> diff --git a/doc/develop/bootstd/overview.rst b/doc/develop/bootstd/overview.rst
> @@ -293,6 +293,9 @@ script_offset_f
> +init_boot_comp_addr_r
> +    Address to which to load the init_boot Android image, e.g. 0xd0000000

Since this env var is required for Android 13+ to boot, please also
document it in the relevant board README(s) / sample env, and handle
the missing case gracefully in the code (see comment on
boot_android_normal()).

Regards,
Simon