Hi Igor,

Thank you for the review.

On lun., juin 10, 2024 at 11:20, Igor Opaniuk igor.opaniuk@gmail.com wrote:

Hi Mattijs,

On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek mkorpershoek@baylibre.com wrote:

...

When reading a boot image header, we may need to retrieve the header version.

Add a helper function for it.

Signed-off-by: Mattijs Korpershoek mkorpershoek@baylibre.com

boot/image-android.c | 7 ++++++- include/image.h | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/boot/image-android.c b/boot/image-android.c index ddd8ffd5e540..4f8fb51585eb 100644 --- a/boot/image-android.c +++ b/boot/image-android.c @@ -185,7 +185,7 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr, return false; }

•   if (((struct andr_boot_img_hdr_v0 *)boot_hdr)->header_version > 2) {
  

•   if (android_image_get_version(boot_hdr) > 2) {
            if (!vendor_boot_hdr) {
                    printf("For boot header v3+ vendor boot image has to be provided\n");
                    return false;
  

@@ -203,6 +203,11 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr, return true; }

+u32 android_image_get_version(const void *hdr) +{

•   return ((struct andr_boot_img_hdr_v0 *)hdr)->header_version;
  

+}

static ulong android_image_get_kernel_addr(struct andr_image_data *img_data) { /* diff --git a/include/image.h b/include/image.h index acffd17e0dfd..18e5ced5ab42 100644 --- a/include/image.h +++ b/include/image.h @@ -1963,6 +1963,13 @@ bool is_android_boot_image_header(const void *hdr); */ bool is_android_vendor_boot_image_header(const void *vendor_boot_img);

+/**

• • android_image_get_version() - Retrieve the boot.img version
• • Return: Android boot image header version.
• */

+u32 android_image_get_version(const void *hdr);

/**

• get_abootimg_addr() - Get Android boot image address

-- 2.45.0

why introduce a helper function if there is only one user of it?

I added a second user of the helper function in patch 5/6.

android_image_get_data() expects andr_boot_img_hdr_v0 anyway, as it has an explicit check for it in the very beginning (is_android_boot_image_header()).

Right.

Have you considered adjusting android_image_get_data() declaration, and just use andr_boot_img_hdr_v0 *boot_hdr as first param instead (like it's done for example in android_boot_image_v0_v1_v2_parse_hdr()) and then rely on implicit cast when this function is used.

this is of course all a matter of preference, just thinking out loud

I've given this some more thought, and since I'm already using struct andr_boot_img_hdr_v0 in bootmeth_android/scan_boot_part(), I will drop this patch for v2.

The helper seems indeed a bit useless given that we can use the struct's member for this.

Thanks!

-- Best regards - Atentamente - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk

Hi Mattijs,

On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek mkorpershoek@baylibre.com wrote:

Some bootflows might be able to only boot from MMC devices.

Add a helper function these bootflows can use.

Signed-off-by: Mattijs Korpershoek mkorpershoek@baylibre.com

boot/bootflow.c | 12 ++++++++++++ include/bootflow.h | 9 +++++++++ 2 files changed, 21 insertions(+)

diff --git a/boot/bootflow.c b/boot/bootflow.c index 9aa3179c3881..59d77d2385f4 100644 --- a/boot/bootflow.c +++ b/boot/bootflow.c @@ -575,6 +575,18 @@ int bootflow_iter_check_blk(const struct bootflow_iter *iter) return -ENOTSUPP; }

+int bootflow_iter_check_mmc(const struct bootflow_iter *iter) +{

•   const struct udevice *media = dev_get_parent(iter->dev);
  

•   enum uclass_id id = device_get_uclass_id(media);
  

•   log_debug("uclass %d: %s\n", id, uclass_get_name(id));
  

•   if (id == UCLASS_MMC)
  

•           return 0;
  

•   return -ENOTSUPP;
  

+}

int bootflow_iter_check_sf(const struct bootflow_iter *iter) { const struct udevice *media = dev_get_parent(iter->dev); diff --git a/include/bootflow.h b/include/bootflow.h index 080ee8501225..6058ddd89b16 100644 --- a/include/bootflow.h +++ b/include/bootflow.h @@ -407,6 +407,15 @@ void bootflow_remove(struct bootflow *bflow); */ int bootflow_iter_check_blk(const struct bootflow_iter *iter);

+/**

• • bootflow_iter_check_mmc() - Check that a bootflow uses a MMC device
• • This checks the bootdev in the bootflow to make sure it uses a mmc device
• • Return: 0 if OK, -ENOTSUPP if some other device is used (e.g. ethernet)
• */

+int bootflow_iter_check_mmc(const struct bootflow_iter *iter);

/**

• bootflow_iter_check_sf() - Check that a bootflow uses SPI FLASH

-- 2.45.0

Reviewed-by: Igor Opaniuk igor.opaniuk@gmail.com

A bit offtopic (just an idea for future refactoring), but I think all these bootflow_iter_check_* helpers should be replaced by just one int bootflow_iter_check_id(const struct bootflow_iter *iter, enum uclass_id id) to avoid code duplication or at least keep all these bootmedia-specific functions as wrappers with one-line call to bootflow_iter_check_id(iter, UCLASS_SPI_*) inside.

Hi Igor,

Thank you for your quick review.

On lun., juin 10, 2024 at 17:15, Igor Opaniuk igor.opaniuk@gmail.com wrote:

Hi Mattijs,

On Thu, Jun 6, 2024 at 2:24 PM Mattijs Korpershoek mkorpershoek@baylibre.com wrote:

...

Android boot flow is a bit different than a regular Linux distro. Android relies on multiple partitions in order to boot.

A typical boot flow would be:

1. Parse the Bootloader Control Block (BCB, misc partition)
2. If BCB requested bootonce-bootloader, start fastboot and wait.
3. If BCB requested recovery or normal android, run the following:

3.a. Get slot (A/B) from BCB 3.b. Run AVB (Android Verified Boot) on boot partitions 3.c. Load boot and vendor_boot partitions 3.d. Load device-tree, ramdisk and boot

The AOSP documentation has more details at [1], [2], [3]

This has been implemented via complex boot scripts such as [4]. However, these boot script are neither very maintainable nor generic. Moreover, DISTRO_DEFAULTS is being deprecated [5].

Add a generic Android bootflow implementation for bootstd. For this initial version, only boot image v4 is supported.

[1] https://source.android.com/docs/core/architecture/bootloader [2] https://source.android.com/docs/core/architecture/partitions [3] https://source.android.com/docs/core/architecture/partitions/generic-boot [4] https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_a... [5] https://lore.kernel.org/r/all/20230914165615.1058529-17-sjg@chromium.org/

Signed-off-by: Mattijs Korpershoek mkorpershoek@baylibre.com

MAINTAINERS | 7 + boot/Kconfig | 14 ++ boot/Makefile | 2 + boot/bootmeth_android.c | 522 ++++++++++++++++++++++++++++++++++++++++++++++++ boot/bootmeth_android.h | 27 +++ doc/develop/bootstd.rst | 6 + 6 files changed, 578 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS index 66783d636e3d..6d2b87720565 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -939,6 +939,13 @@ F: include/bootstd.h F: net/eth_bootdevice.c F: test/boot/

+BOOTMETH_ANDROID +M: Mattijs Korpershoek mkorpershoek@baylibre.com +S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git +F: boot/bootmeth_android.c +F: boot/bootmeth_android.h

BTRFS M: Marek Behún kabel@kernel.org R: Qu Wenruo wqu@suse.com diff --git a/boot/Kconfig b/boot/Kconfig index 6f3096c15a6f..5fa6f3b8315d 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -494,6 +494,20 @@ config BOOTMETH_GLOBAL EFI bootmgr, since they take full control over which bootdevs are selected to boot.

+config BOOTMETH_ANDROID

•   bool "Bootdev support for Android"
  

•   depends on X86 || ARM || SANDBOX
  

•   select ANDROID_AB
  

•   select ANDROID_BOOT_IMAGE
  

•   select CMD_BCB
  

•   select PARTITION_TYPE_GUID
  

•   select PARTITION_UUIDS
  

•   help
  

•     Enables support for booting Android using bootdevs. Android requires
  

•     multiple partitions (misc, boot, vbmeta, ...) in storage for booting.
  

•     Note that only MMC bootdevs are supported at present.
  

config BOOTMETH_CROS bool "Bootdev support for Chromium OS" depends on X86 || ARM || SANDBOX diff --git a/boot/Makefile b/boot/Makefile index 84ccfeaecec4..75d1cd46fabf 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -66,3 +66,5 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += vbe_request.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o

+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_ANDROID) += bootmeth_android.o diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c new file mode 100644 index 000000000000..26d548d2fd6e --- /dev/null +++ b/boot/bootmeth_android.c @@ -0,0 +1,522 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Bootmethod for Android
• • Copyright (C) 2024 BayLibre, SAS
• • Written by Mattijs Korpershoek mkorpershoek@baylibre.com
• */

+#define LOG_CATEGORY UCLASS_BOOTSTD

+#include <android_ab.h> +#include <android_image.h> +#if CONFIG_IS_ENABLED(AVB_VERIFY) +#include <avb_verify.h> +#endif +#include <bcb.h> +#include <blk.h> +#include <bootflow.h> +#include <bootm.h> +#include <bootmeth.h> +#include <dm.h> +#include <image.h> +#include <malloc.h> +#include <mapmem.h> +#include <part.h> +#include "bootmeth_android.h"

+#define BCB_FIELD_COMMAND_SZ 32 +#define BCB_PART_NAME "misc" +#define BOOT_PART_NAME "boot" +#define VENDOR_BOOT_PART_NAME "vendor_boot"

+/**

• • struct android_priv - Private data
• • This is read from the disk and recorded for use when the full Android
• • kernel must be loaded and booted
• */

+struct android_priv {

•   int boot_mode;
  

•   char slot[2];
  

•   u32 header_version;
  

+};

+static int android_check(struct udevice *dev, struct bootflow_iter *iter) +{

•   /* This only works on mmc devices */
  

•   if (bootflow_iter_check_mmc(iter))
  

•           return log_msg_ret("mmc", -ENOTSUPP);
  

•   /* This only works on whole devices, as multiple
  

•    * partitions are needed to boot Android
  

•    */
  

Please use Linux kernel coding style for long comments [1]. Same in all occurrences below

[1] https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting

Sorry about that that. Will do. Odd that running checkpatch with --u-boot did not caught this.

$ ./scripts/checkpatch.pl --strict --u-boot --git HEAD^^..HEAD

...

•   if (iter->part != 0)
  

•           return log_msg_ret("mmc part", -ENOTSUPP);
  

•   return 0;
  

+}

+static int scan_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;
  

•   sprintf(partname, BOOT_PART_NAME "_%s", priv->slot);
  

•   ret = part_get_info_by_name(desc, partname, &partition);
  

•   if (ret < 0)
  

•           return log_msg_ret("part info", ret);
  

•   num_blks = DIV_ROUND_UP(sizeof(struct andr_boot_img_hdr_v0), desc->blksz);
  

•   bufsz = num_blks * desc->blksz;
  

•   buf = malloc(bufsz);
  

•   if (!buf)
  

•           return log_msg_ret("buf", -ENOMEM);
  

•   ret = blk_read(blk, partition.start, num_blks, buf);
  

•   if (ret != num_blks) {
  

•           free(buf);
  

•           return log_msg_ret("part read", -EIO);
  

•   }
  

•   if (!is_android_boot_image_header(buf)) {
  

•           free(buf);
  

•           return log_msg_ret("header", -ENOENT);
  

•   }
  

•   priv->header_version = android_image_get_version(buf);
  

•   return 0;
  

Shouldn't we free(buf) also here?

Yes we should. Will do for v2. Thanks for catching this!

...

+}

+static int scan_vendor_boot_part(struct udevice *blk, const char slot[2]) +{

•   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;
  

•   sprintf(partname, VENDOR_BOOT_PART_NAME "_%s", slot);
  

•   ret = part_get_info_by_name(desc, partname, &partition);
  

•   if (ret < 0)
  

•           return log_msg_ret("part info", ret);
  

•   num_blks = DIV_ROUND_UP(sizeof(struct andr_vnd_boot_img_hdr), desc->blksz);
  

•   bufsz = num_blks * desc->blksz;
  

•   buf = malloc(bufsz);
  

•   if (!buf)
  

•           return log_msg_ret("buf", -ENOMEM);
  

•   ret = blk_read(blk, partition.start, num_blks, buf);
  

•   if (ret != num_blks) {
  

•           free(buf);
  

•           return log_msg_ret("part read", -EIO);
  

•   }
  

•   if (!is_android_vendor_boot_image_header(buf)) {
  

•           free(buf);
  

•           return log_msg_ret("header", -ENOENT);
  

•   }
  

•   return 0;
  

free(buf)?

Yes we should. Will do for v2. Thanks for catching this!

...

+}

+static int android_read_slot_from_bcb(struct bootflow *bflow, bool decrement) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   struct disk_partition misc;
  

•   char slot_suffix[3];
  

•   int ret;
  

•   ret = part_get_info_by_name(desc, BCB_PART_NAME, &misc);
  

•   if (ret < 0)
  

•           return log_msg_ret("part", ret);
  

•   ret = ab_select_slot(desc, &misc, decrement);
  

•   if (ret < 0)
  

•           return log_msg_ret("slot", ret);
  

•   priv->slot[0] = BOOT_SLOT_NAME(ret);
  

•   priv->slot[1] = '\0';
  

•   sprintf(slot_suffix, "_%s", priv->slot);
  

•   ret = bootflow_cmdline_set_arg(bflow, "androidboot.slot_suffix",
  

•                                  slot_suffix, false);
  

•   if (ret < 0)
  

•           return log_msg_ret("slot", ret);
  

•   return 0;
  

+}

+static int configure_serialno(struct bootflow *bflow) +{

•   char *serialno = env_get("serial#");
  

•   if (!serialno)
  

•           return log_msg_ret("serial", -ENOENT);
  

•   return bootflow_cmdline_set_arg(bflow, "androidboot.serialno", serialno, false);
  

+}

+static int android_read_bootflow(struct udevice *dev, struct bootflow *bflow) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct disk_partition misc;
  

•   struct android_priv *priv;
  

•   char command[BCB_FIELD_COMMAND_SZ];
  

•   int ret;
  

•   bflow->state = BOOTFLOWST_MEDIA;
  

•   /* bcb_find_partition_and_load() will print errors to stdout
  

•    * if BCB_PART_NAME is not found. To avoid that, check if the
  

•    * partition exists first.
  

•    */
  

•   ret = part_get_info_by_name(desc, BCB_PART_NAME, &misc);
  

•   if (ret < 0)
  

•           return log_msg_ret("part", ret);
  

•   ret = bcb_find_partition_and_load("mmc", desc->devnum, BCB_PART_NAME);
  

•   if (ret < 0)
  

•           return log_msg_ret("bcb load", ret);
  

•   ret = bcb_get(BCB_FIELD_COMMAND, command, sizeof(command));
  

•   if (ret < 0)
  

•           return log_msg_ret("bcb read", ret);
  

•   priv = malloc(sizeof(struct android_priv));
  

•   if (!priv)
  

•           return log_msg_ret("buf", -ENOMEM);
  

•   bflow->bootmeth_priv = priv;
  

Probably we should do that just before successfully returning from the function, otherwise we will end up with a dangling pointer.

Right, thanks for the suggestion. Will do for v2.

...

•   if (!strcmp("bootonce-bootloader", command)) {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_BOOTLOADER;
  

•           bflow->os_name = strdup("Android (bootloader)");
  

•   } else if (!strcmp("boot-fastboot", command)) {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_RECOVERY;
  

•           bflow->os_name = strdup("Android (fastbootd)");
  

•   } else if (!strcmp("boot-recovery", command)) {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_RECOVERY;
  

•           bflow->os_name = strdup("Android (recovery)");
  

•   } else {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_NORMAL;
  

•           bflow->os_name = strdup("Android");
  

•   }
  

•   if (!bflow->os_name)
  

•           return log_msg_ret("os", -ENOMEM);
  

•   if (priv->boot_mode == ANDROID_BOOT_MODE_BOOTLOADER) {
  

•           /* Clear BCB */
  

•           memset(command, 0, sizeof(command));
  

•           ret = bcb_set(BCB_FIELD_COMMAND, command);
  

•           if (ret < 0) {
  

•                   free(priv);
  

•                   return log_msg_ret("bcb set", ret);
  

•           }
  

•           ret = bcb_store();
  

•           if (ret < 0) {
  

•                   free(priv);
  

...

•                   return log_msg_ret("bcb store", ret);
  

•           }
  

is free(bflow->bootmeth_priv) handled in functions that call android_read_bootflow() ?

It's not. Future functions (called later) like boot_android() need to access this structure.

I've modeled this after bootmeth_cros/cros_boot() but I don't see a reason for not freeing this just before calling bootm(). On the other hand, we will be booting an OS (and this quitting U-Boot) so it's not entirely needed.

I don't have a strong opinion on this so I can add the free() in boot_android_normal() and boot_android_bootloader().

...

•           bflow->state = BOOTFLOWST_READY;
  

•           return 0;
  

•   }
  

•   /* For recovery and normal boot, we need to scan the partitions */
  

•   ret = android_read_slot_from_bcb(bflow, false);
  

•   if (ret < 0) {
  

•           free(priv);
  

•           return log_msg_ret("read slot", ret);
  

•   }
  

•   ret = scan_boot_part(bflow->blk, priv);
  

•   if (ret < 0) {
  

•           printf("- scan boot failed: err=%d\n", ret);
  

•           free(priv);
  

•           return log_msg_ret("scan boot", ret);
  

•   }
  

•   if (priv->header_version != 4) {
  

•           printf("- Only boot.img v4 is supported\n");
  

•           free(priv);
  

•           return log_msg_ret("version", -EINVAL);
  

•   }
  

•   ret = scan_vendor_boot_part(bflow->blk, priv->slot);
  

•   if (ret < 0) {
  

•           printf("- scan vendor_boot failed: err=%d\n", ret);
  

•           free(priv);
  

•           return log_msg_ret("scan vendor_boot", ret);
  

•   }
  

•   /* Ignoring return code: setting serial number is not mandatory for booting */
  

•   configure_serialno(bflow);
  

•   if (priv->boot_mode == ANDROID_BOOT_MODE_NORMAL) {
  

•           ret = bootflow_cmdline_set_arg(bflow, "androidboot.force_normal_boot", "1", false);
  

•           if (ret < 0) {
  

•                   free(priv);
  

•                   return log_msg_ret("normal_boot", ret);
  

•           }
  

•   }
  

•   bflow->state = BOOTFLOWST_READY;
  

•   return 0;
  

+}

