[PATCH v2 2/2] cmd: avb: introduce optional interface parameter to avb init

Igor Opaniuk igor.opaniuk at gmail.com
Wed May 4 22:40:18 CEST 2022


Hi Andrii,

On Tue, Apr 19, 2022 at 9:46 AM Andrii Chepurnyi
<andrii.chepurnyi82 at gmail.com> wrote:
>
> From: Andrii Chepurnyi <andrii.chepurnyi82 at gmail.com>
>
> From: Andrii Chepurnyi <andrii_chepurnyi at epam.com>
>
> Originally, avb implementation relay on mmc block devices.
> The interface parameter will give the ability to use avb with
> various block devices by choosing the exact interface type.
> By default (if no interface parameter is provided) mmc interface
> will be used.
>
> Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi at epam.com>
> ---
>  cmd/avb.c            | 13 +++++--------
>  common/avb_verify.c  | 28 ++++++++++------------------
>  doc/android/avb2.rst |  2 +-
>  include/avb_verify.h | 11 ++++++++++-
>  4 files changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/cmd/avb.c b/cmd/avb.c
> index 783f51b816..6fdbdc708f 100644
> --- a/cmd/avb.c
> +++ b/cmd/avb.c
> @@ -17,17 +17,14 @@ static struct AvbOps *avb_ops;
>
>  int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  {
> -       unsigned long mmc_dev;
> -
> -       if (argc != 2)
> +       if (argc != 2 && argc != 3)
>                 return CMD_RET_USAGE;
>
> -       mmc_dev = hextoul(argv[1], NULL);
> -
>         if (avb_ops)
>                 avb_ops_free(avb_ops);
>
> -       avb_ops = avb_ops_alloc(mmc_dev);
> +       avb_ops = avb_ops_alloc(argv[1], (argc == 3) ? argv[2] : "mmc");
> +
>         if (avb_ops)
>                 return CMD_RET_SUCCESS;
>
> @@ -419,7 +416,7 @@ int do_avb_write_pvalue(struct cmd_tbl *cmdtp, int flag, int argc,
>  }
>
>  static struct cmd_tbl cmd_avb[] = {
> -       U_BOOT_CMD_MKENT(init, 2, 0, do_avb_init, "", ""),
> +       U_BOOT_CMD_MKENT(init, 3, 0, do_avb_init, "", ""),
>         U_BOOT_CMD_MKENT(read_rb, 2, 0, do_avb_read_rb, "", ""),
>         U_BOOT_CMD_MKENT(write_rb, 3, 0, do_avb_write_rb, "", ""),
>         U_BOOT_CMD_MKENT(is_unlocked, 1, 0, do_avb_is_unlocked, "", ""),
> @@ -455,7 +452,7 @@ static int do_avb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  U_BOOT_CMD(
>         avb, 29, 0, do_avb,
>         "Provides commands for testing Android Verified Boot 2.0 functionality",
> -       "init <dev> - initialize avb2 for <dev>\n"
> +       "init <dev> [<interface>] - initialize avb2 for <dev> [<interface>]\n"
>         "avb read_rb <num> - read rollback index at location <num>\n"
>         "avb write_rb <num> <rb> - write rollback index <rb> to <num>\n"
>         "avb is_unlocked - returns unlock status of the device\n"
> diff --git a/common/avb_verify.c b/common/avb_verify.c
> index 0520a71455..e086dc6760 100644
> --- a/common/avb_verify.c
> +++ b/common/avb_verify.c
> @@ -338,7 +338,6 @@ static struct mmc_part *get_partition(AvbOps *ops, const char *partition)
>  {
>         int ret;
>         u8 dev_num;
> -       int part_num = 0;
>         struct mmc_part *part;
>         struct blk_desc *mmc_blk;
>
> @@ -347,22 +346,8 @@ static struct mmc_part *get_partition(AvbOps *ops, const char *partition)
>                 return NULL;
>
>         dev_num = get_boot_device(ops);
> -       part->mmc = find_mmc_device(dev_num);
> -       if (!part->mmc) {
> -               printf("No MMC device at slot %x\n", dev_num);
> -               goto err;
> -       }
> -
> -       if (mmc_init(part->mmc)) {
> -               printf("MMC initialization failed\n");
> -               goto err;
> -       }
> +       mmc_blk = get_blk(ops);
>
> -       ret = mmc_switch_part(part->mmc, part_num);
> -       if (ret)
> -               goto err;
> -
> -       mmc_blk = mmc_get_blk_desc(part->mmc);
>         if (!mmc_blk) {
>                 printf("Error - failed to obtain block descriptor\n");
>                 goto err;
> @@ -976,7 +961,8 @@ free_name:
>   * AVB2.0 AvbOps alloc/initialisation/free
>   * ============================================================================
>   */
> -AvbOps *avb_ops_alloc(int boot_device)
> +
> +AvbOps *avb_ops_alloc(const char *boot_device, const char *interface)
>  {
>         struct AvbOpsData *ops_data;
>
> @@ -999,7 +985,13 @@ AvbOps *avb_ops_alloc(int boot_device)
>         ops_data->ops.read_persistent_value = read_persistent_value;
>  #endif
>         ops_data->ops.get_size_of_partition = get_size_of_partition;
> -       ops_data->mmc_dev = boot_device;
> +       ops_data->mmc_dev = simple_strtoul(boot_device, NULL, 16);
> +       ops_data->blk = NULL;
> +       if (interface && (blk_get_device_by_str(interface, boot_device, &ops_data->blk) < 0)) {
> +               printf("Error - failed to obtain block descriptor for devce=%s if=%s\n",
> +                      boot_device, interface);
> +               return NULL;
> +       }
>
>         return &ops_data->ops;
>  }
> diff --git a/doc/android/avb2.rst b/doc/android/avb2.rst
> index a072119574..8fa54338fd 100644
> --- a/doc/android/avb2.rst
> +++ b/doc/android/avb2.rst
> @@ -38,7 +38,7 @@ AVB 2.0 U-Boot shell commands
>  Provides CLI interface to invoke AVB 2.0 verification + misc. commands for
>  different testing purposes::
>
> -    avb init <dev> - initialize avb 2.0 for <dev>
> +    avb init <dev> [<interface>] - initialize avb2 for <dev> [<interface>]
>      avb verify - run verification process using hash data from vbmeta structure
>      avb read_rb <num> - read rollback index at location <num>
>      avb write_rb <num> <rb> - write rollback index <rb> to <num>
> diff --git a/include/avb_verify.h b/include/avb_verify.h
> index 1e787ba666..ff70cb26f8 100644
> --- a/include/avb_verify.h
> +++ b/include/avb_verify.h
> @@ -32,6 +32,7 @@ struct AvbOpsData {
>         struct udevice *tee;
>         u32 session;
>  #endif
> +       struct blk_desc *blk;
>  };
>
>  struct mmc_part {
> @@ -46,7 +47,7 @@ enum mmc_io_type {
>         IO_WRITE
>  };
>
> -AvbOps *avb_ops_alloc(int boot_device);
> +AvbOps *avb_ops_alloc(const char *boot_device, const char *interface);
>  void avb_ops_free(AvbOps *ops);
>
>  char *avb_set_state(AvbOps *ops, enum avb_boot_state boot_state);
> @@ -98,4 +99,12 @@ static inline int get_boot_device(AvbOps *ops)
>         return -1;
>  }
>
> +static inline struct blk_desc *get_blk(AvbOps *ops)
> +{
> +       if (ops && ops->user_data)
> +               return ((struct AvbOpsData *)ops->user_data)->blk;
> +
> +       return NULL;
> +}
> +
>  #endif /* _AVB_VERIFY_H */
> --
> 2.25.1
>

Could you please also rename all other functions in common/avb_verify.c
and drop "mmc_" prefix in their names (mmc_read_and_flush(), mmc_byte_io()).
Thanks!

With the comment addressed:
Acked-by: Igor Opaniuk <igor.opaniuk at gmail.com>

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


More information about the U-Boot mailing list