Re:Re: [PATCH v1 1/2] boot: android: Add An droid 13+ bootflow support to bootmeth.
刘垣辰 <[email protected]>
| Newsgroups | org.u-boot-project.lists.u-boot |
|---|---|
| Message-ID | <[email protected]> |
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? 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