[U-Boot] [PATCH v3 0/7] android: implement A/B boot process

[U-Boot] [PATCH v3 0/7] android: implement A/B boot process

[Igor Opaniuk]

This patch series adds support for Android A/B boot process [1].
Main steps of A/B boot process are:
  - A/B metadata integrity check
  - looking for the current slot (where the system should be
    booting from)
  - getting the name of the current boot partition (boot_a or boot_b) 
    and loading the corresponding Android boot image
  - getting the name of the current system partition (system_a or 
    system_b) and passing of its full name via kernel command line
    (like 'root=/dev/mmcblk1p11')
  - passing current slot via kernel command line (like
    'androidboot.slot_suffix=_a') and via A/B metadata (e.g. via
    misc partition)
  - A/B metadata processing: setting the boot success flag for
    current slot, handling the retry counter, etc

A/B metadata is organized according to Android reference [2] and stored
on 'misc' partition. On the first A/B boot process, when 'misc'
partition doesn't contain required data, default A/B metadata will be
created and stored in 'misc' partition. In the end of the Android boot,
'update_verifier' and 'update_engine' services are processing the
A/B metadata through the Boot Control HAL. To confirm the boot was
successful using current slot, "boot success" flag must be set on
Android side.

To enable Android A/B support in U-Boot:
  1. Set the following config options:

         CONFIG_ANDROID_AB=y
         CONFIG_CMD_AB_SELECT=y

  2. Change the disk layout so that it has sloted boot partitions.
     E.g. instead of 'boot' and 'system' partitions there should be
     'boot_a', 'boot_b', 'system_a' and 'system_b' partitions.

To be able to actually test this patch series, the A/B features must
be implemented and enabled in Android as well (see [1] for details).

Documentation and corresponding test for A/B boot is present here. The
last patch in this series integrates A/B boot support on AM57xx based
boards (though it's not enabled by default). Future users of A/B boot
feature can use it as a reference.

This series is a part of previous submission [3] by Alex Deymo. It
contains only A/B feature that was stripped out from there with some
modifications for using with "bootm" command preferred in upstream.

Changes in v3:
  * Minor fixes in the ab metadata handling (added additional sanity checks).
  * As Ruslan Trofymenko left Linaro and won't address comments anymore,
    continue (added my S-b tag) upstreaming patches on my own.

Changes in v2:
  * 'android_ab_select' command is renamed to 'ab_select' command and
     moved to separate 'Android support commands' menu
  * For am57xx boards slotted sections (e.g. system_a and system_b) are
    added to the default sections if CONFIG_CMD_AB_SELECT flag is
    defined
  * Returned function error codes are clarified (errno using)
  * Some types constants and files are renamed
  * Assertion condition is clarified in test case
  * 'debug' calls are changed to 'log_debug'
  * The Guide is clarified by the results of changes

[1] https://source.android.com/devices/tech/ota/ab/ab_implement
[2] bootable/recovery/bootloader_message/include/bootloader_message/bootloader_message.h
[3] https://lists.denx.de/pipermail/u-boot/2017-April/285841.html

Ruslan Trofymenko (7):
  cmd: part: Add 'number' sub-command
  disk: part: Extend API to get partition info
  common: Implement A/B metadata
  cmd: Add 'ab_select' command
  test/py: Add base test case for A/B updates
  doc: android: Add simple guide for A/B updates
  env: am57xx: Implement A/B boot process

 cmd/Kconfig                   |  15 +++
 cmd/Makefile                  |   1 +
 cmd/ab_select.c               |  52 ++++++++
 cmd/part.c                    |  16 ++-
 common/Kconfig                |  10 ++
 common/Makefile               |   1 +
 common/android_ab.c           | 290 ++++++++++++++++++++++++++++++++++++++++++
 configs/sandbox_defconfig     |   2 +
 disk/part.c                   |  68 ++++++++++
 doc/README.android-ab         |  67 ++++++++++
 include/android_ab.h          |  34 +++++
 include/android_bl_msg.h      | 169 ++++++++++++++++++++++++
 include/environment/ti/boot.h |  58 ++++++++-
 include/part.h                |  21 +++
 test/py/tests/test_ab.py      |  74 +++++++++++
 15 files changed, 871 insertions(+), 7 deletions(-)
 create mode 100644 cmd/ab_select.c
 create mode 100644 common/android_ab.c
 create mode 100644 doc/README.android-ab
 create mode 100644 include/android_ab.h
 create mode 100644 include/android_bl_msg.h
 create mode 100644 test/py/tests/test_ab.py

-- 
2.7.4

[U-Boot] [PATCH v3 1/7] cmd: part: Add 'number' sub-command

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

This sub-command serves for getting the partition index from
partition name. Also it can be used to test the existence of specified
partition.

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2: None

 cmd/part.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/cmd/part.c b/cmd/part.c
index bfb6488..653e13c 100644
--- a/cmd/part.c
+++ b/cmd/part.c
@@ -24,6 +24,7 @@
 enum cmd_part_info {
 	CMD_PART_INFO_START = 0,
 	CMD_PART_INFO_SIZE,
+	CMD_PART_INFO_NUMBER
 };
 
 static int do_part_uuid(int argc, char * const argv[])
@@ -149,6 +150,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param)
 	case CMD_PART_INFO_SIZE:
 		snprintf(buf, sizeof(buf), LBAF, info.size);
 		break;
+	case CMD_PART_INFO_NUMBER:
+		snprintf(buf, sizeof(buf), "%d", part);
+		break;
 	default:
 		printf("** Unknown cmd_part_info value: %d\n", param);
 		return 1;
@@ -172,6 +176,11 @@ static int do_part_size(int argc, char * const argv[])
 	return do_part_info(argc, argv, CMD_PART_INFO_SIZE);
 }
 
+static int do_part_number(int argc, char * const argv[])
+{
+	return do_part_info(argc, argv, CMD_PART_INFO_NUMBER);
+}
+
 static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	if (argc < 2)
@@ -185,6 +194,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return do_part_start(argc - 2, argv + 2);
 	else if (!strcmp(argv[1], "size"))
 		return do_part_size(argc - 2, argv + 2);
+	else if (!strcmp(argv[1], "number"))
+		return do_part_number(argc - 2, argv + 2);
 
 	return CMD_RET_USAGE;
 }
@@ -206,5 +217,8 @@ U_BOOT_CMD(
 	"      part can be either partition number or partition name\n"
 	"part size <interface> <dev> <part> <varname>\n"
 	"    - set environment variable to the size of the partition (in blocks)\n"
-	"      part can be either partition number or partition name"
+	"      part can be either partition number or partition name\n"
+	"part number <interface> <dev> <part> <varname>\n"
+	"    - set environment variable to the partition number using the partition name\n"
+	"      part must be specified as partition name"
 );
-- 
2.7.4

[U-Boot] [PATCH v3 2/7] disk: part: Extend API to get partition info

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

This patch adds part_get_info_by_dev_and_name_or_num() function which
allows us to get partition info from its number or name. Partition of
interest is specified by string like "device_num:partition_number" or
"device_num#partition_name".

The patch was extracted from [1].

[1] https://android-review.googlesource.com/c/platform/external/u-boot/+/729880/2

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None
Changes in v2:
  * Error codes are changed to -EINVAL instead of -1

 disk/part.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/part.h | 21 ++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/disk/part.c b/disk/part.c
index f30f9e9..7b739ad 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -675,6 +675,74 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name,
 	return part_get_info_by_name_type(dev_desc, name, info, PART_TYPE_ALL);
 }
 
