[U-Boot] [PATCH v2 2/2] cmd: Add 'bcb' command to read/modify/write BCB fields

Simon Glass sjg at chromium.org
Wed May 22 00:53:29 UTC 2019


Hi Eugeniu,

On Fri, 17 May 2019 at 08:46, Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
>
> 'Bootloader Control Block' (BCB) is a well established term/acronym in
> the Android namespace which refers to a location in a dedicated raw
> (i.e. FS-unaware) flash (e.g. eMMC) partition, usually called "misc",
> which is used as media for exchanging messages between Android userspace
> (particularly recovery [1]) and an Android-capable bootloader.
>
> On higher level, this allows implementing a subset of Android Bootloader
> Requirements [2], amongst which is the Android-specific bootloader
> flow [3]. Regardless how the latter is implemented in U-Boot ([3] being
> the most memorable example), reading/writing/dumping the BCB fields in
> the development process from inside the U-Boot is a convenient feature.
> Hence, make it available to the users.
>
> Some usage examples of the new command recorded on R-Car H3ULCB-KF
> ('>>>' is an overlay on top of the original console output):
>
> => bcb
> bcb - Load/set/clear/test/dump/store Android BCB fields
>
> Usage:
> bcb load  <dev> <part>       - load  BCB from mmc <dev>:<part>
> bcb set   <field> <val>      - set   BCB <field> to <val>
> bcb clear [<field>]          - clear BCB <field> or all fields
> bcb test  <field> <op> <val> - test  BCB <field> against <val>
> bcb dump  <field>            - dump  BCB <field>
> bcb store                    - store BCB back to mmc
>
> Legend:
> <dev>   - MMC device index containing the BCB partition
> <part>  - MMC partition index or name containing the BCB
> <field> - one of {command,status,recovery,stage,reserved}
> <op>    - the binary operator used in 'bcb test':
>           '=' returns true if <val> matches the string stored in <field>
>           '~' returns true if <val> matches a subset of <field>'s string
> <val>   - string/text provided as input to bcb {set,test}
>           NOTE: any ':' character in <val> will be replaced by line feed
>           during 'bcb set' and used as separator by upper layers
>
> => bcb dump command
> Error: BCB not loaded!
>  >>> Users must specify mmc device and partition before any other call
>
> => bcb load 1 misc
> => bcb load 1 1
>  >>> The two calls are equivalent (assuming "misc" has index 1)
>
> => bcb dump command
> 00000000: 62 6f 6f 74 6f 6e 63 65 2d 73 68 65 6c 6c 00 72    bootonce-shell.r
> 00000010: 79 00 72 00 00 00 00 00 00 00 00 00 00 00 00 00    y.r.............
>  >>> The output is in binary/string format for convenience
>  >>> The output size matches the size of inspected BCB field
>  >>> (32 bytes in case of 'command')
>
> => bcb test command = bootonce-shell && echo true
> true
> => bcb test command = bootonce-shell- && echo true
> => bcb test command = bootonce-shel && echo true
>  >>> The '=' operator returns 'true' on perfect match
>
> => bcb test command ~ bootonce-shel && echo true
> true
> => bcb test command ~ bootonce-shell && echo true
> true
>  >>> The '~' operator returns 'true' on substring match
>
> => bcb set command recovery
> => bcb dump command
> 00000000: 72 65 63 6f 76 65 72 79 00 73 68 65 6c 6c 00 72    recovery.shell.r
> 00000010: 79 00 72 00 00 00 00 00 00 00 00 00 00 00 00 00    y.r.............
>  >>> The new value is NULL-terminated and stored in the BCB field
>
> => bcb set recovery "msg1:msg2:msg3"
> => bcb dump recovery
> 00000040: 6d 73 67 31 0a 6d 73 67 32 0a 6d 73 67 33 00 00    msg1.msg2.msg3..
> 00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> 00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
>  >>> --- snip ---
>  >>> Every ':' is replaced by line-feed '\n' (0xA). The latter is used
>  >>> as separator between individual commands by Android userspace
>
> => bcb store
>  >>> Flush/store the BCB structure to MMC
>
> [1] https://android.googlesource.com/platform/bootable/recovery
> [2] https://source.android.com/devices/bootloader
> [3] https://patchwork.ozlabs.org/patch/746835/
>     ("[U-Boot,5/6] Initial support for the Android Bootloader flow")

