[PATCH v1 0/3] Support various block interfaces for avb and bcb

[PATCH v1 0/3] Support various block interfaces for avb and bcb

[Andrii Chepurnyi]

Hello.

Originally bcb and avb utilities implementation relay on mmc block devices.
This patch series adds an optional interface parameter to those utilities, 
which gives the ability to use bcb and avb on various block devices.
The patch set was tested using xenguest_arm64 based board and pvblock interface.

Andrii Chepurnyi (3):
  cmd: bcb: introduce optional interface parameter to bcb
  cmd: avb: introduce optional interface parameter to avb init
  cmd: avb: remove warning during avb build

 cmd/avb.c            | 15 ++++------
 cmd/bcb.c            | 65 ++++++++++++++++++++++----------------------
 common/avb_verify.c  | 28 +++++++------------
 doc/android/avb2.rst |  2 +-
 doc/android/bcb.rst  | 33 ++++++++++++----------
 include/avb_verify.h | 11 +++++++-
 6 files changed, 77 insertions(+), 77 deletions(-)

-- 
2.25.1

[PATCH v1 1/3] cmd: bcb: introduce optional interface parameter to bcb

[Andrii Chepurnyi]

From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>

From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

Originally, bcb implementation relay on mmc block devices.
The interface parameter will give the ability to use bcb 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/bcb.c           | 65 +++++++++++++++++++++++----------------------
 doc/android/bcb.rst | 33 ++++++++++++-----------
 2 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 92f4d27990..bfe395558e 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -26,6 +26,7 @@ enum bcb_cmd {
 static int bcb_dev = -1;
 static int bcb_part = -1;
 static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
+static struct blk_desc *bcb_blk_desc;
 
 static int bcb_cmd_get(char *cmd)
 {
@@ -51,6 +52,9 @@ static int bcb_is_misused(int argc, char *const argv[])
 
 	switch (cmd) {
 	case BCB_CMD_LOAD:
+		if (argc != 3 && argc != 4)
+			goto err;
+		break;
 	case BCB_CMD_FIELD_SET:
 		if (argc != 3)
 			goto err;
@@ -115,25 +119,23 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep)
 
 static int __bcb_load(int devnum, const char *partp)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	char *endp;
 	int part, ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, devnum);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err_read_fail;
 	}
 
 	part = simple_strtoul(partp, &endp, 0);
 	if (*endp == '\0') {
-		ret = part_get_info(desc, part, &info);
+		ret = part_get_info(bcb_blk_desc, part, &info);
 		if (ret)
 			goto err_read_fail;
 	} else {
-		part = part_get_info_by_name(desc, partp, &info);
+		part = part_get_info_by_name(bcb_blk_desc, partp, &info);
 		if (part < 0) {
 			ret = part;
 			goto err_read_fail;
@@ -144,12 +146,12 @@ static int __bcb_load(int devnum, const char *partp)
 	if (cnt > info.size)
 		goto err_too_small;
 
-	if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dread(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err_read_fail;
 	}
 
-	bcb_dev = desc->devnum;
+	bcb_dev = bcb_blk_desc->devnum;
 	bcb_part = part;
 	debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
 
@@ -170,15 +172,15 @@ err:
 static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 		       char * const argv[])
 {
-	char *endp;
-	int devnum = simple_strtoul(argv[1], &endp, 0);
+	int ret = blk_get_device_by_str((argv[3]) ? argv[3] : "mmc", argv[1], &bcb_blk_desc);
 
-	if (*endp != '\0') {
-		printf("Error: Device id '%s' not a number\n", argv[1]);
+	if (ret < 0) {
+		printf("Error: Device id '%s' or interface '%s' is not valid\n", argv[1],
+		       (argv[3]) ? argv[3] : "mmc");
 		return CMD_RET_FAILURE;
 	}
 
-	return __bcb_load(devnum, argv[2]);
+	return __bcb_load(bcb_blk_desc->devnum, argv[2]);
 }
 
 static int __bcb_set(char *fieldp, const char *valp)
@@ -281,24 +283,22 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
 
 static int __bcb_store(void)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	int ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, bcb_dev);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err;
 	}
 
-	ret = part_get_info(desc, bcb_part, &info);
+	ret = part_get_info(bcb_blk_desc, bcb_part, &info);
 	if (ret)
 		goto err;
 
 	cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
 
-	if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dwrite(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err;
 	}
@@ -373,21 +373,22 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 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"
+	"load  <dev> <part> [<interface>]   - load  BCB from <dev>:<part>[<interface>]\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"
+	"<dev>          - device index containing the BCB partition\n"
+	"<part>         - partition index or name containing the BCB\n"
+	"<interface>    - interface name of the block device 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"
 );
diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst
index 8861608300..e6d201f439 100644
--- a/doc/android/bcb.rst
+++ b/doc/android/bcb.rst
@@ -41,23 +41,26 @@ requirements enumerated above. Below is the command's help message::
    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
+   bcb load  <dev> <part> [<iftype>]    - load  BCB from <dev>:<part>[<iftype>]
+   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 <iftype>
 
    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
+   <dev>    - device index containing the BCB partition
+   <iftype> - Optional parameter of the interface type for the specified
+              block device like: mmc,sd,virtio see blk.h for details.
+              The default value is mmc.
+   <part>   - 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'. Example of getting reboot reason
-- 
2.25.1

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

[Andrii Chepurnyi]

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

[PATCH v1 3/3] cmd: avb: remove warning during avb build

[Andrii Chepurnyi]

From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>

From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

Remove warning during avb build:
cmd/avb.c:247:8: warning: unused variable ‘extra_args’ [-Wunused-variable]
  247 |  char *extra_args;
      |        ^~~~~~~~~~
cmd/avb.c:246:8: warning: unused variable ‘cmdline’ [-Wunused-variable]
  246 |  char *cmdline;
      |        ^~~~~~~

Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
---
 cmd/avb.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/cmd/avb.c b/cmd/avb.c
index 6fdbdc708f..a5ac224b22 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -231,8 +231,6 @@ int do_avb_verify_part(struct cmd_tbl *cmdtp, int flag,
 	const char * const requested_partitions[] = {"boot", NULL};
 	AvbSlotVerifyResult slot_result;
 	AvbSlotVerifyData *out_data;
-	char *cmdline;
-	char *extra_args;
 	char *slot_suffix = "";
 
 	bool unlocked = false;
-- 
2.25.1

[PATCH v2 0/2] Support various block interfaces for avb and bcb

[Andrii Chepurnyi]

Hello.

Originally bcb and avb utilities implementation relay on mmc block devices.
This patch series adds an optional interface parameter to those utilities,
which gives the ability to use bcb and avb on various block devices.
The patch set was tested using xenguest_arm64 based board and pvblock interface.

Changes for v2:
  - Removed patch #3 as not actual for mainline

Andrii Chepurnyi (2):
  cmd: bcb: introduce optional interface parameter to bcb
  cmd: avb: introduce optional interface parameter to avb init

 cmd/avb.c            | 13 ++++-----
 cmd/bcb.c            | 65 ++++++++++++++++++++++----------------------
 common/avb_verify.c  | 28 +++++++------------
 doc/android/avb2.rst |  2 +-
 doc/android/bcb.rst  | 33 ++++++++++++----------
 include/avb_verify.h | 11 +++++++-
 6 files changed, 77 insertions(+), 75 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] cmd: bcb: introduce optional interface parameter to bcb

[Andrii Chepurnyi]

From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>

From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

