[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata
Simon Glass
sjg at chromium.org
Sun Mar 10 21:50:47 UTC 2019
Hi Igor,
On Mon, 18 Feb 2019 at 10:22, Igor Opaniuk <igor.opaniuk at linaro.org> wrote:
>
> From: Ruslan Trofymenko <ruslan.trofymenko at linaro.org>
>
> This patch determines the A/B-specific bootloader message structure
> that is the basis for implementation of recovery and A/B update
> functions. A/B metadata is stored in this structure and used to decide
> which slot should we use to boot the device. Also some basic functions
> for A/B metadata manipulation are implemented (like slot selection).
>
> The patch was extracted from commits [1], [2] with some coding style
> fixes.
>
> [1] https://android-review.googlesource.com/c/platform/external/u-boot/+/729878/2
> [2] https://android-review.googlesource.com/c/platform/external/u-boot/+/729880/2
>
> Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko at linaro.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk at linaro.org>
> Reviewed-by: Sam Protsenko <semen.protsenko at linaro.org>
> ---
>
> Changes in v3:
> * Add multiple sanity checks
> * Fix mix. minor code formatting issues
>
> Changes in v2:
> * Function return codes are clarified
> * Some types and constants are renamed (for compactness)
> * android_bootloader_message.h is renamed to android_bl_msg.h
> * 'debug' calls are changed to 'log_debug'
> * Order of headers is changed
> * android_bl_msg.h was synced with AOSP master counterpart
>
> common/Kconfig | 10 ++
> common/Makefile | 1 +
> common/android_ab.c | 290 +++++++++++++++++++++++++++++++++++++++++++++++
> include/android_ab.h | 34 ++++++
> include/android_bl_msg.h | 169 +++++++++++++++++++++++++++
> 5 files changed, 504 insertions(+)
> create mode 100644 common/android_ab.c
> create mode 100644 include/android_ab.h
> create mode 100644 include/android_bl_msg.h
>
Reviewed-by: Simon Glass <sjg at chromium.org>
Minor comments below, could be addressed later.
> diff --git a/common/Kconfig b/common/Kconfig
> index 0a14bde..fc08e31 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -767,6 +767,16 @@ config UPDATE_TFTP_MSEC_MAX
> default 100
> depends on UPDATE_TFTP
>
> +config ANDROID_AB
> + bool "Android A/B updates"
> + default n
> + help
> + If enabled, adds support for the new Android A/B update model. This
> + allows the bootloader to select which slot to boot from based on the
> + information provided by userspace via the Android boot_ctrl HAL. This
> + allows a bootloader to try a new version of the system but roll back
> + to previous version if the new one didn't boot all the way.
> +
> endmenu
>
> menu "Blob list"
> diff --git a/common/Makefile b/common/Makefile
> index ad390d0..dfa348c 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -106,6 +106,7 @@ endif
> endif
>
> obj-y += image.o
> +obj-$(CONFIG_ANDROID_AB) += android_ab.o
> obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o
> obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o
> obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o
> diff --git a/common/android_ab.c b/common/android_ab.c
> new file mode 100644
> index 0000000..3a6a52c
> --- /dev/null
> +++ b/common/android_ab.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Copyright (C) 2017 The Android Open Source Project
> + */
> +
> +#include <android_ab.h>
> +#include <android_bl_msg.h>
> +#include <common.h>
Please put common.h first.
> +#include <memalign.h>
> +#include <u-boot/crc.h>
> +
> +/**
> + * Compute the CRC-32 of the bootloader control struct.
> + *
> + * Only the bytes up to the crc32_le field are considered for the CRC-32
> + * calculation.
For function comments please add @abc for arg and @return for return
value in every case.
> + */
> +static uint32_t ab_control_compute_crc(struct andr_bl_control *abc)
> +{
> + return crc32(0, (void *)abc, offsetof(typeof(*abc), crc32_le));
> +}
> +
> +/**
> + * Initialize andr_bl_control to the default value.
> + *
> + * It allows us to boot all slots in order from the first one. This value
> + * should be used when the bootloader message is corrupted, but not when
> + * a valid message indicates that all slots are unbootable.
> + */
> +static int ab_control_default(struct andr_bl_control *abc)
> +{
> + int i;
> + const struct andr_slot_metadata metadata = {
> + .priority = 15,
> + .tries_remaining = 7,
> + .successful_boot = 0,
> + .verity_corrupted = 0,
> + .reserved = 0
> + };
> +
> + if (!abc)
> + return -EINVAL;
-EFAULT?
Also, does this actually happen>
> +
> + memcpy(abc->slot_suffix, "a\0\0\0", 4);
> + abc->magic = ANDROID_BOOT_CTRL_MAGIC;
> + abc->version = ANDROID_BOOT_CTRL_VERSION;
> + abc->nb_slot = ANDROID_NUM_SLOTS;
> + memset(abc->reserved0, 0, sizeof(abc->reserved0));
> + for (i = 0; i < abc->nb_slot; ++i)
> + abc->slot_info[i] = metadata;
> +
> + memset(abc->reserved1, 0, sizeof(abc->reserved1));
> + abc->crc32_le = ab_control_compute_crc(abc);
> +
> + return 0;
> +}
> +
> +/**
> + * Load the boot_control struct from disk into newly allocated memory.
> + *
> + * This function allocates and returns an integer number of disk blocks,
> + * based on the block size of the passed device to help performing a
> + * read-modify-write operation on the boot_control struct.
> + * The boot_control struct offset (2 KiB) must be a multiple of the device
> + * block size, for simplicity.
> + *
> + * @param[in] dev_desc Device where to read the boot_control struct from
> + * @param[in] part_info Partition in 'dev_desc' where to read from, normally
> + * the "misc" partition should be used
> + * @param[out] pointer to pointer to andr_bl_control data
> + * @return 0 on success and a negative on error
> + */
> +static int ab_control_create_from_disk(struct blk_desc *dev_desc,
> + const disk_partition_t *part_info,
> + struct andr_bl_control **abc)
> +{
> + ulong abc_offset, abc_blocks;
> +
> + abc_offset = offsetof(struct andr_bl_msg_ab, slot_suffix);
> + if (abc_offset % part_info->blksz) {
> + printf("ANDROID: Boot control block not block aligned.\n");
These strings bloat the code. Would it be worth changing them to
debug(), or recording a boot failure code somewhere?
> + return -EINVAL;
> + }
> + abc_offset /= part_info->blksz;
> +
> + abc_blocks = DIV_ROUND_UP(sizeof(struct andr_bl_control),
> + part_info->blksz);
> + if (abc_offset + abc_blocks > part_info->size) {
> + printf("ANDROID: boot control partition too small. Need at");
> + printf(" least %lu blocks but have %lu blocks.\n",
> + abc_offset + abc_blocks, part_info->size);
> + return -EINVAL;
> + }
> + *abc = malloc_cache_aligned(abc_blocks * part_info->blksz);
> + if (!*abc)
> + return -ENOMEM;
> +
> + if (blk_dread(dev_desc, part_info->start + abc_offset, abc_blocks,
> + *abc) != abc_blocks) {
blk_dread() actually returns an error number, so you should use
IS_ERR_VALUE() to check for error. No, that is not obvious from the
comment for blk_dread() unfortunately (patch welcome) but see struct
blk_ops.
> + printf("ANDROID: Could not read from boot control partition\n");
> + free(*abc);
> + return -EIO;
> + }
> +
> + log_debug("ANDROID: Loaded ABC, %lu blocks\n", abc_blocks);
> +
> + return 0;
> +}
> +
> +/**
> + * Store the loaded boot_control block.
> + *
> + * Store back to the same location it was read from with
> + * ab_control_create_from_misc().
> + *
> + * @param[in] dev_desc Device where we should write the boot_control struct
> + * @param[in] part_info Partition on the 'dev_desc' where to write
> + * @param[in] abc Pointer to the boot control struct and the extra bytes after
> + * it up to the nearest block boundary
> + * @return 0 on success and a negative on error
> + */
> +static int ab_control_store(struct blk_desc *dev_desc,
> + const disk_partition_t *part_info,
> + struct andr_bl_control *abc)
> +{
> + ulong abc_offset, abc_blocks;
> +
> + abc_offset = offsetof(struct andr_bl_msg_ab, slot_suffix) /
> + part_info->blksz;
> + abc_blocks = DIV_ROUND_UP(sizeof(struct andr_bl_control),
> + part_info->blksz);
> + if (blk_dwrite(dev_desc, part_info->start + abc_offset, abc_blocks,
> + abc) != abc_blocks) {
IS_ERR_VALUE()
> + printf("ANDROID: Could not write back the misc partition\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * Compare two slots.
> + *
> + * The function determines slot which is should we boot from among the two.
> + *
> + * @param[in] a The first bootable slot metadata
> + * @param[in] b The second bootable slot metadata
> + * @return Negative if the slot "a" is better, positive of the slot "b" is
> + * better or 0 if they are equally good.
> + */
> +static int ab_compare_slots(const struct andr_slot_metadata *a,
> + const struct andr_slot_metadata *b)
> +{
> + /* Higher priority is better */
> + if (a->priority != b->priority)
> + return b->priority - a->priority;
> +
> + /* Higher successful_boot value is better, in case of same priority */
> + if (a->successful_boot != b->successful_boot)
> + return b->successful_boot - a->successful_boot;
> +
> + /* Higher tries_remaining is better to ensure round-robin */
> + if (a->tries_remaining != b->tries_remaining)
> + return b->tries_remaining - a->tries_remaining;
> +
> + return 0;
> +}
> +
> +int ab_select_slot(struct blk_desc *dev_desc, disk_partition_t *part_info)
> +{
> + struct andr_bl_control *abc = NULL;
> + u32 crc32_le;
> + int slot, i, ret;
> + bool store_needed = false;
> + char slot_suffix[4];
> +
> + ret = ab_control_create_from_disk(dev_desc, part_info, &abc);
> + if (ret < 0) {
> + /*
> + * This condition represents an actual problem with the code or
> + * the board setup, like an invalid partition information.
> + * Signal a repair mode and do not try to boot from either slot.
> + */
> + return ret;
> + }
> +
> + crc32_le = ab_control_compute_crc(abc);
> + if (abc->crc32_le != crc32_le) {
> + printf("ANDROID: Invalid CRC-32 (expected %.8x, found %.8x), ",
> + crc32_le, abc->crc32_le);
> + printf("re-initializing A/B metadata.\n");
> +
> + ret = ab_control_default(abc);
> + if (ret < 0) {
> + free(abc);
> + return -ENODATA;
> + }
> + store_needed = true;
> + }
> +
> + if (abc->magic != ANDROID_BOOT_CTRL_MAGIC) {
> + printf("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
> + free(abc);
> + return -ENODATA;
> + }
> +
> + if (abc->version > ANDROID_BOOT_CTRL_VERSION) {
> + printf("ANDROID: Unsupported A/B metadata version: %.8x\n",
> + abc->version);
> + free(abc);
> + return -ENODATA;
> + }
> +
> + /*
> + * At this point a valid boot control metadata is stored in abc,
> + * followed by other reserved data in the same block. We select a with
> + * the higher priority slot that
> + * - is not marked as corrupted and
> + * - either has tries_remaining > 0 or successful_boot is true.
> + * If the selected slot has a false successful_boot, we also decrement
> + * the tries_remaining until it eventually becomes unbootable because
> + * tries_remaining reaches 0. This mechanism produces a bootloader
> + * induced rollback, typically right after a failed update.
> + */
> +
> + /* Safety check: limit the number of slots. */
> + if (abc->nb_slot > ARRAY_SIZE(abc->slot_info)) {
> + abc->nb_slot = ARRAY_SIZE(abc->slot_info);
> + store_needed = true;
> + }
> +
> + slot = -1;
> + for (i = 0; i < abc->nb_slot; ++i) {
> + if (abc->slot_info[i].verity_corrupted ||
> + !abc->slot_info[i].tries_remaining) {
> + log_debug("ANDROID: unbootable slot %d tries: %d, ",
> + i, abc->slot_info[i].tries_remaining);
> + log_debug("corrupt: %d\n",
> + abc->slot_info[i].verity_corrupted);
> + continue;
> + }
> + log_debug("ANDROID: bootable slot %d pri: %d, tries: %d, ",
> + i, abc->slot_info[i].priority,
> + abc->slot_info[i].tries_remaining);
> + log_debug("corrupt: %d, successful: %d\n",
> + abc->slot_info[i].verity_corrupted,
> + abc->slot_info[i].successful_boot);
> +
> + if (slot < 0 ||
> + ab_compare_slots(&abc->slot_info[i],
> + &abc->slot_info[slot]) < 0) {
> + slot = i;
> + }
> + }
> +
> + if (slot >= 0 && !abc->slot_info[slot].successful_boot) {
> + printf("ANDROID: Attempting slot %c, tries remaining %d\n",
> + ANDROID_BOOT_SLOT_NAME(slot),
> + abc->slot_info[slot].tries_remaining);
> + abc->slot_info[slot].tries_remaining--;
> + store_needed = true;
> + }
> +
> + if (slot >= 0) {
> + /*
> + * Legacy user-space requires this field to be set in the BCB.
> + * Newer releases load this slot suffix from the command line
> + * or the device tree.
> + */
> + memset(slot_suffix, 0, sizeof(slot_suffix));
> + slot_suffix[0] = ANDROID_BOOT_SLOT_NAME(slot);
> + if (memcmp(abc->slot_suffix, slot_suffix,
> + sizeof(slot_suffix))) {
> + memcpy(abc->slot_suffix, slot_suffix,
> + sizeof(slot_suffix));
> + store_needed = true;
> + }
> + }
> +
> + if (store_needed) {
> + abc->crc32_le = ab_control_compute_crc(abc);
> + ab_control_store(dev_desc, part_info, abc);
Can this fail?
> + }
> + free(abc);
> +
> + if (slot < 0)
> + return -EINVAL;
I feel this error code is overused - is there another one that would
give more info?
> +
> + return slot;
> +}
> diff --git a/include/android_ab.h b/include/android_ab.h
> new file mode 100644
> index 0000000..c1b901d
> --- /dev/null
> +++ b/include/android_ab.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2017 The Android Open Source Project
> + */
> +
> +#ifndef __ANDROID_AB_H
> +#define __ANDROID_AB_H
> +
> +#include <common.h>
> +
> +/* Android standard boot slot names are 'a', 'b', 'c', ... */
> +#define ANDROID_BOOT_SLOT_NAME(slot_num) ('a' + (slot_num))
> +
> +/* Number of slots */
> +#define ANDROID_NUM_SLOTS 2
> +
> +/**
> + * Select the slot where to boot from.
> + *
> + * On Android devices with more than one boot slot (multiple copies of the
> + * kernel and system images) selects which slot should be used to boot from and
> + * registers the boot attempt. This is used in by the new A/B update model where
> + * one slot is updated in the background while running from the other slot. If
> + * the selected slot did not successfully boot in the past, a boot attempt is
> + * registered before returning from this function so it isn't selected
> + * indefinitely.
> + *
> + * @param[in] dev_desc Place to store the device description pointer
> + * @param[in] part_info Place to store the partition information
> + * @return The slot number (>= 0) on success, or a negative on error
> + */
> +int ab_select_slot(struct blk_desc *dev_desc, disk_partition_t *part_info);
> +
> +#endif /* __ANDROID_AB_H */
> diff --git a/include/android_bl_msg.h b/include/android_bl_msg.h
> new file mode 100644
> index 0000000..f37e01a
> --- /dev/null
> +++ b/include/android_bl_msg.h
> @@ -0,0 +1,169 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * This file was taken from the AOSP Project.
> + * Repository: https://android.googlesource.com/platform/bootable/recovery/
> + * File: bootloader_message/include/bootloader_message/bootloader_message.h
> + * Commit: 8b309f6970ab3b7c53cc529c51a2cb44e1c7a7e1
> + *
> + * Copyright (C) 2008 The Android Open Source Project
> + */
> +
> +#ifndef __ANDROID_BL_MSG_H
> +#define __ANDROID_BL_MSG_H
> +
> +/*
> + * compiler.h defines the types that otherwise are included from stdint.h and
> + * stddef.h
> + */
> +#include <compiler.h>
> +#include <linux/sizes.h>
Note if common.h is always included first, these are not needed.
Regards,
Simon
More information about the U-Boot
mailing list