As discussed, please add these docs somewhere in the U-Boot tree (can
be in a separate patch)

>
> Signed-off-by: Eugeniu Rosca <erosca at de.adit-jv.com>
> ---
> v2:
>  - [Heinrich Schuchardt] Implement sub-commands via U_BOOT_CMD_MKENT.
>  - Polished the code. Ensured no warnings returned by sparse, smatch,
>    `cppcheck --force --enable=all --inconclusive`, make W=1.
>  - Tested on R-Car-H3-ES20 ULCB-KF.
>
> v1:
>  - https://patchwork.ozlabs.org/patch/1080395/
> ---
>  cmd/Kconfig  |  17 +++
>  cmd/Makefile |   1 +
>  cmd/bcb.c    | 330 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 348 insertions(+)
>  create mode 100644 cmd/bcb.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 0d36da2a5c43..495bc1052f50 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -631,6 +631,23 @@ config CMD_ADC
>           Shows ADC device info and permit printing one-shot analog converted
>           data from a named Analog to Digital Converter.
>
> +config CMD_BCB
> +       bool "bcb"
> +       depends on MMC
> +       depends on PARTITIONS
> +       help
> +         Read/modify/write the fields of Bootloader Control Block, usually
> +         stored on the flash "misc" partition with its structure defined in:
> +         https://android.googlesource.com/platform/bootable/recovery/+/master/
> +         bootloader_message/include/bootloader_message/bootloader_message.h
> +
> +         Some real-life use-cases include (but are not limited to):
> +         - Determine the "boot reason" (and act accordingly):
> +           https://source.android.com/devices/bootloader/boot-reason
> +         - Get/pass a list of commands from/to recovery:
> +           https://android.googlesource.com/platform/bootable/recovery
> +         - Inspect/dump the contents of the BCB fields
> +
>  config CMD_BIND
>         bool "bind/unbind - Bind or unbind a device to/from a driver"
>         depends on DM
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 7864fcf95c36..8f31b478eb1b 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_CMD_ADC) += adc.o
>  obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
>  obj-y += blk_common.o
>  obj-$(CONFIG_CMD_SOURCE) += source.o
> +obj-$(CONFIG_CMD_BCB) += bcb.o
>  obj-$(CONFIG_CMD_BDI) += bdinfo.o
>  obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
>  obj-$(CONFIG_CMD_BIND) += bind.o
> diff --git a/cmd/bcb.c b/cmd/bcb.c
> new file mode 100644
> index 000000000000..5d8486684542
> --- /dev/null
> +++ b/cmd/bcb.c
> @@ -0,0 +1,330 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Eugeniu Rosca <rosca.eugeniu at gmail.com>
> + *
> + * Command to read/modify/write Android BCB fields
> + */
> +
> +#include <android_bootloader_message.h>
> +#include <command.h>
> +#include <common.h>
> +#include <malloc.h>
> +
> +enum bcb_cmd {
> +       BCB_CMD_LOAD,
> +       BCB_CMD_FIELD_SET,
> +       BCB_CMD_FIELD_CLEAR,
> +       BCB_CMD_FIELD_TEST,
> +       BCB_CMD_FIELD_DUMP,
> +       BCB_CMD_STORE,
> +};
> +
> +static int bcb_dev = -1;
> +static int bcb_part = -1;
> +static struct bootloader_message bcb = { { 0 } };
> +
> +static int bcb_cmd_get(char *cmd)
> +{
> +       if (!strncmp(cmd, "load", sizeof("load")))
> +               return BCB_CMD_LOAD;
> +       if (!strncmp(cmd, "set", sizeof("set")))
> +               return BCB_CMD_FIELD_SET;
> +       if (!strncmp(cmd, "clear", sizeof("clear")))
> +               return BCB_CMD_FIELD_CLEAR;
> +       if (!strncmp(cmd, "test", sizeof("test")))
> +               return BCB_CMD_FIELD_TEST;
> +       if (!strncmp(cmd, "store", sizeof("store")))
> +               return BCB_CMD_STORE;
> +       if (!strncmp(cmd, "dump", sizeof("dump")))
> +               return BCB_CMD_FIELD_DUMP;
> +       else
> +               return -1;
> +}
> +
> +static int do_bcb_load(cmd_tbl_t *cmdtp, int flag, int argc,
> +                      char * const argv[])
> +{
> +       struct blk_desc *desc;
> +       disk_partition_t info;
> +       u64 cnt;
> +       char *endp;
> +       int part;
> +
> +       if (argc != 3)
> +               return CMD_RET_USAGE;
> +
> +       if (blk_get_device_by_str("mmc", argv[1], &desc) < 0)

Error codes should be reported.

> +               goto err_1;
> +
> +       part = simple_strtoul(argv[2], &endp, 0);
> +       if (*endp == '\0') {
> +               if (part_get_info(desc, part, &info))
> +                       goto err_1;
> +       } else {
> +               part = part_get_info_by_name(desc, argv[2], &info);
> +               if (part < 0)
> +                       goto err_1;
> +       }
> +
> +       cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
> +       if (cnt > info.size)
> +               goto err_2;
> +
> +       if (blk_dread(desc, info.start, cnt, &bcb) != cnt)
> +               goto err_1;
> +
> +       bcb_dev = desc->devnum;
> +       bcb_part = part;
> +       debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
> +
> +       return CMD_RET_SUCCESS;
> +err_1:
> +       printf("Error: Failed to read from mmc %s:%s\n", argv[1], argv[2]);

Add error code here.

> +       goto err;
> +err_2:
> +       printf("Error: mmc %s:%s too small!", argv[1], argv[2]);
> +       goto err;
> +err:
> +       bcb_dev = -1;
> +       bcb_part = -1;
> +       return CMD_RET_FAILURE;
> +}
> +
> +static int bcb_is_misused(int argc, char *const argv[])
> +{
> +       if (bcb_dev < 0 || bcb_part < 0) {
> +               printf("Error: BCB not loaded!\n");
> +               return -1;
> +       }
> +       switch (bcb_cmd_get(argv[0])) {

Why have a switch() here, when you could just put the below code in
each function? Or put the call to this function from the main
do_bcb_set() function.

> +       case BCB_CMD_LOAD:
> +               /* Dedicated arg handling */
> +               break;
> +       case BCB_CMD_FIELD_SET:
> +               if (argc != 3)
> +                       goto err;
> +               break;
> +       case BCB_CMD_FIELD_TEST:
> +               if (argc != 4)
> +                       goto err;
> +               break;
> +       case BCB_CMD_FIELD_CLEAR:
> +               if (argc != 1 && argc != 2)
> +                       goto err;
> +               break;
> +       case BCB_CMD_STORE:
> +               if (argc != 1)
> +                       goto err;
> +               break;
> +       case BCB_CMD_FIELD_DUMP:
> +               if (argc != 2)
> +                       goto err;
> +               break;
> +       default:
> +               debug("%s: Error: Unexpected BCB command\n", __func__);
> +               return -1;
> +       }
> +
> +       return 0;
> +err:
> +       printf("Error: Bad usage of 'bcb %s'\n", argv[0]);
> +       return -1;
> +}
> +
> +static int
> +bcb_field_get(char *name, char **field, int *size)
> +{
> +       if (!strncmp(name, "command", sizeof("command"))) {
> +               *field = bcb.command;
> +               *size = sizeof(bcb.command);
> +       } else if (!strncmp(name, "status", sizeof("status"))) {
> +               *field = bcb.status;
> +               *size = sizeof(bcb.status);
> +       } else if (!strncmp(name, "recovery", sizeof("recovery"))) {
> +               *field = bcb.recovery;
> +               *size = sizeof(bcb.recovery);
> +       } else if (!strncmp(name, "stage", sizeof("stage"))) {
> +               *field = bcb.stage;
> +               *size = sizeof(bcb.stage);
> +       } else if (!strncmp(name, "reserved", sizeof("reserved"))) {
> +               *field = bcb.reserved;
> +               *size = sizeof(bcb.reserved);
> +       } else {
> +               printf("Error: Unknown field '%s'\n", name);
> +               return -1;
> +       }
> +       return 0;
> +}
> +
> +static int do_bcb_set(cmd_tbl_t *cmdtp, int flag, int argc,
> +                     char * const argv[])
> +{
> +       int size, len;
> +       char *field, *str, *found, *keep;
> +
> +       if (bcb_is_misused(argc, argv))
> +               return CMD_RET_FAILURE;
> +
> +       if (bcb_field_get(argv[1], &field, &size))
> +               return CMD_RET_FAILURE;
> +
> +       len = strlen(argv[2]);
> +       if (len >= size) {
> +               printf("Error: sizeof('%s') = %d >= %d = sizeof(bcb.%s)\n",
> +                      argv[2], len, size, argv[1]);
> +               return CMD_RET_FAILURE;
> +       }
> +       str = strdup(argv[2]);

It is OK to change the args if you like.


> +       keep = str;
> +
> +       field[0] = '\0';
> +       while ((found = strsep(&str, ":"))) {
> +               if (field[0] != '\0')
> +                       strcat(field, "\n");
> +               strcat(field, found);
> +       }
> +
> +       free(keep);
> +       return CMD_RET_SUCCESS;
> +}
> +
> +static int do_bcb_clear(cmd_tbl_t *cmdtp, int flag, int argc,
> +                       char * const argv[])
> +{
> +       int size;
> +       char *field;
> +
> +       if (bcb_is_misused(argc, argv))
> +               return CMD_RET_FAILURE;
> +
> +       if (argc == 1) {
> +               memset(&bcb, 0, sizeof(bcb));
> +               return CMD_RET_SUCCESS;
> +       }
> +
> +       if (bcb_field_get(argv[1], &field, &size))
> +               return CMD_RET_FAILURE;
> +
> +       memset(field, 0, size);
> +       return CMD_RET_SUCCESS;
> +}
> +
> +static int do_bcb_dump(cmd_tbl_t *cmdtp, int flag, int argc,
> +                      char * const argv[])
> +{
> +       int size;
> +       char *field;
> +
> +       if (bcb_is_misused(argc, argv))
> +               return CMD_RET_FAILURE;

You should return CMD_RET_USAGE if there is a usage problem.

> +
> +       if (bcb_field_get(argv[1], &field, &size))
> +               return CMD_RET_FAILURE;
> +
> +       print_buffer((ulong)field - (ulong)&bcb, (void *)field, 1, size, 16);

Please add newline before return

> +       return CMD_RET_SUCCESS;
> +}
> +
> +static int do_bcb_test(cmd_tbl_t *cmdtp, int flag, int argc,
> +                      char * const argv[])
> +{
> +       int size;
> +       char *field;
> +
> +       if (bcb_is_misused(argc, argv))
> +               return CMD_RET_FAILURE;
> +
> +       if (bcb_field_get(argv[1], &field, &size))
> +               return CMD_RET_FAILURE;
> +
> +       if (!strncmp(argv[2], "=", sizeof("="))) {

Think about code size:

if (*argv[2] == '=')

> +               if (!strncmp(argv[3], field, size))
> +                       return CMD_RET_SUCCESS;
> +               else
> +                       return CMD_RET_FAILURE;
> +       } else if (!strncmp(argv[2], "~", sizeof("~"))) {
> +               if (!strstr(field, argv[3]))
> +                       return CMD_RET_FAILURE;
> +               else
> +                       return CMD_RET_SUCCESS;
> +       } else {
> +               printf("Error: Unknown operator '%s'\n", argv[2]);
> +       }
> +       return CMD_RET_FAILURE;
> +}
> +
> +static int do_bcb_store(cmd_tbl_t *cmdtp, int flag, int argc,
> +                       char * const argv[])
> +{
> +       struct blk_desc *desc;
> +       disk_partition_t info;
> +       u64 cnt;
> +
> +       if (bcb_is_misused(argc, argv))
> +               return CMD_RET_FAILURE;
> +
> +       desc = blk_get_devnum_by_type(IF_TYPE_MMC, bcb_dev);
> +       if (!desc)
> +               goto err;
> +
> +       if (part_get_info(desc, bcb_part, &info))
> +               goto err;

Again, error numbers need to be printed.

> +
> +       cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
> +
> +       if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt)
> +               goto err;
> +
> +       return CMD_RET_SUCCESS;
> +err:
> +       printf("Error: Failed to write to mmc %d:%d\n", bcb_dev, bcb_part);
> +       return CMD_RET_FAILURE;
> +}
> +
> +static cmd_tbl_t cmd_bcb_sub[] = {
> +       U_BOOT_CMD_MKENT(load, CONFIG_SYS_MAXARGS, 1, do_bcb_load, "", ""),
> +       U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 1, do_bcb_set, "", ""),
> +       U_BOOT_CMD_MKENT(clear, CONFIG_SYS_MAXARGS, 1, do_bcb_clear, "", ""),
> +       U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_bcb_test, "", ""),
> +       U_BOOT_CMD_MKENT(dump, CONFIG_SYS_MAXARGS, 1, do_bcb_dump, "", ""),
> +       U_BOOT_CMD_MKENT(store, CONFIG_SYS_MAXARGS, 1, do_bcb_store, "", ""),
> +};
> +
> +static int do_bcb(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
> +{
> +       cmd_tbl_t *c;
> +
> +       if (argc < 2)
> +               return CMD_RET_USAGE;
> +
> +       argc--;
> +       argv++;
> +
> +       c = find_cmd_tbl(argv[0], cmd_bcb_sub, ARRAY_SIZE(cmd_bcb_sub));
> +       if (c)
> +               return c->cmd(cmdtp, flag, argc, argv);
> +
> +       return CMD_RET_USAGE;
> +}
> +
> +U_BOOT_CMD(
> +       bcb, CONFIG_SYS_MAXARGS, 1, do_bcb,
> +       "Load/set/clear/test/dump/store Android BCB fields",
> +       "load  <dev> <part>       - load  BCB from mmc <dev>:<part>\n"
> +       "bcb set   <field> <val>      - set   BCB <field> to <val>\n"
> +       "bcb clear [<field>]          - clear BCB <field> or all fields\n"
> +       "bcb test  <field> <op> <val> - test  BCB <field> against <val>\n"
> +       "bcb dump  <field>            - dump  BCB <field>\n"
> +       "bcb store                    - store BCB back to mmc\n"
> +       "\n"
> +       "Legend:\n"
> +       "<dev>   - MMC device index containing the BCB partition\n"
> +       "<part>  - MMC partition index or name containing the BCB\n"
> +       "<field> - one of {command,status,recovery,stage,reserved}\n"
> +       "<op>    - the binary operator used in 'bcb test':\n"
> +       "          '=' returns true if <val> matches the string stored in <field>\n"
> +       "          '~' returns true if <val> matches a subset of <field>'s string\n"
> +       "<val>   - string/text provided as input to bcb {set,test}\n"
> +       "          NOTE: any ':' character in <val> will be replaced by line feed\n"
> +       "          during 'bcb set' and used as separator by upper layers\n"
> +);
> --
> 2.21.0
>

Regards,
Simon


More information about the U-Boot mailing list