+/**
+ * Get partition info from device number and partition name.
+ *
+ * Parse a device number and partition name string in the form of
+ * "device_num#partition_name", for example "0#misc". If the partition
+ * is found, sets dev_desc and part_info accordingly with the information
+ * of the partition with the given partition_name.
+ *
+ * @param[in] dev_iface Device interface
+ * @param[in] dev_part_str Input string argument, like "0#misc"
+ * @param[out] dev_desc Place to store the device description pointer
+ * @param[out] part_info Place to store the partition information
+ * @return 0 on success, or a negative on error
+ */
+static int part_get_info_by_dev_and_name(const char *dev_iface,
+					 const char *dev_part_str,
+					 struct blk_desc **dev_desc,
+					 disk_partition_t *part_info)
+{
+	char *ep;
+	const char *part_str;
+	int dev_num;
+
+	part_str = strchr(dev_part_str, '#');
+	if (!part_str || part_str == dev_part_str)
+		return -EINVAL;
+
+	dev_num = simple_strtoul(dev_part_str, &ep, 16);
+	if (ep != part_str) {
+		/* Not all the first part before the # was parsed. */
+		return -EINVAL;
+	}
+	part_str++;
+
+	*dev_desc = blk_get_dev(dev_iface, dev_num);
+	if (!*dev_desc) {
+		printf("Could not find %s %d\n", dev_iface, dev_num);
+		return -EINVAL;
+	}
+	if (part_get_info_by_name(*dev_desc, part_str, part_info) < 0) {
+		printf("Could not find \"%s\" partition\n", part_str);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
+					 const char *dev_part_str,
+					 struct blk_desc **dev_desc,
+					 disk_partition_t *part_info)
+{
+	/* Split the part_name if passed as "$dev_num#part_name". */
+	if (!part_get_info_by_dev_and_name(dev_iface, dev_part_str,
+					   dev_desc, part_info))
+		return 0;
+	/*
+	 * Couldn't lookup by name, try looking up the partition description
+	 * directly.
+	 */
+	if (blk_get_device_part_str(dev_iface, dev_part_str,
+				    dev_desc, part_info, 1) < 0) {
+		printf("Couldn't find partition %s %s\n",
+		       dev_iface, dev_part_str);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 void part_set_generic_name(const struct blk_desc *dev_desc,
 	int part_num, char *name)
 {
diff --git a/include/part.h b/include/part.h
index ebca546..0b5cf3d 100644
--- a/include/part.h
+++ b/include/part.h
@@ -202,6 +202,27 @@ int part_get_info_by_name(struct blk_desc *dev_desc,
 			      const char *name, disk_partition_t *info);
 
 /**
+ * Get partition info from dev number + part name, or dev number + part number.
+ *
+ * Parse a device number and partition description (either name or number)
+ * in the form of device number plus partition name separated by a "#"
+ * (like "device_num#partition_name") or a device number plus a partition number
+ * separated by a ":". For example both "0#misc" and "0:1" can be valid
+ * partition descriptions for a given interface. If the partition is found, sets
+ * dev_desc and part_info accordingly with the information of the partition.
+ *
+ * @param[in] dev_iface	Device interface
+ * @param[in] dev_part_str Input partition description, like "0#misc" or "0:1"
+ * @param[out] dev_desc	Place to store the device description pointer
+ * @param[out] part_info Place to store the partition information
+ * @return 0 on success, or a negative on error
+ */
+int part_get_info_by_dev_and_name_or_num(const char *dev_iface,
+					 const char *dev_part_str,
+					 struct blk_desc **dev_desc,
+					 disk_partition_t *part_info);
+
+/**
  * part_set_generic_name() - create generic partition like hda1 or sdb2
  *
  * Helper function for partition tables, which don't hold partition names
-- 
2.7.4

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@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@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
Reviewed-by: Sam Protsenko <semen.protsenko@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

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>
+#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.
+ */
+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;
+
+	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");
+		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) {
+		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) {
+		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);
+	}
+	free(abc);
+
+	if (slot < 0)
+		return -EINVAL;
+
+	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>
+
+/*
+ * Spaces used by misc partition are as below:
+ * 0   - 2K     Bootloader Message
+ * 2K  - 16K    Used by Vendor's bootloader (the 2K - 4K range may be optionally
+ *              used as bootloader_message_ab struct)
+ * 16K - 64K    Used by uncrypt and recovery to store wipe_package for A/B
+ *              devices
+ * Note that these offsets are admitted by bootloader, recovery and uncrypt, so
+ * they are not configurable without changing all of them.
+ */
+#define ANDROID_MISC_BM_OFFSET		0
+#define ANDROID_MISC_WIPE_OFFSET	SZ_16K
+
+/**
+ * Bootloader Message (2-KiB).
+ *
+ * This structure describes the content of a block in flash
+ * that is used for recovery and the bootloader to talk to
+ * each other.
+ *
+ * The command field is updated by linux when it wants to
+ * reboot into recovery or to update radio or bootloader firmware.
+ * It is also updated by the bootloader when firmware update
+ * is complete (to boot into recovery for any final cleanup)
+ *
+ * The status field is written by the bootloader after the
+ * completion of an "update-radio" or "update-hboot" command.
+ *
+ * The recovery field is only written by linux and used
+ * for the system to send a message to recovery or the
+ * other way around.
+ *
+ * The stage field is written by packages which restart themselves
+ * multiple times, so that the UI can reflect which invocation of the
+ * package it is.  If the value is of the format "#/#" (eg, "1/3"),
+ * the UI will add a simple indicator of that status.
+ *
+ * We used to have slot_suffix field for A/B boot control metadata in
+ * this struct, which gets unintentionally cleared by recovery or
+ * uncrypt. Move it into struct bootloader_message_ab to avoid the
+ * issue.
+ */
+struct andr_bl_msg {
+	char command[32];
+	char status[32];
+	char recovery[768];
+
+	/*
+	 * The 'recovery' field used to be 1024 bytes.  It has only ever
+	 * been used to store the recovery command line, so 768 bytes
+	 * should be plenty.  We carve off the last 256 bytes to store the
+	 * stage string (for multistage packages) and possible future
+	 * expansion.
+	 */
+	char stage[32];
+
+	/*
+	 * The 'reserved' field used to be 224 bytes when it was initially
+	 * carved off from the 1024-byte recovery field. Bump it up to
+	 * 1184-byte so that the entire bootloader_message struct rounds up
+	 * to 2048-byte.
+	 */
+	char reserved[1184];
+};
+
+/**
+ * The A/B-specific bootloader message structure (4-KiB).
+ *
+ * We separate A/B boot control metadata from the regular bootloader
+ * message struct and keep it here. Everything that's A/B-specific
+ * stays after struct bootloader_message, which should be managed by
+ * the A/B-bootloader or boot control HAL.
+ *
+ * The slot_suffix field is used for A/B implementations where the
+ * bootloader does not set the androidboot.ro.boot.slot_suffix kernel
+ * commandline parameter. This is used by fs_mgr to mount /system and
+ * other partitions with the slotselect flag set in fstab. A/B
+ * implementations are free to use all 32 bytes and may store private
+ * data past the first NUL-byte in this field. It is encouraged, but
+ * not mandatory, to use 'struct bootloader_control' described below.
+ *
+ * The update_channel field is used to store the Omaha update channel
+ * if update_engine is compiled with Omaha support.
+ */
+struct andr_bl_msg_ab {
+	struct andr_bl_msg message;
+	char slot_suffix[32];
+	char update_channel[128];
+
+	/* Round up the entire struct to 4096-byte */
+	char reserved[1888];
+};
+
+#define ANDROID_BOOT_CTRL_MAGIC   0x42414342 /* Bootloader Control AB */
+#define ANDROID_BOOT_CTRL_VERSION 1
+
+struct andr_slot_metadata {
+	/*
+	 * Slot priority with 15 meaning highest priority, 1 lowest
+	 * priority and 0 the slot is unbootable
+	 */
+	u8 priority : 4;
+	/* Number of times left attempting to boot this slot */
+	u8 tries_remaining : 3;
+	/* 1 if this slot has booted successfully, 0 otherwise */
+	u8 successful_boot : 1;
+	/*
+	 * 1 if this slot is corrupted from a dm-verity corruption,
+	 * 0 otherwise
+	 */
+	u8 verity_corrupted : 1;
+	/* Reserved for further use */
+	u8 reserved : 7;
+} __packed;
+
+/**
+ * Bootloader Control AB.
+ *
+ * This struct can be used to manage A/B metadata. It is designed to
+ * be put in the 'slot_suffix' field of the 'bootloader_message'
+ * structure described above. It is encouraged to use the
+ * 'bootloader_control' structure to store the A/B metadata, but not
+ * mandatory.
+ */
+struct andr_bl_control {
+	/* NULL terminated active slot suffix */
+	char slot_suffix[4];
+	/* Bootloader Control AB magic number (see BOOT_CTRL_MAGIC) */
+	u32 magic;
+	/* Version of struct being used (see BOOT_CTRL_VERSION) */
+	u8 version;
+	/* Number of slots being managed */
+	u8 nb_slot : 3;
+	/* Number of times left attempting to boot recovery */
+	u8 recovery_tries_remaining : 3;
+	/* Ensure 4-bytes alignment for slot_info field */
+	u8 reserved0[2];
+	/* Per-slot information. Up to 4 slots */
+	struct andr_slot_metadata slot_info[4];
+	/* Reserved for further use */
+	u8 reserved1[8];
+	/*
+	 * CRC32 of all 28 bytes preceding this field (little endian
+	 * format)
+	 */
+	u32 crc32_le;
+} __packed;
+
+#endif  /* __ANDROID_BL_MSG_H */
-- 
2.7.4

[U-Boot] [PATCH v3 4/7] cmd: Add 'ab_select' command

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

For A/B system update support the Android boot process requires to send
'androidboot.slot_suffix' parameter as a command line argument. This
patch implementes 'ab_select' command which allows us to obtain current
slot by processing the A/B metadata.

The patch was extracted from commit [1] with one modification: the
separator for specifying the name of metadata partition was changed
from ';' to '#', because ';' is used for commands separation.

[1] https://android-review.googlesource.com/c/platform/external/u-boot/+/729880/2

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v3: None
Changes in v2:
  * 'android_ab_select' command is renamed to 'ab_select' command
  * command is moved to the separate 'Android support commands' menu

 cmd/Kconfig     | 15 +++++++++++++++
 cmd/Makefile    |  1 +
 cmd/ab_select.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)
 create mode 100644 cmd/ab_select.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 3ea42e4..290a570 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1123,6 +1123,21 @@ config CMD_SETEXPR
 
 endmenu
 
+menu "Android support commands"
+
+config CMD_AB_SELECT
+	bool "ab_select"
+	default n
+	depends on ANDROID_AB
+	help
+	  On Android devices with more than one boot slot (multiple copies of
+	  the kernel and system images) this provides a command to select which
+	  slot should be used to boot from and register the boot attempt. This
+	  is used by the new A/B update model where one slot is updated in the
+	  background while running from the other slot.
+
+endmenu
+
 if NET
 
 menuconfig CMD_NET
diff --git a/cmd/Makefile b/cmd/Makefile
index 15ae4d2..ea03eb0 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -12,6 +12,7 @@ obj-y += version.o
 
 # command
 obj-$(CONFIG_CMD_AES) += aes.o
+obj-$(CONFIG_CMD_AB_SELECT) += ab_select.o
 obj-$(CONFIG_CMD_ADC) += adc.o
 obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
 obj-y += blk_common.o
diff --git a/cmd/ab_select.c b/cmd/ab_select.c
new file mode 100644
index 0000000..2a9e524
--- /dev/null
+++ b/cmd/ab_select.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ */
+
+#include <android_ab.h>
+#include <command.h>
+
+static int do_ab_select(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	int ret;
+	struct blk_desc *dev_desc;
+	disk_partition_t part_info;
+	char slot[2];
+
+	if (argc != 4)
+		return CMD_RET_USAGE;
+
+	/* Lookup the "misc" partition from argv[2] and argv[3] */
+	if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3],
+						 &dev_desc, &part_info) < 0) {
+		return CMD_RET_FAILURE;
+	}
+
+	ret = ab_select_slot(dev_desc, &part_info);
+	if (ret < 0) {
+		printf("Android boot failed, error %d.\n", ret);
+		return CMD_RET_FAILURE;
+	}
+
+	/* Android standard slot names are 'a', 'b', ... */
+	slot[0] = ANDROID_BOOT_SLOT_NAME(ret);
+	slot[1] = '\0';
+	env_set(argv[1], slot);
+	printf("ANDROID: Booting slot: %s\n", slot);
+	return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(ab_select, 4, 0, do_ab_select,
+	   "Select the slot used to boot from and register the boot attempt.",
+	   "<slot_var_name> <interface> <dev[:part|#part_name]>\n"
+	   "    - Load the slot metadata from the partition 'part' on\n"
+	   "      device type 'interface' instance 'dev' and store the active\n"
+	   "      slot in the 'slot_var_name' variable. This also updates the\n"
+	   "      Android slot metadata with a boot attempt, which can cause\n"
+	   "      successive calls to this function to return a different result\n"
+	   "      if the returned slot runs out of boot attempts.\n"
+	   "    - If 'part_name' is passed, preceded with a # instead of :, the\n"
+	   "      partition name whose label is 'part_name' will be looked up in\n"
+	   "      the partition table. This is commonly the \"misc\" partition.\n"
+);
-- 
2.7.4

