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

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

[Ruslan Trofymenko]

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_ANDROID_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.

[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 'android_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                          |  11 ++
 cmd/Makefile                         |   1 +
 cmd/android_ab_select.c              |  53 +++++++
 cmd/part.c                           |  16 +-
 common/Kconfig                       |  10 ++
 common/Makefile                      |   1 +
 common/android_ab.c                  | 278 +++++++++++++++++++++++++++++++++++
 configs/sandbox_defconfig            |   2 +
 disk/part.c                          |  68 +++++++++
 doc/README.android-ab                |  67 +++++++++
 include/android_ab.h                 |  34 +++++
 include/android_bootloader_message.h | 164 +++++++++++++++++++++
 include/environment/ti/boot.h        |  44 +++++-
 include/part.h                       |  21 +++
 test/py/tests/test_ab.py             |  74 ++++++++++
 15 files changed, 839 insertions(+), 5 deletions(-)
 create mode 100644 cmd/android_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_bootloader_message.h
 create mode 100644 test/py/tests/test_ab.py

-- 
2.7.4

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

[Ruslan Trofymenko]

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>
---
 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 2/7] disk: part: Extend API to get partition info

[Ruslan Trofymenko]

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>
---
 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..ad4239d 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 -1 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 -1;
+
+	dev_num = simple_strtoul(dev_part_str, &ep, 16);
+	if (ep != part_str) {
+		/* Not all the first part before the # was parsed. */
+		return -1;
+	}
+	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 -1;
+	}
+	if (part_get_info_by_name(*dev_desc, part_str, part_info) < 0) {
+		printf("Could not find \"%s\" partition\n", part_str);
+		return -1;
+	}
+	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 -1;
+	}
+	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 0750aee..331b3d5 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 -1 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 3/7] common: Implement A/B metadata

[Ruslan Trofymenko]

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>
---
 common/Kconfig                       |  10 ++
 common/Makefile                      |   1 +
 common/android_ab.c                  | 278 +++++++++++++++++++++++++++++++++++
 include/android_ab.h                 |  34 +++++
 include/android_bootloader_message.h | 164 +++++++++++++++++++++
 5 files changed, 487 insertions(+)
 create mode 100644 common/android_ab.c
 create mode 100644 include/android_ab.h
 create mode 100644 include/android_bootloader_message.h