+static int android_read_file(struct udevice *dev, struct bootflow *bflow,

•                        const char *file_path, ulong addr, ulong *sizep)
  

+{

•   /* Reading individual files is not supported since we only
  

•    * operate on whole mmc devices (because we require multiple partitions)
  

•    */
  

•   return log_msg_ret("Unsupported", -ENOSYS);
  

+}

+static int read_slotted_partition(struct blk_desc *desc, const char *const name,

•                             const char slot[2], ulong addr)
  

+{

•   struct disk_partition partition;
  

•   char partname[PART_NAME_LEN];
  

•   int ret;
  

•   u32 n;
  

•   /* Ensure name fits in partname it should be: <name>_<slot>\0 */
  

•   if (strlen(name) > (PART_NAME_LEN - 2 - 1))
  

•           return log_msg_ret("name too long", -EINVAL);
  

•   sprintf(partname, "%s_%s", name, slot);
  

•   ret = part_get_info_by_name(desc, partname, &partition);
  

•   if (ret < 0)
  

•           return log_msg_ret("part", ret);
  

•   n = blk_dread(desc, partition.start, partition.size, map_sysmem(addr, 0));
  

•   if (n < partition.size)
  

•           return log_msg_ret("part read", -EIO);
  

•   return 0;
  

+}