[U-Boot] [PATCH v3 5/7] test/py: Add base test case for A/B updates

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

Add sandbox test for 'ab_select' command.

Test: ./test/py/test.py --bd sandbox --build -k test_ab

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v3: None

Changes in v2:
* Changes related to command renaming (android_ab_select->ab_select).
* Assertion condition was clarified. Full command output is controlled.

 configs/sandbox_defconfig |  2 ++
 test/py/tests/test_ab.py  | 74 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 test/py/tests/test_ab.py

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 193e418..31c18ad 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -20,6 +20,7 @@ CONFIG_PRE_CON_BUF_ADDR=0x100000
 CONFIG_LOG_MAX_LEVEL=6
 CONFIG_LOG_ERROR_RETURN=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_ANDROID_AB=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
@@ -48,6 +49,7 @@ CONFIG_CMD_SF=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_AXI=y
+CONFIG_CMD_AB_SELECT=y
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_TFTPSRV=y
 CONFIG_CMD_RARP=y
diff --git a/test/py/tests/test_ab.py b/test/py/tests/test_ab.py
new file mode 100644
index 0000000..b90ca87
--- /dev/null
+++ b/test/py/tests/test_ab.py
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: GPL-2.0
+# (C) Copyright 2018 Texas Instruments, <www.ti.com>
+
+# Test A/B update commands.
+
+import os
+import pytest
+import u_boot_utils
+
+class ABTestDiskImage(object):
+    """Disk Image used by the A/B tests."""
+
+    def __init__(self, u_boot_console):
+        """Initialize a new ABTestDiskImage object.
+
+        Args:
+            u_boot_console: A U-Boot console.
+
+        Returns:
+            Nothing.
+        """
+
+        filename = 'test_ab_disk_image.bin'
+
+        persistent = u_boot_console.config.persistent_data_dir + '/' + filename
+        self.path = u_boot_console.config.result_dir  + '/' + filename
+
+        with u_boot_utils.persistent_file_helper(u_boot_console.log, persistent):
+            if os.path.exists(persistent):
+                u_boot_console.log.action('Disk image file ' + persistent +
+                    ' already exists')
+            else:
+                u_boot_console.log.action('Generating ' + persistent)
+                fd = os.open(persistent, os.O_RDWR | os.O_CREAT)
+                os.ftruncate(fd, 524288)
+                os.close(fd)
+                cmd = ('sgdisk', persistent)
+                u_boot_utils.run_and_log(u_boot_console, cmd)
+
+                cmd = ('sgdisk', '--new=1:64:512', '-c 1:misc', persistent)
+                u_boot_utils.run_and_log(u_boot_console, cmd)
+                cmd = ('sgdisk', '-l', persistent)
+                u_boot_utils.run_and_log(u_boot_console, cmd)
+
+        cmd = ('cp', persistent, self.path)
+        u_boot_utils.run_and_log(u_boot_console, cmd)
+
+di = None
+ at pytest.fixture(scope='function')
+def ab_disk_image(u_boot_console):
+    global di
+    if not di:
+        di = ABTestDiskImage(u_boot_console)
+    return di
+
+ at pytest.mark.boardspec('sandbox')
+ at pytest.mark.buildconfigspec('android_ab')
+ at pytest.mark.buildconfigspec('cmd_ab_select')
+ at pytest.mark.requiredtool('sgdisk')
+def test_ab(ab_disk_image, u_boot_console):
+    """Test the 'ab_select' command."""
+
+    u_boot_console.run_command('host bind 0 ' + ab_disk_image.path)
+
+    output = u_boot_console.run_command('ab_select slot_name host 0#misc')
+    assert 're-initializing A/B metadata' in output
+    assert 'Attempting slot a, tries remaining 7' in output
+    output = u_boot_console.run_command('printenv slot_name')
+    assert 'slot_name=a' in output
+
+    output = u_boot_console.run_command('ab_select slot_name host 0:1')
+    assert 'Attempting slot b, tries remaining 7' in output
+    output = u_boot_console.run_command('printenv slot_name')
+    assert 'slot_name=b' in output
-- 
2.7.4

