Hi 刘垣Valentin, On Thu, 20 Aug 2026 at 10:41, 刘垣辰 <[email protected]> wrote: > > Hi Simon, > > Thanks for your reviewing and suggestions. > > I can merge all scanning function into one "helper", just introduce a new > variable to condition which partition we needed. > > But I have no idea about test and document and I have not seen anything in > test/ or doc/ folder. Can you give me some advice?
Yes you can see the documentation here: https://docs.u-boot-project.org/en/latest/develop/index.html#testing The android tests are in test/boot/bootflow.c and test/py/tests/test_android/test_abootimg.py for example. Let me know if you have trouble running them. BTW we try to avoid top-posting as it makes the thread confusing. Regards, Simon > > I will refactor the AVB logic in the next patch in future. > > Best regards, > Valentin Liu > > 2026年8月20日下午8:31，Simon Glass <[email protected]> 写道： > > > 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 > >
