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

Hi Andrii,

On Tue, Apr 19, 2022 at 9:46 AM Andrii Chepurnyi andrii.chepurnyi82@gmail.com wrote:

From: Andrii Chepurnyi andrii.chepurnyi82@gmail.com

From: Andrii Chepurnyi andrii_chepurnyi@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@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@gmail.com