+#if CONFIG_IS_ENABLED(AVB_VERIFY) +static int avb_append_commandline_arg(struct bootflow *bflow, char *arg) +{

•   char *key = strsep(&arg, "=");
  

•   char *value = arg;
  

•   int ret;
  

•   ret = bootflow_cmdline_set_arg(bflow, key, value, false);
  

•   if (ret < 0)
  

•           return log_msg_ret("avb cmdline", ret);
  

•   return 0;
  

+}

+static int avb_append_commandline(struct bootflow *bflow, char *cmdline) +{

•   char *arg = strsep(&cmdline, " ");
  

•   int ret;
  

•   while (arg) {
  

•           ret = avb_append_commandline_arg(bflow, arg);
  

•           if (ret < 0)
  

•                   return ret;
  

•           arg = strsep(&cmdline, " ");
  

•   }
  

•   return 0;
  

+}

+static int run_avb_verification(struct bootflow *bflow) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   const char * const requested_partitions[] = {"boot", "vendor_boot"};
  

•   struct AvbOps *avb_ops;
  

•   AvbSlotVerifyResult result;
  

•   AvbSlotVerifyData *out_data;
  

•   enum avb_boot_state boot_state;
  

•   char *extra_args;
  

•   char slot_suffix[3];
  

•   bool unlocked = false;
  

