Hi Jan,
On Tue, Oct 6, 2026 at 3:00 PM Jan Kehren <[email protected]> wrote:
>
> AVB assumed its partitions were on mmc: avb_ops_alloc() took a bare device
> number and get_partition() called find_mmc_device(). bootmeth_android
> accepts any bootdev but passes only desc->devnum, so an Android bootflow
> found on nvme 0 or usb 0 is verified against mmc 0.
I don't think this path can be reached today: android_check() rejects
every non-mmc bootdev through bootflow_iter_check_mmc(), and
android_read_bootflow() loads the BCB with
bcb_find_partition_and_load("mmc", ...).
So the bootmeth hunk below is right, but it does not change behaviour yet.
The user that does benefit is "avb init <interface> <dev>", e.g.
board/armltd/total_compute/total_compute.env, which has been calling
"avb init ${blk_dev} 0" since 2024 and gets a usage error on next. Could
you reword the motivation along those lines and mention total_compute?
>
> Carry a uclass id alongside the device number and resolve the block device
> with blk_get_devnum_by_uclass_id(), keeping mmc_init() and the hardware
> partition switch on the mmc path only. "avb init <dev>" still means mmc,
> "avb init <interface> <dev>" selects another bus, and both now look the
> device up before allocating. get_boot_interface() returns UCLASS_INVALID
> without ops, so a caller that skipped "avb init" fails rather than reading
> mmc.
>
> Signed-off-by: Jan Kehren <[email protected]>
> ---
> Changes in v2:
> - Split: the renames moved to the preceding patch
> - Put the "avb init <dev>" comment on the branch it describes, by testing
> argc == 2 rather than argc == 3
> - Drop the single-use part_num and move the user-area comment next to
> mmc_switch_part()
>
> boot/bootmeth_android.c | 2 +-
> cmd/avb.c | 24 ++++++---
> common/Kconfig | 7 ++-
> common/avb_verify.c | 71 +++++++++++++++++---------
> doc/android/avb2.rst | 7 ++-
> include/avb_verify.h | 26 +++++++---
> test/py/tests/test_android/test_avb.py | 28 ++++++++++
> 7 files changed, 127 insertions(+), 38 deletions(-)
>
> diff --git a/boot/bootmeth_android.c b/boot/bootmeth_android.c
> index 0db08d4f86..ae05c08488 100644
> --- a/boot/bootmeth_android.c
> +++ b/boot/bootmeth_android.c
> @@ -443,7 +443,7 @@ static int run_avb_verification(struct bootflow *bflow)
> bool unlocked = false;
> int ret;
>
> - avb_ops = avb_ops_alloc(desc->devnum);
> + avb_ops = avb_ops_alloc(desc->uclass_id, desc->devnum);
> if (!avb_ops)
> return log_msg_ret("avb ops", -ENOMEM);
>
> diff --git a/cmd/avb.c b/cmd/avb.c
> index 8fbd48ee5a..4c1f56a33a 100644
> --- a/cmd/avb.c
> +++ b/cmd/avb.c
> @@ -16,17 +16,28 @@ static struct AvbOps *avb_ops;
>
> int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> argv[])
> {
> - unsigned long mmc_dev;
> + struct blk_desc *desc;
>
> - if (argc != 2)
> + if (argc < 2 || argc > 3)
> return CMD_RET_USAGE;
>
> - mmc_dev = hextoul(argv[1], NULL);
> + /* "avb init <dev>" keeps its old meaning of mmc <dev> */
> + if (argc == 2)
> + desc = blk_get_devnum_by_uclass_id(UCLASS_MMC,
> + hextoul(argv[1], NULL));
> + else
> + desc = blk_get_devnum_by_uclass_idname(argv[1],
> + hextoul(argv[2],
> NULL));
> +
This up-front lookup is what breaks the existing sandbox tests: with the
series, test_avb_verify, test_avb_read_rb, test_avb_is_unlocked,
test_avb_persistent_values and the new test_avb_init_interface fail with
Can't map file 'mmc1.img': Invalid argument
sandbox_mmc_probe() mmc1: Unable to map file 'mmc1.img'
Can't find the requested device
while next gives 4 passed, 2 skipped for test_avb.py.
Sandbox mmc1 has no backing image when these tests run
(test_android/ is collected before the tests that create mmc images),
and most of them only talk to the sandbox TA, so they never needed
the device before. Would you consider keeping
"avb init" lazy: validate the interface name here, store uclass + number,
and let get_partition() resolve the device on first access as before?
That keeps the old semantics, and a missing device is still reported, by
the first partition access. Alternatively the tests would need to create
mmc1.img first.
> + if (!desc) {
> + printf("Can't find the requested device\n");
> + return CMD_RET_FAILURE;
> + }
>
> if (avb_ops)
> avb_ops_free(avb_ops);
>
> - avb_ops = avb_ops_alloc(mmc_dev);
> + avb_ops = avb_ops_alloc(desc->uclass_id, desc->devnum);
> if (avb_ops)
> return CMD_RET_SUCCESS;
> else
> @@ -428,7 +439,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, "", ""),
> @@ -464,7 +475,8 @@ 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 [<interface>] <dev> - initialize avb2 for hex <dev> on
> <interface>\n"
> + " (mmc, nvme, usb, ...; defaults to mmc)\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/Kconfig b/common/Kconfig
> index c739225621..5590fbfdb2 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -885,9 +885,14 @@ config AVB_VERIFY
> This option enables compilation of bootloader-dependent operations,
> used by Android Verified Boot 2.0 library (libavb). Includes:
> * Helpers to process strings in order to build OS bootargs.
> - * Helpers to access MMC, similar to drivers/fastboot/fb_mmc.c.
> + * Helpers to access block devices, similar to
> + drivers/fastboot/fb_mmc.c.
> * Helpers to alloc/init/free avb ops.
>
> + Only mmc is initialised by AVB itself. Any other interface must
> + already be enumerated when AVB runs, either by the board code or by
> + a command such as "usb start".
> +
> if AVB_VERIFY
>
> config AVB_BUF_ADDR
> diff --git a/common/avb_verify.c b/common/avb_verify.c
> index fc29433c3c..b898ebb78f 100644
> --- a/common/avb_verify.c
> +++ b/common/avb_verify.c
> @@ -8,6 +8,7 @@
> #include <cpu_func.h>
> #include <image.h>
> #include <malloc.h>
> +#include <mmc.h>
> #include <part.h>
> #include <tee.h>
> #include <tee/optee_ta_avb.h>
> @@ -386,44 +387,68 @@ static unsigned long avb_blk_write(struct avb_blk_part
> *part, lbaint_t start,
> start, sectors, tmp_buf);
> }
>
> -static struct avb_blk_part *get_partition(AvbOps *ops, const char *partition)
> +static int avb_init_mmc(int dev_num)
> {
> + struct mmc *mmc;
> int ret;
> - u8 dev_num;
> - int part_num = 0;
> - struct avb_blk_part *part;
> - struct blk_desc *blk;
> -
> - part = malloc(sizeof(struct avb_blk_part));
> - if (!part)
> - return NULL;
>
> - dev_num = get_boot_device(ops);
> - part->mmc = find_mmc_device(dev_num);
> - if (!part->mmc) {
> + mmc = find_mmc_device(dev_num);
> + if (!mmc) {
> printf("%s: no MMC device at slot %x\n", __func__, dev_num);
> - goto err;
> + return -ENODEV;
> }
>
> - ret = mmc_init(part->mmc);
> + ret = mmc_init(mmc);
> if (ret) {
> printf("%s: MMC initialization failed, err = %d\n",
> __func__, ret);
> - goto err;
> + return ret;
> }
>
> - if (IS_MMC(part->mmc)) {
> - ret = mmc_switch_part(part->mmc, part_num);
> + if (IS_MMC(mmc)) {
> + /* Select the user area, where the GPT and our images live */
> + ret = mmc_switch_part(mmc, 0);
> if (ret) {
> printf("%s: MMC part switch failed, err = %d\n",
> __func__, ret);
> - goto err;
> + return ret;
> }
> }
>
> - blk = mmc_get_blk_desc(part->mmc);
> + return 0;
> +}
> +
> +static struct avb_blk_part *get_partition(AvbOps *ops, const char *partition)
> +{
> + enum uclass_id uclass_id;
> + struct avb_blk_part *part;
> + struct blk_desc *blk;
> + int dev_num;
> + int ret;
> +
> + part = malloc(sizeof(struct avb_blk_part));
> + if (!part)
> + return NULL;
> +
> + dev_num = get_boot_device(ops);
> + uclass_id = get_boot_interface(ops);
> + if (uclass_id == UCLASS_INVALID || dev_num < 0) {
> + printf("%s: no storage interface, run 'avb init' first\n",
> + __func__);
> + goto err;
> + }
> +
> + if (uclass_id == UCLASS_MMC) {
> + ret = avb_init_mmc(dev_num);
> + if (ret)
> + goto err;
> + }
> +
> + blk = blk_get_devnum_by_uclass_id(uclass_id, dev_num);
> if (!blk) {
> - printf("%s: failed to obtain block descriptor\n", __func__);
> + /* Every bus but mmc must already be started when AVB runs */
> + printf("%s: no %s device %d, is the bus started?\n", __func__,
> + blk_get_uclass_name(uclass_id), dev_num);
A note for the doc/Kconfig text rather than the code: with the images on
virtio/NVMe/USB and the AVB TA keeping its state in eMMC RPMB, the TA's
first RPMB request becomes the first eMMC access and happens from inside
the supplicant RPC.
On next that hangs: "avb init virtio 0; avb read_rb 0"
leaves OP-TEE spinning in do_notif, the nested RPMB announcement deadlock
fixed by "optee: do not announce RPMB from within a supplicant RPC"
(currently v2 on the list). Not a bug in this series, but that
configuration only works with that fix, so it may be worth a line in
avb2.rst.
> goto err;
> }
>
> @@ -433,7 +458,6 @@ static struct avb_blk_part *get_partition(AvbOps *ops,
> const char *partition)
> goto err;
> }
>
> - part->dev_num = dev_num;
> part->blk = blk;
>
> return part;
> @@ -1111,7 +1135,7 @@ __weak AvbIOResult avb_read_root_key_digest(AvbOps
> *ops, uint8_t *digest)
> * AVB2.0 AvbOps alloc/initialisation/free
> *
> ============================================================================
> */
> -AvbOps *avb_ops_alloc(int boot_device)
> +AvbOps *avb_ops_alloc(enum uclass_id uclass_id, int dev_num)
> {
> struct AvbOpsData *ops_data;
>
> @@ -1134,7 +1158,8 @@ 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->uclass_id = uclass_id;
> + ops_data->dev_num = dev_num;
>
> return &ops_data->ops;
> }
> diff --git a/doc/android/avb2.rst b/doc/android/avb2.rst
> index a514701241..cef49d3e0d 100644
> --- a/doc/android/avb2.rst
> +++ b/doc/android/avb2.rst
> @@ -29,6 +29,10 @@ integration in ``common/avb_verify.c`` and the platform
> port under
>
> For additional details check [1]_.
>
> +Only mmc is started by AVB itself. Any other interface must already be
> +enumerated when AVB runs, either by the board code or by a command such as
> +``usb start``, or the partition lookup fails.
> +
> AVB using OP-TEE (optional)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> @@ -80,7 +84,8 @@ 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 for <dev>
> + avb init [<interface>] <dev> - initialize avb 2 for hex <dev> on
> + <interface> (mmc, nvme, usb, ...; defaults to mmc)
This is the behaviour change that bites, see the test comment at the end:
the lookup probes the device (blk_get_device() -> device_probe(), and for
mmc the blk probe runs mmc_init()), whereas "avb init" used to only store
the number. Please at least state that it probes now.
> avb read_rb <num> - read rollback index at location <num>
> avb write_rb <num> <rb> - write rollback index <rb> to <num>
> avb is_unlocked - returns unlock status of the device
> diff --git a/include/avb_verify.h b/include/avb_verify.h
> index 8c97adbfbf..6dbbb39a8a 100644
> --- a/include/avb_verify.h
> +++ b/include/avb_verify.h
> @@ -7,8 +7,9 @@
> #define _AVB_VERIFY_H
>
> #include <../lib/libavb/libavb.h>
> +#include <blk.h>
> #include <mapmem.h>
> -#include <mmc.h>
> +#include <part.h>
>
> #define AVB_MAX_ARGS 1024
> #define VERITY_TABLE_OPT_RESTART "restart_on_corruption"
> @@ -24,7 +25,8 @@ enum avb_boot_state {
>
> struct AvbOpsData {
> struct AvbOps ops;
> - int mmc_dev;
> + enum uclass_id uclass_id;
> + int dev_num;
> enum avb_boot_state boot_state;
> #ifdef CONFIG_OPTEE_TA_AVB
> struct udevice *tee;
> @@ -33,8 +35,6 @@ struct AvbOpsData {
> };
>
> struct avb_blk_part {
> - int dev_num;
> - struct mmc *mmc;
> struct blk_desc *blk;
> struct disk_partition info;
> };
> @@ -44,7 +44,7 @@ enum avb_io_type {
> AVB_IO_WRITE
> };
>
> -AvbOps *avb_ops_alloc(int boot_device);
> +AvbOps *avb_ops_alloc(enum uclass_id uclass_id, int dev_num);
> void avb_ops_free(AvbOps *ops);
>
> /**
> @@ -110,10 +110,24 @@ static inline int get_boot_device(AvbOps *ops)
> if (ops) {
> data = ops->user_data;
> if (data)
> - return data->mmc_dev;
> + return data->dev_num;
> }
>
> return -1;
> }
>
> +/* Mirrors get_boot_device(): no ops means no interface, not a default one */
> +static inline enum uclass_id get_boot_interface(AvbOps *ops)
> +{
> + struct AvbOpsData *data;
> +
> + if (ops) {
> + data = ops->user_data;
> + if (data)
> + return data->uclass_id;
> + }
> +
> + return UCLASS_INVALID;
> +}
> +
> #endif /* _AVB_VERIFY_H */
> diff --git a/test/py/tests/test_android/test_avb.py
> b/test/py/tests/test_android/test_avb.py
> index 137d83e1de..4c5b3c6f59 100644
> --- a/test/py/tests/test_android/test_avb.py
> +++ b/test/py/tests/test_android/test_avb.py
> @@ -135,3 +135,31 @@ def test_avb_persistent_values(ubman):
>
> response = ubman.run_command('avb read_pvalue test 12')
> assert response == 'Read 12 bytes, value = value_value'
> +
> +
> [email protected]('cmd_avb')
> [email protected]('cmd_mmc')
> +def test_avb_init_interface(ubman):
> + """Check that 'avb init' accepts an explicit interface
> + """
> +
> + response = ubman.run_command('avb init mmc %s' % str(mmc_dev))
> + assert response == ''
Fails on sandbox for the same mmc1.img reason, see above.
With the "avb init" behaviour sorted out, the rest works for me:
* "avb init 0" as the first eMMC access, read_rb, is_unlocked and
"avb verify _a" pass;
* "avb init nvme 0", "avb init mmc 5" and "avb init bogus 0" fail cleanly;
* with the Android images on virtio only (the eMMC holding just RPMB),
"avb init virtio 0; avb verify _a" passes, and "avb init 0" on the
empty eMMC correctly fails to find vbmeta_a;
> +
> + response = ubman.run_command('avb init %s' % str(mmc_dev))
> + assert response == ''
> +
> +
> [email protected]('cmd_avb')
> +def test_avb_init_bad_device(ubman):
> + """Check that 'avb init' rejects a device that does not exist
> + """
> +
> + response = ubman.run_command('avb init mmc ff')
> + assert response.find("Can't find the requested device") != -1
> +
> + response = ubman.run_command('avb init ff')
> + assert response.find("Can't find the requested device") != -1
> +
> + response = ubman.run_command('avb init nonsense 0')
> + assert response.find("Can't find the requested device") != -1
> --
> 2.43.0
>
