[U-Boot] [PATCH v2 5/6] rename GPT partitions to detect boot failure

Lukasz Majewski
2017-05-31T08:12:46Z

Hi Alison,

> From: Alison Chaiken <alison@she-devel.com>
>
> This patch provides support in u-boot for renaming GPT
> partitions. The renaming is accomplished via a new 'gpt flip'
> command which is enabled via a CONFIG_CMD_GPT_FLIP option.
>
> The concept for the bootloader state machine is the following:
>
> -- u-boot renames ‘primary’ partitions as ‘candidate’ and tries
> to boot them.
> -- Linux, at boot, will rename ‘candidate’ partitions as
> ‘primary’.
> -- If u-boot sees a ‘candidate’ partition after a boot attempt,
> it renames it failed’ and renames the ‘backup’ partition as
> ‘candidate’.
>
> Logic:
> -- Partitions can go to ‘failed’ only from ‘candidate’ and only
> via u-boot. Partitions can go to ‘backup’ only from ‘primary’
> and vice-versa, only via Linux. Partitions go to ‘candidate’
> from ‘primary’ or ‘backup’ only via u-boot. Only system
> update software will rename 'failed' partitions.
>
> Rewriting the partition table has the side-effect that all partitions
> end up with "msftdata" flag set. The reason is that partition type
> PARTITION_BASIC_DATA_GUID is hard-coded in the gpt_fill_pte()
> function. This does not appear to cause any harm.
>
> Signed-off-by: Alison Chaiken <alison@peloton-tech.com>
> ---
> cmd/Kconfig | 7 ++
> cmd/gpt.c | 199
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> doc/README.gpt | 13 ++++ 3 files changed, 215 insertions(+), 4
> deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 5ee52f6..a8f7716 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -575,6 +575,13 @@ config CMD_GPT
> Enable the 'gpt' command to ready and write GPT style
> partition tables.
>
> +config CMD_GPT_FLIP
> + bool "GPT flip-partitions command"
> + depends on CMD_GPT
> + help
> + Enables the 'gpt' command to write modified GPT partition
> + tables via the 'gpt flip' command.
> +
> config CMD_ARMFLASH
> #depends on FLASH_CFI_DRIVER
> bool "armflash"
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index c61d2b1..6a0b70f 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -20,6 +20,7 @@
> #include <div64.h>
> #include <memalign.h>
> #include <linux/compat.h>
> +#include <linux/sizes.h>
>
> static LIST_HEAD(disk_partitions);
>
> @@ -190,16 +191,33 @@ static struct disk_part
> *allocate_disk_part(disk_partition_t *info, int partnum) return
> newpart; }
>
> +static void prettyprint_part_size(char *sizestr, unsigned long
> partsize,
> + unsigned long blksize)
> +{
> + unsigned long long partbytes;
> + unsigned long partmegabytes;
> +
> + partbytes = partsize * blksize;
> + partmegabytes = lldiv(partbytes, SZ_1M);
> + snprintf(sizestr, 16, "%luMiB", partmegabytes);
> +}
> +
> static void print_gpt_info(void)
> {
> struct list_head *pos;
> struct disk_part *curr;
> + char partstartstr[16];
> + char partsizestr[16];
>
> list_for_each(pos, &disk_partitions) {
> curr = list_entry(pos, struct disk_part, list);
> + prettyprint_part_size(partstartstr, (unsigned
> long)curr->gpt_part_info.start,
> + (unsigned long)
> curr->gpt_part_info.blksz);
> + prettyprint_part_size(partsizestr, (unsigned
> long)curr->gpt_part_info.size,
> + (unsigned long)
> curr->gpt_part_info.blksz); +
> printf("Partition %d:\n", curr->partnum);
> - printf("1st block %x, size %x\n",
> (unsigned)curr->gpt_part_info.start,
> - (unsigned)curr->gpt_part_info.size);
> + printf("Start %s, size %s\n", partstartstr,
> partsizestr); printf("Block size %lu, name %s\n",
> curr->gpt_part_info.blksz, curr->gpt_part_info.name);
> printf("Type %s, bootable %d\n",
> curr->gpt_part_info.type, @@ -211,6 +229,85 @@ static void
> print_gpt_info(void) }
> }
>
> +#ifdef CONFIG_CMD_GPT_FLIP
> +static int calc_parts_list_len(int numparts)
> +{
> + /*
> + * prefatory string:
> + * doc/README.GPT, suggests that
> + * int partlistlen = UUID_STR_LEN + 1 +
> strlen("partitions=uuid_disk=");
> + * is correct, but extract_val() expects "uuid_disk" first.
> + */
> + int partlistlen = UUID_STR_LEN + 1 + strlen("uuid_disk=");
> + /* for the comma */
> + partlistlen++;
> +
> + /* per-partition additions; numparts starts at 1, so this
> should be correct */
> + partlistlen += numparts * (strlen("name=,") + PART_NAME_LEN
> + 1);
> + /* 17 because partstr in create_gpt_partitions_list() is 16
> chars */
> + partlistlen += numparts * (strlen("start=MiB,") + 17);
> + partlistlen += numparts * (strlen("size=MiB,") + 17);
> + partlistlen += numparts * (strlen("uuid=;") + UUID_STR_LEN +
> 1);
> + /* for the terminating null */
> + partlistlen++;
> + debug("Length of partitions_list is %d for %d partitions\n",
> partlistlen,
> + numparts);
> + return partlistlen;
> +}
> +
> +/*
> + * create the string that upstream 'gpt write' command will accept
> as an
> + * argument
> + *
> + * From doc/README.gpt, Format of partitions layout:
> + * "partitions=uuid_disk=...;name=u-boot,size=60MiB,uuid=...;
> + * name=kernel,size=60MiB,uuid=...;"
> + * The fields 'name' and 'size' are mandatory for every partition.
> + * The field 'start' is optional. The fields 'uuid' and 'uuid_disk'
> + * are optional if CONFIG_RANDOM_UUID is enabled.
> + */
> +static int create_gpt_partitions_list(int numparts, const char
> *guid, char *partitions_list) +{
> + struct list_head *pos;
> + struct disk_part *curr;
> + char partstr[PART_NAME_LEN + 1];
> +
> + if (!partitions_list)
> + return -1;
> +
> + /*
> + * README.gpt specifies starting with "partitions=" like so:
> + * strcpy(partitions_list, "partitions=uuid_disk=");
> + * but that breaks extract_val, which doesn't skip over
> 'partitions='.
> + */
> + strcpy(partitions_list, "uuid_disk=");
> + strncat(partitions_list, guid, UUID_STR_LEN + 1);
> + strcat(partitions_list, ";");
> +
> + list_for_each(pos, &disk_partitions) {
> + curr = list_entry(pos, struct disk_part, list);
> + strcat(partitions_list, "name=");
> + strncat(partitions_list, (const char
> *)curr->gpt_part_info.name, PART_NAME_LEN + 1);
> + strcat(partitions_list, ",start=");
> + prettyprint_part_size(partstr, (unsigned
> long)curr->gpt_part_info.start,
> + (unsigned long)
> curr->gpt_part_info.blksz);
> + /* one extra byte for NULL */
> + strncat(partitions_list, partstr, PART_NAME_LEN + 1);
> + strcat(partitions_list, ",size=");
> + /* lbaint_t is unsigned long, per include/ide.h */
> + prettyprint_part_size(partstr, (unsigned
> long)curr->gpt_part_info.size,
> + (unsigned long)
> curr->gpt_part_info.blksz);
> + strncat(partitions_list, partstr, PART_NAME_LEN + 1);
> +
> + strcat(partitions_list, ",uuid=");
> + strncat(partitions_list, (const char
> *)curr->gpt_part_info.uuid,
> + UUID_STR_LEN + 1);
> + strcat(partitions_list, ";");
> + }
> + return 0;
> +}
> +#endif
> +
> /*
> * read partition info into disk_partitions list where
> * it can be printed or modified
> @@ -222,8 +319,11 @@ static int get_gpt_info(struct blk_desc
> *dev_desc) disk_partition_t info;
> struct disk_part *new_disk_part;
>
> - if (disk_partitions.next == NULL)
> - INIT_LIST_HEAD(&disk_partitions);
> + /*
> + * Always re-read partition info from device, in case
> + * it has changed
> + */
> + INIT_LIST_HEAD(&disk_partitions);
>
> for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
> ret = part_get_info(dev_desc, p, &info);
> @@ -294,6 +394,8 @@ static int set_gpt_info(struct blk_desc *dev_desc,
> return -1;
>
> str = strdup(str_part);
> + if (str == NULL)
> + return -ENOMEM;
>
> /* extract disk guid */
> s = str;
> @@ -523,6 +625,86 @@ static int do_disk_guid(struct blk_desc
> *dev_desc, char * const namestr) return 0;
> }
>
> +#ifdef CONFIG_CMD_GPT_FLIP
> +static int do_flip_gpt_parts(struct blk_desc *dev_desc)
> +{
> + struct list_head *pos;
> + struct disk_part *curr;
> + disk_partition_t *new_partitions = NULL;
> + char disk_guid[UUID_STR_LEN + 1];
> + char *partitions_list, *str_disk_guid;
> + u8 part_count = 0;
> + int partlistlen, ret, numparts = 0;
> +
> + ret = get_disk_guid(dev_desc, disk_guid);
> + if (ret < 0)
> + return ret;
> +
> + numparts = get_gpt_info(dev_desc);
> + if (numparts < 0)
> + return numparts;
> + printf("Current partition table with %d partitions is:\n",
> numparts);
> + print_gpt_info();
> +
> + partlistlen = calc_parts_list_len(numparts);
> + partitions_list = (char *)malloc(partlistlen);
> + memset(partitions_list, '\0', partlistlen);
> +
> + ret = create_gpt_partitions_list(numparts, (const char *)
> disk_guid,
> + partitions_list);
> + if (ret < 0)
> + return ret;
> + debug("OLD partitions_list is %s with %d chars\n",
> partitions_list, strlen(partitions_list)); +
> + ret = set_gpt_info(dev_desc, (const char *)partitions_list,
> &str_disk_guid,
> + &new_partitions, &part_count);
> + if (ret < 0)
> + return ret;
> +
> + list_for_each(pos, &disk_partitions) {
> + curr = list_entry(pos, struct disk_part, list);
> + if (!strcmp((char *)curr->gpt_part_info.name,
> "backup_kernel"))
> + strcpy((char *)curr->gpt_part_info.name,
> "candidate_kernel");
> + if (!strcmp((char *)curr->gpt_part_info.name,
> "primary_kernel"))
> + strcpy((char *)curr->gpt_part_info.name,
> "backup_kernel");
> + if (!strcmp((char *)curr->gpt_part_info.name,
> "backup_rootfs"))
> + strcpy((char *)curr->gpt_part_info.name,
> "candidate_rootfs");
> + if (!strcmp((char *)curr->gpt_part_info.name,
> "primary_rootfs"))
> + strcpy((char *)curr->gpt_part_info.name,
> "backup_rootfs");
> + }
> +
> + ret = create_gpt_partitions_list(numparts, (const char *)
> disk_guid, partitions_list);
> + if (ret < 0)
> + return ret;
> + debug("NEW partitions_list is %s with %d chars\n",
> partitions_list, strlen(partitions_list)); +
> + ret = set_gpt_info(dev_desc, (const char *)partitions_list,
> &str_disk_guid,
> + &new_partitions, &part_count);
> + if (ret < 0)
> + return ret;
> +
> + debug("Writing new partition table\n");
> + ret = gpt_restore(dev_desc, disk_guid, new_partitions,
> numparts);
> + if (ret < 0) {
> + printf("Writing new partition table failed\n");
> + return ret;
> + }
> +
> + debug("Reading back new partition table\n");
> + numparts = get_gpt_info(dev_desc);
> + if (numparts < 0)
> + return numparts;
> + printf("new partition table with %d partitions is:\n",
> numparts);
> + print_gpt_info();
> +
> + del_gpt_info();
> + free(partitions_list);
> + free(str_disk_guid);
> + free(new_partitions);
> + return ret;
> +}
> +#endif
> +
> /**
> * do_gpt(): Perform GPT operations
> *
> @@ -567,6 +749,10 @@ static int do_gpt(cmd_tbl_t *cmdtp, int flag,
> int argc, char * const argv[]) return do_disk_guid(blk_dev_desc,
> varname); } else if (strcmp(argv[1], "read") == 0) {
> return do_get_gpt_info(blk_dev_desc);
> +#ifdef CONFIG_CMD_GPT_FLIP
> + } else if (strcmp(argv[1], "flip") == 0) {
> + return do_flip_gpt_parts(blk_dev_desc);
> +#endif
> } else {
> return CMD_RET_USAGE;
> }
> @@ -598,4 +784,9 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
> " Example usage:\n"
> " gpt guid mmc 0\n"
> " gpt guid mmc 0 varname\n"
> +#ifdef CONFIG_CMD_GPT_FLIP
> + "gpt partition-flip command\n"
> + "gpt flip <interface> <dev>\n"
> + " - exchange device's 'primary' and 'backup' partition
> names\n" +#endif
> );
> diff --git a/doc/README.gpt b/doc/README.gpt
> index c0779a4..e29b188 100644
> --- a/doc/README.gpt
> +++ b/doc/README.gpt
> @@ -210,6 +210,19 @@ Following line can be used to assess if GPT
> verification has succeed: U-BOOT> gpt verify mmc 0 $partitions
> U-BOOT> if test $? = 0; then echo "GPT OK"; else echo "GPT ERR"; fi
>
> +Renaming GPT partitions from U-Boot:
> +====================================
> +
> +GPT partition names are a mechanism via which userspace and U-Boot
> can +communicate about software updates and boot failure. The 'gpt
> guid', +'gpt read' and 'gpt flip' commands facilitate programmatic
> renaming of +partitions from bootscripts by generating and modifying
> the partitions +layout string. The code in gpt_flip() illustrates
> the case of +swapping 'primary' and 'backup' partitions via:
> +
> +U-BOOT> gpt flip mmc 0

Maybe it would be better to have

gpt flip mmc 0 <optional parameter 'name'>

(By default we have "primary" and "backup")

In that way we could set other names to GPT partitions without the
need to modify the code.

And another request -> Could you consider adding tests for those new
gpt commands to the 'sandbox' (sandbox_defconfig) ?

Then you can 'mount' some gpt test image ('host' command) and use it
with:
gpt <command> host X .....

Despite above comments - you did a great job :-)

Reviewed-by: Lukasz Majewski <lukma@denx.de>

> +
> +Choose different partition names by modifying these strings in gpt.c.
>
> Partition type GUID:
> ====================

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de