•   int ret;
  

•   avb_ops = avb_ops_alloc(desc->devnum);
  

•   if (!avb_ops)
  

•           return log_msg_ret("avb ops", -ENOMEM);.
  

•   sprintf(slot_suffix, "_%s", priv->slot);
  

•   ret = avb_ops->read_is_device_unlocked(avb_ops, &unlocked);
  

•   if (ret != AVB_IO_RESULT_OK)
  

•           return log_msg_ret("avb lock", -EIO);
  

•   result = avb_slot_verify(avb_ops,
  

•                            requested_partitions,
  

•                            slot_suffix,
  

•                            unlocked,
  

•                            AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
  

•                            &out_data);
  

•   if (result != AVB_SLOT_VERIFY_RESULT_OK) {
  

•           printf("Verification failed, reason: %s\n",
  

•                  str_avb_slot_error(result));
  

avb_ops_free(avb_ops) ?

Yes we should. Will do for v2. Thanks for catching this!

...

•           return log_msg_ret("avb verify", -EIO);
  

•   }
  

•   if (unlocked)
  

•           boot_state = AVB_ORANGE;
  

•   else
  

•           boot_state = AVB_GREEN;
  

•   extra_args = avb_set_state(avb_ops, boot_state);
  

•   if (extra_args) {
  

•           ret = avb_append_commandline_arg(bflow, extra_args);
  

•           if (ret < 0)
  

•                   goto free_out_data;
  

•   }
  

•   ret = avb_append_commandline(bflow, out_data->cmdline);
  

•   if (ret < 0)
  

•           goto free_out_data;
  

•   return 0;
  

• free_out_data:

•   if (out_data)
  

•           avb_slot_verify_data_free(out_data);
  

•   return log_msg_ret("avb cmdline", ret);
  

+} +#else +static int run_avb_verification(struct bootflow *bflow) +{

•   int ret;
  

•   /* When AVB is unsupported, pass ORANGE state  */
  

•   ret = bootflow_cmdline_set_arg(bflow,
  

•                                  "androidboot.verifiedbootstate",
  

•                                  "orange", false);
  

•   if (ret < 0)
  

•           return log_msg_ret("avb cmdline", ret);
  

•   return 0;
  

+} +#endif /* AVB_VERIFY */

+static int boot_android_normal(struct bootflow *bflow) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   int ret;
  

•   ulong loadaddr = env_get_hex("loadaddr", 0);
  

•   ulong vloadaddr = env_get_hex("vendor_boot_comp_addr_r", 0);
  

•   ret = run_avb_verification(bflow);
  

•   if (ret < 0)
  

•           return log_msg_ret("avb", ret);
  

•   /* Read slot once more to decrement counter from BCB */
  

•   ret = android_read_slot_from_bcb(bflow, true);
  

•   if (ret < 0)
  

•           return log_msg_ret("read slot", ret);
  

•   ret = read_slotted_partition(desc, "boot", priv->slot, loadaddr);
  

•   if (ret < 0)
  

•           return log_msg_ret("read boot", ret);
  

•   ret = read_slotted_partition(desc, "vendor_boot", priv->slot, vloadaddr);
  

•   if (ret < 0)
  

•           return log_msg_ret("read vendor_boot", ret);
  

•   set_abootimg_addr(loadaddr);
  

•   set_avendor_bootimg_addr(vloadaddr);
  

•   ret = bootm_boot_start(loadaddr, bflow->cmdline);
  

•   return log_msg_ret("boot", ret);
  

+}

+static int boot_android_recovery(struct bootflow *bflow) +{

•   int ret;
  

•   ret = boot_android_normal(bflow);
  

•   return log_msg_ret("boot", ret);
  

+}

+static int boot_android_bootloader(struct bootflow *bflow) +{

•   int ret;
  

•   ret = run_command("fastboot usb 0", 0);
  

select CMD_FASTBOOT in Kconfig (config BOOTMETH_ANDROID)?

Yes, I will add this for v2. Thank you for the suggestion.

...

•   do_reset(NULL, 0, 0, NULL);
  

•   return log_msg_ret("boot", ret);
  

+}

+static int android_boot(struct udevice *dev, struct bootflow *bflow) +{

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   int ret;
  

•   switch (priv->boot_mode) {
  

•   case ANDROID_BOOT_MODE_NORMAL:
  

•           ret = boot_android_normal(bflow);
  

•           break;
  

•   case ANDROID_BOOT_MODE_RECOVERY:
  

•           ret = boot_android_recovery(bflow);
  

•           break;
  

•   case ANDROID_BOOT_MODE_BOOTLOADER:
  

•           ret = boot_android_bootloader(bflow);
  

•           break;
  

•   default:
  

•           printf("ANDROID: Unknown boot mode %d. Running fastboot...\n",
  

•                  priv->boot_mode);
  

•           boot_android_bootloader(bflow);
  

•           /* Tell we failed to boot since boot mode is unknown */
  

•           ret = -EFAULT;
  

•   }
  

•   return ret;
  

+}

+static int android_bootmeth_bind(struct udevice *dev) +{

•   struct bootmeth_uc_plat *plat = dev_get_uclass_plat(dev);
  

•   plat->desc = "Android boot";
  

•   plat->flags = BOOTMETHF_ANY_PART;
  

•   return 0;
  

+}

+static struct bootmeth_ops android_bootmeth_ops = {

•   .check          = android_check,
  

•   .read_bootflow  = android_read_bootflow,
  

•   .read_file      = android_read_file,
  

•   .boot           = android_boot,
  

+};

+static const struct udevice_id android_bootmeth_ids[] = {

•   { .compatible = "u-boot,android" },
  

•   { }
  

+};

+U_BOOT_DRIVER(bootmeth_android) = {

•   .name           = "bootmeth_android",
  

•   .id             = UCLASS_BOOTMETH,
  

•   .of_match       = android_bootmeth_ids,
  

•   .ops            = &android_bootmeth_ops,
  

•   .bind           = android_bootmeth_bind,
  

+}; diff --git a/boot/bootmeth_android.h b/boot/bootmeth_android.h new file mode 100644 index 000000000000..411c2f2d15e7 --- /dev/null +++ b/boot/bootmeth_android.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Bootmethod for Android
• • Copyright (C) 2024 BayLibre, SAS
• • Written by Mattijs Korpershoek mkorpershoek@baylibre.com
• */