diff --git a/common/Kconfig b/common/Kconfig
index 57bd16d..0ff4679 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -748,6 +748,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 88079d1..a1a252c 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -104,6 +104,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..25daba7
--- /dev/null
+++ b/common/android_ab.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ */
+
+#include <android_ab.h>
+
+#include <android_bootloader_message.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 android_bootloader_control *abc)
+{
+	return crc32(0, (void *)abc, offsetof(typeof(*abc), crc32_le));
+}
+
+/**
+ * Initialize android_bootloader_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 void ab_control_default(struct android_bootloader_control *abc)
+{
+	int i;
+	const struct android_slot_metadata metadata = {
+		.priority = 15,
+		.tries_remaining = 7,
+		.successful_boot = 0,
+		.verity_corrupted = 0,
+		.reserved = 0
+	};
+
+	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);
+}
+
+/**
+ * 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
+ * @return boot_control loaded from disk or NULL on error
+ */
+static struct android_bootloader_control *ab_control_create_from_disk(
+		struct blk_desc *dev_desc,  const disk_partition_t *part_info)
+{
+	ulong abc_offset, abc_blocks;
+	struct android_bootloader_control *abc;
+
+	abc_offset = offsetof(struct android_bootloader_message_ab,
+			      slot_suffix);
+	if (abc_offset % part_info->blksz) {
+		printf("ANDROID: Boot control block not block aligned.\n");
+		return NULL;
+	}
+	abc_offset /= part_info->blksz;
+
+	abc_blocks = DIV_ROUND_UP(sizeof(struct android_bootloader_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 NULL;
+	}
+	abc = malloc_cache_aligned(abc_blocks * part_info->blksz);
+	if (!abc)
+		return NULL;
+
+	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 NULL;
+	}
+	debug("ANDROID: Loaded ABC, %lu blocks.\n", abc_blocks);
+	return abc;
+}
+
+/**
+ * Store the loaded boot_control block.
+ *
+ * Store back to the same location it was read from with
+ * ab_control_create_from_misc().
+ *
+ * @param[in] abc Pointer to the boot_control struct and the extra bytes after
+ *                it up to the nearest block boundary
+ * @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
+ * @return 0 on success and -1 on error
+ */
+static int ab_control_store(struct android_bootloader_control *abc,
+			    struct blk_desc *dev_desc,
+			    const disk_partition_t *part_info)
+{
+	ulong abc_offset, abc_blocks;
+
+	abc_offset = offsetof(struct android_bootloader_message_ab,
+			      slot_suffix) / part_info->blksz;
+	abc_blocks = DIV_ROUND_UP(sizeof(struct android_bootloader_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 -1;
+	}
+	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 android_slot_metadata *a,
+			    const struct android_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 android_bootloader_control *abc;
+	u32 crc32_le;
+	int slot, i;
+	bool store_needed = false;
+	char slot_suffix[4];
+
+	abc = ab_control_create_from_disk(dev_desc, part_info);
+	if (!abc) {
+		/*
+		 * 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 -1;
+	}
+
+	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");
+		ab_control_default(abc);
+		store_needed = true;
+	}
+
+	if (abc->magic != ANDROID_BOOT_CTRL_MAGIC) {
+		printf("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
+		free(abc);
+		return -1;
+	}
+
+	if (abc->version > ANDROID_BOOT_CTRL_VERSION) {
+		printf("ANDROID: Unsupported A/B metadata version: %.8x\n",
+		       abc->version);
+		free(abc);
+		return -1;
+	}
+
+	/*
+	 * 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) {
+			debug("ANDROID: unbootable slot %d tries: %d, ",
+			      i, abc->slot_info[i].tries_remaining);
+			debug("corrupt: %d\n",
+			      abc->slot_info[i].verity_corrupted);
+			continue;
+		}
+		debug("ANDROID: bootable slot %d pri: %d, tries: %d, ",
+		      i, abc->slot_info[i].priority,
+		      abc->slot_info[i].tries_remaining);
+		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(abc, dev_desc, part_info);
+	}
+	free(abc);
+
+	if (slot < 0)
+		return -1;
+
+	return slot;
+}
diff --git a/include/android_ab.h b/include/android_ab.h
new file mode 100644
index 0000000..53f6dc6
--- /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 -1 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_bootloader_message.h b/include/android_bootloader_message.h
new file mode 100644
index 0000000..1f19273
--- /dev/null
+++ b/include/android_bootloader_message.h
@@ -0,0 +1,164 @@
+/* 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_BOOTLOADER_MESSAGE_H
+#define __ANDROID_BOOTLOADER_MESSAGE_H
+
+/*
+ * compiler.h defines the types that otherwise are included from stdint.h and
+ * stddef.h
+ */
+#include <compiler.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.
+ */
+static const size_t ANDROID_BOOTLOADER_MESSAGE_OFFSET_IN_MISC;
+static const size_t ANDROID_WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;
+
+/*
+ * 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 android_bootloader_message {
+	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.
+ */
+struct android_bootloader_message_ab {
+	struct android_bootloader_message message;
+	char slot_suffix[32];
+
+	/* Round up the entire struct to 4096-byte */
+	char reserved[2016];
+};
+
+#define ANDROID_BOOT_CTRL_MAGIC   0x42414342 /* Bootloader Control AB */
+#define ANDROID_BOOT_CTRL_VERSION 1
+
+struct android_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 android_bootloader_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 android_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_BOOTLOADER_MESSAGE_H */
-- 
2.7.4

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

[Ruslan Trofymenko]

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 'android_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>
---
 cmd/Kconfig             | 11 ++++++++++
 cmd/Makefile            |  1 +
 cmd/android_ab_select.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)
 create mode 100644 cmd/android_ab_select.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index e2973b3..e96a5e4 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1476,6 +1476,17 @@ config CMD_UUID
 	  The two commands are very similar except for the endianness of the
 	  output.
 
+config CMD_ANDROID_AB_SELECT
+	bool "android_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
 
 source "cmd/ti/Kconfig"
diff --git a/cmd/Makefile b/cmd/Makefile
index 5ec2f9e..23a78c0 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_ANDROID_AB_SELECT) += android_ab_select.o
 obj-$(CONFIG_CMD_ADC) += adc.o
 obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
 obj-y += blk_common.o
diff --git a/cmd/android_ab_select.c b/cmd/android_ab_select.c
new file mode 100644
index 0000000..7966361
--- /dev/null
+++ b/cmd/android_ab_select.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ */
+
+#include <android_ab.h>
+#include <command.h>
+
+static int do_android_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(android_ab_select, 4, 0, do_android_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 5/7] test/py: Add base test case for A/B updates

[Ruslan Trofymenko]

Add sandbox test for 'android_ab_select' command.

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

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
---
 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 5a744f4..f246f89 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
@@ -61,6 +62,7 @@ CONFIG_CMD_TIME=y
 CONFIG_CMD_TIMER=y
 CONFIG_CMD_SOUND=y
 CONFIG_CMD_QFW=y
+CONFIG_CMD_ANDROID_AB_SELECT=y
 CONFIG_CMD_BOOTSTAGE=y
 CONFIG_CMD_PMIC=y
 CONFIG_CMD_REGULATOR=y
diff --git a/test/py/tests/test_ab.py b/test/py/tests/test_ab.py
new file mode 100644
index 0000000..f27538e
--- /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_android_ab_select')
+ at pytest.mark.requiredtool('sgdisk')
+def test_ab(ab_disk_image, u_boot_console):
+    """Test the 'android_ab_select' command."""
+
+    u_boot_console.run_command('host bind 0 ' + ab_disk_image.path)
+
+    output = u_boot_console.run_command('android_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 'a' in output
+
+    output = u_boot_console.run_command('android_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 'b' in output
-- 
2.7.4

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

[Ruslan Trofymenko]

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

Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
---
 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..230088c
--- /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_ANDROID_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 'android_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
+-------------
+
+    android_ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
+
+for example:
+
+    => android_ab_select slot_name mmc 1:4
+
+or
+
+    => android_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 7/7] env: am57xx: Implement A/B boot process

[Ruslan Trofymenko]

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

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

'android_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_ANDROID_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>
---
 include/environment/ti/boot.h | 44 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/include/environment/ti/boot.h b/include/environment/ti/boot.h
index 3c9c87f..3ace316 100644
--- a/include/environment/ti/boot.h
+++ b/include/environment/ti/boot.h
@@ -63,6 +63,36 @@
 #define AVB_VERIFY_CMD ""
 #endif
 
+#define CONTROL_PARTITION "misc"
+
+#if defined(CONFIG_CMD_ANDROID_AB_SELECT)
+#define AB_SELECT \
+	"if part number mmc 1 " CONTROL_PARTITION " control_part_number; " \
+	"then " \
+		"echo " CONTROL_PARTITION \
+			" partition number:${control_part_number};" \
+		"android_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" \
@@ -91,10 +121,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 0/7] android: Implement A/B boot process

[Alistair Strachan]

On Tue, Nov 27, 2018 at 11:57 AM Ruslan Trofymenko
<ruslan.trofymenko@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_ANDROID_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.
>
> [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 'android_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                          |  11 ++
>  cmd/Makefile                         |   1 +
>  cmd/android_ab_select.c              |  53 +++++++
>  cmd/part.c                           |  16 +-
>  common/Kconfig                       |  10 ++
>  common/Makefile                      |   1 +
>  common/android_ab.c                  | 278 +++++++++++++++++++++++++++++++++++
>  configs/sandbox_defconfig            |   2 +
>  disk/part.c                          |  68 +++++++++
>  doc/README.android-ab                |  67 +++++++++
>  include/android_ab.h                 |  34 +++++
>  include/android_bootloader_message.h | 164 +++++++++++++++++++++
>  include/environment/ti/boot.h        |  44 +++++-
>  include/part.h                       |  21 +++
>  test/py/tests/test_ab.py             |  74 ++++++++++
>  15 files changed, 839 insertions(+), 5 deletions(-)
>  create mode 100644 cmd/android_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_bootloader_message.h
>  create mode 100644 test/py/tests/test_ab.py

Looks good, thanks Ruslan!

Reviewed-by: Alistair Strachan <astrachan@google.com>

>
> --
> 2.7.4
>

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

[Sam Protsenko]

Hi Ruslan,

On Tue, Nov 27, 2018 at 9:57 PM Ruslan Trofymenko
<ruslan.trofymenko@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_ANDROID_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.
>
> [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 'android_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                          |  11 ++
>  cmd/Makefile                         |   1 +
>  cmd/android_ab_select.c              |  53 +++++++
>  cmd/part.c                           |  16 +-
>  common/Kconfig                       |  10 ++
>  common/Makefile                      |   1 +
>  common/android_ab.c                  | 278 +++++++++++++++++++++++++++++++++++
>  configs/sandbox_defconfig            |   2 +
>  disk/part.c                          |  68 +++++++++
>  doc/README.android-ab                |  67 +++++++++
>  include/android_ab.h                 |  34 +++++
>  include/android_bootloader_message.h | 164 +++++++++++++++++++++
>  include/environment/ti/boot.h        |  44 +++++-
>  include/part.h                       |  21 +++
>  test/py/tests/test_ab.py             |  74 ++++++++++
>  15 files changed, 839 insertions(+), 5 deletions(-)
>  create mode 100644 cmd/android_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_bootloader_message.h
>  create mode 100644 test/py/tests/test_ab.py
>
> --
> 2.7.4
>

For the whole series:
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

Thanks.

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

[Simon Glass]

On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
<ruslan.trofymenko@linaro.org> wrote:
>
> 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>
> ---
>  cmd/part.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)

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

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

[Simon Glass]

Hi Ruslan,

On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
<ruslan.trofymenko@linaro.org> wrote:
>
> 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>
> ---
>  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..ad4239d 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 -1 on error

Can you please return an error like -EINVAL? -1 is defined as -EPERM
whcih seems wrong.

Regards,
Simon

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

[Simon Glass]

Hi Ruslan,

On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
<ruslan.trofymenko@linaro.org> wrote:
>
> 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>
> ---
>  common/Kconfig                       |  10 ++
>  common/Makefile                      |   1 +
>  common/android_ab.c                  | 278 +++++++++++++++++++++++++++++++++++
>  include/android_ab.h                 |  34 +++++
>  include/android_bootloader_message.h | 164 +++++++++++++++++++++
>  5 files changed, 487 insertions(+)
>  create mode 100644 common/android_ab.c
>  create mode 100644 include/android_ab.h
>  create mode 100644 include/android_bootloader_message.h
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 57bd16d..0ff4679 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -748,6 +748,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 88079d1..a1a252c 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -104,6 +104,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..25daba7
> --- /dev/null
> +++ b/common/android_ab.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Copyright (C) 2017 The Android Open Source Project
> + */
> +
> +#include <android_ab.h>

Please put common.h first

> +
> +#include <android_bootloader_message.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 android_bootloader_control *abc)
> +{
> +       return crc32(0, (void *)abc, offsetof(typeof(*abc), crc32_le));
> +}
> +
> +/**
> + * Initialize android_bootloader_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 void ab_control_default(struct android_bootloader_control *abc)
> +{
> +       int i;
> +       const struct android_slot_metadata metadata = {
> +               .priority = 15,
> +               .tries_remaining = 7,
> +               .successful_boot = 0,
> +               .verity_corrupted = 0,
> +               .reserved = 0
> +       };
> +
> +       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);
> +}
> +
> +/**
> + * 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
> + * @return boot_control loaded from disk or NULL on error

You have different error types in this function. Can you change it to
return an int?

> + */
> +static struct android_bootloader_control *ab_control_create_from_disk(
> +               struct blk_desc *dev_desc,  const disk_partition_t *part_info)
> +{
> +       ulong abc_offset, abc_blocks;
> +       struct android_bootloader_control *abc;
> +
> +       abc_offset = offsetof(struct android_bootloader_message_ab,
> +                             slot_suffix);
> +       if (abc_offset % part_info->blksz) {
> +               printf("ANDROID: Boot control block not block aligned.\n");
> +               return NULL;
> +       }
> +       abc_offset /= part_info->blksz;
> +
> +       abc_blocks = DIV_ROUND_UP(sizeof(struct android_bootloader_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 NULL;
> +       }
> +       abc = malloc_cache_aligned(abc_blocks * part_info->blksz);
> +       if (!abc)
> +               return NULL;
> +
> +       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 NULL;
> +       }
> +       debug("ANDROID: Loaded ABC, %lu blocks.\n", abc_blocks);
> +       return abc;
> +}
> +
> +/**
> + * Store the loaded boot_control block.
> + *
> + * Store back to the same location it was read from with
> + * ab_control_create_from_misc().
> + *
> + * @param[in] abc Pointer to the boot_control struct and the extra bytes after
> + *                it up to the nearest block boundary
> + * @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
> + * @return 0 on success and -1 on error
> + */
> +static int ab_control_store(struct android_bootloader_control *abc,
> +                           struct blk_desc *dev_desc,
> +                           const disk_partition_t *part_info)
> +{
> +       ulong abc_offset, abc_blocks;
> +
> +       abc_offset = offsetof(struct android_bootloader_message_ab,
> +                             slot_suffix) / part_info->blksz;
> +       abc_blocks = DIV_ROUND_UP(sizeof(struct android_bootloader_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 -1;

Please use an errno error number

> +       }
> +       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 android_slot_metadata *a,
> +                           const struct android_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 android_bootloader_control *abc;
> +       u32 crc32_le;
> +       int slot, i;
> +       bool store_needed = false;
> +       char slot_suffix[4];
> +
> +       abc = ab_control_create_from_disk(dev_desc, part_info);
> +       if (!abc) {
> +               /*
> +                * 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 -1;
> +       }
> +
> +       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");
> +               ab_control_default(abc);
> +               store_needed = true;
> +       }
> +
> +       if (abc->magic != ANDROID_BOOT_CTRL_MAGIC) {
> +               printf("ANDROID: Unknown A/B metadata: %.8x\n", abc->magic);
> +               free(abc);
> +               return -1;
> +       }
> +
> +       if (abc->version > ANDROID_BOOT_CTRL_VERSION) {
> +               printf("ANDROID: Unsupported A/B metadata version: %.8x\n",
> +                      abc->version);
> +               free(abc);
> +               return -1;
> +       }
> +
> +       /*
> +        * 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) {
> +                       debug("ANDROID: unbootable slot %d tries: %d, ",
> +                             i, abc->slot_info[i].tries_remaining);
> +                       debug("corrupt: %d\n",
> +                             abc->slot_info[i].verity_corrupted);
> +                       continue;
> +               }
> +               debug("ANDROID: bootable slot %d pri: %d, tries: %d, ",
> +                     i, abc->slot_info[i].priority,
> +                     abc->slot_info[i].tries_remaining);

Consider using log_debug() with a suitable LOGC_... or UCLASS.

> +               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(abc, dev_desc, part_info);
> +       }
> +       free(abc);
> +
> +       if (slot < 0)
> +               return -1;
> +
> +       return slot;
> +}
> diff --git a/include/android_ab.h b/include/android_ab.h
> new file mode 100644
> index 0000000..53f6dc6
> --- /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 -1 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_bootloader_message.h b/include/android_bootloader_message.h
> new file mode 100644
> index 0000000..1f19273
> --- /dev/null
> +++ b/include/android_bootloader_message.h

Too long. How about android_bl_msg.h ?

> @@ -0,0 +1,164 @@
> +/* 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_BOOTLOADER_MESSAGE_H
> +#define __ANDROID_BOOTLOADER_MESSAGE_H
> +
> +/*
> + * compiler.h defines the types that otherwise are included from stdint.h and
> + * stddef.h
> + */
> +#include <compiler.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.
> + */
> +static const size_t ANDROID_BOOTLOADER_MESSAGE_OFFSET_IN_MISC;
> +static const size_t ANDROID_WIPE_PACKAGE_OFFSET_IN_MISC = 16 * 1024;

OMH way too long :-)

> +
> +/*
> + * 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 android_bootloader_message {

How about andr_bl_msg ? Similarly below


> +       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.
> + */
> +struct android_bootloader_message_ab {
> +       struct android_bootloader_message message;
> +       char slot_suffix[32];
> +
> +       /* Round up the entire struct to 4096-byte */
> +       char reserved[2016];
> +};
> +
> +#define ANDROID_BOOT_CTRL_MAGIC   0x42414342 /* Bootloader Control AB */
> +#define ANDROID_BOOT_CTRL_VERSION 1
> +
> +struct android_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 android_bootloader_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 android_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_BOOTLOADER_MESSAGE_H */
> --
> 2.7.4
>

Regards,
Simon

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

[Simon Glass]

On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
<ruslan.trofymenko@linaro.org> wrote:
>
> Add sandbox test for 'android_ab_select' command.
>
> Test: ./test/py/test.py --bd sandbox --build -k test_ab
>
> Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> ---
>  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

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

Please see below

>
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 5a744f4..f246f89 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
> @@ -61,6 +62,7 @@ CONFIG_CMD_TIME=y
>  CONFIG_CMD_TIMER=y
>  CONFIG_CMD_SOUND=y
>  CONFIG_CMD_QFW=y
> +CONFIG_CMD_ANDROID_AB_SELECT=y
>  CONFIG_CMD_BOOTSTAGE=y
>  CONFIG_CMD_PMIC=y
>  CONFIG_CMD_REGULATOR=y
> diff --git a/test/py/tests/test_ab.py b/test/py/tests/test_ab.py
> new file mode 100644
> index 0000000..f27538e
> --- /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_android_ab_select')
> + at pytest.mark.requiredtool('sgdisk')
> +def test_ab(ab_disk_image, u_boot_console):
> +    """Test the 'android_ab_select' command."""
> +
> +    u_boot_console.run_command('host bind 0 ' + ab_disk_image.path)
> +
> +    output = u_boot_console.run_command('android_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 'a' in output

I just worry their mind be other next there.

Maybe assert 'b' not in output ?

> +
> +    output = u_boot_console.run_command('android_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 'b' in output
> --
> 2.7.4
>

Regards,
Simon

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

[Simon Glass]

Hi,

On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
<ruslan.trofymenko@linaro.org> wrote:
>
> Add a short documentation for A/B enablement and 'android_ab_select'
> command usage.
>
> Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> ---
>  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..230088c
> --- /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_ANDROID_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 'android_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
> +-------------
> +
> +    android_ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>

Can we have a shorter command?

Perhaps we need a new 'android' command with an 'ab_select'
subcommand? Then the automatica abbreviation will work.

> +
> +for example:
> +
> +    => android_ab_select slot_name mmc 1:4
> +
> +or
> +
> +    => android_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
>

Regards,
Simon

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

[Ruslan Trofymenko]

Hi Simon,

On Thu, 6 Dec 2018 at 03:31, Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
> <ruslan.trofymenko@linaro.org> wrote:
> >
> > Add a short documentation for A/B enablement and 'android_ab_select'
> > command usage.
> >
> > Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> > ---
> >  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..230088c
> > --- /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_ANDROID_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 'android_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
> > +-------------
> > +
> > +    android_ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
>
> Can we have a shorter command?
>
> Perhaps we need a new 'android' command with an 'ab_select'
> subcommand? Then the automatica abbreviation will work.
>

Agree with all your previous comments, will send v2 shortly. But this
one I'd like to leave as is (I will drop android_ prefix though). I
think adding "android" command may lead to unneeded architecture
complexity in future, e.g.:
  - we will need to re-work next commands as sub-commands of "android"
command: fastboot, dtimg, mmc_swrite (which can be hard as ab_select
command file has BSD license and other commands have GPL license)
 - we will need to implement sub-sub-commands (e.g. for "android dtimg
dump ..." etc.)
 - having "android" command can be inconvenient for users and may
break existing API (e.g. it will force users to use "android fastboot"
instead of just "fastboot" command)
 - actually I don't see any namespace issues that can be solved by
adding "android" command

Also, in subsequent patch, I will add the dedicated menu option for
Android-related commands and will pull all existing Android commands
(along with ab_select) to that menu entry.

So I hope it's fine with you if I leave this command as "ab_select"?

Thanks.

> > +
> > +for example:
> > +
> > +    => android_ab_select slot_name mmc 1:4
> > +
> > +or
> > +
> > +    => android_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
> >
>
> Regards,
> Simon

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

[Simon Glass]

Hi,

On Mon, 10 Dec 2018 at 12:11, Ruslan Trofymenko
<ruslan.trofymenko@linaro.org> wrote:
>
> Hi Simon,
>
> On Thu, 6 Dec 2018 at 03:31, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi,
> >
> > On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
> > <ruslan.trofymenko@linaro.org> wrote:
> > >
> > > Add a short documentation for A/B enablement and 'android_ab_select'
> > > command usage.
> > >
> > > Signed-off-by: Ruslan Trofymenko <ruslan.trofymenko@linaro.org>
> > > ---
> > >  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..230088c
> > > --- /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_ANDROID_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 'android_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
> > > +-------------
> > > +
> > > +    android_ab_select <slot_var_name> <interface> <dev[:part_number|#part_name]>
> >
> > Can we have a shorter command?
> >
> > Perhaps we need a new 'android' command with an 'ab_select'
> > subcommand? Then the automatica abbreviation will work.
> >
>
> Agree with all your previous comments, will send v2 shortly. But this
> one I'd like to leave as is (I will drop android_ prefix though). I
> think adding "android" command may lead to unneeded architecture
> complexity in future, e.g.:
>   - we will need to re-work next commands as sub-commands of "android"
> command: fastboot, dtimg, mmc_swrite (which can be hard as ab_select
> command file has BSD license and other commands have GPL license)
>  - we will need to implement sub-sub-commands (e.g. for "android dtimg
> dump ..." etc.)
>  - having "android" command can be inconvenient for users and may
> break existing API (e.g. it will force users to use "android fastboot"
> instead of just "fastboot" command)
>  - actually I don't see any namespace issues that can be solved by
> adding "android" command
>
> Also, in subsequent patch, I will add the dedicated menu option for
> Android-related commands and will pull all existing Android commands
> (along with ab_select) to that menu entry.
>
> So I hope it's fine with you if I leave this command as "ab_select"?

Yes I think that is OK.

Regards,
Simon

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

[Eugeniu Rosca]

Hi Simon, Igor, Ruslan,

On Thu, Dec 6, 2018 at 4:05 AM Simon Glass <sjg@chromium.org> wrote:
> Hi Ruslan,
>
> On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
> <ruslan.trofymenko@linaro.org> wrote:
> >
[..]
> > +struct android_bootloader_message {
>
> How about andr_bl_msg ? Similarly below

Simon, I am currently working on a new U-Boot command which requires
the same AOSP header in-tree. Since the v4 of the whole "A/B support"
series is still WIP by Igor (Ruslan?), may I kindly ask you whether
you feel strong about these specific header and struct renames? We've
recently got some feedback from Tom [1] that it should be OK to keep
the out-of-tree headers untouched. My main motivation is 1) minimizing
the effort of updating this specific header from upstream and 2) using
the U-Boot-compliant header/struct names in my WIP changes. I am open
minded if the original filename is not preserved, but the struct
renames imply some amount of changes in the comments (see [2]). Also,
renaming the structs will imply parsing and verifying the comments
each time the header is updated. It's this kind of tiny bit of
integration effort which you always want to avoid, since it doesn't
require any creativity and can't be automated easily. I am looking
forward for your feedback.

Dear Igor, Ruslan,

How should we handle the import of
bootloader_message/include/bootloader_message/bootloader_message.h ?
If it takes more time for you to submit the next version of A/B
support, would it be fine for you if I do the importing of the header
myself along with my other patches which depend on it?

[1] https://patchwork.ozlabs.org/patch/1044158/#2129429
[2] https://patchwork.ozlabs.org/patch/1044158/#2109299

Many thanks,
Eugeniu.

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

[Eugeniu Rosca]

Hi Simon, Igor, All,

On Wed, Apr 03, 2019 at 07:55:30PM +0200, Eugeniu Rosca wrote:
> Hi Simon, Igor, Ruslan,
> 
> On Thu, Dec 6, 2018 at 4:05 AM Simon Glass <sjg@chromium.org> wrote:
> > Hi Ruslan,
> >
> > On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
> > <ruslan.trofymenko@linaro.org> wrote:
> > >
> [..]
> > > +struct android_bootloader_message {
> >
> > How about andr_bl_msg ? Similarly below
> 
> Simon, I am currently working on a new U-Boot command which requires
> the same AOSP header in-tree. Since the v4 of the whole "A/B support"
> series is still WIP by Igor (Ruslan?), may I kindly ask you whether
> you feel strong about these specific header and struct renames? We've
> recently got some feedback from Tom [1] that it should be OK to keep
> the out-of-tree headers untouched. My main motivation is 1) minimizing
> the effort of updating this specific header from upstream and 2) using
> the U-Boot-compliant header/struct names in my WIP changes. I am open
> minded if the original filename is not preserved, but the struct
> renames imply some amount of changes in the comments (see [2]). Also,
> renaming the structs will imply parsing and verifying the comments
> each time the header is updated. It's this kind of tiny bit of
> integration effort which you always want to avoid, since it doesn't
> require any creativity and can't be automated easily. I am looking
> forward for your feedback.
> 
> Dear Igor, Ruslan,
> 
> How should we handle the import of
> bootloader_message/include/bootloader_message/bootloader_message.h ?
> If it takes more time for you to submit the next version of A/B
> support, would it be fine for you if I do the importing of the header
> myself along with my other patches which depend on it?
> 
> [1] https://patchwork.ozlabs.org/patch/1044158/#2129429
> [2] https://patchwork.ozlabs.org/patch/1044158/#2109299

Apologize for making another iteration on this, but what about below
solution WRT struct renames upon importing bootloader_message.h in-tree.
The following typedef statements (placed in the imported header) would
allow keeping the original structures and the associated comments
untouched (hence speeding up the updates from the source), while U-Boot
would use use the new/rightmost types mirroring the upstream ones.

typedef struct bootloader_message       andr_bl_msg;
typedef struct bootloader_message_ab    andr_bl_msg_ab;
typedef struct bootloader_control       andr_bl_control;

> 
> Many thanks,
> Eugeniu.

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

[Igor Opaniuk]

Hi Eugeniu,

Sorry for the late reply.

Please go ahead,
I'm currently on a business trip, so I won't have access to my setup
with A/B till April 19 (so I definetely won't send any updates till
that time).

I'll sync with the patches you're going to send.

Thanks!

On Wed, Apr 3, 2019 at 7:55 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
>
> Hi Simon, Igor, Ruslan,
>
> On Thu, Dec 6, 2018 at 4:05 AM Simon Glass <sjg@chromium.org> wrote:
> > Hi Ruslan,
> >
> > On Tue, 27 Nov 2018 at 12:57, Ruslan Trofymenko
> > <ruslan.trofymenko@linaro.org> wrote:
> > >
> [..]
> > > +struct android_bootloader_message {
> >
> > How about andr_bl_msg ? Similarly below
>
> Simon, I am currently working on a new U-Boot command which requires
> the same AOSP header in-tree. Since the v4 of the whole "A/B support"
> series is still WIP by Igor (Ruslan?), may I kindly ask you whether
> you feel strong about these specific header and struct renames? We've
> recently got some feedback from Tom [1] that it should be OK to keep
> the out-of-tree headers untouched. My main motivation is 1) minimizing
> the effort of updating this specific header from upstream and 2) using
> the U-Boot-compliant header/struct names in my WIP changes. I am open
> minded if the original filename is not preserved, but the struct
> renames imply some amount of changes in the comments (see [2]). Also,
> renaming the structs will imply parsing and verifying the comments
> each time the header is updated. It's this kind of tiny bit of
> integration effort which you always want to avoid, since it doesn't
> require any creativity and can't be automated easily. I am looking
> forward for your feedback.
>
> Dear Igor, Ruslan,
>
> How should we handle the import of
> bootloader_message/include/bootloader_message/bootloader_message.h ?
> If it takes more time for you to submit the next version of A/B
> support, would it be fine for you if I do the importing of the header
> myself along with my other patches which depend on it?
>
> [1] https://patchwork.ozlabs.org/patch/1044158/#2129429
> [2] https://patchwork.ozlabs.org/patch/1044158/#2109299
>
> Many thanks,
> Eugeniu.



-- 
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 3/7] common: Implement A/B metadata

[Eugeniu Rosca]

Hi Igor,

On Sun, Apr 07, 2019 at 09:11:39PM +0200, Igor Opaniuk wrote:
> Hi Eugeniu,
> 
> Sorry for the late reply.
> 
> Please go ahead,
> I'm currently on a business trip, so I won't have access to my setup
> with A/B till April 19 (so I definetely won't send any updates till
> that time).
> 
> I'll sync with the patches you're going to send.

Thanks for feedback!

jFTR, here is the link to the newly submitted patches making use of
the same AOSP header: https://patchwork.ozlabs.org/cover/1080393/ .

Best regards,
Eugeniu.