[U-Boot] [PATCH v3 6/7] doc: android: Add simple guide for A/B updates

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

Add a short documentation for A/B enablement and 'ab_select' command
usage.

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v3: None
Changes in v2:
* Changes related to command renaming

 doc/README.android-ab | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 doc/README.android-ab

diff --git a/doc/README.android-ab b/doc/README.android-ab
new file mode 100644
index 0000000..9f37ed5
--- /dev/null
+++ b/doc/README.android-ab
@@ -0,0 +1,67 @@
+Android A/B updates
+===================
+
+Overview
+--------
+
+A/B system updates ensures modern approach for system update. This feature
+allows one to use two sets (or more) of partitions referred to as slots
+(normally slot A and slot B). The system runs from the current slot while the
+partitions in the unused slot can be updated [1].
+
+A/B enablement
+--------------
+
+The A/B updates support can be activated by specifying next options in
+your board configuration file:
+
+    CONFIG_ANDROID_AB=y
+    CONFIG_CMD_AB_SELECT=y
+
+The disk space on target device must be partitioned in a way so that each
+partition which needs to be updated has two or more instances. The name of
+each instance must be formed by adding suffixes: _a, _b, _c, etc.
+For example: boot_a, boot_b, system_a, system_b, vendor_a, vendor_b.
+
+As a result you can use 'ab_select' command to ensure A/B boot process in your
+boot script. This command analyzes and processes A/B metadata stored on a
+special partition (e.g. "misc") and determines which slot should be used for
+booting up.
+
+Command usage
+-------------
+
+    ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
+
+for example:
+
+    => ab_select slot_name mmc 1:4
+
+or
+
+    => ab_select slot_name mmc 1#misc
+
+Result:
+
+    => printenv slot_name
+    slot_name=a
+
+Based on this slot information, the current boot partition should be defined,
+and next kernel command line parameters should be generated:
+
+ - androidboot.slot_suffix=
+ - root=
+
+For example:
+
+    androidboot.slot_suffix=_a root=/dev/mmcblk1p12
+
+A/B metadata is organized according to AOSP reference [2]. On the first system
+start with A/B enabled, when 'misc' partition doesn't contain required data,
+the default A/B metadata will be created and written to 'misc' partition.
+
+References
+----------
+
+[1] https://source.android.com/devices/tech/ota/ab
+[2] bootable/recovery/bootloader_message/include/bootloader_message/bootloader_message.h
-- 
2.7.4

[U-Boot] [PATCH v3 7/7] env: am57xx: Implement A/B boot process

[Igor Opaniuk]

From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>

Add support for A/B boot process on AM57xx based boards:

  1. Define 'slot_suffix' variable (using 'ab_select' command)
  2. Extend 'emmc_android_boot' boot command (add commands for A/B boot
     process)

'ab_select' command is used to decide which slot should be used for
booting up. A/B metadata resides in 'misc' partition.

To activate the A/B boot process, the following config options must be
set:

    CONFIG_ANDROID_AB=y
    CONFIG_CMD_AB_SELECT=y

For successful A/B boot, the corresponding A/B infrastructure must be
involved on Android side [1] (including mounting system as root), and
disk must be partitioned accordingly.

When A/B boot is enabled, there are some known limitations currently
exist (not related to A/B patches, need to be implemented later):

  1. The 'Verified Boot' sequence is not supported
  2. dev path to system partition (system_a or system_b) is passed via
     'bootargs' as 'root=' argument like 'root=/dev/mmcblk1p12', but
     further we'll need to rework it with respect to dm-verity
     requirements [2]

In case when A/B partitions are not present in system (and A/B boot is
enabled), boot up process will be terminated and next message will be
shown:

    "boot_a(b) partition not found"

[1] https://source.android.com/devices/tech/ota/ab
[2] https://source.android.com/devices/tech/ota/ab/ab_implement#kernel

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
Reviewed-by: Alistair Strachan <astrachan@google.com>
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
---

Changes in v3: None

Changes in v2:
* Add changes related to command renaming (android_ab_select -> ab_select).
* Slotted sections (e.g. system_a and system_b) are added to the
  default sections if CONFIG_CMD_AB_SELECT flag is defined
* Rebased on top of master
* system partitions sizes increased to 1024 MiB (to be consistent with
  recent changes to boot.h file)

 include/environment/ti/boot.h | 58 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/include/environment/ti/boot.h b/include/environment/ti/boot.h
index 05bdbbc..bc05bca 100644
--- a/include/environment/ti/boot.h
+++ b/include/environment/ti/boot.h
@@ -23,6 +23,18 @@
 #define VBMETA_PART			""
 #endif
 
+#if defined(CONFIG_CMD_AB_SELECT)
+#define COMMON_PARTS \
+	"name=boot_a,size=10M,uuid=${uuid_gpt_boot_a};" \
+	"name=boot_b,size=10M,uuid=${uuid_gpt_boot_b};" \
+	"name=system_a,size=1024M,uuid=${uuid_gpt_system_a};" \
+	"name=system_b,size=1024M,uuid=${uuid_gpt_system_b};"
+#else
+#define COMMON_PARTS \
+	"name=boot,size=10M,uuid=${uuid_gpt_boot};" \
+	"name=system,size=1024M,uuid=${uuid_gpt_system};"
+#endif
+
 #ifndef PARTS_DEFAULT
 /* Define the default GPT table for eMMC */
 #define PARTS_DEFAULT \
@@ -38,8 +50,7 @@
 	"name=uboot-env,start=2432K,size=256K,uuid=${uuid_gpt_reserved};" \
 	"name=misc,size=128K,uuid=${uuid_gpt_misc};" \
 	"name=recovery,size=40M,uuid=${uuid_gpt_recovery};" \
-	"name=boot,size=10M,uuid=${uuid_gpt_boot};" \
-	"name=system,size=1024M,uuid=${uuid_gpt_system};" \
+	COMMON_PARTS \
 	"name=vendor,size=256M,uuid=${uuid_gpt_vendor};" \
 	VBMETA_PART \
 	"name=userdata,size=-,uuid=${uuid_gpt_userdata}"