+enum android_boot_mode {

•   ANDROID_BOOT_MODE_NORMAL = 0,
  

•   /* Android "recovery" is a special boot mode that uses another ramdisk.
  

•    * It can be used to "factory reset" a board or to flash logical partitions
  

•    * It operates in 2 modes: adb or fastbootd
  

•    * To enter recovery from Android, we can do:
  

•    * $ adb reboot recovery
  

•    * $ adb reboot fastboot
  

•    */
  

•   ANDROID_BOOT_MODE_RECOVERY,
  

•   /* Android "bootloader" is for accessing/reflashing physical partitions
  

•    * Typically, this will launch a fastboot process in U-Boot.
  

•    * To enter "bootloader" from Android, we can do:
  

•    * $ adb reboot bootloader
  

•    */
  

•   ANDROID_BOOT_MODE_BOOTLOADER,
  

+}; diff --git a/doc/develop/bootstd.rst b/doc/develop/bootstd.rst index a07a72581e7a..709fa9e64ff3 100644 --- a/doc/develop/bootstd.rst +++ b/doc/develop/bootstd.rst @@ -95,6 +95,7 @@ bootflows.

Note: it is possible to have a bootmeth that uses a partition or a whole device directly, but it is more common to use a filesystem. +For example, the Android bootmeth uses a whole device.

Note that some bootmeths are 'global', meaning that they select the bootdev themselves. Examples include VBE and EFI boot manager. In this case, they @@ -277,6 +278,9 @@ script_offset_f script_size_f Size of the script to load, e.g. 0x2000

+vendor_boot_comp_addr_r

• Address to which to load the vendor_boot Android image, e.g. 0xe0000000

Some variables are set by script bootmeth:

devtype @@ -418,6 +422,7 @@ Bootmeth drivers are provided for: - EFI boot using bootefi from disk - VBE - EFI boot using boot manager

• • Android bootflow (boot image v4)

Command interface @@ -786,6 +791,7 @@ To do Some things that need to be done to completely replace the distro-boot scripts:

• implement extensions (devicetree overlays with add-on boards)

+- implement legacy (boot image v2) android boot flow

Other ideas:

-- 2.45.0

Some comments after a quick "scan". Will take a more detailed look a bit later today/tomorrow.

Thanks a bunch for your quick scan. You caught quite some issues already. I will implement these suggestions and will be awaiting the detailed look before sending a v2.

-- Best regards - Atentamente - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com skype: igor.opanyuk https://www.linkedin.com/in/iopaniuk

Hi Simon,

Thank you for your review.

On mar., juin 11, 2024 at 12:52, Simon Glass sjg@chromium.org wrote:

Hi Mattijs,

On Thu, 6 Jun 2024 at 06:24, Mattijs Korpershoek mkorpershoek@baylibre.com wrote:

...

Android boot flow is a bit different than a regular Linux distro. Android relies on multiple partitions in order to boot.

A typical boot flow would be:

1. Parse the Bootloader Control Block (BCB, misc partition)
2. If BCB requested bootonce-bootloader, start fastboot and wait.
3. If BCB requested recovery or normal android, run the following:

3.a. Get slot (A/B) from BCB 3.b. Run AVB (Android Verified Boot) on boot partitions 3.c. Load boot and vendor_boot partitions 3.d. Load device-tree, ramdisk and boot

The AOSP documentation has more details at [1], [2], [3]

This has been implemented via complex boot scripts such as [4]. However, these boot script are neither very maintainable nor generic. Moreover, DISTRO_DEFAULTS is being deprecated [5].

Add a generic Android bootflow implementation for bootstd. For this initial version, only boot image v4 is supported.

[1] https://source.android.com/docs/core/architecture/bootloader [2] https://source.android.com/docs/core/architecture/partitions [3] https://source.android.com/docs/core/architecture/partitions/generic-boot [4] https://source.denx.de/u-boot/u-boot/-/blob/master/include/configs/meson64_a... [5] https://lore.kernel.org/r/all/20230914165615.1058529-17-sjg@chromium.org/

Signed-off-by: Mattijs Korpershoek mkorpershoek@baylibre.com

MAINTAINERS | 7 + boot/Kconfig | 14 ++ boot/Makefile | 2 + boot/bootmeth_android.c | 522 ++++++++++++++++++++++++++++++++++++++++++++++++ boot/bootmeth_android.h | 27 +++ doc/develop/bootstd.rst | 6 + 6 files changed, 578 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

nits below

...

diff --git a/MAINTAINERS b/MAINTAINERS index 66783d636e3d..6d2b87720565 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -939,6 +939,13 @@ F: include/bootstd.h F: net/eth_bootdevice.c F: test/boot/

+BOOTMETH_ANDROID +M: Mattijs Korpershoek mkorpershoek@baylibre.com +S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-dfu.git +F: boot/bootmeth_android.c +F: boot/bootmeth_android.h

BTRFS M: Marek Behún kabel@kernel.org R: Qu Wenruo wqu@suse.com diff --git a/boot/Kconfig b/boot/Kconfig index 6f3096c15a6f..5fa6f3b8315d 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -494,6 +494,20 @@ config BOOTMETH_GLOBAL EFI bootmgr, since they take full control over which bootdevs are selected to boot.

+config BOOTMETH_ANDROID

•   bool "Bootdev support for Android"
  

•   depends on X86 || ARM || SANDBOX
  

•   select ANDROID_AB
  

•   select ANDROID_BOOT_IMAGE
  

•   select CMD_BCB
  

•   select PARTITION_TYPE_GUID
  

•   select PARTITION_UUIDS
  

•   help
  

•     Enables support for booting Android using bootdevs. Android requires
  

using standard boot (or using bootstd).

Will do for v2.

...

•     multiple partitions (misc, boot, vbmeta, ...) in storage for booting.
  

•     Note that only MMC bootdevs are supported at present.
  

Why is that limitation present? Can you please mention what is needed to remove it?

Mainly because we use AVB and AVB is hard-coded for MMC. Alistair submitted changes to convert to generic block devices here:

https://lore.kernel.org/all/20220926220211.868968-1-adelva@google.com/

There were some review comments but I did not see any v2 on the list.

I will add a comment in the KConfig description.

...

config BOOTMETH_CROS bool "Bootdev support for Chromium OS" depends on X86 || ARM || SANDBOX diff --git a/boot/Makefile b/boot/Makefile index 84ccfeaecec4..75d1cd46fabf 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -66,3 +66,5 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += vbe_request.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o

+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_ANDROID) += bootmeth_android.o diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c new file mode 100644 index 000000000000..26d548d2fd6e --- /dev/null +++ b/boot/bootmeth_android.c @@ -0,0 +1,522 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Bootmethod for Android

This is my fault, but I think we should settle on Bootmeth throughout.

Yes, thank you for noticing. Will do for v2.

...

• • Copyright (C) 2024 BayLibre, SAS
• • Written by Mattijs Korpershoek mkorpershoek@baylibre.com
• */

+#define LOG_CATEGORY UCLASS_BOOTSTD

+#include <android_ab.h> +#include <android_image.h> +#if CONFIG_IS_ENABLED(AVB_VERIFY) +#include <avb_verify.h>

Can you include that header always?

I can't, it does not link if I do.

It would be possible if we implemented stub functions but that's not the case today.

Since this is a nitpick, I will consider this for a future improvement.

...

