[PATCH v1 4/8] cmd: gpt: enable extension to manipulate GPT PTE attributes

[Quentin Schulz]

Hi Denis,

On 6/5/26 5:39 AM, dmukhin at ford.com wrote:
> From: Denis Mukhin <dmukhin at ford.com>
> 
> Enable extension for existing 'gpt' command to manipulate top 16
> bits of GPT PTE attributes to assist ChromiumOS-style boot counters.
> 
> Follow the existing syntax for specifying <interface, device, partition>
> tuple.
> 
> Signed-off-by: Denis Mukhin <dmukhin at ford.com>
> ---
>   cmd/Kconfig |  8 +++++
>   cmd/gpt.c   | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 105 insertions(+)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index c71c6824a196..83409e60da8b 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1319,6 +1319,14 @@ config CMD_GPT
>   	  Enable the 'gpt' command to ready and write GPT style partition
>   	  tables.
>   
> +config CMD_GPT_FLAGS
> +	bool "GPT partition flags commands"
> +	depends on CMD_GPT
> +	select PARTITION_TYPE_GUID
> +	help
> +	  Enables the GUID partition table attributes manipulation (bits 48:63)
> +	  in a ChromiumOS-style manner.
> +
>   config CMD_GPT_RENAME
>   	bool "GPT partition renaming commands"
>   	depends on CMD_GPT
> diff --git a/cmd/gpt.c b/cmd/gpt.c
> index 205f696c8072..4d55cb588ac2 100644
> --- a/cmd/gpt.c
> +++ b/cmd/gpt.c
> @@ -11,6 +11,9 @@
>    */
>   
>   #include <blk.h>
> +#ifdef CONFIG_CMD_GPT_FLAGS
> +#include <cros_flags.h>
> +#endif
>   #include <env.h>
>   #include <log.h>
>   #include <malloc.h>
> @@ -1116,6 +1119,64 @@ out:
>   }
>   #endif
>   
> +#ifdef CONFIG_CMD_GPT_FLAGS
> +static int do_get_gpt_flags(struct blk_desc *desc, int partnum)
> +{
> +	u16 guid_flags;
> +	int rc;
> +
> +	rc = read_disk_flags(desc, partnum, &guid_flags);
> +	if (rc < 0) {
> +		printf("ERROR: partnum %d: failed to read GPT flags: %d\n",
> +		       partnum, rc);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	printf("P=%d S=%d T=%d\n",
> +	       cros_flags_get_priority(guid_flags),
> +	       cros_flags_get_successful(guid_flags),
> +	       cros_flags_get_tries(guid_flags));
> +
> +	return 0;
> +}
> +
> +static int do_set_gpt_flags(struct blk_desc *desc, int partnum,
> +                            const char *key, int value)
> +{
> +	u16 guid_flags;
> +	int rc;
> +
> +	rc = read_disk_flags(desc, partnum, &guid_flags);
> +	if (rc < 0) {
> +		printf("ERROR: partnum %d: failed to read GPT flags: %d\n",
> +		       partnum, rc);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	if (!strcmp(key, "set-flags")) {
> +		guid_flags = (u16)(value & 0xffff);
> +	} else if (!strcmp(key, "set-priority")) {
> +		guid_flags = cros_flags_set_priority(guid_flags, value);
> +	} else if (!strcmp(key, "set-successful")) {
> +		guid_flags = cros_flags_set_successful(guid_flags, value);
> +	} else if (!strcmp(key, "set-tries")) {
> +		guid_flags = cros_flags_set_tries(guid_flags, value);
> +	} else {
> +		printf("ERROR: unsupported command '%s'\n", key);
> +		return CMD_RET_FAILURE;

CMD_RET_USAGE

> +	}
> +
> +	rc = write_disk_flags(desc, partnum, guid_flags);
> +	if (rc < 0) {
> +		pr_err("ERROR: partnum %d: failed to write GPT flags: %d\n",
> +               partnum, rc);
> +		return CMD_RET_FAILURE;
> +    }
> +
> +	return 0;
> +}
> +#endif /* CONFIG_CMD_GPT_FLAGS */
> +
>   /**
>    * do_gpt(): Perform GPT operations
>    *
> @@ -1176,6 +1237,23 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   		ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4], argv[5]);
>   	} else if ((strcmp(argv[1], "set-bootable") == 0)) {
>   		ret = gpt_set_bootable(blk_dev_desc, argv[4]);
> +#endif
> +#ifdef CONFIG_CMD_GPT_FLAGS
> +	} else if (!strcmp(argv[1], "get-flags")) {
> +		if (argc < 5)
> +			return CMD_RET_FAILURE;

CMD_RET_USAGE

> +		ret = do_get_gpt_flags(blk_dev_desc,
> +		                       simple_strtol(argv[4], NULL, 0));
> +	} else if (!strcmp(argv[1], "set-flags") ||
> +		   !strcmp(argv[1], "set-priority") ||
> +		   !strcmp(argv[1], "set-successful") ||
> +		   !strcmp(argv[1], "set-tries")) {
> +		if (argc < 6)
> +			return CMD_RET_FAILURE;

CMD_RET_USAGE

> +		ret = do_set_gpt_flags(blk_dev_desc, /* argv[2,3] */
> +		                       simple_strtol(argv[4], NULL, 0),
> +		                       argv[1],
> +		                       simple_strtol(argv[5], NULL, 0));
>   #endif
>   	} else {
>   		return CMD_RET_USAGE;
> @@ -1237,4 +1315,23 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
>   	" gpt set-bootable mmc 0 boot_a,boot_b\n"
>   	" gpt transpose mmc 0 1 2\n"
>   #endif
> +#ifdef CONFIG_CMD_GPT_FLAGS
> +	"gpt partition boot flag manipulation:\n"
> +	" gpt get-flags <interface> <dev> <partid>\n"
> +	"    - read top 16-bit of partition attributes (bits 48:63)\n"
> +	" gpt set-flags <interface> <dev> <partid> <flags-in-hex>\n"
> +	"    - write top 16-bit of partition attributes (bits 48:63)\n"
> +	" gpt set-successful <interface> <dev> <partid> {0,1}\n"
> +	"    - set 'boot successful' bit\n"
> +	" gpt set-priority <interface> <dev> <partid> [0..15]\n"
> +	"    - set priority; 0 - non-bootable\n"
> +	" gpt set-tries <interface> <dev> <partid> [0..15]\n"
> +	"    - set tries (boot counter)\n"

These are the first subcommands that use "partid" as an argument 
identifier, can you rather use a name we've already used in another 
subcommand (if applicable).

I'm wondering if we need the additional get-/set- prefix? It could be 
enough to pass a 4th argument to know whether we want a get or a set. 
Either way is fine I guess.

I really recommend to not go with decimal values, U-Boot has a mix of 
hexadecimal and base 10 numbers all over the place but we typically say 
the default should be hex, c.f. 
https://docs.u-boot-project.org/en/latest/usage/cmdline.html#representing-numbers

In any case, the base needs to be made explicit in doc/usage/cmd/gpt.rst 
so there is no possible confusion.

> +	" Example usage:\n"
> +	" gpt get-flags nvme 0 1\n"
> +	" gpt set-flags nvme 0 1 0xff\n"
> +	" gpt set-priority nvme 0 1 0\n"
> +	" gpt set-successful nvme 0 1 1\n"
> +	" gpt set-tries nvme 0 1 15\n"

I don't think we want examples in the help text in U-Boot, it'll 
unnecessarily increase the size of the binary. I see we have it for 
other gpt subcommands so maybe that's fine /me shrugs.

We absolutely do need an update to doc/usage/cmd/gpt.rst to reflect 
those changes though (and there you can have examples!).

Cheers,
Quentin