@@ -58,6 +69,35 @@
 #define AVB_VERIFY_CMD ""
 #endif
 
+#define CONTROL_PARTITION "misc"
+
+#if defined(CONFIG_CMD_AB_SELECT)
+#define AB_SELECT \
+	"if part number mmc 1 " CONTROL_PARTITION " control_part_number; " \
+	"then " \
+		"echo " CONTROL_PARTITION \
+			" partition number:${control_part_number};" \
+		"ab_select slot_name mmc ${mmcdev}:${control_part_number};" \
+	"else " \
+		"echo " CONTROL_PARTITION " partition not found;" \
+		"exit;" \
+	"fi;" \
+	"setenv slot_suffix _${slot_name};" \
+	"if part number mmc ${mmcdev} system${slot_suffix} " \
+	"system_part_number; then " \
+		"setenv bootargs_ab " \
+		"ro root=/dev/mmcblk${mmcdev}p${system_part_number} " \
+		"rootwait init=/init skip_initramfs " \
+		"androidboot.slot_suffix=${slot_suffix};" \
+		"echo A/B cmdline addition: ${bootargs_ab};" \
+		"setenv bootargs ${bootargs} ${bootargs_ab};" \
+	"else " \
+		"echo system${slot_suffix} partition not found;" \
+	"fi;"
+#else
+#define AB_SELECT ""
+#endif
+
 #define DEFAULT_COMMON_BOOT_TI_ARGS \
 	"console=" CONSOLEDEV ",115200n8\0" \
 	"fdtfile=undefined\0" \
@@ -86,10 +126,16 @@
 		"mmc dev $mmcdev; " \
 		"mmc rescan; " \
 		AVB_VERIFY_CHECK \
-		"part start mmc ${mmcdev} boot boot_start; " \
-		"part size mmc ${mmcdev} boot boot_size; " \
-		"mmc read ${loadaddr} ${boot_start} ${boot_size}; " \
-		"bootm ${loadaddr}#${fdtfile};\0 "
+		AB_SELECT \
+		"if part start mmc ${mmcdev} boot${slot_suffix} boot_start; " \
+		"then " \
+			"part size mmc ${mmcdev} boot${slot_suffix} " \
+				"boot_size; " \
+			"mmc read ${loadaddr} ${boot_start} ${boot_size}; " \
+			"bootm ${loadaddr}#${fdtfile}; " \
+		"else " \
+			"echo boot${slot_suffix} partition not found; " \
+		"fi;\0"
 
 #ifdef CONFIG_OMAP54XX
 
-- 
2.7.4

[U-Boot] [PATCH v3 0/7] android: implement A/B boot process

[Sam Protsenko]

For the whole series:

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

On Mon, Feb 18, 2019 at 6:22 PM Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
>
> This patch series adds support for Android A/B boot process [1].
> Main steps of A/B boot process are:
>   - A/B metadata integrity check
>   - looking for the current slot (where the system should be
>     booting from)
>   - getting the name of the current boot partition (boot_a or boot_b)
>     and loading the corresponding Android boot image
>   - getting the name of the current system partition (system_a or
>     system_b) and passing of its full name via kernel command line
>     (like 'root=/dev/mmcblk1p11')
>   - passing current slot via kernel command line (like
>     'androidboot.slot_suffix=_a') and via A/B metadata (e.g. via
>     misc partition)
>   - A/B metadata processing: setting the boot success flag for
>     current slot, handling the retry counter, etc
>
> A/B metadata is organized according to Android reference [2] and stored
> on 'misc' partition. On the first A/B boot process, when 'misc'
> partition doesn't contain required data, default A/B metadata will be
> created and stored in 'misc' partition. In the end of the Android boot,
> 'update_verifier' and 'update_engine' services are processing the
> A/B metadata through the Boot Control HAL. To confirm the boot was
> successful using current slot, "boot success" flag must be set on
> Android side.
>
> To enable Android A/B support in U-Boot:
>   1. Set the following config options:
>
>          CONFIG_ANDROID_AB=y
>          CONFIG_CMD_AB_SELECT=y
>
>   2. Change the disk layout so that it has sloted boot partitions.
>      E.g. instead of 'boot' and 'system' partitions there should be
>      'boot_a', 'boot_b', 'system_a' and 'system_b' partitions.
>
> To be able to actually test this patch series, the A/B features must
> be implemented and enabled in Android as well (see [1] for details).
>
> Documentation and corresponding test for A/B boot is present here. The
> last patch in this series integrates A/B boot support on AM57xx based
> boards (though it's not enabled by default). Future users of A/B boot
> feature can use it as a reference.
>
> This series is a part of previous submission [3] by Alex Deymo. It
> contains only A/B feature that was stripped out from there with some
> modifications for using with "bootm" command preferred in upstream.
>
> Changes in v3:
>   * Minor fixes in the ab metadata handling (added additional sanity checks).
>   * As Ruslan Trofymenko left Linaro and won't address comments anymore,
>     continue (added my S-b tag) upstreaming patches on my own.
>
> Changes in v2:
>   * 'android_ab_select' command is renamed to 'ab_select' command and
>      moved to separate 'Android support commands' menu
>   * For am57xx boards slotted sections (e.g. system_a and system_b) are
>     added to the default sections if CONFIG_CMD_AB_SELECT flag is
>     defined
>   * Returned function error codes are clarified (errno using)
>   * Some types constants and files are renamed
>   * Assertion condition is clarified in test case
>   * 'debug' calls are changed to 'log_debug'
>   * The Guide is clarified by the results of changes
>
> [1] https://source.android.com/devices/tech/ota/ab/ab_implement
> [2] bootable/recovery/bootloader_message/include/bootloader_message/bootloader_message.h
> [3] https://lists.denx.de/pipermail/u-boot/2017-April/285841.html
>
> Ruslan Trofymenko (7):
>   cmd: part: Add 'number' sub-command
>   disk: part: Extend API to get partition info
>   common: Implement A/B metadata
>   cmd: Add 'ab_select' command
>   test/py: Add base test case for A/B updates
>   doc: android: Add simple guide for A/B updates
>   env: am57xx: Implement A/B boot process
>
>  cmd/Kconfig                   |  15 +++
>  cmd/Makefile                  |   1 +
>  cmd/ab_select.c               |  52 ++++++++
>  cmd/part.c                    |  16 ++-
>  common/Kconfig                |  10 ++
>  common/Makefile               |   1 +
>  common/android_ab.c           | 290 ++++++++++++++++++++++++++++++++++++++++++
>  configs/sandbox_defconfig     |   2 +
>  disk/part.c                   |  68 ++++++++++
>  doc/README.android-ab         |  67 ++++++++++
>  include/android_ab.h          |  34 +++++
>  include/android_bl_msg.h      | 169 ++++++++++++++++++++++++
>  include/environment/ti/boot.h |  58 ++++++++-
>  include/part.h                |  21 +++
>  test/py/tests/test_ab.py      |  74 +++++++++++
>  15 files changed, 871 insertions(+), 7 deletions(-)
>  create mode 100644 cmd/ab_select.c
>  create mode 100644 common/android_ab.c
>  create mode 100644 doc/README.android-ab
>  create mode 100644 include/android_ab.h
>  create mode 100644 include/android_bl_msg.h
>  create mode 100644 test/py/tests/test_ab.py
>
> --
> 2.7.4
>

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

Hello Igor,

Thanks for the series. Some questions below.

First, my understanding is that the patches replace the deprecated
libavb_ab and make it trully obsolete, i.e. there should be no need to
import libavb_ab into U-Boot (unlike some of our suppliers still do).
Can you please confirm?

On Mon, Feb 18, 2019 at 5:25 PM Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
>
[..]

> 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

I won't object on it and it's not my purpose to do any
teaching/mentoring, but I think it makes sense to decouple (i.e.
allocate standalone commits for) these two activities:
 - integration of external headers/libraries (e.g. libavb, dtc,
headers imported from linux/avb/recovery/etc trees)
 - in-tree U-Boot development around those headers/libraries

I think mixing these two activities creates more overhead for the
reviewers, so it's easy for various mistakes to slip in unnoticed. See
next comment as example.

> + * Commit: 8b309f6970ab3b7c53cc529c51a2cb44e1c7a7e1

The contents of out-of-tree "bootloader_message.h" at this commit
doesn't appear to contain any "Omaha" references:
https://android.googlesource.com/platform/bootable/recovery.git/+/8b309f6970ab3b/bootloader_message/include/bootloader_message/bootloader_message.h

The addition of "Omaha" comment is done in commit:
$ git log --oneline -G Omaha  8b309f6970ab..recovery/master --
bootloader_message/include/bootloader_message/bootloader_message.h
7191bf049216 Add update_channel field to bootloader_message_ab.

So, I believe there is a mismatch between the contents of the newly
created file and its documented version.

[..]

> +       u8 priority : 4;
> +       /* Number of times left attempting to boot this slot */
> +       u8 tries_remaining : 3;
> +       /* 1 if this slot has booted successfully, 0 otherwise */
> +       u8 successful_boot : 1;

The s/uint8_t/u8/ and s/uint32_t/u32/ conversion creates noise
comparing the in-tree versus out-of-tree files and will add some
overhead during integration. I still see a lot of U-Boot code saying
uint8_t/uint32_t, so I wonder if these can be preserved. BTW, some of
the patches from this series add code using uint32_t.

Looking forward to pick the patches from mainline!

Thanks and best regards,
Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Simon Glass]