+#endif +#include <bcb.h> +#include <blk.h> +#include <bootflow.h> +#include <bootm.h> +#include <bootmeth.h> +#include <dm.h> +#include <image.h> +#include <malloc.h> +#include <mapmem.h> +#include <part.h> +#include "bootmeth_android.h"

+#define BCB_FIELD_COMMAND_SZ 32 +#define BCB_PART_NAME "misc" +#define BOOT_PART_NAME "boot" +#define VENDOR_BOOT_PART_NAME "vendor_boot"

+/**

• • struct android_priv - Private data
• • This is read from the disk and recorded for use when the full Android
• • kernel must be loaded and booted

please add member comments as I don't know what these fields are

Will do for v2.

...

• */

+struct android_priv {

•   int boot_mode;
  

•   char slot[2];
  

•   u32 header_version;
  

+};

+static int android_check(struct udevice *dev, struct bootflow_iter *iter) +{

•   /* This only works on mmc devices */
  

•   if (bootflow_iter_check_mmc(iter))
  

•           return log_msg_ret("mmc", -ENOTSUPP);
  

•   /* This only works on whole devices, as multiple
  

•    * partitions are needed to boot Android
  

•    */
  

•   if (iter->part != 0)
  

•           return log_msg_ret("mmc part", -ENOTSUPP);
  

•   return 0;
  

+}

+static int scan_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;
  

•   sprintf(partname, BOOT_PART_NAME "_%s", priv->slot);
  

•   ret = part_get_info_by_name(desc, partname, &partition);
  

•   if (ret < 0)
  

•           return log_msg_ret("part info", ret);
  

•   num_blks = DIV_ROUND_UP(sizeof(struct andr_boot_img_hdr_v0), desc->blksz);
  

•   bufsz = num_blks * desc->blksz;
  

•   buf = malloc(bufsz);
  

•   if (!buf)
  

•           return log_msg_ret("buf", -ENOMEM);
  

•   ret = blk_read(blk, partition.start, num_blks, buf);
  

•   if (ret != num_blks) {
  

•           free(buf);
  

•           return log_msg_ret("part read", -EIO);
  

•   }
  

•   if (!is_android_boot_image_header(buf)) {
  

•           free(buf);
  

•           return log_msg_ret("header", -ENOENT);
  

•   }
  

•   priv->header_version = android_image_get_version(buf);
  

•   return 0;
  

+}

+static int scan_vendor_boot_part(struct udevice *blk, const char slot[2]) +{

•   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;
  

•   sprintf(partname, VENDOR_BOOT_PART_NAME "_%s", slot);
  

•   ret = part_get_info_by_name(desc, partname, &partition);
  

•   if (ret < 0)
  

•           return log_msg_ret("part info", ret);
  

•   num_blks = DIV_ROUND_UP(sizeof(struct andr_vnd_boot_img_hdr), desc->blksz);
  

•   bufsz = num_blks * desc->blksz;
  

•   buf = malloc(bufsz);
  

•   if (!buf)
  

•           return log_msg_ret("buf", -ENOMEM);
  

•   ret = blk_read(blk, partition.start, num_blks, buf);
  

•   if (ret != num_blks) {
  

•           free(buf);
  

•           return log_msg_ret("part read", -EIO);
  

•   }
  

•   if (!is_android_vendor_boot_image_header(buf)) {
  

•           free(buf);
  

•           return log_msg_ret("header", -ENOENT);
  

•   }
  

•   return 0;
  

+}

+static int android_read_slot_from_bcb(struct bootflow *bflow, bool decrement) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   struct disk_partition misc;
  

•   char slot_suffix[3];
  

•   int ret;
  

•   ret = part_get_info_by_name(desc, BCB_PART_NAME, &misc);
  

•   if (ret < 0)
  

•           return log_msg_ret("part", ret);
  

•   ret = ab_select_slot(desc, &misc, decrement);
  

•   if (ret < 0)
  

•           return log_msg_ret("slot", ret);
  

•   priv->slot[0] = BOOT_SLOT_NAME(ret);
  

•   priv->slot[1] = '\0';
  

•   sprintf(slot_suffix, "_%s", priv->slot);
  

•   ret = bootflow_cmdline_set_arg(bflow, "androidboot.slot_suffix",
  

•                                  slot_suffix, false);
  

•   if (ret < 0)
  

•           return log_msg_ret("slot", ret);
  

perhaps "cmdl" ?

Yes, will do for v2.

...

•   return 0;
  

+}

+static int configure_serialno(struct bootflow *bflow) +{

•   char *serialno = env_get("serial#");
  

•   if (!serialno)
  

•           return log_msg_ret("serial", -ENOENT);
  

•   return bootflow_cmdline_set_arg(bflow, "androidboot.serialno", serialno, false);
  

+}

+static int android_read_bootflow(struct udevice *dev, struct bootflow *bflow) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct disk_partition misc;
  

•   struct android_priv *priv;
  

•   char command[BCB_FIELD_COMMAND_SZ];
  

•   int ret;
  

•   bflow->state = BOOTFLOWST_MEDIA;
  

•   /* bcb_find_partition_and_load() will print errors to stdout
  

I believe this first line should just be /*

You are right. I've made this mistake in other parts of the file. Will fix all occurences for v2.

...

•    * if BCB_PART_NAME is not found. To avoid that, check if the
  

•    * partition exists first.
  

•    */
  

•   ret = part_get_info_by_name(desc, BCB_PART_NAME, &misc);
  

•   if (ret < 0)
  

•           return log_msg_ret("part", ret);
  

•   ret = bcb_find_partition_and_load("mmc", desc->devnum, BCB_PART_NAME);
  

•   if (ret < 0)
  

•           return log_msg_ret("bcb load", ret);
  

•   ret = bcb_get(BCB_FIELD_COMMAND, command, sizeof(command));
  

•   if (ret < 0)
  

•           return log_msg_ret("bcb read", ret);
  

•   priv = malloc(sizeof(struct android_priv));
  

•   if (!priv)
  

•           return log_msg_ret("buf", -ENOMEM);
  

•   bflow->bootmeth_priv = priv;
  

ideally this would happen at the end of the function

Yes, Igor pointed out as well. It's dangerous to have a dangling pointer. Will do for v2.

...

•   if (!strcmp("bootonce-bootloader", command)) {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_BOOTLOADER;
  

•           bflow->os_name = strdup("Android (bootloader)");
  

•   } else if (!strcmp("boot-fastboot", command)) {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_RECOVERY;
  

•           bflow->os_name = strdup("Android (fastbootd)");
  

•   } else if (!strcmp("boot-recovery", command)) {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_RECOVERY;
  

•           bflow->os_name = strdup("Android (recovery)");
  

•   } else {
  

•           priv->boot_mode = ANDROID_BOOT_MODE_NORMAL;
  

•           bflow->os_name = strdup("Android");
  

•   }
  

•   if (!bflow->os_name)
  

•           return log_msg_ret("os", -ENOMEM);
  