Originally, bcb implementation relay on mmc block devices.
The interface parameter will give the ability to use bcb 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/bcb.c           | 65 +++++++++++++++++++++++----------------------
 doc/android/bcb.rst | 33 ++++++++++++-----------
 2 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 92f4d27990..bfe395558e 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -26,6 +26,7 @@ enum bcb_cmd {
 static int bcb_dev = -1;
 static int bcb_part = -1;
 static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
+static struct blk_desc *bcb_blk_desc;
 
 static int bcb_cmd_get(char *cmd)
 {
@@ -51,6 +52,9 @@ static int bcb_is_misused(int argc, char *const argv[])
 
 	switch (cmd) {
 	case BCB_CMD_LOAD:
+		if (argc != 3 && argc != 4)
+			goto err;
+		break;
 	case BCB_CMD_FIELD_SET:
 		if (argc != 3)
 			goto err;
@@ -115,25 +119,23 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep)
 
 static int __bcb_load(int devnum, const char *partp)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	char *endp;
 	int part, ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, devnum);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err_read_fail;
 	}
 
 	part = simple_strtoul(partp, &endp, 0);
 	if (*endp == '\0') {
-		ret = part_get_info(desc, part, &info);
+		ret = part_get_info(bcb_blk_desc, part, &info);
 		if (ret)
 			goto err_read_fail;
 	} else {
-		part = part_get_info_by_name(desc, partp, &info);
+		part = part_get_info_by_name(bcb_blk_desc, partp, &info);
 		if (part < 0) {
 			ret = part;
 			goto err_read_fail;
@@ -144,12 +146,12 @@ static int __bcb_load(int devnum, const char *partp)
 	if (cnt > info.size)
 		goto err_too_small;
 
-	if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dread(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err_read_fail;
 	}
 
-	bcb_dev = desc->devnum;
+	bcb_dev = bcb_blk_desc->devnum;
 	bcb_part = part;
 	debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
 
@@ -170,15 +172,15 @@ err:
 static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 		       char * const argv[])
 {
-	char *endp;
-	int devnum = simple_strtoul(argv[1], &endp, 0);
+	int ret = blk_get_device_by_str((argv[3]) ? argv[3] : "mmc", argv[1], &bcb_blk_desc);
 
-	if (*endp != '\0') {
-		printf("Error: Device id '%s' not a number\n", argv[1]);
+	if (ret < 0) {
+		printf("Error: Device id '%s' or interface '%s' is not valid\n", argv[1],
+		       (argv[3]) ? argv[3] : "mmc");
 		return CMD_RET_FAILURE;
 	}
 
-	return __bcb_load(devnum, argv[2]);
+	return __bcb_load(bcb_blk_desc->devnum, argv[2]);
 }
 
 static int __bcb_set(char *fieldp, const char *valp)
@@ -281,24 +283,22 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
 
 static int __bcb_store(void)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	int ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, bcb_dev);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err;
 	}
 
-	ret = part_get_info(desc, bcb_part, &info);
+	ret = part_get_info(bcb_blk_desc, bcb_part, &info);
 	if (ret)
 		goto err;
 
 	cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
 
-	if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dwrite(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err;
 	}
@@ -373,21 +373,22 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 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"
+	"load  <dev> <part> [<interface>]   - load  BCB from <dev>:<part>[<interface>]\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"
+	"<dev>          - device index containing the BCB partition\n"
+	"<part>         - partition index or name containing the BCB\n"
+	"<interface>    - interface name of the block device 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"
 );
diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst
index 8861608300..e6d201f439 100644
--- a/doc/android/bcb.rst
+++ b/doc/android/bcb.rst
@@ -41,23 +41,26 @@ requirements enumerated above. Below is the command's help message::
    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
+   bcb load  <dev> <part> [<iftype>]    - load  BCB from <dev>:<part>[<iftype>]
+   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 <iftype>
 
    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
+   <dev>    - device index containing the BCB partition
+   <iftype> - Optional parameter of the interface type for the specified
+              block device like: mmc,sd,virtio see blk.h for details.
+              The default value is mmc.
+   <part>   - 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'. Example of getting reboot reason
-- 
2.25.1

Re: [PATCH v2 1/2] cmd: bcb: introduce optional interface parameter to bcb