Hi Igor,

On Mon, 18 Feb 2019 at 10:22, Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
>
> From: Ruslan Trofymenko <ruslan.trofymenko@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@linaro.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
> Reviewed-by: Sam Protsenko <semen.protsenko@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@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

[U-Boot] [PATCH v3 4/7] cmd: Add 'ab_select' command

[Simon Glass]

On Mon, 18 Feb 2019 at 09:22, Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
>
> From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
>
> For A/B system update support the Android boot process requires to send
> 'androidboot.slot_suffix' parameter as a command line argument. This
> patch implementes 'ab_select' command which allows us to obtain current
> slot by processing the A/B metadata.
>
> The patch was extracted from commit [1] with one modification: the
> separator for specifying the name of metadata partition was changed
> from ';' to '#', because ';' is used for commands separation.
>
> [1] https://android-review.googlesource.com/c/platform/external/u-boot/+/729880/2
>
> Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
> Reviewed-by: Alistair Strachan <astrachan@google.com>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v3: None
> Changes in v2:
>   * 'android_ab_select' command is renamed to 'ab_select' command
>   * command is moved to the separate 'Android support commands' menu
>
>  cmd/Kconfig     | 15 +++++++++++++++
>  cmd/Makefile    |  1 +
>  cmd/ab_select.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+)
>  create mode 100644 cmd/ab_select.c

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3 7/7] env: am57xx: Implement A/B boot process

[Simon Glass]

On Mon, 18 Feb 2019 at 09:22, Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
>
> From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
>
> Add support for A/B boot process on AM57xx based boards:
>
>   1. Define 'slot_suffix' variable (using 'ab_select' command)
>   2. Extend 'emmc_android_boot' boot command (add commands for A/B boot
>      process)
>
> 'ab_select' command is used to decide which slot should be used for
> booting up. A/B metadata resides in 'misc' partition.
>
> To activate the A/B boot process, the following config options must be
> set:
>
>     CONFIG_ANDROID_AB=y
>     CONFIG_CMD_AB_SELECT=y
>
> For successful A/B boot, the corresponding A/B infrastructure must be
> involved on Android side [1] (including mounting system as root), and
> disk must be partitioned accordingly.
>
> When A/B boot is enabled, there are some known limitations currently
> exist (not related to A/B patches, need to be implemented later):
>
>   1. The 'Verified Boot' sequence is not supported
>   2. dev path to system partition (system_a or system_b) is passed via
>      'bootargs' as 'root=' argument like 'root=/dev/mmcblk1p12', but
>      further we'll need to rework it with respect to dm-verity
>      requirements [2]
>
> In case when A/B partitions are not present in system (and A/B boot is
> enabled), boot up process will be terminated and next message will be
> shown:
>
>     "boot_a(b) partition not found"
>
> [1] https://source.android.com/devices/tech/ota/ab
> [2] https://source.android.com/devices/tech/ota/ab/ab_implement#kernel
>
> Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@linaro.org>
> Reviewed-by: Alistair Strachan <astrachan@google.com>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>
> Changes in v3: None
>
> Changes in v2:
> * Add changes related to command renaming (android_ab_select -> ab_select).
> * Slotted sections (e.g. system_a and system_b) are added to the
>   default sections if CONFIG_CMD_AB_SELECT flag is defined
> * Rebased on top of master
> * system partitions sizes increased to 1024 MiB (to be consistent with
>   recent changes to boot.h file)
>
>  include/environment/ti/boot.h | 58 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 52 insertions(+), 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3 1/7] cmd: part: Add 'number' sub-command

[Eugeniu Rosca]

Hi Igor,

On Mon, Feb 18, 2019 at 06:21:51PM +0200, Igor Opaniuk wrote:
> From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> 
> This sub-command serves for getting the partition index from
> partition name.

[..]

>  	CMD_PART_INFO_SIZE,
> +	CMD_PART_INFO_NUMBER

IMHO 's/number/index/' (when referring to the id of a partition) would
make code and documentation more clear since 'number' can be confused
with 'count' while 'index' is unequivocal.

Feel free to ignore it though, as the terms seem to be intermixed in
various places of Linux/U-Boot (my opinion still stands).

The review comment also applies to other patches from this series:
 - https://patchwork.ozlabs.org/patch/1044153/ ("[U-Boot,v3,2/7] disk:
   part: Extend API to get partition info")
 - https://patchwork.ozlabs.org/patch/1044162/ ("[U-Boot,v3,6/7] doc:
   android: Add simple guide for A/B updates")
 - https://patchwork.ozlabs.org/patch/1044160/ ("[U-Boot,v3,7/7] env:
   am57xx: Implement A/B boot process")

Best regards,
Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

FWIW, below are some dangling struct name references due to:
 - s/bootloader_message/andr_bl_msg/
 - s/bootloader_message_ab/andr_bl_msg_ab/
 - s/bootloader_control/andr_bl_control/

On Mon, Feb 18, 2019 at 5:25 PM Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
[..]
> + * uncrypt. Move it into struct bootloader_message_ab to avoid the
[..]
> +        * 1184-byte so that the entire bootloader_message struct rounds up
[..]
> + * stays after struct bootloader_message, which should be managed by
[..]
> + * not mandatory, to use 'struct bootloader_control' described below.
[..]
> + * be put in the 'slot_suffix' field of the 'bootloader_message'
[..]
> + * 'bootloader_control' structure to store the A/B metadata, but not

Best regards,
Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

jFYI/FWIW, AOSP U-Boot [1] seems to currently import the bootloader
message header [2] at least twice:
 - as include/android_bl_msg.h via
   https://android.googlesource.com/platform/external/u-boot/+/86a4b492b5db%5E!/
 - as include/android_bootloader_message.h via
   https://android.googlesource.com/platform/external/u-boot/+/c7f85c5f75f9%5E!/

[1] https://android.googlesource.com/platform/external/u-boot
[2] bootable/recovery/bootloader_message/include/bootloader_message/bootloader_message.h

> Best regards,
> Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

I received the following bounce:

> Thank you for your email.
>
> Igor Opaniuk no longer works for Linaro.
>
> If your email is related to Linaro business, please use the Contact
> form (https://www.linaro.org/contact/) if you do not have another
> Linaro email address to use.

I hope this is a glitch? Otherwise, any suggestions how to proceed here?

Thanks,
Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Bajjuri, Praneeth]