•   if (priv->boot_mode == ANDROID_BOOT_MODE_BOOTLOADER) {
  

•           /* Clear BCB */
  

•           memset(command, 0, sizeof(command));
  

•           ret = bcb_set(BCB_FIELD_COMMAND, command);
  

•           if (ret < 0) {
  

•                   free(priv);
  

•                   return log_msg_ret("bcb set", ret);
  

•           }
  

•           ret = bcb_store();
  

•           if (ret < 0) {
  

•                   free(priv);
  

•                   return log_msg_ret("bcb store", ret);
  

•           }
  

•           bflow->state = BOOTFLOWST_READY;
  

•           return 0;
  

•   }
  

•   /* For recovery and normal boot, we need to scan the partitions */
  

•   ret = android_read_slot_from_bcb(bflow, false);
  

•   if (ret < 0) {
  

•           free(priv);
  

•           return log_msg_ret("read slot", ret);
  

•   }
  

•   ret = scan_boot_part(bflow->blk, priv);
  

•   if (ret < 0) {
  

•           printf("- scan boot failed: err=%d\n", ret);
  

•           free(priv);
  

•           return log_msg_ret("scan boot", ret);
  

•   }
  

•   if (priv->header_version != 4) {
  

•           printf("- Only boot.img v4 is supported\n");
  

shouldn't this be log_debug() ?

Hmm, I've modeled this to be similar to bootmeth_cros, but I can convert to log_debug(). I don't have a strong opinion on this.

...

•           free(priv);
  

•           return log_msg_ret("version", -EINVAL);
  

•   }
  

•   ret = scan_vendor_boot_part(bflow->blk, priv->slot);
  

•   if (ret < 0) {
  

•           printf("- scan vendor_boot failed: err=%d\n", ret);
  

•           free(priv);
  

•           return log_msg_ret("scan vendor_boot", ret);
  

•   }
  

•   /* Ignoring return code: setting serial number is not mandatory for booting */
  

•   configure_serialno(bflow);
  

•   if (priv->boot_mode == ANDROID_BOOT_MODE_NORMAL) {
  

•           ret = bootflow_cmdline_set_arg(bflow, "androidboot.force_normal_boot", "1", false);
  

check line length

Yes, somehow I missed this with checkpatch. Will fix for v2.

...

•           if (ret < 0) {
  

•                   free(priv);
  

•                   return log_msg_ret("normal_boot", ret);
  

•           }
  

•   }
  

•   bflow->state = BOOTFLOWST_READY;
  

•   return 0;
  

+}

+static int android_read_file(struct udevice *dev, struct bootflow *bflow,

•                        const char *file_path, ulong addr, ulong *sizep)
  

+{

•   /* Reading individual files is not supported since we only
  

•    * operate on whole mmc devices (because we require multiple partitions)
  

•    */
  

•   return log_msg_ret("Unsupported", -ENOSYS);
  

+}

+static int read_slotted_partition(struct blk_desc *desc, const char *const name,

•                             const char slot[2], ulong addr)
  

What is a slotted partition? This function could use a comment

Most modern Android devices use "Seamless updates", where each partition is duplicated. For example, the boot partition has boot_a and boot_b. This is what I called "slotted partition".

See: https://source.android.com/docs/core/ota/ab https://source.android.com/docs/core/ota/ab/ab_implement

I will add a comment pointing to the above documentation and some additional explanation in v2.

...

+{

•   struct disk_partition partition;
  

•   char partname[PART_NAME_LEN];
  

•   int ret;
  

•   u32 n;
  

•   /* Ensure name fits in partname it should be: <name>_<slot>\0 */
  

•   if (strlen(name) > (PART_NAME_LEN - 2 - 1))
  

•           return log_msg_ret("name too long", -EINVAL);
  

•   sprintf(partname, "%s_%s", name, slot);
  

•   ret = part_get_info_by_name(desc, partname, &partition);
  

•   if (ret < 0)
  

•           return log_msg_ret("part", ret);
  

•   n = blk_dread(desc, partition.start, partition.size, map_sysmem(addr, 0));
  

•   if (n < partition.size)
  

•           return log_msg_ret("part read", -EIO);
  

•   return 0;
  

+}

+#if CONFIG_IS_ENABLED(AVB_VERIFY)

Can you use if() instead, and either rename the functions below or add this if() to each function?

I will try, but most avb_ops related functions are not stubbed so I'm not sure we can do this easily.

Since this is a nitpick, I might consider this a future improvement (If stubs are needed, for example).

...

+static int avb_append_commandline_arg(struct bootflow *bflow, char *arg) +{

•   char *key = strsep(&arg, "=");
  

•   char *value = arg;
  

•   int ret;
  

•   ret = bootflow_cmdline_set_arg(bflow, key, value, false);
  

•   if (ret < 0)
  

•           return log_msg_ret("avb cmdline", ret);
  

•   return 0;
  

+}

+static int avb_append_commandline(struct bootflow *bflow, char *cmdline) +{

•   char *arg = strsep(&cmdline, " ");
  

•   int ret;
  

•   while (arg) {
  

•           ret = avb_append_commandline_arg(bflow, arg);
  

•           if (ret < 0)
  

•                   return ret;
  

•           arg = strsep(&cmdline, " ");
  

•   }
  

•   return 0;
  

+}

+static int run_avb_verification(struct bootflow *bflow) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   const char * const requested_partitions[] = {"boot", "vendor_boot"};
  

•   struct AvbOps *avb_ops;
  

•   AvbSlotVerifyResult result;
  

•   AvbSlotVerifyData *out_data;
  

•   enum avb_boot_state boot_state;
  

•   char *extra_args;
  

•   char slot_suffix[3];
  

•   bool unlocked = false;
  

•   int ret;
  

•   avb_ops = avb_ops_alloc(desc->devnum);
  

•   if (!avb_ops)
  

•           return log_msg_ret("avb ops", -ENOMEM);
  

•   sprintf(slot_suffix, "_%s", priv->slot);
  

•   ret = avb_ops->read_is_device_unlocked(avb_ops, &unlocked);
  

•   if (ret != AVB_IO_RESULT_OK)
  

•           return log_msg_ret("avb lock", -EIO);
  

•   result = avb_slot_verify(avb_ops,
  

•                            requested_partitions,
  

•                            slot_suffix,
  

•                            unlocked,
  

•                            AVB_HASHTREE_ERROR_MODE_RESTART_AND_INVALIDATE,
  

•                            &out_data);
  

•   if (result != AVB_SLOT_VERIFY_RESULT_OK) {
  

•           printf("Verification failed, reason: %s\n",
  

•                  str_avb_slot_error(result));
  

•           return log_msg_ret("avb verify", -EIO);
  

•   }
  

•   if (unlocked)
  

•           boot_state = AVB_ORANGE;
  

•   else
  

•           boot_state = AVB_GREEN;
  

•   extra_args = avb_set_state(avb_ops, boot_state);
  

•   if (extra_args) {
  

•           ret = avb_append_commandline_arg(bflow, extra_args);
  

This function will write to extra_args...is that OK?

Yes, extra_args is no longer used after this. I will add a comment to state that this is safe in v2.

...

•           if (ret < 0)
  

•                   goto free_out_data;
  

•   }
  

•   ret = avb_append_commandline(bflow, out_data->cmdline);
  

•   if (ret < 0)
  