[Igor Opaniuk]

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, bcb implementation relay on mmc block devices.
> The interface parameter will give the ability to use bcb 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/bcb.c           | 65 +++++++++++++++++++++++----------------------
>  doc/android/bcb.rst | 33 ++++++++++++-----------
>  2 files changed, 51 insertions(+), 47 deletions(-)
>
> diff --git a/cmd/bcb.c b/cmd/bcb.c
> index 92f4d27990..bfe395558e 100644
> --- a/cmd/bcb.c
> +++ b/cmd/bcb.c
> @@ -26,6 +26,7 @@ enum bcb_cmd {
>  static int bcb_dev = -1;
>  static int bcb_part = -1;
>  static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
> +static struct blk_desc *bcb_blk_desc;
>
>  static int bcb_cmd_get(char *cmd)
>  {
> @@ -51,6 +52,9 @@ static int bcb_is_misused(int argc, char *const argv[])
>
>         switch (cmd) {
>         case BCB_CMD_LOAD:
> +               if (argc != 3 && argc != 4)
> +                       goto err;
> +               break;
>         case BCB_CMD_FIELD_SET:
>                 if (argc != 3)
>                         goto err;
> @@ -115,25 +119,23 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep)
>
>  static int __bcb_load(int devnum, const char *partp)
>  {
> -       struct blk_desc *desc;
>         struct disk_partition info;
>         u64 cnt;
>         char *endp;
>         int part, ret;
>
> -       desc = blk_get_devnum_by_type(IF_TYPE_MMC, devnum);
> -       if (!desc) {
> +       if (!bcb_blk_desc) {
>                 ret = -ENODEV;
>                 goto err_read_fail;
>         }
>
>         part = simple_strtoul(partp, &endp, 0);
>         if (*endp == '\0') {
> -               ret = part_get_info(desc, part, &info);
> +               ret = part_get_info(bcb_blk_desc, part, &info);
>                 if (ret)
>                         goto err_read_fail;
>         } else {
> -               part = part_get_info_by_name(desc, partp, &info);
> +               part = part_get_info_by_name(bcb_blk_desc, partp, &info);
>                 if (part < 0) {
>                         ret = part;
>                         goto err_read_fail;
> @@ -144,12 +146,12 @@ static int __bcb_load(int devnum, const char *partp)
>         if (cnt > info.size)
>                 goto err_too_small;
>
> -       if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
> +       if (blk_dread(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
>                 ret = -EIO;
>                 goto err_read_fail;
>         }
>
> -       bcb_dev = desc->devnum;
> +       bcb_dev = bcb_blk_desc->devnum;
>         bcb_part = part;
>         debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
>
> @@ -170,15 +172,15 @@ err:
>  static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
>                        char * const argv[])
>  {
> -       char *endp;
> -       int devnum = simple_strtoul(argv[1], &endp, 0);
> +       int ret = blk_get_device_by_str((argv[3]) ? argv[3] : "mmc", argv[1], &bcb_blk_desc);
>
> -       if (*endp != '\0') {
> -               printf("Error: Device id '%s' not a number\n", argv[1]);
> +       if (ret < 0) {
> +               printf("Error: Device id '%s' or interface '%s' is not valid\n", argv[1],
> +                      (argv[3]) ? argv[3] : "mmc");
>                 return CMD_RET_FAILURE;
>         }
>
> -       return __bcb_load(devnum, argv[2]);
> +       return __bcb_load(bcb_blk_desc->devnum, argv[2]);
>  }
>
>  static int __bcb_set(char *fieldp, const char *valp)
> @@ -281,24 +283,22 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
>
>  static int __bcb_store(void)
>  {
> -       struct blk_desc *desc;
>         struct disk_partition info;
>         u64 cnt;
>         int ret;
>
> -       desc = blk_get_devnum_by_type(IF_TYPE_MMC, bcb_dev);
> -       if (!desc) {
> +       if (!bcb_blk_desc) {
>                 ret = -ENODEV;
>                 goto err;
>         }
>
> -       ret = part_get_info(desc, bcb_part, &info);
> +       ret = part_get_info(bcb_blk_desc, bcb_part, &info);
>         if (ret)
>                 goto err;
>
>         cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
>
> -       if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
> +       if (blk_dwrite(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
>                 ret = -EIO;
>                 goto err;
>         }
> @@ -373,21 +373,22 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  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"
> +       "load  <dev> <part> [<interface>]   - load  BCB from <dev>:<part>[<interface>]\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"
> +       "<dev>          - device index containing the BCB partition\n"
> +       "<part>         - partition index or name containing the BCB\n"
> +       "<interface>    - interface name of the block device 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"
>  );
> diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst
> index 8861608300..e6d201f439 100644
> --- a/doc/android/bcb.rst
> +++ b/doc/android/bcb.rst
> @@ -41,23 +41,26 @@ requirements enumerated above. Below is the command's help message::
>     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
> +   bcb load  <dev> <part> [<iftype>]    - load  BCB from <dev>:<part>[<iftype>]
> +   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 <iftype>
>
>     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
> +   <dev>    - device index containing the BCB partition
> +   <iftype> - Optional parameter of the interface type for the specified
> +              block device like: mmc,sd,virtio see blk.h for details.
> +              The default value is mmc.
> +   <part>   - 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'. Example of getting reboot reason
> --
> 2.25.1
>

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

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

[Andrii Chepurnyi]

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

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

[Igor Opaniuk]

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>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[PATCH v3 0/3] Support various block interfaces for avb and bcb

[Andrii Chepurnyi]

Hello.

Originally bcb and avb utilities implementation relay on mmc block devices.
This patch series adds an optional interface parameter to those utilities,
which gives the ability to use bcb and avb on various block devices.
The patch set was tested using xenguest_arm64 based board and pvblock interface.

Changes for v3:
  - Added RB and ACK
  - Added patch #3 by the request

Changes for v2:
  - Removed patch #3 as not actual for mainline

Andrii Chepurnyi (3):
  cmd: bcb: introduce optional interface parameter to bcb
  cmd: avb: introduce optional interface parameter to avb init
  cmd: avb: remove mmc naming from generic block code

 cmd/avb.c            | 13 +++-----
 cmd/bcb.c            | 65 ++++++++++++++++++-------------------
 common/avb_verify.c  | 76 ++++++++++++++++++++------------------------
 doc/android/avb2.rst |  2 +-
 doc/android/bcb.rst  | 33 ++++++++++---------
 include/avb_verify.h | 24 +++++++++-----
 6 files changed, 107 insertions(+), 106 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] cmd: bcb: introduce optional interface parameter to bcb

[Andrii Chepurnyi]

From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>

From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

Originally, bcb implementation relay on mmc block devices.
The interface parameter will give the ability to use bcb with
various block devices by choosing the exact interface type.
By default (if no interface parameter is provided) mmc interface
will be used.

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
---
 cmd/bcb.c           | 65 +++++++++++++++++++++++----------------------
 doc/android/bcb.rst | 33 ++++++++++++-----------
 2 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 92f4d27990..bfe395558e 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -26,6 +26,7 @@ enum bcb_cmd {
 static int bcb_dev = -1;
 static int bcb_part = -1;
 static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
+static struct blk_desc *bcb_blk_desc;
 
 static int bcb_cmd_get(char *cmd)
 {
@@ -51,6 +52,9 @@ static int bcb_is_misused(int argc, char *const argv[])
 
 	switch (cmd) {
 	case BCB_CMD_LOAD:
+		if (argc != 3 && argc != 4)
+			goto err;
+		break;
 	case BCB_CMD_FIELD_SET:
 		if (argc != 3)
 			goto err;
@@ -115,25 +119,23 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep)
 
 static int __bcb_load(int devnum, const char *partp)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	char *endp;
 	int part, ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, devnum);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err_read_fail;
 	}
 
 	part = simple_strtoul(partp, &endp, 0);
 	if (*endp == '\0') {
-		ret = part_get_info(desc, part, &info);
+		ret = part_get_info(bcb_blk_desc, part, &info);
 		if (ret)
 			goto err_read_fail;
 	} else {
-		part = part_get_info_by_name(desc, partp, &info);
+		part = part_get_info_by_name(bcb_blk_desc, partp, &info);
 		if (part < 0) {
 			ret = part;
 			goto err_read_fail;
@@ -144,12 +146,12 @@ static int __bcb_load(int devnum, const char *partp)
 	if (cnt > info.size)
 		goto err_too_small;
 
-	if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dread(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err_read_fail;
 	}
 
-	bcb_dev = desc->devnum;
+	bcb_dev = bcb_blk_desc->devnum;
 	bcb_part = part;
 	debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
 
@@ -170,15 +172,15 @@ err:
 static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 		       char * const argv[])
 {
-	char *endp;
-	int devnum = simple_strtoul(argv[1], &endp, 0);
+	int ret = blk_get_device_by_str((argv[3]) ? argv[3] : "mmc", argv[1], &bcb_blk_desc);
 
-	if (*endp != '\0') {
-		printf("Error: Device id '%s' not a number\n", argv[1]);
+	if (ret < 0) {
+		printf("Error: Device id '%s' or interface '%s' is not valid\n", argv[1],
+		       (argv[3]) ? argv[3] : "mmc");
 		return CMD_RET_FAILURE;
 	}
 
-	return __bcb_load(devnum, argv[2]);
+	return __bcb_load(bcb_blk_desc->devnum, argv[2]);
 }
 
 static int __bcb_set(char *fieldp, const char *valp)
@@ -281,24 +283,22 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
 
 static int __bcb_store(void)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	int ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, bcb_dev);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err;
 	}
 
-	ret = part_get_info(desc, bcb_part, &info);
+	ret = part_get_info(bcb_blk_desc, bcb_part, &info);
 	if (ret)
 		goto err;
 
 	cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
 
-	if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dwrite(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err;
 	}
@@ -373,21 +373,22 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 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"
+	"load  <dev> <part> [<interface>]   - load  BCB from <dev>:<part>[<interface>]\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"
+	"<dev>          - device index containing the BCB partition\n"
+	"<part>         - partition index or name containing the BCB\n"
+	"<interface>    - interface name of the block device 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"
 );
diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst
index 8861608300..e6d201f439 100644
--- a/doc/android/bcb.rst
+++ b/doc/android/bcb.rst
@@ -41,23 +41,26 @@ requirements enumerated above. Below is the command's help message::
    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
+   bcb load  <dev> <part> [<iftype>]    - load  BCB from <dev>:<part>[<iftype>]
+   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 <iftype>
 
    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
+   <dev>    - device index containing the BCB partition
+   <iftype> - Optional parameter of the interface type for the specified
+              block device like: mmc,sd,virtio see blk.h for details.
+              The default value is mmc.
+   <part>   - 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'. Example of getting reboot reason
-- 
2.25.1

Re: [PATCH v3 1/3] cmd: bcb: introduce optional interface parameter to bcb

[Igor Opaniuk]

Hello Andrii,

On Wed, Jul 20, 2022 at 6:00 PM Andrii Chepurnyi
<andrii.chepurnyi82@gmail.com> wrote:
>
> From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>
>
> From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
>
> Originally, bcb implementation relay on mmc block devices.
> The interface parameter will give the ability to use bcb with
> various block devices by choosing the exact interface type.
> By default (if no interface parameter is provided) mmc interface
> will be used.
>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> ---
>  cmd/bcb.c           | 65 +++++++++++++++++++++++----------------------
>  doc/android/bcb.rst | 33 ++++++++++++-----------
>  2 files changed, 51 insertions(+), 47 deletions(-)
>
> diff --git a/cmd/bcb.c b/cmd/bcb.c
> index 92f4d27990..bfe395558e 100644
> --- a/cmd/bcb.c
> +++ b/cmd/bcb.c
> @@ -26,6 +26,7 @@ enum bcb_cmd {
>  static int bcb_dev = -1;
>  static int bcb_part = -1;
>  static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
> +static struct blk_desc *bcb_blk_desc;
>
>  static int bcb_cmd_get(char *cmd)
>  {
> @@ -51,6 +52,9 @@ static int bcb_is_misused(int argc, char *const argv[])
>
>         switch (cmd) {
>         case BCB_CMD_LOAD:
> +               if (argc != 3 && argc != 4)
> +                       goto err;
> +               break;
>         case BCB_CMD_FIELD_SET:
>                 if (argc != 3)
>                         goto err;
> @@ -115,25 +119,23 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep)
>
>  static int __bcb_load(int devnum, const char *partp)
>  {
> -       struct blk_desc *desc;
>         struct disk_partition info;
>         u64 cnt;
>         char *endp;
>         int part, ret;
>
> -       desc = blk_get_devnum_by_type(IF_TYPE_MMC, devnum);
> -       if (!desc) {
> +       if (!bcb_blk_desc) {
>                 ret = -ENODEV;
>                 goto err_read_fail;
>         }
>
>         part = simple_strtoul(partp, &endp, 0);
>         if (*endp == '\0') {
> -               ret = part_get_info(desc, part, &info);
> +               ret = part_get_info(bcb_blk_desc, part, &info);
>                 if (ret)
>                         goto err_read_fail;
>         } else {
> -               part = part_get_info_by_name(desc, partp, &info);
> +               part = part_get_info_by_name(bcb_blk_desc, partp, &info);
>                 if (part < 0) {
>                         ret = part;
>                         goto err_read_fail;
> @@ -144,12 +146,12 @@ static int __bcb_load(int devnum, const char *partp)
>         if (cnt > info.size)
>                 goto err_too_small;
>
> -       if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
> +       if (blk_dread(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
>                 ret = -EIO;
>                 goto err_read_fail;
>         }
>
> -       bcb_dev = desc->devnum;
> +       bcb_dev = bcb_blk_desc->devnum;
>         bcb_part = part;
>         debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
>
> @@ -170,15 +172,15 @@ err:
>  static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
>                        char * const argv[])
>  {
> -       char *endp;
> -       int devnum = simple_strtoul(argv[1], &endp, 0);
> +       int ret = blk_get_device_by_str((argv[3]) ? argv[3] : "mmc", argv[1], &bcb_blk_desc);
>
> -       if (*endp != '\0') {
> -               printf("Error: Device id '%s' not a number\n", argv[1]);
> +       if (ret < 0) {
> +               printf("Error: Device id '%s' or interface '%s' is not valid\n", argv[1],
> +                      (argv[3]) ? argv[3] : "mmc");
>                 return CMD_RET_FAILURE;
>         }
>
> -       return __bcb_load(devnum, argv[2]);
> +       return __bcb_load(bcb_blk_desc->devnum, argv[2]);
>  }
>
>  static int __bcb_set(char *fieldp, const char *valp)
> @@ -281,24 +283,22 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
>
>  static int __bcb_store(void)
>  {
> -       struct blk_desc *desc;
>         struct disk_partition info;
>         u64 cnt;
>         int ret;
>
> -       desc = blk_get_devnum_by_type(IF_TYPE_MMC, bcb_dev);
> -       if (!desc) {
> +       if (!bcb_blk_desc) {
>                 ret = -ENODEV;
>                 goto err;
>         }
>
> -       ret = part_get_info(desc, bcb_part, &info);
> +       ret = part_get_info(bcb_blk_desc, bcb_part, &info);
>         if (ret)
>                 goto err;
>
>         cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
>
> -       if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
> +       if (blk_dwrite(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
>                 ret = -EIO;
>                 goto err;
>         }
> @@ -373,21 +373,22 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  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"
> +       "load  <dev> <part> [<interface>]   - load  BCB from <dev>:<part>[<interface>]\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"
> +       "<dev>          - device index containing the BCB partition\n"
> +       "<part>         - partition index or name containing the BCB\n"
> +       "<interface>    - interface name of the block device 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"
>  );
> diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst
> index 8861608300..e6d201f439 100644
> --- a/doc/android/bcb.rst
> +++ b/doc/android/bcb.rst
> @@ -41,23 +41,26 @@ requirements enumerated above. Below is the command's help message::
>     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
> +   bcb load  <dev> <part> [<iftype>]    - load  BCB from <dev>:<part>[<iftype>]
> +   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 <iftype>
>
>     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
> +   <dev>    - device index containing the BCB partition
> +   <iftype> - Optional parameter of the interface type for the specified
> +              block device like: mmc,sd,virtio see blk.h for details.
> +              The default value is mmc.
> +   <part>   - 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'. Example of getting reboot reason
> --
> 2.25.1
>

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

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

[Andrii Chepurnyi]

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.

Acked-by: Igor Opaniuk <igor.opaniuk@gmail.com>
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

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

[Igor Opaniuk]

Hi Andrii,

On Wed, Jul 20, 2022 at 6:00 PM 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.
>
> Acked-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> 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
>

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[PATCH v3 3/3] cmd: avb: remove mmc naming from generic block code

[Andrii Chepurnyi]

From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>

From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

Part of avb code uses mmc notation, but in fact
it uses generic block functions.

Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
---
 common/avb_verify.c  | 52 ++++++++++++++++++++++----------------------
 include/avb_verify.h | 13 +++++------
 2 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/common/avb_verify.c b/common/avb_verify.c
index e086dc6760..3c9594d6d7 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -253,10 +253,10 @@ char *avb_set_enforce_verity(const char *cmdline)
 
 /**
  * ============================================================================
- * IO(mmc) auxiliary functions
+ * IO auxiliary functions
  * ============================================================================
  */
-static unsigned long mmc_read_and_flush(struct mmc_part *part,
+static unsigned long blk_read_and_flush(struct blk_part *part,
 					lbaint_t start,
 					lbaint_t sectors,
 					void *buffer)
@@ -291,7 +291,7 @@ static unsigned long mmc_read_and_flush(struct mmc_part *part,
 		tmp_buf = buffer;
 	}
 
-	blks = blk_dread(part->mmc_blk,
+	blks = blk_dread(part->blk,
 			 start, sectors, tmp_buf);
 	/* flush cache after read */
 	flush_cache((ulong)tmp_buf, sectors * part->info.blksz);
@@ -302,7 +302,7 @@ static unsigned long mmc_read_and_flush(struct mmc_part *part,
 	return blks;
 }
 
-static unsigned long mmc_write(struct mmc_part *part, lbaint_t start,
+static unsigned long blk_write(struct blk_part *part, lbaint_t start,
 			       lbaint_t sectors, void *buffer)
 {
 	void *tmp_buf;
@@ -330,37 +330,37 @@ static unsigned long mmc_write(struct mmc_part *part, lbaint_t start,
 		tmp_buf = buffer;
 	}
 
-	return blk_dwrite(part->mmc_blk,
+	return blk_dwrite(part->blk,
 			  start, sectors, tmp_buf);
 }
 
-static struct mmc_part *get_partition(AvbOps *ops, const char *partition)
+static struct blk_part *get_partition(AvbOps *ops, const char *partition)
 {
 	int ret;
 	u8 dev_num;
-	struct mmc_part *part;
-	struct blk_desc *mmc_blk;
+	struct blk_part *part;
+	struct blk_desc *blk;
 
-	part = malloc(sizeof(struct mmc_part));
+	part = malloc(sizeof(struct blk_part));
 	if (!part)
 		return NULL;
 
 	dev_num = get_boot_device(ops);
-	mmc_blk = get_blk(ops);
+	blk = get_blk(ops);
 
-	if (!mmc_blk) {
+	if (!blk) {
 		printf("Error - failed to obtain block descriptor\n");
 		goto err;
 	}
 
-	ret = part_get_info_by_name(mmc_blk, partition, &part->info);
+	ret = part_get_info_by_name(blk, partition, &part->info);
 	if (ret < 0) {
 		printf("Can't find partition '%s'\n", partition);
 		goto err;
 	}
 
 	part->dev_num = dev_num;
-	part->mmc_blk = mmc_blk;
+	part->blk = blk;
 
 	return part;
 err:
@@ -368,16 +368,16 @@ err:
 	return NULL;
 }
 
-static AvbIOResult mmc_byte_io(AvbOps *ops,
+static AvbIOResult blk_byte_io(AvbOps *ops,
 			       const char *partition,
 			       s64 offset,
 			       size_t num_bytes,
 			       void *buffer,
 			       size_t *out_num_read,
-			       enum mmc_io_type io_type)
+			       enum io_type io_type)
 {
 	ulong ret;
-	struct mmc_part *part;
+	struct blk_part *part;
 	u64 start_offset, start_sector, sectors, residue;
 	u8 *tmp_buf;
 	size_t io_cnt = 0;
@@ -410,7 +410,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 			}
 
 			if (io_type == IO_READ) {
-				ret = mmc_read_and_flush(part,
+				ret = blk_read_and_flush(part,
 							 part->info.start +
 							 start_sector,
 							 1, tmp_buf);
@@ -427,7 +427,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 				tmp_buf += (start_offset % part->info.blksz);
 				memcpy(buffer, (void *)tmp_buf, residue);
 			} else {
-				ret = mmc_read_and_flush(part,
+				ret = blk_read_and_flush(part,
 							 part->info.start +
 							 start_sector,
 							 1, tmp_buf);
@@ -441,7 +441,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 					start_offset % part->info.blksz,
 					buffer, residue);
 
-				ret = mmc_write(part, part->info.start +
+				ret = blk_write(part, part->info.start +
 						start_sector, 1, tmp_buf);
 				if (ret != 1) {
 					printf("%s: write error (%ld, %lld)\n",
@@ -459,12 +459,12 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 
 		if (sectors) {
 			if (io_type == IO_READ) {
-				ret = mmc_read_and_flush(part,
+				ret = blk_read_and_flush(part,
 							 part->info.start +
 							 start_sector,
 							 sectors, buffer);
 			} else {
-				ret = mmc_write(part,
+				ret = blk_write(part,
 						part->info.start +
 						start_sector,
 						sectors, buffer);
@@ -520,7 +520,7 @@ static AvbIOResult read_from_partition(AvbOps *ops,
 				       void *buffer,
 				       size_t *out_num_read)
 {
-	return mmc_byte_io(ops, partition_name, offset_from_partition,
+	return blk_byte_io(ops, partition_name, offset_from_partition,
 			   num_bytes, buffer, out_num_read, IO_READ);
 }
 
@@ -547,7 +547,7 @@ static AvbIOResult write_to_partition(AvbOps *ops,
 				      size_t num_bytes,
 				      const void *buffer)
 {
-	return mmc_byte_io(ops, partition_name, offset_from_partition,
+	return blk_byte_io(ops, partition_name, offset_from_partition,
 			   num_bytes, (void *)buffer, NULL, IO_WRITE);
 }
 
@@ -788,7 +788,7 @@ static AvbIOResult get_unique_guid_for_partition(AvbOps *ops,
 						 char *guid_buf,
 						 size_t guid_buf_size)
 {
-	struct mmc_part *part;
+	struct blk_part *part;
 	size_t uuid_size;
 
 	part = get_partition(ops, partition);
@@ -822,7 +822,7 @@ static AvbIOResult get_size_of_partition(AvbOps *ops,
 					 const char *partition,
 					 u64 *out_size_num_bytes)
 {
-	struct mmc_part *part;
+	struct blk_part *part;
 
 	if (!out_size_num_bytes)
 		return AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE;
@@ -985,7 +985,7 @@ AvbOps *avb_ops_alloc(const char *boot_device, const char *interface)
 	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 = simple_strtoul(boot_device, NULL, 16);
+	ops_data->blk_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",
diff --git a/include/avb_verify.h b/include/avb_verify.h
index ff70cb26f8..d095649c0a 100644
--- a/include/avb_verify.h
+++ b/include/avb_verify.h
@@ -26,7 +26,7 @@ enum avb_boot_state {
 
 struct AvbOpsData {
 	struct AvbOps ops;
-	int mmc_dev;
+	int blk_dev;
 	enum avb_boot_state boot_state;
 #ifdef CONFIG_OPTEE_TA_AVB
 	struct udevice *tee;
@@ -35,14 +35,13 @@ struct AvbOpsData {
 	struct blk_desc *blk;
 };
 
-struct mmc_part {
+struct blk_part {
 	int dev_num;
-	struct mmc *mmc;
-	struct blk_desc *mmc_blk;
+	struct blk_desc *blk;
 	struct disk_partition info;
 };
 
-enum mmc_io_type {
+enum io_type {
 	IO_READ,
 	IO_WRITE
 };
@@ -61,7 +60,7 @@ char *append_cmd_line(char *cmdline_orig, char *cmdline_new);
  * I/O helper inline functions
  * ============================================================================
  */
-static inline uint64_t calc_offset(struct mmc_part *part, int64_t offset)
+static inline uint64_t calc_offset(struct blk_part *part, int64_t offset)
 {
 	u64 part_size = part->info.size * part->info.blksz;
 
@@ -93,7 +92,7 @@ static inline int get_boot_device(AvbOps *ops)
 	if (ops) {
 		data = ops->user_data;
 		if (data)
-			return data->mmc_dev;
+			return data->blk_dev;
 	}
 
 	return -1;
-- 
2.25.1

Re: [PATCH v3 3/3] cmd: avb: remove mmc naming from generic block code

[Igor Opaniuk]

Hello Andrii,

On Wed, Jul 20, 2022 at 6:00 PM Andrii Chepurnyi
<andrii.chepurnyi82@gmail.com> wrote:
>
> From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>
>
> From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
>
> Part of avb code uses mmc notation, but in fact
> it uses generic block functions.
>
> Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> ---
>  common/avb_verify.c  | 52 ++++++++++++++++++++++----------------------
>  include/avb_verify.h | 13 +++++------
>  2 files changed, 32 insertions(+), 33 deletions(-)
>
> diff --git a/common/avb_verify.c b/common/avb_verify.c
> index e086dc6760..3c9594d6d7 100644
> --- a/common/avb_verify.c
> +++ b/common/avb_verify.c
> @@ -253,10 +253,10 @@ char *avb_set_enforce_verity(const char *cmdline)
>
>  /**
>   * ============================================================================
> - * IO(mmc) auxiliary functions
> + * IO auxiliary functions
>   * ============================================================================
>   */
> -static unsigned long mmc_read_and_flush(struct mmc_part *part,
> +static unsigned long blk_read_and_flush(struct blk_part *part,
>                                         lbaint_t start,
>                                         lbaint_t sectors,
>                                         void *buffer)
> @@ -291,7 +291,7 @@ static unsigned long mmc_read_and_flush(struct mmc_part *part,
>                 tmp_buf = buffer;
>         }
>
> -       blks = blk_dread(part->mmc_blk,
> +       blks = blk_dread(part->blk,
>                          start, sectors, tmp_buf);
>         /* flush cache after read */
>         flush_cache((ulong)tmp_buf, sectors * part->info.blksz);
> @@ -302,7 +302,7 @@ static unsigned long mmc_read_and_flush(struct mmc_part *part,
>         return blks;
>  }
>
> -static unsigned long mmc_write(struct mmc_part *part, lbaint_t start,
> +static unsigned long blk_write(struct blk_part *part, lbaint_t start,
>                                lbaint_t sectors, void *buffer)
>  {
>         void *tmp_buf;
> @@ -330,37 +330,37 @@ static unsigned long mmc_write(struct mmc_part *part, lbaint_t start,
>                 tmp_buf = buffer;
>         }
>
> -       return blk_dwrite(part->mmc_blk,
> +       return blk_dwrite(part->blk,
>                           start, sectors, tmp_buf);
>  }
>
> -static struct mmc_part *get_partition(AvbOps *ops, const char *partition)
> +static struct blk_part *get_partition(AvbOps *ops, const char *partition)
>  {
>         int ret;
>         u8 dev_num;
> -       struct mmc_part *part;
> -       struct blk_desc *mmc_blk;
> +       struct blk_part *part;
> +       struct blk_desc *blk;
>
> -       part = malloc(sizeof(struct mmc_part));
> +       part = malloc(sizeof(struct blk_part));
>         if (!part)
>                 return NULL;
>
>         dev_num = get_boot_device(ops);
> -       mmc_blk = get_blk(ops);
> +       blk = get_blk(ops);
>
> -       if (!mmc_blk) {
> +       if (!blk) {
>                 printf("Error - failed to obtain block descriptor\n");
>                 goto err;
>         }
>
> -       ret = part_get_info_by_name(mmc_blk, partition, &part->info);
> +       ret = part_get_info_by_name(blk, partition, &part->info);
>         if (ret < 0) {
>                 printf("Can't find partition '%s'\n", partition);
>                 goto err;
>         }
>
>         part->dev_num = dev_num;
> -       part->mmc_blk = mmc_blk;
> +       part->blk = blk;
>
>         return part;
>  err:
> @@ -368,16 +368,16 @@ err:
>         return NULL;
>  }
>
> -static AvbIOResult mmc_byte_io(AvbOps *ops,
> +static AvbIOResult blk_byte_io(AvbOps *ops,
>                                const char *partition,
>                                s64 offset,
>                                size_t num_bytes,
>                                void *buffer,
>                                size_t *out_num_read,
> -                              enum mmc_io_type io_type)
> +                              enum io_type io_type)
>  {
>         ulong ret;
> -       struct mmc_part *part;
> +       struct blk_part *part;
>         u64 start_offset, start_sector, sectors, residue;
>         u8 *tmp_buf;
>         size_t io_cnt = 0;
> @@ -410,7 +410,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
>                         }
>
>                         if (io_type == IO_READ) {
> -                               ret = mmc_read_and_flush(part,
> +                               ret = blk_read_and_flush(part,
>                                                          part->info.start +
>                                                          start_sector,
>                                                          1, tmp_buf);
> @@ -427,7 +427,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
>                                 tmp_buf += (start_offset % part->info.blksz);
>                                 memcpy(buffer, (void *)tmp_buf, residue);
>                         } else {
> -                               ret = mmc_read_and_flush(part,
> +                               ret = blk_read_and_flush(part,
>                                                          part->info.start +
>                                                          start_sector,
>                                                          1, tmp_buf);
> @@ -441,7 +441,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
>                                         start_offset % part->info.blksz,
>                                         buffer, residue);
>
> -                               ret = mmc_write(part, part->info.start +
> +                               ret = blk_write(part, part->info.start +
>                                                 start_sector, 1, tmp_buf);
>                                 if (ret != 1) {
>                                         printf("%s: write error (%ld, %lld)\n",
> @@ -459,12 +459,12 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
>
>                 if (sectors) {
>                         if (io_type == IO_READ) {
> -                               ret = mmc_read_and_flush(part,
> +                               ret = blk_read_and_flush(part,
>                                                          part->info.start +
>                                                          start_sector,
>                                                          sectors, buffer);
>                         } else {
> -                               ret = mmc_write(part,
> +                               ret = blk_write(part,
>                                                 part->info.start +
>                                                 start_sector,
>                                                 sectors, buffer);
> @@ -520,7 +520,7 @@ static AvbIOResult read_from_partition(AvbOps *ops,
>                                        void *buffer,
>                                        size_t *out_num_read)
>  {
> -       return mmc_byte_io(ops, partition_name, offset_from_partition,
> +       return blk_byte_io(ops, partition_name, offset_from_partition,
>                            num_bytes, buffer, out_num_read, IO_READ);
>  }
>
> @@ -547,7 +547,7 @@ static AvbIOResult write_to_partition(AvbOps *ops,
>                                       size_t num_bytes,
>                                       const void *buffer)
>  {
> -       return mmc_byte_io(ops, partition_name, offset_from_partition,
> +       return blk_byte_io(ops, partition_name, offset_from_partition,
>                            num_bytes, (void *)buffer, NULL, IO_WRITE);
>  }
>
> @@ -788,7 +788,7 @@ static AvbIOResult get_unique_guid_for_partition(AvbOps *ops,
>                                                  char *guid_buf,
>                                                  size_t guid_buf_size)
>  {
> -       struct mmc_part *part;
> +       struct blk_part *part;
>         size_t uuid_size;
>
>         part = get_partition(ops, partition);
> @@ -822,7 +822,7 @@ static AvbIOResult get_size_of_partition(AvbOps *ops,
>                                          const char *partition,
>                                          u64 *out_size_num_bytes)
>  {
> -       struct mmc_part *part;
> +       struct blk_part *part;
>
>         if (!out_size_num_bytes)
>                 return AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE;
> @@ -985,7 +985,7 @@ AvbOps *avb_ops_alloc(const char *boot_device, const char *interface)
>         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 = simple_strtoul(boot_device, NULL, 16);
> +       ops_data->blk_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",
> diff --git a/include/avb_verify.h b/include/avb_verify.h
> index ff70cb26f8..d095649c0a 100644
> --- a/include/avb_verify.h
> +++ b/include/avb_verify.h
> @@ -26,7 +26,7 @@ enum avb_boot_state {
>
>  struct AvbOpsData {
>         struct AvbOps ops;
> -       int mmc_dev;
> +       int blk_dev;
>         enum avb_boot_state boot_state;
>  #ifdef CONFIG_OPTEE_TA_AVB
>         struct udevice *tee;
> @@ -35,14 +35,13 @@ struct AvbOpsData {
>         struct blk_desc *blk;
>  };
>
> -struct mmc_part {
> +struct blk_part {
>         int dev_num;
> -       struct mmc *mmc;
> -       struct blk_desc *mmc_blk;
> +       struct blk_desc *blk;
>         struct disk_partition info;
>  };
>
> -enum mmc_io_type {
> +enum io_type {
>         IO_READ,
>         IO_WRITE
>  };
> @@ -61,7 +60,7 @@ char *append_cmd_line(char *cmdline_orig, char *cmdline_new);
>   * I/O helper inline functions
>   * ============================================================================
>   */
> -static inline uint64_t calc_offset(struct mmc_part *part, int64_t offset)
> +static inline uint64_t calc_offset(struct blk_part *part, int64_t offset)
>  {
>         u64 part_size = part->info.size * part->info.blksz;
>
> @@ -93,7 +92,7 @@ static inline int get_boot_device(AvbOps *ops)
>         if (ops) {
>                 data = ops->user_data;
>                 if (data)
> -                       return data->mmc_dev;
> +                       return data->blk_dev;
>         }
>
>         return -1;
> --
> 2.25.1
>

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk

[PATCH v4 0/3] Support various block interfaces for avb and bcb

[Andrii Chepurnyi]

Hello.

Originally bcb and avb utilities implementation relay on mmc block devices.
This patch series adds an optional interface parameter to those utilities,
which gives the ability to use bcb and avb on various block devices.
The patch set was tested using xenguest_arm64 based board and pvblock interface.

Changes for v4:
  - Added RB to all patches

Changes for v3:
  - Added RB and ACK
  - Added patch #3 by the request

Changes for v2:
  - Removed patch #3 as not actual for mainline

Andrii Chepurnyi (3):
  cmd: bcb: introduce optional interface parameter to bcb
  cmd: avb: introduce optional interface parameter to avb init
  cmd: avb: remove mmc naming from generic block code

 cmd/avb.c            | 13 +++-----
 cmd/bcb.c            | 65 ++++++++++++++++++-------------------
 common/avb_verify.c  | 76 ++++++++++++++++++++------------------------
 doc/android/avb2.rst |  2 +-
 doc/android/bcb.rst  | 33 ++++++++++---------
 include/avb_verify.h | 24 +++++++++-----
 6 files changed, 107 insertions(+), 106 deletions(-)

-- 
2.25.1

[PATCH v4 1/3] cmd: bcb: introduce optional interface parameter to bcb

[Andrii Chepurnyi]

From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>

From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

Originally, bcb implementation relay on mmc block devices.
The interface parameter will give the ability to use bcb with
various block devices by choosing the exact interface type.
By default (if no interface parameter is provided) mmc interface
will be used.

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
---
 cmd/bcb.c           | 65 +++++++++++++++++++++++----------------------
 doc/android/bcb.rst | 33 ++++++++++++-----------
 2 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 92f4d27990..bfe395558e 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -26,6 +26,7 @@ enum bcb_cmd {
 static int bcb_dev = -1;
 static int bcb_part = -1;
 static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
+static struct blk_desc *bcb_blk_desc;
 
 static int bcb_cmd_get(char *cmd)
 {
@@ -51,6 +52,9 @@ static int bcb_is_misused(int argc, char *const argv[])
 
 	switch (cmd) {
 	case BCB_CMD_LOAD:
+		if (argc != 3 && argc != 4)
+			goto err;
+		break;
 	case BCB_CMD_FIELD_SET:
 		if (argc != 3)
 			goto err;
@@ -115,25 +119,23 @@ static int bcb_field_get(char *name, char **fieldp, int *sizep)
 
 static int __bcb_load(int devnum, const char *partp)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	char *endp;
 	int part, ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, devnum);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err_read_fail;
 	}
 
 	part = simple_strtoul(partp, &endp, 0);
 	if (*endp == '\0') {
-		ret = part_get_info(desc, part, &info);
+		ret = part_get_info(bcb_blk_desc, part, &info);
 		if (ret)
 			goto err_read_fail;
 	} else {
-		part = part_get_info_by_name(desc, partp, &info);
+		part = part_get_info_by_name(bcb_blk_desc, partp, &info);
 		if (part < 0) {
 			ret = part;
 			goto err_read_fail;
@@ -144,12 +146,12 @@ static int __bcb_load(int devnum, const char *partp)
 	if (cnt > info.size)
 		goto err_too_small;
 
-	if (blk_dread(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dread(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err_read_fail;
 	}
 
-	bcb_dev = desc->devnum;
+	bcb_dev = bcb_blk_desc->devnum;
 	bcb_part = part;
 	debug("%s: Loaded from mmc %d:%d\n", __func__, bcb_dev, bcb_part);
 
@@ -170,15 +172,15 @@ err:
 static int do_bcb_load(struct cmd_tbl *cmdtp, int flag, int argc,
 		       char * const argv[])
 {
-	char *endp;
-	int devnum = simple_strtoul(argv[1], &endp, 0);
+	int ret = blk_get_device_by_str((argv[3]) ? argv[3] : "mmc", argv[1], &bcb_blk_desc);
 
-	if (*endp != '\0') {
-		printf("Error: Device id '%s' not a number\n", argv[1]);
+	if (ret < 0) {
+		printf("Error: Device id '%s' or interface '%s' is not valid\n", argv[1],
+		       (argv[3]) ? argv[3] : "mmc");
 		return CMD_RET_FAILURE;
 	}
 
-	return __bcb_load(devnum, argv[2]);
+	return __bcb_load(bcb_blk_desc->devnum, argv[2]);
 }
 
 static int __bcb_set(char *fieldp, const char *valp)
@@ -281,24 +283,22 @@ static int do_bcb_dump(struct cmd_tbl *cmdtp, int flag, int argc,
 
 static int __bcb_store(void)
 {
-	struct blk_desc *desc;
 	struct disk_partition info;
 	u64 cnt;
 	int ret;
 
-	desc = blk_get_devnum_by_type(IF_TYPE_MMC, bcb_dev);
-	if (!desc) {
+	if (!bcb_blk_desc) {
 		ret = -ENODEV;
 		goto err;
 	}
 
-	ret = part_get_info(desc, bcb_part, &info);
+	ret = part_get_info(bcb_blk_desc, bcb_part, &info);
 	if (ret)
 		goto err;
 
 	cnt = DIV_ROUND_UP(sizeof(struct bootloader_message), info.blksz);
 
-	if (blk_dwrite(desc, info.start, cnt, &bcb) != cnt) {
+	if (blk_dwrite(bcb_blk_desc, info.start, cnt, &bcb) != cnt) {
 		ret = -EIO;
 		goto err;
 	}
@@ -373,21 +373,22 @@ static int do_bcb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 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"
+	"load  <dev> <part> [<interface>]   - load  BCB from <dev>:<part>[<interface>]\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"
+	"<dev>          - device index containing the BCB partition\n"
+	"<part>         - partition index or name containing the BCB\n"
+	"<interface>    - interface name of the block device 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"
 );
diff --git a/doc/android/bcb.rst b/doc/android/bcb.rst
index 8861608300..e6d201f439 100644
--- a/doc/android/bcb.rst
+++ b/doc/android/bcb.rst
@@ -41,23 +41,26 @@ requirements enumerated above. Below is the command's help message::
    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
+   bcb load  <dev> <part> [<iftype>]    - load  BCB from <dev>:<part>[<iftype>]
+   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 <iftype>
 
    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
+   <dev>    - device index containing the BCB partition
+   <iftype> - Optional parameter of the interface type for the specified
+              block device like: mmc,sd,virtio see blk.h for details.
+              The default value is mmc.
+   <part>   - 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'. Example of getting reboot reason
-- 
2.25.1

Re: [PATCH v4 1/3] cmd: bcb: introduce optional interface parameter to bcb

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Mon, Aug 01, 2022 at 11:07:15AM +0300, Andrii Chepurnyi wrote:

> From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>
> 
> From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> 
> Originally, bcb implementation relay on mmc block devices.
> The interface parameter will give the ability to use bcb with
> various block devices by choosing the exact interface type.
> By default (if no interface parameter is provided) mmc interface
> will be used.
> 
> Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
> Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

The tests now fail:
https://source.denx.de/u-boot/u-boot/-/jobs/490627

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v4 1/3] cmd: bcb: introduce optional interface parameter to bcb

[Andrii Chepurnyi]

Hello Tom,

I've used a doker from mentioned job to reproduce issue.
I am able to see same errors and test fail as in jobs/490627.

Here is my investigation:

The error that appeared is not related to AVB:

fs_devread read outside partition 2
Failed to mount ext2 filesystem...
BTRFS: superblock end 69632 is larger than device size 512
fs_devread read outside partition 0
fs_devread read outside partition 2
Failed to mount ext2 filesystem...
BTRFS: superblock end 69632 is larger than device size 512
fs_devread read outside partition 0

I found a test case that caused this situation: dm_test_part.
So if you run all tests with -k "not dm_test_part" - all tests will 
pass, including AVB tests.

dm_test_part  modify GPT (during testing used mmc1.img for emulation, as 
I understand), and after if some part of code touch blk you may see a 
mentioned error. I.e. run only dm_test_part test, start u-boot and run 
"part list mmc 1" you will see the same error.
So I suppose this problem(if it is a problem since I see it like a test 
case that didn't restore the environment back) was present prior to my 
patches.

But why it appears after my patches?
Because I've changed the internal logic of avb command and now on avb 
init will interact with blk(blk_get_device_by_str) which causes those 
prints. In the previous versions of avb commands "avb init" and "avb 
read_rb " will not touch blk, so no error is present.

So from my point of view, this is more like dm_test_part problem, but 
not bcb or avb patches.

Andrii.

On 03.09.22 04:55, Tom Rini wrote:
> On Mon, Aug 01, 2022 at 11:07:15AM +0300, Andrii Chepurnyi wrote:
>
>> From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>
>>
>> From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
>>
>> Originally, bcb implementation relay on mmc block devices.
>> The interface parameter will give the ability to use bcb with
>> various block devices by choosing the exact interface type.
>> By default (if no interface parameter is provided) mmc interface
>> will be used.
>>
>> Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
>> Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> The tests now fail:
> https://source.denx.de/u-boot/u-boot/-/jobs/490627
>

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

[Andrii Chepurnyi]

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.

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
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

[PATCH v4 3/3] cmd: avb: remove mmc naming from generic block code

[Andrii Chepurnyi]

From: Andrii Chepurnyi <andrii.chepurnyi82@gmail.com>

From: Andrii Chepurnyi <andrii_chepurnyi@epam.com>

Part of avb code uses mmc notation, but in fact
it uses generic block functions.

Reviewed-by: Igor Opaniuk <igor.opaniuk@gmail.com>
Signed-off-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
---
 common/avb_verify.c  | 52 ++++++++++++++++++++++----------------------
 include/avb_verify.h | 13 +++++------
 2 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/common/avb_verify.c b/common/avb_verify.c
index e086dc6760..3c9594d6d7 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -253,10 +253,10 @@ char *avb_set_enforce_verity(const char *cmdline)
 
 /**
  * ============================================================================
- * IO(mmc) auxiliary functions
+ * IO auxiliary functions
  * ============================================================================
  */
-static unsigned long mmc_read_and_flush(struct mmc_part *part,
+static unsigned long blk_read_and_flush(struct blk_part *part,
 					lbaint_t start,
 					lbaint_t sectors,
 					void *buffer)
@@ -291,7 +291,7 @@ static unsigned long mmc_read_and_flush(struct mmc_part *part,
 		tmp_buf = buffer;
 	}
 
-	blks = blk_dread(part->mmc_blk,
+	blks = blk_dread(part->blk,
 			 start, sectors, tmp_buf);
 	/* flush cache after read */
 	flush_cache((ulong)tmp_buf, sectors * part->info.blksz);
@@ -302,7 +302,7 @@ static unsigned long mmc_read_and_flush(struct mmc_part *part,
 	return blks;
 }
 
-static unsigned long mmc_write(struct mmc_part *part, lbaint_t start,
+static unsigned long blk_write(struct blk_part *part, lbaint_t start,
 			       lbaint_t sectors, void *buffer)
 {
 	void *tmp_buf;
@@ -330,37 +330,37 @@ static unsigned long mmc_write(struct mmc_part *part, lbaint_t start,
 		tmp_buf = buffer;
 	}
 
-	return blk_dwrite(part->mmc_blk,
+	return blk_dwrite(part->blk,
 			  start, sectors, tmp_buf);
 }
 
-static struct mmc_part *get_partition(AvbOps *ops, const char *partition)
+static struct blk_part *get_partition(AvbOps *ops, const char *partition)
 {
 	int ret;
 	u8 dev_num;
-	struct mmc_part *part;
-	struct blk_desc *mmc_blk;
+	struct blk_part *part;
+	struct blk_desc *blk;
 
-	part = malloc(sizeof(struct mmc_part));
+	part = malloc(sizeof(struct blk_part));
 	if (!part)
 		return NULL;
 
 	dev_num = get_boot_device(ops);
-	mmc_blk = get_blk(ops);
+	blk = get_blk(ops);
 
-	if (!mmc_blk) {
+	if (!blk) {
 		printf("Error - failed to obtain block descriptor\n");
 		goto err;
 	}
 
-	ret = part_get_info_by_name(mmc_blk, partition, &part->info);
+	ret = part_get_info_by_name(blk, partition, &part->info);
 	if (ret < 0) {
 		printf("Can't find partition '%s'\n", partition);
 		goto err;
 	}
 
 	part->dev_num = dev_num;
-	part->mmc_blk = mmc_blk;
+	part->blk = blk;
 
 	return part;
 err:
@@ -368,16 +368,16 @@ err:
 	return NULL;
 }
 
-static AvbIOResult mmc_byte_io(AvbOps *ops,
+static AvbIOResult blk_byte_io(AvbOps *ops,
 			       const char *partition,
 			       s64 offset,
 			       size_t num_bytes,
 			       void *buffer,
 			       size_t *out_num_read,
-			       enum mmc_io_type io_type)
+			       enum io_type io_type)
 {
 	ulong ret;
-	struct mmc_part *part;
+	struct blk_part *part;
 	u64 start_offset, start_sector, sectors, residue;
 	u8 *tmp_buf;
 	size_t io_cnt = 0;
@@ -410,7 +410,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 			}
 
 			if (io_type == IO_READ) {
-				ret = mmc_read_and_flush(part,
+				ret = blk_read_and_flush(part,
 							 part->info.start +
 							 start_sector,
 							 1, tmp_buf);
@@ -427,7 +427,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 				tmp_buf += (start_offset % part->info.blksz);
 				memcpy(buffer, (void *)tmp_buf, residue);
 			} else {
-				ret = mmc_read_and_flush(part,
+				ret = blk_read_and_flush(part,
 							 part->info.start +
 							 start_sector,
 							 1, tmp_buf);
@@ -441,7 +441,7 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 					start_offset % part->info.blksz,
 					buffer, residue);
 
-				ret = mmc_write(part, part->info.start +
+				ret = blk_write(part, part->info.start +
 						start_sector, 1, tmp_buf);
 				if (ret != 1) {
 					printf("%s: write error (%ld, %lld)\n",
@@ -459,12 +459,12 @@ static AvbIOResult mmc_byte_io(AvbOps *ops,
 
 		if (sectors) {
 			if (io_type == IO_READ) {
-				ret = mmc_read_and_flush(part,
+				ret = blk_read_and_flush(part,
 							 part->info.start +
 							 start_sector,
 							 sectors, buffer);
 			} else {
-				ret = mmc_write(part,
+				ret = blk_write(part,
 						part->info.start +
 						start_sector,
 						sectors, buffer);
@@ -520,7 +520,7 @@ static AvbIOResult read_from_partition(AvbOps *ops,
 				       void *buffer,
 				       size_t *out_num_read)
 {
-	return mmc_byte_io(ops, partition_name, offset_from_partition,
+	return blk_byte_io(ops, partition_name, offset_from_partition,
 			   num_bytes, buffer, out_num_read, IO_READ);
 }
 
@@ -547,7 +547,7 @@ static AvbIOResult write_to_partition(AvbOps *ops,
 				      size_t num_bytes,
 				      const void *buffer)
 {
-	return mmc_byte_io(ops, partition_name, offset_from_partition,
+	return blk_byte_io(ops, partition_name, offset_from_partition,
 			   num_bytes, (void *)buffer, NULL, IO_WRITE);
 }
 
@@ -788,7 +788,7 @@ static AvbIOResult get_unique_guid_for_partition(AvbOps *ops,
 						 char *guid_buf,
 						 size_t guid_buf_size)
 {
-	struct mmc_part *part;
+	struct blk_part *part;
 	size_t uuid_size;
 
 	part = get_partition(ops, partition);
@@ -822,7 +822,7 @@ static AvbIOResult get_size_of_partition(AvbOps *ops,
 					 const char *partition,
 					 u64 *out_size_num_bytes)
 {
-	struct mmc_part *part;
+	struct blk_part *part;
 
 	if (!out_size_num_bytes)
 		return AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE;
@@ -985,7 +985,7 @@ AvbOps *avb_ops_alloc(const char *boot_device, const char *interface)
 	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 = simple_strtoul(boot_device, NULL, 16);
+	ops_data->blk_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",
diff --git a/include/avb_verify.h b/include/avb_verify.h
index ff70cb26f8..d095649c0a 100644
--- a/include/avb_verify.h
+++ b/include/avb_verify.h
@@ -26,7 +26,7 @@ enum avb_boot_state {
 
 struct AvbOpsData {
 	struct AvbOps ops;
-	int mmc_dev;
+	int blk_dev;
 	enum avb_boot_state boot_state;
 #ifdef CONFIG_OPTEE_TA_AVB
 	struct udevice *tee;
@@ -35,14 +35,13 @@ struct AvbOpsData {
 	struct blk_desc *blk;
 };
 
-struct mmc_part {
+struct blk_part {
 	int dev_num;
-	struct mmc *mmc;
-	struct blk_desc *mmc_blk;
+	struct blk_desc *blk;
 	struct disk_partition info;
 };
 
-enum mmc_io_type {
+enum io_type {
 	IO_READ,
 	IO_WRITE
 };
@@ -61,7 +60,7 @@ char *append_cmd_line(char *cmdline_orig, char *cmdline_new);
  * I/O helper inline functions
  * ============================================================================
  */
-static inline uint64_t calc_offset(struct mmc_part *part, int64_t offset)
+static inline uint64_t calc_offset(struct blk_part *part, int64_t offset)
 {
 	u64 part_size = part->info.size * part->info.blksz;
 
@@ -93,7 +92,7 @@ static inline int get_boot_device(AvbOps *ops)
 	if (ops) {
 		data = ops->user_data;
 		if (data)
-			return data->mmc_dev;
+			return data->blk_dev;
 	}
 
 	return -1;
-- 
2.25.1