Eugeniu,

On 3/18/2019 3:21 PM, Eugeniu Rosca wrote:
> I received the following bounce:
>
>> Thank you for your email.
>>
>> Igor Opaniuk no longer works for Linaro.
>>
>> If your email is related to Linaro business, please use the Contact
>> form (https://www.linaro.org/contact/) if you do not have another
>> Linaro email address to use.
> I hope this is a glitch? Otherwise, any suggestions how to proceed here?


Not a glitch, Igor's linaro email id is no longer active. He might 
respond with updated email
address soon.


>
> Thanks,
> Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

Hi Praneeth, Igor,

On Mon, Mar 18, 2019 at 04:20:16PM -0500, Bajjuri, Praneeth wrote:
> Eugeniu,
[..]
> Not a glitch, Igor's linaro email id is no longer active. He might respond
> with updated email
> address soon.

Great news. Looking forward for v4 of this series.

Many thanks,
Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Igor Opaniuk]

Hi Eugeniu,

On Fri, Mar 8, 2019 at 7:29 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
> Hello Igor,
>
> Thanks for the series. Some questions below.
>
> First, my understanding is that the patches replace the deprecated
> libavb_ab and make it trully obsolete, i.e. there should be no need to
> import libavb_ab into U-Boot (unlike some of our suppliers still do).
> Can you please confirm?

That's correct. Currently there is no any integration with AVB yet,
but it's planned to be the next step when these patches are merged.

>
> On Mon, Feb 18, 2019 at 5:25 PM Igor Opaniuk <igor.opaniuk@linaro.org> wrote:
> >
> [..]
>
> > 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
>
> I won't object on it and it's not my purpose to do any
> teaching/mentoring, but I think it makes sense to decouple (i.e.
> allocate standalone commits for) these two activities:
>  - integration of external headers/libraries (e.g. libavb, dtc,
> headers imported from linux/avb/recovery/etc trees)
>  - in-tree U-Boot development around those headers/libraries
>
> I think mixing these two activities creates more overhead for the
> reviewers, so it's easy for various mistakes to slip in unnoticed. See
> next comment as example.
>
> > + * Commit: 8b309f6970ab3b7c53cc529c51a2cb44e1c7a7e1
>
> The contents of out-of-tree "bootloader_message.h" at this commit
> doesn't appear to contain any "Omaha" references:
> https://android.googlesource.com/platform/bootable/recovery.git/+/8b309f6970ab3b/bootloader_message/include/bootloader_message/bootloader_message.h
>
> The addition of "Omaha" comment is done in commit:
> $ git log --oneline -G Omaha  8b309f6970ab..recovery/master --
> bootloader_message/include/bootloader_message/bootloader_message.h
> 7191bf049216 Add update_channel field to bootloader_message_ab.
>
> So, I believe there is a mismatch between the contents of the newly
> created file and its documented version.

Totally agree. These patch-series were initially introduced (v1 and
v2) by Ruslan (who was actually the main author),
so unfortunately I'm barely aware why in these particular commit both
external headers/libraries and in-tree U-boot stuff are mixed.
Will be fixed in v4.

>
> [..]
>
> > +       u8 priority : 4;
> > +       /* Number of times left attempting to boot this slot */
> > +       u8 tries_remaining : 3;
> > +       /* 1 if this slot has booted successfully, 0 otherwise */
> > +       u8 successful_boot : 1;
>
> The s/uint8_t/u8/ and s/uint32_t/u32/ conversion creates noise
> comparing the in-tree versus out-of-tree files and will add some
> overhead during integration. I still see a lot of U-Boot code saying
> uint8_t/uint32_t, so I wonder if these can be preserved. BTW, some of
> the patches from this series add code using uint32_t.

Agree.

>
> Looking forward to pick the patches from mainline!
>
> Thanks and best regards,
> Eugeniu.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

Thanks for the review!

[U-Boot] [PATCH v3 1/7] cmd: part: Add 'number' sub-command

[Igor Opaniuk]

Hi Eugeniu,


On Mon, Mar 11, 2019 at 12:42 AM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
>
> Hi Igor,
>
> On Mon, Feb 18, 2019 at 06:21:51PM +0200, Igor Opaniuk wrote:
> > From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> >
> > This sub-command serves for getting the partition index from
> > partition name.
>
> [..]
>
> >       CMD_PART_INFO_SIZE,
> > +     CMD_PART_INFO_NUMBER
>
> IMHO 's/number/index/' (when referring to the id of a partition) would
> make code and documentation more clear since 'number' can be confused
> with 'count' while 'index' is unequivocal.
>
> Feel free to ignore it though, as the terms seem to be intermixed in
> various places of Linux/U-Boot (my opinion still stands).

Agree, will be fixed in v4.

> The review comment also applies to other patches from this series:
>  - https://patchwork.ozlabs.org/patch/1044153/ ("[U-Boot,v3,2/7] disk:
>    part: Extend API to get partition info")
>  - https://patchwork.ozlabs.org/patch/1044162/ ("[U-Boot,v3,6/7] doc:
>    android: Add simple guide for A/B updates")
>  - https://patchwork.ozlabs.org/patch/1044160/ ("[U-Boot,v3,7/7] env:
>    am57xx: Implement A/B boot process")
>
> Best regards,
> Eugeniu.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

Hello Igor, All,

On Thu, Mar 21, 2019 at 05:39:36PM +0200, Igor Opaniuk wrote:
> Hi Eugeniu,
> 
> On Fri, Mar 8, 2019 at 7:29 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
[..]
> >
> > The s/uint8_t/u8/ and s/uint32_t/u32/ conversion creates noise
> > comparing the in-tree versus out-of-tree files and will add some
> > overhead during integration. I still see a lot of U-Boot code saying
> > uint8_t/uint32_t, so I wonder if these can be preserved. BTW, some of
> > the patches from this series add code using uint32_t.
> 
> Agree.

Igor, many thanks for the recent replies.

WRT preserving the contents of the original bootloader_message.h from
https://android.googlesource.com/platform/bootable/recovery.git/ , I
wonder what's the chance of keeping the CPP comments in place when
importing the file in-tree, since it will make the diffs and subsequent
integration/updates easier from the source repository. Furthermore, we
don't expect any U-Boot specific development to be done in this file
in-tree. It should purely reflect the upstream state.

I wonder if there is any official position regarding that from the
U-Boot maintainers?

FTR/FWIW, there are around 24 .c/.h files in U-Boot master, carrying CPP
comments (SDPX identifiers carefully excluded):

$ git grep "^\s*//\s" -- "*.c" "*.h" | grep -v "SPDX" | awk '{print $1}' | sort -u | cut -d ':' -f 1
arch/arm/mach-tegra/board2.c
arch/riscv/include/asm/encoding.h
arch/xtensa/include/asm/arch-dc233c/tie-asm.h
arch/xtensa/include/asm/arch-de212/tie-asm.h
board/freescale/lx2160a/eth_lx2160aqds.c
board/renesas/blanche/qos.c
board/xilinx/zynq/zynq-microzed/ps7_init_gpl.c
board/xilinx/zynq/zynq-zc702/ps7_init_gpl.c
board/xilinx/zynq/zynq-zc706/ps7_init_gpl.c
board/xilinx/zynq/zynq-zed/ps7_init_gpl.c
drivers/video/stb_truetype.h
drivers/video/stb_truetype.h
fs/ubifs/ubifs.h
include/efi_api.h
include/linux/mtd/flashchip.h
include/linux/mtd/mtd.h
lib/efi_loader/efi_hii.c
lib/efi_selftest/efi_selftest_crc32.c
lib/efi_selftest/efi_selftest_hii_data.c
lib/libavb/avb_slot_verify.c
scripts/kconfig/expr.c
scripts/kconfig/gconf.c
scripts/kconfig/qconf.h
tools/mingw_support.c