•           goto free_out_data;
  

•   return 0;
  

• free_out_data:

•   if (out_data)
  

•           avb_slot_verify_data_free(out_data);
  

•   return log_msg_ret("avb cmdline", ret);
  

+} +#else +static int run_avb_verification(struct bootflow *bflow) +{

•   int ret;
  

•   /* When AVB is unsupported, pass ORANGE state  */
  

•   ret = bootflow_cmdline_set_arg(bflow,
  

•                                  "androidboot.verifiedbootstate",
  

•                                  "orange", false);
  

•   if (ret < 0)
  

•           return log_msg_ret("avb cmdline", ret);
  

•   return 0;
  

+} +#endif /* AVB_VERIFY */

+static int boot_android_normal(struct bootflow *bflow) +{

•   struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
  

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   int ret;
  

drop blank line

Will do for v2.

...

•   ulong loadaddr = env_get_hex("loadaddr", 0);
  

•   ulong vloadaddr = env_get_hex("vendor_boot_comp_addr_r", 0);
  

•   ret = run_avb_verification(bflow);
  

•   if (ret < 0)
  

•           return log_msg_ret("avb", ret);
  

•   /* Read slot once more to decrement counter from BCB */
  

•   ret = android_read_slot_from_bcb(bflow, true);
  

•   if (ret < 0)
  

•           return log_msg_ret("read slot", ret);
  

•   ret = read_slotted_partition(desc, "boot", priv->slot, loadaddr);
  

•   if (ret < 0)
  

•           return log_msg_ret("read boot", ret);
  

•   ret = read_slotted_partition(desc, "vendor_boot", priv->slot, vloadaddr);
  

•   if (ret < 0)
  

•           return log_msg_ret("read vendor_boot", ret);
  

•   set_abootimg_addr(loadaddr);
  

•   set_avendor_bootimg_addr(vloadaddr);
  

•   ret = bootm_boot_start(loadaddr, bflow->cmdline);
  

•   return log_msg_ret("boot", ret);
  

+}

+static int boot_android_recovery(struct bootflow *bflow) +{

•   int ret;
  

•   ret = boot_android_normal(bflow);
  

•   return log_msg_ret("boot", ret);
  

+}

+static int boot_android_bootloader(struct bootflow *bflow) +{

•   int ret;
  

•   ret = run_command("fastboot usb 0", 0);
  

•   do_reset(NULL, 0, 0, NULL);
  

•   return log_msg_ret("boot", ret);
  

+}

+static int android_boot(struct udevice *dev, struct bootflow *bflow) +{

•   struct android_priv *priv = bflow->bootmeth_priv;
  

•   int ret;
  

•   switch (priv->boot_mode) {
  

•   case ANDROID_BOOT_MODE_NORMAL:
  

•           ret = boot_android_normal(bflow);
  

•           break;
  

•   case ANDROID_BOOT_MODE_RECOVERY:
  

•           ret = boot_android_recovery(bflow);
  

•           break;
  

•   case ANDROID_BOOT_MODE_BOOTLOADER:
  

•           ret = boot_android_bootloader(bflow);
  

•           break;
  

•   default:
  

•           printf("ANDROID: Unknown boot mode %d. Running fastboot...\n",
  

•                  priv->boot_mode);
  

•           boot_android_bootloader(bflow);
  

•           /* Tell we failed to boot since boot mode is unknown */
  

•           ret = -EFAULT;
  

•   }
  

•   return ret;
  

+}

+static int android_bootmeth_bind(struct udevice *dev) +{

•   struct bootmeth_uc_plat *plat = dev_get_uclass_plat(dev);
  

•   plat->desc = "Android boot";
  

•   plat->flags = BOOTMETHF_ANY_PART;
  

•   return 0;
  

+}

+static struct bootmeth_ops android_bootmeth_ops = {

•   .check          = android_check,
  

•   .read_bootflow  = android_read_bootflow,
  

•   .read_file      = android_read_file,
  

•   .boot           = android_boot,
  

+};

+static const struct udevice_id android_bootmeth_ids[] = {

•   { .compatible = "u-boot,android" },
  

•   { }
  

+};

+U_BOOT_DRIVER(bootmeth_android) = {

•   .name           = "bootmeth_android",
  

•   .id             = UCLASS_BOOTMETH,
  

•   .of_match       = android_bootmeth_ids,
  

•   .ops            = &android_bootmeth_ops,
  

•   .bind           = android_bootmeth_bind,
  

+}; diff --git a/boot/bootmeth_android.h b/boot/bootmeth_android.h new file mode 100644 index 000000000000..411c2f2d15e7 --- /dev/null +++ b/boot/bootmeth_android.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Bootmethod for Android
• • Copyright (C) 2024 BayLibre, SAS
• • Written by Mattijs Korpershoek mkorpershoek@baylibre.com
• */

+enum android_boot_mode {

•   ANDROID_BOOT_MODE_NORMAL = 0,
  

•   /* Android "recovery" is a special boot mode that uses another ramdisk.
  

•    * It can be used to "factory reset" a board or to flash logical partitions
  

•    * It operates in 2 modes: adb or fastbootd
  

•    * To enter recovery from Android, we can do:
  

•    * $ adb reboot recovery
  

•    * $ adb reboot fastboot
  

•    */
  

•   ANDROID_BOOT_MODE_RECOVERY,
  

•   /* Android "bootloader" is for accessing/reflashing physical partitions
  

•    * Typically, this will launch a fastboot process in U-Boot.
  

•    * To enter "bootloader" from Android, we can do:
  

•    * $ adb reboot bootloader
  

•    */
  

•   ANDROID_BOOT_MODE_BOOTLOADER,
  

+}; diff --git a/doc/develop/bootstd.rst b/doc/develop/bootstd.rst index a07a72581e7a..709fa9e64ff3 100644 --- a/doc/develop/bootstd.rst +++ b/doc/develop/bootstd.rst @@ -95,6 +95,7 @@ bootflows.

Note: it is possible to have a bootmeth that uses a partition or a whole device directly, but it is more common to use a filesystem. +For example, the Android bootmeth uses a whole device.

Note that some bootmeths are 'global', meaning that they select the bootdev themselves. Examples include VBE and EFI boot manager. In this case, they @@ -277,6 +278,9 @@ script_offset_f script_size_f Size of the script to load, e.g. 0x2000

+vendor_boot_comp_addr_r

• Address to which to load the vendor_boot Android image, e.g. 0xe0000000

Some variables are set by script bootmeth:

devtype @@ -418,6 +422,7 @@ Bootmeth drivers are provided for: - EFI boot using bootefi from disk - VBE - EFI boot using boot manager

• • Android bootflow (boot image v4)

Command interface @@ -786,6 +791,7 @@ To do Some things that need to be done to completely replace the distro-boot scripts:

• implement extensions (devicetree overlays with add-on boards)

+- implement legacy (boot image v2) android boot flow

Nice!

Thank you! It was easier to implement that I imagined, mainly because the bootflow framework is well written (imo) and the bootmeth_cros example was very helpful as well.

...

Other ideas:

-- 2.45.0

Regards, Simon