> 
> > Thanks and best regards,
> > Eugeniu.

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Tom Rini]

On Thu, Mar 28, 2019 at 05:30:30PM +0100, Eugeniu Rosca wrote:
> Hello Igor, All,
> 
> On Thu, Mar 21, 2019 at 05:39:36PM +0200, Igor Opaniuk wrote:
> > Hi Eugeniu,
> > 
> > On Fri, Mar 8, 2019 at 7:29 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> [..]
> > >
> > > The s/uint8_t/u8/ and s/uint32_t/u32/ conversion creates noise
> > > comparing the in-tree versus out-of-tree files and will add some
> > > overhead during integration. I still see a lot of U-Boot code saying
> > > uint8_t/uint32_t, so I wonder if these can be preserved. BTW, some of
> > > the patches from this series add code using uint32_t.
> > 
> > Agree.
> 
> Igor, many thanks for the recent replies.
> 
> WRT preserving the contents of the original bootloader_message.h from
> https://android.googlesource.com/platform/bootable/recovery.git/ , I
> wonder what's the chance of keeping the CPP comments in place when
> importing the file in-tree, since it will make the diffs and subsequent
> integration/updates easier from the source repository. Furthermore, we
> don't expect any U-Boot specific development to be done in this file
> in-tree. It should purely reflect the upstream state.
> 
> I wonder if there is any official position regarding that from the
> U-Boot maintainers?

Yes, for stuff that we're importing from another place, and really
really aren't touching otherwise, we should avoid changing it.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190328/389df801/attachment.sig>

[U-Boot] [PATCH v3 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

Hello,

On Thu, Mar 28, 2019 at 03:58:48PM -0400, Tom Rini wrote:
> On Thu, Mar 28, 2019 at 05:30:30PM +0100, Eugeniu Rosca wrote:
> > Hello Igor, All,
> > 
> > On Thu, Mar 21, 2019 at 05:39:36PM +0200, Igor Opaniuk wrote:
> > > Hi Eugeniu,
> > > 
> > > On Fri, Mar 8, 2019 at 7:29 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> > [..]
> > > >
> > > > The s/uint8_t/u8/ and s/uint32_t/u32/ conversion creates noise
> > > > comparing the in-tree versus out-of-tree files and will add some
> > > > overhead during integration. I still see a lot of U-Boot code saying
> > > > uint8_t/uint32_t, so I wonder if these can be preserved. BTW, some of
> > > > the patches from this series add code using uint32_t.
> > > 
> > > Agree.
> > 
> > Igor, many thanks for the recent replies.
> > 
> > WRT preserving the contents of the original bootloader_message.h from
> > https://android.googlesource.com/platform/bootable/recovery.git/ , I
> > wonder what's the chance of keeping the CPP comments in place when
> > importing the file in-tree, since it will make the diffs and subsequent
> > integration/updates easier from the source repository. Furthermore, we
> > don't expect any U-Boot specific development to be done in this file
> > in-tree. It should purely reflect the upstream state.
> > 
> > I wonder if there is any official position regarding that from the
> > U-Boot maintainers?
> 
> Yes, for stuff that we're importing from another place, and really
> really aren't touching otherwise, we should avoid changing it.

Tom, thanks for your prompt feedback.

Igor, unless you have any objections/comments, would you kindly leave
the CPP comments in the next revision of "bootloader_message.h", so
that we minimize the future integration/back-porting efforts?

I would also re-iterate on the need to import the header file in a
standalone commit, since certain users need it for implementing
A/B-unrelated use-cases/features (e.g. accessing A/B-unrelated
fields of the BCB, like boot reason [1]).

[1] https://source.android.com/devices/bootloader/boot-reason

Best regards,
Eugeniu.

[U-Boot] [PATCH v3 1/7] cmd: part: Add 'number' sub-command

[Igor Opaniuk]

+ Roman


On Thu, Mar 21, 2019 at 5:42 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> Hi Eugeniu,
>
>
> On Mon, Mar 11, 2019 at 12:42 AM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> >
> >
> > Hi Igor,
> >
> > On Mon, Feb 18, 2019 at 06:21:51PM +0200, Igor Opaniuk wrote:
> > > From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> > >
> > > This sub-command serves for getting the partition index from
> > > partition name.
> >
> > [..]
> >
> > >       CMD_PART_INFO_SIZE,
> > > +     CMD_PART_INFO_NUMBER
> >
> > IMHO 's/number/index/' (when referring to the id of a partition) would
> > make code and documentation more clear since 'number' can be confused
> > with 'count' while 'index' is unequivocal.
> >
> > Feel free to ignore it though, as the terms seem to be intermixed in
> > various places of Linux/U-Boot (my opinion still stands).
>
> Agree, will be fixed in v4.
>
> > The review comment also applies to other patches from this series:
> >  - https://patchwork.ozlabs.org/patch/1044153/ ("[U-Boot,v3,2/7] disk:
> >    part: Extend API to get partition info")
> >  - https://patchwork.ozlabs.org/patch/1044162/ ("[U-Boot,v3,6/7] doc:
> >    android: Add simple guide for A/B updates")
> >  - https://patchwork.ozlabs.org/patch/1044160/ ("[U-Boot,v3,7/7] env:
> >    am57xx: Implement A/B boot process")
> >
> > Best regards,
> > Eugeniu.
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot



--
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk at gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[U-Boot] [PATCH v3 1/7] cmd: part: Add 'number' sub-command

[Roman Stratiienko]

Are there any updates of this patch?

We need this functionality to retrieve UUID of the partition using it's name.
But there are lot of commands that supports only partition index as an
argument, so I assume it could be very useful there.
This command can be also used alone can be used to find root partition, e.g.:
gpt index mmc 0 system system_index
setenv bootargs="... root=/dev/mmcblk0p$system_index"

On Mon, May 20, 2019 at 4:27 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> + Roman
>
>
> On Thu, Mar 21, 2019 at 5:42 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> >
> > Hi Eugeniu,
> >
> >
> > On Mon, Mar 11, 2019 at 12:42 AM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> > >
> > >
> > > Hi Igor,
> > >
> > > On Mon, Feb 18, 2019 at 06:21:51PM +0200, Igor Opaniuk wrote:
> > > > From: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> > > >
> > > > This sub-command serves for getting the partition index from
> > > > partition name.
> > >
> > > [..]
> > >
> > > >       CMD_PART_INFO_SIZE,
> > > > +     CMD_PART_INFO_NUMBER
> > >
> > > IMHO 's/number/index/' (when referring to the id of a partition) would
> > > make code and documentation more clear since 'number' can be confused
> > > with 'count' while 'index' is unequivocal.
> > >
> > > Feel free to ignore it though, as the terms seem to be intermixed in
> > > various places of Linux/U-Boot (my opinion still stands).
> >
> > Agree, will be fixed in v4.
> >
> > > The review comment also applies to other patches from this series:
> > >  - https://patchwork.ozlabs.org/patch/1044153/ ("[U-Boot,v3,2/7] disk:
> > >    part: Extend API to get partition info")
> > >  - https://patchwork.ozlabs.org/patch/1044162/ ("[U-Boot,v3,6/7] doc:
> > >    android: Add simple guide for A/B updates")
> > >  - https://patchwork.ozlabs.org/patch/1044160/ ("[U-Boot,v3,7/7] env:
> > >    am57xx: Implement A/B boot process")
> > >
> > > Best regards,
> > > Eugeniu.
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot
>
>
>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opaniuk at gmail.com
> skype: igor.opanyuk
> +380 (93) 836 40 67
> http://ua.linkedin.com/in/iopaniuk



-- 
Best regards,
Roman Stratiienko
Global Logic
ADIT Team