[PATCH v2 00/10] Fix AVB pytest suite for sandbox

[PATCH v2 00/10] Fix AVB pytest suite for sandbox

[Mattijs Korpershoek]

The AVB tests have the following limitations on sandbox:

* The mmc1.img required by sandbox is not created when the avb test runs
* mmc1.img contains a Fedora image, so avb verification cannot be done
* The tests silently pass due to bad usage of string.find()

This series improve the test suite to have more useful results on
sandbox.

Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
Changes in v2:
- Add missing find() return code check in test_avb_mmc_uuid (Guillaume)
- Skip avb tests when qemu-img or cgpt is not installed (Igor)
- Link to v1: https://lore.kernel.org/r/20261015-test-avb-fixes-v1-0-9e5b82644359@kernel.org

---
Mattijs Korpershoek (10):
      avb: Use map_to_sysmem() for flush_cache() address
      test/py: android: Move android image creation to a dedicated file
      test/py: android: Drop verbose printing and unused return
      test/py: android: Use for loop to create the images / update disks
      test/py: android: Add create_gpt_image() for image creation
      test/py: android: Add setup_android_v{4,2}_image subfunctions
      test/py: avb: Use format specifiers instead of str() for mmc_dev
      test/py: avb: Move mmc device number to a pytest fixture
      test/py: avb: Add sandbox AVB testing with dedicated mmc12 device
      test/py: avb: Fix find() logic when matching response strings

 arch/sandbox/dts/test.dts                   |   8 ++
 common/avb_verify.c                         |   3 +-
 test/py/tests/test_android/android_image.py | 104 ++++++++++++++++++++++
 test/py/tests/test_android/test_abootimg.py |  77 +++++++++++++++++
 test/py/tests/test_android/test_avb.py      |  78 +++++++++++------
 test/py/tests/test_ut.py                    | 129 +---------------------------
 6 files changed, 246 insertions(+), 153 deletions(-)
---
base-commit: fed92546960a92177d67c0b8d7b22385f50fed29
change-id: 20261015-test-avb-fixes-9a4682e8057d

Best regards,
--  
Mattijs Korpershoek <mkorpershoek@kernel.org>

[PATCH v2 01/10] avb: Use map_to_sysmem() for flush_cache() address

[Mattijs Korpershoek]

flush_cache() expects a U-Boot physical address, but avb_verify's
mmc_read_and_flush() passes a raw pointer cast to ulong.
On sandbox, host heap pointers (e.g. 0x1a27ee70) can be outside the
emulated SDRAM region (0x0 to gd->ram_size), causing map_sysmem()
inside flush_cache() to crash:

  phys_to_virt: Cannot map sandbox address 1a27ee70
    (SDRAM from 0 to 10000000)

Fix the crash by using map_to_sysmem() to convert the buffer
pointer to a proper U-Boot address.

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 common/avb_verify.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/avb_verify.c b/common/avb_verify.c
index 09f1534c3cc5..d157adc4af07 100644
--- a/common/avb_verify.c
+++ b/common/avb_verify.c
@@ -7,6 +7,7 @@
 #include <blk.h>
 #include <cpu_func.h>
 #include <image.h>
+#include <mapmem.h>
 #include <malloc.h>
 #include <part.h>
 #include <tee.h>
@@ -346,7 +347,7 @@ static unsigned long mmc_read_and_flush(struct mmc_part *part,
 	blks = blk_dread(part->mmc_blk,
 			 start, sectors, tmp_buf);
 	/* flush cache after read */
-	flush_cache((ulong)tmp_buf, sectors * part->info.blksz);
+	flush_cache(map_to_sysmem(tmp_buf), sectors * part->info.blksz);
 
 	if (unaligned)
 		memcpy(buffer, tmp_buf, sectors * part->info.blksz);

-- 
2.56.0

Re: [PATCH v2 01/10] avb: Use map_to_sysmem() for flush_cache() address

[Simon Glass]

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> avb: Use map_to_sysmem() for flush_cache() address
>
> flush_cache() expects a U-Boot physical address, but avb_verify's
> mmc_read_and_flush() passes a raw pointer cast to ulong.
> On sandbox, host heap pointers (e.g. 0x1a27ee70) can be outside the
> emulated SDRAM region (0x0 to gd->ram_size), causing map_sysmem()
> inside flush_cache() to crash:
>
>   phys_to_virt: Cannot map sandbox address 1a27ee70
>     (SDRAM from 0 to 10000000)
>
> Fix the crash by using map_to_sysmem() to convert the buffer
> pointer to a proper U-Boot address.
>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
>  common/avb_verify.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 02/10] test/py: android: Move android image creation to a dedicated file

[Mattijs Korpershoek]

The image creation code in test_ut is huge and difficult to follow.
The android part contain code duplications when making mmc7.img/mmc8.img.

To prepare some code refactoring, move the Android specific bits into a
dedicated file under test_android/.

No functional change.

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/android_image.py | 135 ++++++++++++++++++++++++++++
 test/py/tests/test_ut.py                    | 129 +-------------------------
 2 files changed, 136 insertions(+), 128 deletions(-)

diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
new file mode 100644
index 000000000000..3bea311586bf
--- /dev/null
+++ b/test/py/tests/test_android/android_image.py
@@ -0,0 +1,135 @@
+# SPDX-License-Identifier: GPL-2.0
+"""Helpers to create Android disk images for sandbox testing."""
+
+import collections
+import os
+
+import utils
+from test_android import test_abootimg
+
+def setup_android_image(ubman):
+    """Create a 20MB disk image with Android partitions"""
+    Partition = collections.namedtuple('part', 'start,size,name')
+    parts = {}
+    disk_data = None
+
+    def set_part_data(partnum, data):
+        """Set the contents of a disk partition
+
+        This updates disk_data by putting data in the right place
+
+        Args:
+            partnum (int): Partition number to set
+            data (bytes): Data for that partition
+        """
+        nonlocal disk_data
+
+        start = parts[partnum].start * sect_size
+        disk_data = disk_data[:start] + data + disk_data[start + len(data):]
+
+    mmc_dev = 7
+    fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
+    utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
+    utils.run_and_log(ubman, f'cgpt create {fname}')
+
+    ptr = 40
+
+    # Number of sectors in 1MB
+    sect_size = 512
+    sect_1mb = (1 << 20) // sect_size
+
+    required_parts = [
+        {'num': 1, 'label':'misc', 'size': '1M'},
+        {'num': 2, 'label':'boot_a', 'size': '4M'},
+        {'num': 3, 'label':'boot_b', 'size': '4M'},
+        {'num': 4, 'label':'vendor_boot_a', 'size': '4M'},
+        {'num': 5, 'label':'vendor_boot_b', 'size': '4M'},
+    ]
+
+    for part in required_parts:
+        size_str = part['size']
+        if 'M' in size_str:
+            size = int(size_str[:-1]) * sect_1mb
+        else:
+            size = int(size_str)
+        utils.run_and_log(
+            ubman,
+            f"cgpt add -i {part['num']} -b {ptr} -s {size} -l {part['label']} -t basicdata {fname}")
+        ptr += size
+
+    utils.run_and_log(ubman, f'cgpt boot -p {fname}')
+    out = utils.run_and_log(ubman, f'cgpt show -q {fname}')
+
+    # Create a dict (indexed by partition number) containing the above info
+    for line in out.splitlines():
+        start, size, num, name = line.split(maxsplit=3)
+        parts[int(num)] = Partition(int(start), int(size), name)
+
+    with open(fname, 'rb') as inf:
+        disk_data = inf.read()
+
+    test_abootimg.AbootimgTestDiskImage(ubman, 'bootv4.img', test_abootimg.boot_img_hex)
+    boot_img = os.path.join(ubman.config.result_dir, 'bootv4.img')
+    with open(boot_img, 'rb') as inf:
+        set_part_data(2, inf.read())
+
+    test_abootimg.AbootimgTestDiskImage(ubman, 'vendor_boot.img', test_abootimg.vboot_img_hex)
+    vendor_boot_img = os.path.join(ubman.config.result_dir, 'vendor_boot.img')
+    with open(vendor_boot_img, 'rb') as inf:
+        set_part_data(4, inf.read())
+
+    with open(fname, 'wb') as outf:
+        outf.write(disk_data)
+
+    print(f'wrote to {fname}')
+
+    mmc_dev = 8
+    fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
+    utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
+    utils.run_and_log(ubman, f'cgpt create {fname}')
+
+    ptr = 40
+
+    # Number of sectors in 1MB
+    sect_size = 512
+    sect_1mb = (1 << 20) // sect_size
+
+    required_parts = [
+        {'num': 1, 'label':'misc', 'size': '1M'},
+        {'num': 2, 'label':'boot_a', 'size': '4M'},
+        {'num': 3, 'label':'boot_b', 'size': '4M'},
+    ]
+
+    for part in required_parts:
+        size_str = part['size']
+        if 'M' in size_str:
+            size = int(size_str[:-1]) * sect_1mb
+        else:
+            size = int(size_str)
+        utils.run_and_log(
+            ubman,
+            f"cgpt add -i {part['num']} -b {ptr} -s {size} -l {part['label']} -t basicdata {fname}")
+        ptr += size
+
+    utils.run_and_log(ubman, f'cgpt boot -p {fname}')
+    out = utils.run_and_log(ubman, f'cgpt show -q {fname}')
+
+    # Create a dict (indexed by partition number) containing the above info
+    for line in out.splitlines():
+        start, size, num, name = line.split(maxsplit=3)
+        parts[int(num)] = Partition(int(start), int(size), name)
+
+    with open(fname, 'rb') as inf:
+        disk_data = inf.read()
+
+    test_abootimg.AbootimgTestDiskImage(ubman, 'boot.img', test_abootimg.img_hex)
+    boot_img = os.path.join(ubman.config.result_dir, 'boot.img')
+    with open(boot_img, 'rb') as inf:
+        set_part_data(2, inf.read())
+
+    with open(fname, 'wb') as outf:
+        outf.write(disk_data)
+
+    print(f'wrote to {fname}')
+
+    return fname
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 2a5acc18c612..c1b5bc056023 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -19,7 +19,7 @@ import utils
 # pylint: disable=E0611
 from tests import fs_helper
 from fs_helper import DiskHelper, FsHelper
-from test_android import test_abootimg
+from test_android.android_image import setup_android_image
 
 def mkdir_cond(dirname):
     """Create a directory if it doesn't already exist
@@ -506,133 +506,6 @@ def setup_cros_image(ubman):
 
     return fname
 
-def setup_android_image(ubman):
-    """Create a 20MB disk image with Android partitions"""
-    Partition = collections.namedtuple('part', 'start,size,name')
-    parts = {}
-    disk_data = None
-
-    def set_part_data(partnum, data):
-        """Set the contents of a disk partition
-
-        This updates disk_data by putting data in the right place
-
-        Args:
-            partnum (int): Partition number to set
-            data (bytes): Data for that partition
-        """
-        nonlocal disk_data
-
-        start = parts[partnum].start * sect_size
-        disk_data = disk_data[:start] + data + disk_data[start + len(data):]
-
-    mmc_dev = 7
-    fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
-    utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
-    utils.run_and_log(ubman, f'cgpt create {fname}')
-
-    ptr = 40
-
-    # Number of sectors in 1MB
-    sect_size = 512
-    sect_1mb = (1 << 20) // sect_size
-
-    required_parts = [
-        {'num': 1, 'label':'misc', 'size': '1M'},
-        {'num': 2, 'label':'boot_a', 'size': '4M'},
-        {'num': 3, 'label':'boot_b', 'size': '4M'},
-        {'num': 4, 'label':'vendor_boot_a', 'size': '4M'},
-        {'num': 5, 'label':'vendor_boot_b', 'size': '4M'},
-    ]
-
-    for part in required_parts:
-        size_str = part['size']
-        if 'M' in size_str:
-            size = int(size_str[:-1]) * sect_1mb
-        else:
-            size = int(size_str)
-        utils.run_and_log(
-            ubman,
-            f"cgpt add -i {part['num']} -b {ptr} -s {size} -l {part['label']} -t basicdata {fname}")
-        ptr += size
-
-    utils.run_and_log(ubman, f'cgpt boot -p {fname}')
-    out = utils.run_and_log(ubman, f'cgpt show -q {fname}')
-
-    # Create a dict (indexed by partition number) containing the above info
-    for line in out.splitlines():
-        start, size, num, name = line.split(maxsplit=3)
-        parts[int(num)] = Partition(int(start), int(size), name)
-
-    with open(fname, 'rb') as inf:
-        disk_data = inf.read()
-
-    test_abootimg.AbootimgTestDiskImage(ubman, 'bootv4.img', test_abootimg.boot_img_hex)
-    boot_img = os.path.join(ubman.config.result_dir, 'bootv4.img')
-    with open(boot_img, 'rb') as inf:
-        set_part_data(2, inf.read())
-
-    test_abootimg.AbootimgTestDiskImage(ubman, 'vendor_boot.img', test_abootimg.vboot_img_hex)
-    vendor_boot_img = os.path.join(ubman.config.result_dir, 'vendor_boot.img')
-    with open(vendor_boot_img, 'rb') as inf:
-        set_part_data(4, inf.read())
-
-    with open(fname, 'wb') as outf:
-        outf.write(disk_data)
-
-    print(f'wrote to {fname}')
-
-    mmc_dev = 8
-    fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
-    utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
-    utils.run_and_log(ubman, f'cgpt create {fname}')
-
-    ptr = 40
-
-    # Number of sectors in 1MB
-    sect_size = 512
-    sect_1mb = (1 << 20) // sect_size
-
-    required_parts = [
-        {'num': 1, 'label':'misc', 'size': '1M'},
-        {'num': 2, 'label':'boot_a', 'size': '4M'},
-        {'num': 3, 'label':'boot_b', 'size': '4M'},
-    ]
-
-    for part in required_parts:
-        size_str = part['size']
-        if 'M' in size_str:
-            size = int(size_str[:-1]) * sect_1mb
-        else:
-            size = int(size_str)
-        utils.run_and_log(
-            ubman,
-            f"cgpt add -i {part['num']} -b {ptr} -s {size} -l {part['label']} -t basicdata {fname}")
-        ptr += size
-
-    utils.run_and_log(ubman, f'cgpt boot -p {fname}')
-    out = utils.run_and_log(ubman, f'cgpt show -q {fname}')
-
-    # Create a dict (indexed by partition number) containing the above info
-    for line in out.splitlines():
-        start, size, num, name = line.split(maxsplit=3)
-        parts[int(num)] = Partition(int(start), int(size), name)
-
-    with open(fname, 'rb') as inf:
-        disk_data = inf.read()
-
-    test_abootimg.AbootimgTestDiskImage(ubman, 'boot.img', test_abootimg.img_hex)
-    boot_img = os.path.join(ubman.config.result_dir, 'boot.img')
-    with open(boot_img, 'rb') as inf:
-        set_part_data(2, inf.read())
-
-    with open(fname, 'wb') as outf:
-        outf.write(disk_data)
-
-    print(f'wrote to {fname}')
-
-    return fname
-
 def setup_cedit_file(ubman):
     """Set up a .dtb file for use with testing expo and configuration editor"""
     infname = os.path.join(ubman.config.source_dir,

-- 
2.56.0

Re: [PATCH v2 02/10] test/py: android: Move android image creation to a dedicated file

[Simon Glass]

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: android: Move android image creation to a dedicated file
>
> The image creation code in test_ut is huge and difficult to follow.
> The android part contain code duplications when making mmc7.img/mmc8.img.
>
> To prepare some code refactoring, move the Android specific bits into a
> dedicated file under test_android/.
>
> No functional change.
>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
>  test/py/tests/test_android/android_image.py | 135 ++++++++++++++++++++++++++++
>  test/py/tests/test_ut.py                    | 129 +-------------------------
>  2 files changed, 136 insertions(+), 128 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 03/10] test/py: android: Drop verbose printing and unused return

[Mattijs Korpershoek]

The setup_android_image() is only called once. The callers does not use
the return value.

Drop the unused return. While at it, also remove the verbose printing.

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/android_image.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
index 3bea311586bf..8b536754656e 100644
--- a/test/py/tests/test_android/android_image.py
+++ b/test/py/tests/test_android/android_image.py
@@ -81,8 +81,6 @@ def setup_android_image(ubman):
     with open(fname, 'wb') as outf:
         outf.write(disk_data)
 
-    print(f'wrote to {fname}')
-
     mmc_dev = 8
     fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
     utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
@@ -129,7 +127,3 @@ def setup_android_image(ubman):
 
     with open(fname, 'wb') as outf:
         outf.write(disk_data)
-
-    print(f'wrote to {fname}')
-
-    return fname

-- 
2.56.0

Re: [PATCH v2 03/10] test/py: android: Drop verbose printing and unused return

[Simon Glass]

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: android: Drop verbose printing and unused return
>
> The setup_android_image() is only called once. The callers does not use
> the return value.
>
> Drop the unused return. While at it, also remove the verbose printing.
>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
>  test/py/tests/test_android/android_image.py | 6 ------
>  1 file changed, 6 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 04/10] test/py: android: Use for loop to create the images / update disks

[Mattijs Korpershoek]

setup_android_image() creates 2 mmcX.img files:

* mmc7.img: For boot image v4 support
* mmc8.img: For boot image v2 support

There is quite some code that's common for both images. Currently,
most code is duplicated.

To prepare a cleaner refactoring, move the image descriptions into an
array and create each partition using a loop.

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/android_image.py | 48 +++++++++++++----------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
index 8b536754656e..14dfd26fffd8 100644
--- a/test/py/tests/test_android/android_image.py
+++ b/test/py/tests/test_android/android_image.py
@@ -13,20 +13,6 @@ def setup_android_image(ubman):
     parts = {}
     disk_data = None
 
-    def set_part_data(partnum, data):
-        """Set the contents of a disk partition
-
-        This updates disk_data by putting data in the right place
-
-        Args:
-            partnum (int): Partition number to set
-            data (bytes): Data for that partition
-        """
-        nonlocal disk_data
-
-        start = parts[partnum].start * sect_size
-        disk_data = disk_data[:start] + data + disk_data[start + len(data):]
-
     mmc_dev = 7
     fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
     utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
@@ -68,15 +54,17 @@ def setup_android_image(ubman):
     with open(fname, 'rb') as inf:
         disk_data = inf.read()
 
-    test_abootimg.AbootimgTestDiskImage(ubman, 'bootv4.img', test_abootimg.boot_img_hex)
-    boot_img = os.path.join(ubman.config.result_dir, 'bootv4.img')
-    with open(boot_img, 'rb') as inf:
-        set_part_data(2, inf.read())
-
-    test_abootimg.AbootimgTestDiskImage(ubman, 'vendor_boot.img', test_abootimg.vboot_img_hex)
-    vendor_boot_img = os.path.join(ubman.config.result_dir, 'vendor_boot.img')
-    with open(vendor_boot_img, 'rb') as inf:
-        set_part_data(4, inf.read())
+    images = [
+        (2, 'bootv4.img', test_abootimg.boot_img_hex),
+        (4, 'vendor_boot.img', test_abootimg.vboot_img_hex),
+    ]
+    for partnum, img_name, hex_data in images:
+        test_abootimg.AbootimgTestDiskImage(ubman, img_name, hex_data)
+        img_path = os.path.join(ubman.config.result_dir, img_name)
+        with open(img_path, 'rb') as inf:
+            data = inf.read()
+        start = parts[partnum].start * sect_size
+        disk_data = disk_data[:start] + data + disk_data[start + len(data):]
 
     with open(fname, 'wb') as outf:
         outf.write(disk_data)
@@ -120,10 +108,16 @@ def setup_android_image(ubman):
     with open(fname, 'rb') as inf:
         disk_data = inf.read()
 
-    test_abootimg.AbootimgTestDiskImage(ubman, 'boot.img', test_abootimg.img_hex)
-    boot_img = os.path.join(ubman.config.result_dir, 'boot.img')
-    with open(boot_img, 'rb') as inf:
-        set_part_data(2, inf.read())
+    images = [
+        (2, 'boot.img', test_abootimg.img_hex),
+    ]
+    for partnum, img_name, hex_data in images:
+        test_abootimg.AbootimgTestDiskImage(ubman, img_name, hex_data)
+        img_path = os.path.join(ubman.config.result_dir, img_name)
+        with open(img_path, 'rb') as inf:
+            data = inf.read()
+        start = parts[partnum].start * sect_size
+        disk_data = disk_data[:start] + data + disk_data[start + len(data):]
 
     with open(fname, 'wb') as outf:
         outf.write(disk_data)

-- 
2.56.0

Re: [PATCH v2 04/10] test/py: android: Use for loop to create the images / update disks

[Simon Glass]

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: android: Use for loop to create the images / update disks
>
> setup_android_image() creates 2 mmcX.img files:
>
> * mmc7.img: For boot image v4 support
> * mmc8.img: For boot image v2 support
>
> There is quite some code that's common for both images. Currently,
> most code is duplicated.
>
> To prepare a cleaner refactoring, move the image descriptions into an
> array and create each partition using a loop.
>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
>  test/py/tests/test_android/android_image.py | 48 +++++++++++++----------------
>  1 file changed, 21 insertions(+), 27 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 05/10] test/py: android: Add create_gpt_image() for image creation

[Mattijs Korpershoek]

There is quite some code duplication between the image creation for
mmc7.img and mmc8.img. If we want to add more images, this will be even
more duplication.

Add a common function, create_gpt_image() which removes all the code
duplication.

Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/android_image.py | 78 +++++++----------------------
 1 file changed, 19 insertions(+), 59 deletions(-)

diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
index 14dfd26fffd8..b5fede8ebf0b 100644
--- a/test/py/tests/test_android/android_image.py
+++ b/test/py/tests/test_android/android_image.py
@@ -7,13 +7,12 @@ import os
 import utils
 from test_android import test_abootimg
 
-def setup_android_image(ubman):
+Partition = collections.namedtuple('part', 'start,size,name')
+
+def create_gpt_image(ubman, mmc_dev, required_parts, images):
     """Create a 20MB disk image with Android partitions"""
-    Partition = collections.namedtuple('part', 'start,size,name')
     parts = {}
     disk_data = None
-
-    mmc_dev = 7
     fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
     utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
     utils.run_and_log(ubman, f'cgpt create {fname}')
@@ -24,14 +23,6 @@ def setup_android_image(ubman):
     sect_size = 512
     sect_1mb = (1 << 20) // sect_size
 
-    required_parts = [
-        {'num': 1, 'label':'misc', 'size': '1M'},
-        {'num': 2, 'label':'boot_a', 'size': '4M'},
-        {'num': 3, 'label':'boot_b', 'size': '4M'},
-        {'num': 4, 'label':'vendor_boot_a', 'size': '4M'},
-        {'num': 5, 'label':'vendor_boot_b', 'size': '4M'},
-    ]
-
     for part in required_parts:
         size_str = part['size']
         if 'M' in size_str:
@@ -54,10 +45,6 @@ def setup_android_image(ubman):
     with open(fname, 'rb') as inf:
         disk_data = inf.read()
 
-    images = [
-        (2, 'bootv4.img', test_abootimg.boot_img_hex),
-        (4, 'vendor_boot.img', test_abootimg.vboot_img_hex),
-    ]
     for partnum, img_name, hex_data in images:
         test_abootimg.AbootimgTestDiskImage(ubman, img_name, hex_data)
         img_path = os.path.join(ubman.config.result_dir, img_name)
@@ -69,55 +56,28 @@ def setup_android_image(ubman):
     with open(fname, 'wb') as outf:
         outf.write(disk_data)
 
-    mmc_dev = 8
-    fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
-    utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
-    utils.run_and_log(ubman, f'cgpt create {fname}')
-
-    ptr = 40
-
-    # Number of sectors in 1MB
-    sect_size = 512
-    sect_1mb = (1 << 20) // sect_size
-
+def setup_android_image(ubman):
+    mmc_dev = 7
     required_parts = [
         {'num': 1, 'label':'misc', 'size': '1M'},
         {'num': 2, 'label':'boot_a', 'size': '4M'},
         {'num': 3, 'label':'boot_b', 'size': '4M'},
+        {'num': 4, 'label':'vendor_boot_a', 'size': '4M'},
+        {'num': 5, 'label':'vendor_boot_b', 'size': '4M'},
     ]
+    images = [
+        (2, 'bootv4.img', test_abootimg.boot_img_hex),
+        (4, 'vendor_boot.img', test_abootimg.vboot_img_hex),
+    ]
+    create_gpt_image(ubman, mmc_dev, required_parts, images)
 
-    for part in required_parts:
-        size_str = part['size']
-        if 'M' in size_str:
-            size = int(size_str[:-1]) * sect_1mb
-        else:
-            size = int(size_str)
-        utils.run_and_log(
-            ubman,
-            f"cgpt add -i {part['num']} -b {ptr} -s {size} -l {part['label']} -t basicdata {fname}")
-        ptr += size
-
-    utils.run_and_log(ubman, f'cgpt boot -p {fname}')
-    out = utils.run_and_log(ubman, f'cgpt show -q {fname}')
-
-    # Create a dict (indexed by partition number) containing the above info
-    for line in out.splitlines():
-        start, size, num, name = line.split(maxsplit=3)
-        parts[int(num)] = Partition(int(start), int(size), name)
-
-    with open(fname, 'rb') as inf:
-        disk_data = inf.read()
-
+    mmc_dev = 8
+    required_parts = [
+        {'num': 1, 'label':'misc', 'size': '1M'},
+        {'num': 2, 'label':'boot_a', 'size': '4M'},
+        {'num': 3, 'label':'boot_b', 'size': '4M'},
+    ]
     images = [
         (2, 'boot.img', test_abootimg.img_hex),
     ]
-    for partnum, img_name, hex_data in images:
-        test_abootimg.AbootimgTestDiskImage(ubman, img_name, hex_data)
-        img_path = os.path.join(ubman.config.result_dir, img_name)
-        with open(img_path, 'rb') as inf:
-            data = inf.read()
-        start = parts[partnum].start * sect_size
-        disk_data = disk_data[:start] + data + disk_data[start + len(data):]
-
-    with open(fname, 'wb') as outf:
-        outf.write(disk_data)
+    create_gpt_image(ubman, mmc_dev, required_parts, images)

-- 
2.56.0

Re: [PATCH v2 05/10] test/py: android: Add create_gpt_image() for image creation

[Guillaume La Roque]

> There is quite some code duplication between the image creation for
> mmc7.img and mmc8.img. If we want to add more images, this will be even
> more duplication.
> 
> Add a common function, create_gpt_image() which removes all the code
> duplication.
> 
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
> diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
> index 14dfd26fffd8..b5fede8ebf0b 100644
> --- a/test/py/tests/test_android/android_image.py
> +++ b/test/py/tests/test_android/android_image.py
> @@ -7,13 +7,12 @@ import os
>  import utils
>  from test_android import test_abootimg
>  
> -def setup_android_image(ubman):
> +Partition = collections.namedtuple('part', 'start,size,name')
> +
> +def create_gpt_image(ubman, mmc_dev, required_parts, images):
>      """Create a 20MB disk image with Android partitions"""
> -    Partition = collections.namedtuple('part', 'start,size,name')
>      parts = {}
>      disk_data = None
> -
> -    mmc_dev = 7
>      fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
>      utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
>      utils.run_and_log(ubman, f'cgpt create {fname}')
> @@ -24,14 +23,6 @@ def setup_android_image(ubman):
>      sect_size = 512
>      sect_1mb = (1 << 20) // sect_size
>  
> -    required_parts = [
> -        {'num': 1, 'label':'misc', 'size': '1M'},
> -        {'num': 2, 'label':'boot_a', 'size': '4M'},
> -        {'num': 3, 'label':'boot_b', 'size': '4M'},
> -        {'num': 4, 'label':'vendor_boot_a', 'size': '4M'},
> -        {'num': 5, 'label':'vendor_boot_b', 'size': '4M'},
> -    ]
> -
>      for part in required_parts:
>          size_str = part['size']
>          if 'M' in size_str:
> @@ -54,10 +45,6 @@ def setup_android_image(ubman):
>      with open(fname, 'rb') as inf:
>          disk_data = inf.read()
>  
> -    images = [
> -        (2, 'bootv4.img', test_abootimg.boot_img_hex),
> -        (4, 'vendor_boot.img', test_abootimg.vboot_img_hex),
> -    ]
>      for partnum, img_name, hex_data in images:
>          test_abootimg.AbootimgTestDiskImage(ubman, img_name, hex_data)
>          img_path = os.path.join(ubman.config.result_dir, img_name)
> @@ -69,55 +56,28 @@ def setup_android_image(ubman):
>      with open(fname, 'wb') as outf:
>          outf.write(disk_data)
>  
> -    mmc_dev = 8
> -    fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
> -    utils.run_and_log(ubman, f'qemu-img create {fname} 20M')
> -    utils.run_and_log(ubman, f'cgpt create {fname}')
> -
> -    ptr = 40
> -
> -    # Number of sectors in 1MB
> -    sect_size = 512
> -    sect_1mb = (1 << 20) // sect_size
> -
> +def setup_android_image(ubman):
> +    mmc_dev = 7
>      required_parts = [
>          {'num': 1, 'label':'misc', 'size': '1M'},
>          {'num': 2, 'label':'boot_a', 'size': '4M'},
>          {'num': 3, 'label':'boot_b', 'size': '4M'},
> +        {'num': 4, 'label':'vendor_boot_a', 'size': '4M'},
> +        {'num': 5, 'label':'vendor_boot_b', 'size': '4M'},
>      ]
> +    images = [
> +        (2, 'bootv4.img', test_abootimg.boot_img_hex),
> +        (4, 'vendor_boot.img', test_abootimg.vboot_img_hex),
> +    ]
> +    create_gpt_image(ubman, mmc_dev, required_parts, images)
>  
> -    for part in required_parts:
> -        size_str = part['size']
> -        if 'M' in size_str:
> -            size = int(size_str[:-1]) * sect_1mb
> -        else:
> -            size = int(size_str)
> -        utils.run_and_log(
> -            ubman,
> -            f"cgpt add -i {part['num']} -b {ptr} -s {size} -l {part['label']} -t basicdata {fname}")
> -        ptr += size
> -
> -    utils.run_and_log(ubman, f'cgpt boot -p {fname}')
> -    out = utils.run_and_log(ubman, f'cgpt show -q {fname}')
> -
> -    # Create a dict (indexed by partition number) containing the above info
> -    for line in out.splitlines():
> -        start, size, num, name = line.split(maxsplit=3)
> -        parts[int(num)] = Partition(int(start), int(size), name)
> -
> -    with open(fname, 'rb') as inf:
> -        disk_data = inf.read()
> -
> +    mmc_dev = 8
> +    required_parts = [
> +        {'num': 1, 'label':'misc', 'size': '1M'},
> +        {'num': 2, 'label':'boot_a', 'size': '4M'},
> +        {'num': 3, 'label':'boot_b', 'size': '4M'},
> +    ]
>      images = [
>          (2, 'boot.img', test_abootimg.img_hex),
>      ]
> -    for partnum, img_name, hex_data in images:
> -        test_abootimg.AbootimgTestDiskImage(ubman, img_name, hex_data)
> -        img_path = os.path.join(ubman.config.result_dir, img_name)
> -        with open(img_path, 'rb') as inf:
> -            data = inf.read()
> -        start = parts[partnum].start * sect_size
> -        disk_data = disk_data[:start] + data + disk_data[start + len(data):]
> -
> -    with open(fname, 'wb') as outf:
> -        outf.write(disk_data)
> +    create_gpt_image(ubman, mmc_dev, required_parts, images)

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>

-- 
Guillaume La Roque <glaroque@baylibre.com>

Re: [PATCH v2 05/10] test/py: android: Add create_gpt_image() for image creation

[Simon Glass]

Hi Mattijs,

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: android: Add create_gpt_image() for image creation
>
> There is quite some code duplication between the image creation for
> mmc7.img and mmc8.img. If we want to add more images, this will be even
> more duplication.
>
> Add a common function, create_gpt_image() which removes all the code
> duplication.
>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
>
>  test/py/tests/test_android/android_image.py | 78 +++++++----------------------
>  1 file changed, 19 insertions(+), 59 deletions(-)

> diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
> @@ -7,13 +7,12 @@ import os
> -def setup_android_image(ubman):
> +Partition = collections.namedtuple('part', 'start,size,name')
> +
> +def create_gpt_image(ubman, mmc_dev, required_parts, images):
>      """Create a 20MB disk image with Android partitions"""
> -    Partition = collections.namedtuple('part', 'start,size,name')
>      parts = {}
>      disk_data = None

This is now a general helper that callers have to fill in, but the
format of 'required_parts' (a list of dicts with 'num', 'label' and
'size', where size is either a sector count or 'NM') and 'images'
(tuples of partition number, file name and hex data) is only visible
from reading the body. Please can you add an Args: section to the
docstring describing each parameter, as set_part_data() has earlier in
the series? Since the name now says GPT, the docstring could also say
which mmc image it writes rather than 'Android partitions'.

Also, 'disk_data = None' is now redundant since the variable is only
assigned once, further down. Moving Partition to module level seems
unnecessary too, since only this function uses it.

> diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
> @@ -69,55 +56,28 @@ def setup_android_image(ubman):
> +def setup_android_image(ubman):
> +    mmc_dev = 7

This drops the docstring from setup_android_image(), so pylint
complains at this patch. The next patch adds it back, but it would be
better to keep it here so that each patch is clean on its own.

Regards,
Simon

Re: [PATCH v2 05/10] test/py: android: Add create_gpt_image() for image creation

[Mattijs Korpershoek]

Hi Simon,

Thank you for the review.

On Thu, Oct 22, 2026 at 01:26, Simon Glass <sjg@chromium.org> wrote:

> Hi Mattijs,
>
> On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>> test/py: android: Add create_gpt_image() for image creation
>>
>> There is quite some code duplication between the image creation for
>> mmc7.img and mmc8.img. If we want to add more images, this will be even
>> more duplication.
>>
>> Add a common function, create_gpt_image() which removes all the code
>> duplication.
>>
>> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
>> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
>> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
>> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
>>
>>  test/py/tests/test_android/android_image.py | 78 +++++++----------------------
>>  1 file changed, 19 insertions(+), 59 deletions(-)
>
>> diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
>> @@ -7,13 +7,12 @@ import os
>> -def setup_android_image(ubman):
>> +Partition = collections.namedtuple('part', 'start,size,name')
>> +
>> +def create_gpt_image(ubman, mmc_dev, required_parts, images):
>>      """Create a 20MB disk image with Android partitions"""
>> -    Partition = collections.namedtuple('part', 'start,size,name')
>>      parts = {}
>>      disk_data = None
>
> This is now a general helper that callers have to fill in, but the
> format of 'required_parts' (a list of dicts with 'num', 'label' and
> 'size', where size is either a sector count or 'NM') and 'images'
> (tuples of partition number, file name and hex data) is only visible
> from reading the body. Please can you add an Args: section to the
> docstring describing each parameter, as set_part_data() has earlier in
> the series? Since the name now says GPT, the docstring could also say
> which mmc image it writes rather than 'Android partitions'.

Ok, will do for v3.

>
> Also, 'disk_data = None' is now redundant since the variable is only
> assigned once, further down. Moving Partition to module level seems
> unnecessary too, since only this function uses it.

Ok, dropped disk_data = None for v3.
Ok, put Partition back into the function for v3.

>
>> diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
>> @@ -69,55 +56,28 @@ def setup_android_image(ubman):
>> +def setup_android_image(ubman):
>> +    mmc_dev = 7
>
> This drops the docstring from setup_android_image(), so pylint
> complains at this patch. The next patch adds it back, but it would be
> better to keep it here so that each patch is clean on its own.

Ok, will put the docstring back in in v3.

>
> Regards,
> Simon

[PATCH v2 06/10] test/py: android: Add setup_android_v{4,2}_image subfunctions

[Mattijs Korpershoek]

To split out the logic even further, do each image creation in a
separate function.

This will make it cleaner to add future images as well.

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/android_image.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
index b5fede8ebf0b..abdc6cd28744 100644
--- a/test/py/tests/test_android/android_image.py
+++ b/test/py/tests/test_android/android_image.py
@@ -56,8 +56,8 @@ def create_gpt_image(ubman, mmc_dev, required_parts, images):
     with open(fname, 'wb') as outf:
         outf.write(disk_data)
 
-def setup_android_image(ubman):
-    mmc_dev = 7
+def setup_android_v4_image(ubman):
+    """Create mmc7.img: boot image v4 with vendor_boot partitions."""
     required_parts = [
         {'num': 1, 'label':'misc', 'size': '1M'},
         {'num': 2, 'label':'boot_a', 'size': '4M'},
@@ -69,9 +69,10 @@ def setup_android_image(ubman):
         (2, 'bootv4.img', test_abootimg.boot_img_hex),
         (4, 'vendor_boot.img', test_abootimg.vboot_img_hex),
     ]
-    create_gpt_image(ubman, mmc_dev, required_parts, images)
+    create_gpt_image(ubman, 7, required_parts, images)
 
-    mmc_dev = 8
+def setup_android_v2_image(ubman):
+    """Create mmc8.img: boot image v2 without vendor_boot."""
     required_parts = [
         {'num': 1, 'label':'misc', 'size': '1M'},
         {'num': 2, 'label':'boot_a', 'size': '4M'},
@@ -80,4 +81,9 @@ def setup_android_image(ubman):
     images = [
         (2, 'boot.img', test_abootimg.img_hex),
     ]
-    create_gpt_image(ubman, mmc_dev, required_parts, images)
+    create_gpt_image(ubman, 8, required_parts, images)
+
+def setup_android_image(ubman):
+    """Create disk images with Android partitions."""
+    setup_android_v4_image(ubman)
+    setup_android_v2_image(ubman)

-- 
2.56.0

Re: [PATCH v2 06/10] test/py: android: Add setup_android_v{4, 2}_image subfunctions

[Simon Glass]

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: android: Add setup_android_v{4,2}_image subfunctions
>
> To split out the logic even further, do each image creation in a
> separate function.
>
> This will make it cleaner to add future images as well.
>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
>  test/py/tests/test_android/android_image.py | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 07/10] test/py: avb: Use format specifiers instead of str() for mmc_dev

[Mattijs Korpershoek]

The tests format mmc_dev using str() conversion, which produces a
decimal string. This works when mmc_dev is 1, since "1" is valid in
both decimal and hex, but breaks for other device numbers like 12
(0xc): str(12) produces "12", which avb init would interpret as 0x12.

Use %x for commands that parse hex (avb init) and %d for commands that
parse decimal (mmc dev) to format device numbers correctly regardless
of their value.

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/test_avb.py | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
index 137d83e1deab..c66ef0cc52d5 100644
--- a/test/py/tests/test_android/test_avb.py
+++ b/test/py/tests/test_android/test_avb.py
@@ -29,7 +29,7 @@ def test_avb_verify(ubman):
 
     success_str = "Verification passed successfully"
 
-    response = ubman.run_command('avb init %s' %str(mmc_dev))
+    response = ubman.run_command('avb init %x' % mmc_dev)
     assert response == ''
     response = ubman.run_command('avb verify')
     assert response.find(success_str)
@@ -43,11 +43,10 @@ def test_avb_mmc_uuid(ubman):
     'part list mmc 1' output
     """
 
-    response = ubman.run_command('avb init %s' % str(mmc_dev))
+    response = ubman.run_command('avb init %x' % mmc_dev)
     assert response == ''
 
-    response = ubman.run_command('mmc rescan; mmc dev %s' %
-                                          str(mmc_dev))
+    response = ubman.run_command('mmc rescan; mmc dev %d' % mmc_dev)
     assert response.find('is current device')
 
     part_lines = ubman.run_command('mmc part').splitlines()
@@ -75,7 +74,7 @@ def test_avb_read_rb(ubman):
     """Test reading rollback indexes
     """
 
-    response = ubman.run_command('avb init %s' % str(mmc_dev))
+    response = ubman.run_command('avb init %x' % mmc_dev)
     assert response == ''
 
     response = ubman.run_command('avb read_rb 1')
@@ -87,7 +86,7 @@ def test_avb_is_unlocked(ubman):
     """Test if device is in the unlocked state
     """
 
-    response = ubman.run_command('avb init %s' % str(mmc_dev))
+    response = ubman.run_command('avb init %x' % mmc_dev)
     assert response == ''
 
     response = ubman.run_command('avb is_unlocked')
@@ -101,14 +100,13 @@ def test_avb_mmc_read(ubman):
     """Test mmc read operation
     """
 
-    response = ubman.run_command('mmc rescan; mmc dev %s 0' %
-                                          str(mmc_dev))
+    response = ubman.run_command('mmc rescan; mmc dev %d 0' % mmc_dev)
     assert response.find('is current device')
 
     response = ubman.run_command('mmc read 0x%x 0x100 0x1' % temp_addr)
     assert response.find('read: OK')
 
-    response = ubman.run_command('avb init %s' % str(mmc_dev))
+    response = ubman.run_command('avb init %x' % mmc_dev)
     assert response == ''
 
     response = ubman.run_command('avb read_part xloader 0 100 0x%x' %
@@ -127,7 +125,7 @@ def test_avb_persistent_values(ubman):
     """Test reading/writing persistent storage to avb
     """
 
-    response = ubman.run_command('avb init %s' % str(mmc_dev))
+    response = ubman.run_command('avb init %x' % mmc_dev)
     assert response == ''
 
     response = ubman.run_command('avb write_pvalue test value_value')

-- 
2.56.0

Re: [PATCH v2 07/10] test/py: avb: Use format specifiers instead of str() for mmc_dev

[Simon Glass]

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: avb: Use format specifiers instead of str() for mmc_dev
>
> The tests format mmc_dev using str() conversion, which produces a
> decimal string. This works when mmc_dev is 1, since "1" is valid in
> both decimal and hex, but breaks for other device numbers like 12
> (0xc): str(12) produces "12", which avb init would interpret as 0x12.
>
> Use %x for commands that parse hex (avb init) and %d for commands that
> parse decimal (mmc dev) to format device numbers correctly regardless
> of their value.
>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
>  test/py/tests/test_android/test_avb.py | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 08/10] test/py: avb: Move mmc device number to a pytest fixture

[Mattijs Korpershoek]

The AVB tests hardcode mmc device 1, which is usually correct on real
hardware. On sandbox, this is not the case because mmc1.img contains a
fedora image.

Replace the module-level mmc_dev constant with an avb_device_id
fixture so that a subsequent commit can override the device number
for sandbox.

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/test_avb.py | 38 ++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
index c66ef0cc52d5..34cddc485ebc 100644
--- a/test/py/tests/test_android/test_avb.py
+++ b/test/py/tests/test_android/test_avb.py
@@ -16,20 +16,26 @@ For configuration verification:
 
 import pytest
 
-# defauld mmc id
-mmc_dev = 1
 temp_addr = 0x90000000
 temp_addr2 = 0x90002000
 
+@pytest.fixture(scope='function')
+def avb_device_id(ubman):
+    """Returns the device id number for AVB
+    """
+
+    # default mmc id
+    return 1
+
 @pytest.mark.buildconfigspec('cmd_avb')
 @pytest.mark.buildconfigspec('cmd_mmc')
-def test_avb_verify(ubman):
+def test_avb_verify(ubman, avb_device_id):
     """Run AVB 2.0 boot verification chain with avb subset of commands
     """
 
     success_str = "Verification passed successfully"
 
-    response = ubman.run_command('avb init %x' % mmc_dev)
+    response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
     response = ubman.run_command('avb verify')
     assert response.find(success_str)
@@ -38,15 +44,15 @@ def test_avb_verify(ubman):
 @pytest.mark.buildconfigspec('cmd_avb')
 @pytest.mark.buildconfigspec('cmd_mmc')
 @pytest.mark.notbuildconfigspec('sandbox')
-def test_avb_mmc_uuid(ubman):
+def test_avb_mmc_uuid(ubman, avb_device_id):
     """Check if 'avb get_uuid' works, compare results with
     'part list mmc 1' output
     """
 
-    response = ubman.run_command('avb init %x' % mmc_dev)
+    response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
 
-    response = ubman.run_command('mmc rescan; mmc dev %d' % mmc_dev)
+    response = ubman.run_command('mmc rescan; mmc dev %d' % avb_device_id)
     assert response.find('is current device')
 
     part_lines = ubman.run_command('mmc part').splitlines()
@@ -70,11 +76,11 @@ def test_avb_mmc_uuid(ubman):
 
 
 @pytest.mark.buildconfigspec('cmd_avb')
-def test_avb_read_rb(ubman):
+def test_avb_read_rb(ubman, avb_device_id):
     """Test reading rollback indexes
     """
 
-    response = ubman.run_command('avb init %x' % mmc_dev)
+    response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
 
     response = ubman.run_command('avb read_rb 1')
@@ -82,11 +88,11 @@ def test_avb_read_rb(ubman):
 
 
 @pytest.mark.buildconfigspec('cmd_avb')
-def test_avb_is_unlocked(ubman):
+def test_avb_is_unlocked(ubman, avb_device_id):
     """Test if device is in the unlocked state
     """
 
-    response = ubman.run_command('avb init %x' % mmc_dev)
+    response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
 
     response = ubman.run_command('avb is_unlocked')
@@ -96,17 +102,17 @@ def test_avb_is_unlocked(ubman):
 @pytest.mark.buildconfigspec('cmd_avb')
 @pytest.mark.buildconfigspec('cmd_mmc')
 @pytest.mark.notbuildconfigspec('sandbox')
-def test_avb_mmc_read(ubman):
+def test_avb_mmc_read(ubman, avb_device_id):
     """Test mmc read operation
     """
 
-    response = ubman.run_command('mmc rescan; mmc dev %d 0' % mmc_dev)
+    response = ubman.run_command('mmc rescan; mmc dev %d 0' % avb_device_id)
     assert response.find('is current device')
 
     response = ubman.run_command('mmc read 0x%x 0x100 0x1' % temp_addr)
     assert response.find('read: OK')
 
-    response = ubman.run_command('avb init %x' % mmc_dev)
+    response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
 
     response = ubman.run_command('avb read_part xloader 0 100 0x%x' %
@@ -121,11 +127,11 @@ def test_avb_mmc_read(ubman):
 
 @pytest.mark.buildconfigspec('cmd_avb')
 @pytest.mark.buildconfigspec('optee_ta_avb')
-def test_avb_persistent_values(ubman):
+def test_avb_persistent_values(ubman, avb_device_id):
     """Test reading/writing persistent storage to avb
     """
 
-    response = ubman.run_command('avb init %x' % mmc_dev)
+    response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
 
     response = ubman.run_command('avb write_pvalue test value_value')

-- 
2.56.0

Re: [PATCH v2 08/10] test/py: avb: Move mmc device number to a pytest fixture

[Simon Glass]

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: avb: Move mmc device number to a pytest fixture
>
> The AVB tests hardcode mmc device 1, which is usually correct on real
> hardware. On sandbox, this is not the case because mmc1.img contains a
> fedora image.
>
> Replace the module-level mmc_dev constant with an avb_device_id
> fixture so that a subsequent commit can override the device number
> for sandbox.
>
> Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Tested-by: Guillaume La Roque <glaroque@baylibre.com>
> Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
>
>  test/py/tests/test_android/test_avb.py | 38 ++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 16 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 09/10] test/py: avb: Add sandbox AVB testing with dedicated mmc12 device

[Mattijs Korpershoek]

Right now, sandbox runs the AVB tests using mmc1.img, which contains a
fedora image.
This does not really make sense. We should test on an mmc device which
has a proper vbmeta partition instead.

Add a new mmc12 in sandbox which contains all required partitions for
proper AVB testing.
Use the mmc12 device in the avb tests only when running in sandbox.
The mmc12 DT node gets enabled by the avb_device_id fixture if CMD_BIND
is available.

Note: the vbmeta.img has been generated using:

    $ ./avbtool.py add_hash_footer \
                   --image boot.img \
                   --partition_name boot \
                   --partition_size 4194304

    $ ./avbtool.py make_vbmeta_image \
                   --key test/data/testkey_rsa4096.pem \
                   --algorithm SHA256_RSA4096 \
                   --include_descriptors_from_image boot.img \
                   --padding_size 4096 --flags 1 \
                   --output vbmeta.img

     $ gzip -9 vbmeta.img
     $ xxd -p vbmeta.img.gz > vbmeta.img.gz.hex

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 arch/sandbox/dts/test.dts                   |  8 +++
 test/py/tests/test_android/android_image.py | 15 ++++++
 test/py/tests/test_android/test_abootimg.py | 77 +++++++++++++++++++++++++++++
 test/py/tests/test_android/test_avb.py      | 26 ++++++++++
 4 files changed, 126 insertions(+)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 25c99ef81472..80f505083cd9 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -50,6 +50,7 @@
 		mmc9 = "/mmc9";
 		mmc10 = "/mmc10";
 		mmc11 = "/mmc11";
+		mmc12 = "/mmc12";
 		pci0 = &pci0;
 		pci1 = &pci1;
 		pci2 = &pci2;
@@ -1398,6 +1399,13 @@
 		filename = "mmc11.img";
 	};
 
+	/* This is used for Android AVB tests */
+	mmc12 {
+		status = "disabled";
+		compatible = "sandbox,mmc";
+		filename = "mmc12.img";
+	};
+
 	pch {
 		compatible = "sandbox,pch";
 	};
diff --git a/test/py/tests/test_android/android_image.py b/test/py/tests/test_android/android_image.py
index abdc6cd28744..72725ac9044c 100644
--- a/test/py/tests/test_android/android_image.py
+++ b/test/py/tests/test_android/android_image.py
@@ -83,7 +83,22 @@ def setup_android_v2_image(ubman):
     ]
     create_gpt_image(ubman, 8, required_parts, images)
 
+def setup_android_avb_image(ubman):
+    """Create mmc12.img: boot image v2 with vbmeta.img."""
+
+    required_parts = [
+        {'num': 1, 'label':'boot', 'size': '4M'},
+        {'num': 2, 'label':'system', 'size': '4M'},
+        {'num': 3, 'label':'vbmeta', 'size': '4M'},
+    ]
+    images = [
+        (1, 'boot.img', test_abootimg.img_hex),
+        (3, 'vbmeta.img', test_abootimg.vbmeta_img_hex),
+    ]
+    create_gpt_image(ubman, 12, required_parts, images)
+
 def setup_android_image(ubman):
     """Create disk images with Android partitions."""
     setup_android_v4_image(ubman)
     setup_android_v2_image(ubman)
+    setup_android_avb_image(ubman)
diff --git a/test/py/tests/test_android/test_abootimg.py b/test/py/tests/test_android/test_abootimg.py
index ff54a8318a12..a54e09bb09ba 100644
--- a/test/py/tests/test_android/test_abootimg.py
+++ b/test/py/tests/test_android/test_abootimg.py
@@ -49,6 +49,18 @@ using the same steps above :
                 --pagesize 4096  --vendor_ramdisk ./ramdisk.img \
                 --header_version 4 --vendor_boot ./vboot.img \
 
+3- vbmeta.img
+    $ ./avbtool.py add_hash_footer \
+                   --image boot.img \
+                   --partition_name boot \
+                   --partition_size 4194304
+
+    $ ./avbtool.py make_vbmeta_image \
+                   --key test/data/testkey_rsa4096.pem \
+                   --algorithm SHA256_RSA4096 \
+                   --include_descriptors_from_image boot.img \
+                   --padding_size 4096 --flags 1 \
+                   --output vbmeta.img
 """
 
 # boot.img.gz hex dump
@@ -81,6 +93,71 @@ fae68747835d6dec42bc0df6d74d1fc5a0bfac6e89331797b9564926663a
 000080fa912fcae854c55adbeb2769d4ab34c9ad4d16963f010000000000
 a88d2fb468951800500000"""
 
+vbmeta_img_hex = """
+1f8b0808f36fcf6a020376626d6574612e696d6700edd369301bea1a07f0
+7062dfb758521a55b4b650722807572d41ab2d5a6a4b90a225b64aad69c5
+d67063b968a9725b91632bb12fb11faa45c4762c8928557b7a6c557b0fee
+ccd9e69e993b77eecc9db91feef87df9bfcfbcf3cefbe5f99b385cd10400
+006c803fb0ffe5d7e4f86500fc19e4cff9c775efaf01e4fe2d8701ff5aef
+3f9d7ff9d323d413131888866869e8686802fef7025c122d02b16f9c05b1
+3fd89904c6054cb993631550b20e3d36f0fc97a5fab3e3ab1813c7059894
+3024ec1e6a4c412f9d71de37ebb3132b8f25858b29e0a98d19820724a8ad
+11ef74e93b156edc57630f18b6788530b0de2f661a5f755bda6b28cc5367
+f41f67dc7c008db14b0f961539b149a94adc44317ab0f55f257d50addbb2
+6ca967eb2f0240832e33ad34f0165457652a752b30e1a9ea60bb64458785
+4d17bf22cc1fc07b777210d3f3e139b0377020d02b59ac2d84ccb5f3ac4a
+eeb60e1322f9445a259a6b3c79f2a563e7e16a9c54886ac6323659f8ef85
+82e36fd1b7728f1d645495422df053249da0d2c58fe44b24a250cb8f7306
+faa26fb8231abdb6ce657cf416e183cb4a111de2a8363c1d8a61b5673f86
+01ef83ddc1059b3a370c8a23a5560bd9433d664dde352db09412cab2272f
+5a145b363c347439a13b1ecc535e95f53ea79c401b48efa277065eb21467
+6f104b19d15d8f4790cce128dd673132db7edc29bb63b0899e25ab1778ce
+f14622bd6d9ff0e486336968f432b3c107e9e0fa75044c19e8474936cfec
+c85b0a1cad94ac579b390a995d312280a6d3b0dc2224da514ef5965f2749
+4733a4324d40ec6bb8b8c7ec2d0bbad1a8ffdbf3743231b1125be6ffe07d
+bbf14e2e7c3c7cba09be7b0486e22b708b40cc39d88176eb190bcbb9905c
+09cf1c8dbd282817c483f5b7f7b1cc6147f935fb65c5b7d42ad70b5cb49a
+667f3d51fd70d38a5dcb017c8df917df58c27485c47fb846bf1781f27b3d
+42ee795c827dfb6fdf007feb11e4bfdd61cfc0404cc5d6f032473f5cc44e
+f49025bc09950a607ad7114cf1da446413f27b3fc1c875231c6ba6c5d629
+179c169fd52add2237fc48f17ee770a9b15ca6ade3094e1c001006dc9e04
+9219c0caf9d178f4638124c3aee451d1db2ccfbecf42081ae841991956cd
+cbfa693d4815d0c2b921c6657fede06ed3854870fcf34486cff70a07c557
+df5b0bede94b55dc2a9138ae2297c3980664e5d6ca9551a8aece84ad2ada
+59999ec559e947c1510f35ebaa136a6d5ebf7e03d291a12cf71c1ef08495
+6567fe6456310469ddc4894c29f0c3839bfc27a2dce1789043666397ca8b
+aa6b6cfc9a8c4cfce65a00a15dd365b7a8721a873547a4a56722cc8e29b1
+5ceb11d40ead10cf3bf303ddad5d22b244eb8e63e547c45d1809417def08
+35daec737be2bb5fe95dbc36405923229b731364c6792a9e5e3718ea77ed
+8bc0edd18aac4b7c19a35a2207bbec281519f511196f30c8a4ad6dcfe607
+eb45652b64f49d7965f5c53e6f413b3d90de57d04ff6c64b47ac0284a213
+23c2fce49b6b4a0500e7d42e5ba2c441d4b66b4cae141cc650e760918560
+1809522a81a6ccc4ba2d1c024739f89af56baccca1f01e0113e36c9c8d71
+677d4b10b258b7d2f29ca8b9c68e103e632e9eed4054485ce2fc6052ce7a
+53b7645090c4a7f677e8b39eb5dbea20fc570739f3b28a4b0b88f4357b6f
+44db1a7949f4b2405cdf8a1f5abd70bddbd9bb22399b2e80b6ead9e9ced0
+22f0ac6b334b751baca4f605ddcd7bb5d337208ec2ede1ea714a13548f03
+e86c0787a69db3e81940b11b0dabf1f6198164b069aa8198064c09d4eae6
+940ee4359aeec1cce4bb6283cb525ed79988accaa5fc2c1e9b713dce97d7
+0f867063b9d292672e8f58bb2051a5776779fbf928ced8377c4bedc704a1
+473aa9ecde18363435b41cebc30d3596295273af3d61a65044f901d1b49b
+44d7bbc439e425afbebcf6095e31fc772ae6707eea95d8c324afe5a4bab4
+3db92585c26127dd880cfaa0e1d13c916f089d90eff5c5f59d350a19a0af
+af42f06d80b6ecd75cf56c954957f49ebf4f75b7e2dbfd6bbb51a87d5764
+0950f2e1e72fdf7882136aaa3670120d63d73f04974fa900c7eae49bbd1c
+1c570ee30b1e890956d25fdc5121c5c4a7f0e4fb76b64cee3c296f7be8c1
+b3946c1856b3a6095900c866af74f400c0f2d5a065131f8d55bdd645701d
+a78256f019ab8150ebd06f53484904ebc86d7caf8490eee55d7f9cf4866b
+89f720181421ff81aea5d92d3fade3f2a38caa91dbde272a476a63bf9546
+fefc76b1a5b4701eb078fb02a637100551b538af242d3ba2a6e5ef7cb180
+b3a8c684c6ab9cc593550ef70abf67fb7c8251a49e3fb43a87d98a9dcf9e
+7c3cb6cf17646a581d958d9be8a74bb7a2f88e6c128f7dca29b49b8df76a
+5d26e4147f467e9752ad8dadb562330ebe2d68dbeca174f869b17ee5f5d0
+40d6ab894ea47d3a996306b26ccb7479e6b3b6bd7ec52b488c2f48fc4cdf
+ac3415b9231b7415cd8959add708fb88219272f2135e7e2e673c069c3a75
+ead4a953a7feeffd03523602d400100000
+"""
+
 # Expected response for "abootimg dtb_dump" command
 dtb_dump_resp="""## DTB area contents (concat format):
  - DTB #0:
diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
index 34cddc485ebc..48fa6ee7cf0b 100644
--- a/test/py/tests/test_android/test_avb.py
+++ b/test/py/tests/test_android/test_avb.py
@@ -13,8 +13,11 @@ For configuration verification:
 - Corrupt boot partition and check for failure
 - Corrupt vbmeta partition and check for failure
 """
+import os
 
 import pytest
+import shutil
+from android_image import setup_android_avb_image
 
 temp_addr = 0x90000000
 temp_addr2 = 0x90002000
@@ -22,7 +25,30 @@ temp_addr2 = 0x90002000
 @pytest.fixture(scope='function')
 def avb_device_id(ubman):
     """Returns the device id number for AVB
+
+    On sandbox, creates mmc12.img and binds the disabled mmc12 DT node.
+    On other boards, defaults to device 1.
     """
+    if ubman.config.buildconfig.get('config_sandbox'):
+        if not ubman.config.buildconfig.get('config_cmd_bind'):
+            pytest.skip('CMD_BIND required for AVB on sandbox')
+
+        required_host_tools = ['qemu-img', 'cgpt']
+        for tool in required_host_tools:
+            if shutil.which(tool) is None:
+                pytest.skip(f'tool "{tool}" not in $PATH')
+
+        mmc_dev = 0xc
+        fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
+        if not os.path.exists(fname):
+            setup_android_avb_image(ubman)
+            ubman.restart_uboot()
+
+        ubman.run_command(f'bind /mmc{mmc_dev} mmc_sandbox')
+        # Run mmc list to ensure that mmc_sandbox driver is probed
+        ubman.run_command('mmc list')
+
+        return mmc_dev
 
     # default mmc id
     return 1

-- 
2.56.0

Re: [PATCH v2 09/10] test/py: avb: Add sandbox AVB testing with dedicated mmc12 device

[Simon Glass]

Hi Mattijs,

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: avb: Add sandbox AVB testing with dedicated mmc12 device
>
> Right now, sandbox runs the AVB tests using mmc1.img, which contains a
> fedora image.
> This does not really make sense. We should test on an mmc device which
> has a proper vbmeta partition instead.
>
> Add a new mmc12 in sandbox which contains all required partitions for
> proper AVB testing.
> Use the mmc12 device in the avb tests only when running in sandbox.
> The mmc12 DT node gets enabled by the avb_device_id fixture if CMD_BIND
> is available.
>
> Note: the vbmeta.img has been generated using:
>
>     $ ./avbtool.py add_hash_footer \
>                    --image boot.img \
>                    --partition_name boot \
>                    --partition_size 4194304
>
> [...]
>
>  arch/sandbox/dts/test.dts                   |  8 +++
>  test/py/tests/test_android/android_image.py | 15 ++++++
>  test/py/tests/test_android/test_abootimg.py | 77 +++++++++++++++++++++++++++++
>  test/py/tests/test_android/test_avb.py      | 26 ++++++++++
>  4 files changed, 126 insertions(+)

> diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
> @@ -22,7 +25,30 @@ temp_addr2 = 0x90002000
> +        mmc_dev = 0xc
> +        fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
> +        if not os.path.exists(fname):
> +            setup_android_avb_image(ubman)
> +            ubman.restart_uboot()

Just to check, is the restart needed? The mmc12 node is disabled, so
nothing probes it until the 'bind' below. Since sandbox_mmc_probe() is
where the file gets mapped, the new image should be picked up without
restarting U-Boot.

> diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
> @@ -22,7 +25,30 @@ temp_addr2 = 0x90002000
> +        ubman.run_command(f'bind /mmc{mmc_dev} mmc_sandbox')
> +        # Run mmc list to ensure that mmc_sandbox driver is probed
> +        ubman.run_command('mmc list')

The fixture has function scope, so this bind runs for each AVB test in
the same U-Boot session. Since bind_by_node_path() doesn't check
whether the node already has a device, each later test adds another
mmc12 device with the same alias seq. The tests may still pass because
the first device is found first, but it leaves junk in driver model.
Please can you either unbind after the test (yield, then 'unbind
/mmc12') or bind only if the device isn't already there? It would also
be worth checking the bind output, so that a failure shows up here
rather than later as a confusing 'avb verify' error.

I'm also wondering whether we lose something by not doing the
disk-image setup upfront? Can we still run C tests on it?

Regards,
Simon

Re: [PATCH v2 09/10] test/py: avb: Add sandbox AVB testing with dedicated mmc12 device

[Mattijs Korpershoek]

Hi Simon,

Thank you for the review.

On Thu, Oct 22, 2026 at 01:26, Simon Glass <sjg@chromium.org> wrote:

> Hi Mattijs,
>
> On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>> test/py: avb: Add sandbox AVB testing with dedicated mmc12 device
>>
>> Right now, sandbox runs the AVB tests using mmc1.img, which contains a
>> fedora image.
>> This does not really make sense. We should test on an mmc device which
>> has a proper vbmeta partition instead.
>>
>> Add a new mmc12 in sandbox which contains all required partitions for
>> proper AVB testing.
>> Use the mmc12 device in the avb tests only when running in sandbox.
>> The mmc12 DT node gets enabled by the avb_device_id fixture if CMD_BIND
>> is available.
>>
>> Note: the vbmeta.img has been generated using:
>>
>>     $ ./avbtool.py add_hash_footer \
>>                    --image boot.img \
>>                    --partition_name boot \
>>                    --partition_size 4194304
>>
>> [...]
>>
>>  arch/sandbox/dts/test.dts                   |  8 +++
>>  test/py/tests/test_android/android_image.py | 15 ++++++
>>  test/py/tests/test_android/test_abootimg.py | 77 +++++++++++++++++++++++++++++
>>  test/py/tests/test_android/test_avb.py      | 26 ++++++++++
>>  4 files changed, 126 insertions(+)
>
>> diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
>> @@ -22,7 +25,30 @@ temp_addr2 = 0x90002000
>> +        mmc_dev = 0xc
>> +        fname = os.path.join(ubman.config.source_dir, f'mmc{mmc_dev}.img')
>> +        if not os.path.exists(fname):
>> +            setup_android_avb_image(ubman)
>> +            ubman.restart_uboot()
>
> Just to check, is the restart needed? The mmc12 node is disabled, so
> nothing probes it until the 'bind' below. Since sandbox_mmc_probe() is
> where the file gets mapped, the new image should be picked up without
> restarting U-Boot.

I did not give this enough thought and copied this from somewhere else.
You are right, the restart is not needed. I will drop in in v3.

>
>> diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
>> @@ -22,7 +25,30 @@ temp_addr2 = 0x90002000
>> +        ubman.run_command(f'bind /mmc{mmc_dev} mmc_sandbox')
>> +        # Run mmc list to ensure that mmc_sandbox driver is probed
>> +        ubman.run_command('mmc list')
>
> The fixture has function scope, so this bind runs for each AVB test in
> the same U-Boot session. Since bind_by_node_path() doesn't check
> whether the node already has a device, each later test adds another
> mmc12 device with the same alias seq. The tests may still pass because
> the first device is found first, but it leaves junk in driver model.

Indeed:

=> dm tree mmc12
 Class     Seq    Probed  Driver                Name
-----------------------------------------------------------
 mmc          12  [   ]   mmc_sandbox           mmc12
 blk           3  [   ]   mmc_blk               |-- mmc12.blk
 bootdev      14  [   ]   mmc_bootdev           `-- mmc12.bootdev
=> bind /mmc12 mmc_sandbox
=> dm tree mmc12
 Class     Seq    Probed  Driver                Name
-----------------------------------------------------------
 mmc          12  [   ]   mmc_sandbox           mmc12
 blk           3  [   ]   mmc_blk               |-- mmc12.blk
 bootdev      14  [   ]   mmc_bootdev           |-- mmc12.bootdev
 mmc          12  [   ]   mmc_sandbox           `-- mmc12
 blk           4  [   ]   mmc_blk                   |-- mmc12.blk
 bootdev      15  [   ]   mmc_bootdev               `-- mmc12.bootdev
=>

> please can you either unbind after the test (yield, then 'unbind
> /mmc12') or bind only if the device isn't already there? it would also
> be worth checking the bind output, so that a failure shows up here
> rather than later as a confusing 'avb verify' error.

I will check the bind output in v3.
I'l also do an unbind using yield followed by 'unbind /mmc12'

>
> I'm also wondering whether we lose something by not doing the
> disk-image setup upfront? Can we still run C tests on it?

You mean doing the disk-image setup at the same time we create the other
disk images in test_ut.py (test_ut_dm_init_bootstd) ?

I have considered this. I chose to create the image from the tests
because it allows us to start from a clean state.
For example:

$ git clean -xdf
$ ./test/py/test.py --bd sandbox --build -k test_avb_verify -s -v

If we create the images from another place (like test_ut.py) it means we
have to run those first.

We could run C tests but mmc12.img has been made exclusively for the AVB
tests for now.

If we need to access them from C we can revisit as of where to create
the disk image.

>
> Regards,
> Simon

[PATCH v2 10/10] test/py: avb: Fix find() logic when matching response strings

[Mattijs Korpershoek]

The AVB test suite runs some avb commands (such as "avb verify") and
then parses the console output for success/failure.

It does so by using the string.find() function which returns:
* -1 when the subsystring does not exist
* 0/a positive number when the substring exists

For example, here is the output of:
$ ./test/py/test.py --bd sandbox --build -k test_avb_verify -s -v

Hit any key to stop autoboot: 0
=> =>  avb init 1
=> => avb verify
get_partition: no MMC device at slot 1
get_partition: no MMC device at slot 1
avb_slot_verify.c:644: ERROR: boot: Error loading footer.
Verification failed, reason: I/O error occurred while trying to load data
=> PASSED

Here, the test is marked as PASSED because assert(-1) will return
success.

Fix the logic to fail these tests when the expected string is not found.

Tested-by: Guillaume La Roque <glaroque@baylibre.com>
Tested-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
Signed-off-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
---
 test/py/tests/test_android/test_avb.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
index 48fa6ee7cf0b..b0ecb7deb1f3 100644
--- a/test/py/tests/test_android/test_avb.py
+++ b/test/py/tests/test_android/test_avb.py
@@ -64,7 +64,7 @@ def test_avb_verify(ubman, avb_device_id):
     response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
     response = ubman.run_command('avb verify')
-    assert response.find(success_str)
+    assert response.find(success_str) != -1
 
 
 @pytest.mark.buildconfigspec('cmd_avb')
@@ -79,7 +79,7 @@ def test_avb_mmc_uuid(ubman, avb_device_id):
     assert response == ''
 
     response = ubman.run_command('mmc rescan; mmc dev %d' % avb_device_id)
-    assert response.find('is current device')
+    assert response.find('is current device') != -1
 
     part_lines = ubman.run_command('mmc part').splitlines()
     part_list = {}
@@ -133,22 +133,22 @@ def test_avb_mmc_read(ubman, avb_device_id):
     """
 
     response = ubman.run_command('mmc rescan; mmc dev %d 0' % avb_device_id)
-    assert response.find('is current device')
+    assert response.find('is current device') != -1
 
     response = ubman.run_command('mmc read 0x%x 0x100 0x1' % temp_addr)
-    assert response.find('read: OK')
+    assert response.find('read: OK') != -1
 
     response = ubman.run_command('avb init %x' % avb_device_id)
     assert response == ''
 
     response = ubman.run_command('avb read_part xloader 0 100 0x%x' %
                                            temp_addr2)
-    assert response.find('Read 512 bytes')
+    assert response.find('Read 512 bytes') != -1
 
     # Now lets compare two buffers
     response = ubman.run_command('cmp 0x%x 0x%x 40' %
                                           (temp_addr, temp_addr2))
-    assert response.find('64 word')
+    assert response.find('64 word') != -1
 
 
 @pytest.mark.buildconfigspec('cmd_avb')

-- 
2.56.0

Re: [PATCH v2 10/10] test/py: avb: Fix find() logic when matching response strings

[Guillaume La Roque]

On Tue, 20 Oct 2026 11:25:59 +0200, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> The AVB test suite runs some avb commands (such as "avb verify") and
> then parses the console output for success/failure.
> 
> It does so by using the string.find() function which returns:
> * -1 when the subsystring does not exist
> * 0/a positive number when the substring exists
> 
> [...]

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>

-- 
Guillaume La Roque <glaroque@baylibre.com>

Re: [PATCH v2 10/10] test/py: avb: Fix find() logic when matching response strings

[Simon Glass]

Hi Mattijs,

On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> test/py: avb: Fix find() logic when matching response strings
>
> The AVB test suite runs some avb commands (such as "avb verify") and
> then parses the console output for success/failure.
>
> It does so by using the string.find() function which returns:
> * -1 when the subsystring does not exist

Typo: 'subsystring' should be 'substring'

> * 0/a positive number when the substring exists
>
> For example, here is the output of:
> $ ./test/py/test.py --bd sandbox --build -k test_avb_verify -s -v
>
> Hit any key to stop autoboot: 0
> => =>  avb init 1
> => => avb verify
> get_partition: no MMC device at slot 1
> get_partition: no MMC device at slot 1
> avb_slot_verify.c:644: ERROR: boot: Error loading footer.
> Verification failed, reason: I/O error occurred while trying to load data
> => PASSED
> [...]
>
>  test/py/tests/test_android/test_avb.py | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

> diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
> @@ -64,7 +64,7 @@ def test_avb_verify(ubman, avb_device_id):
> -    assert response.find(success_str)
> +    assert response.find(success_str) != -1

It would be worth using the 'in' operator here and elsewhere, e.g.
'assert success_str in response', since that is the usual Python
idiom. It also helps with debugging: pytest's assertion rewriting then
shows both the expected string and the console output on failure,
whereas '!= -1' just gives '-1 != -1'. What do you think?

Regards,
Simon

Re: [PATCH v2 10/10] test/py: avb: Fix find() logic when matching response strings

[Mattijs Korpershoek]

Hi Simon,

Thank you for the review.

On Thu, Oct 22, 2026 at 01:26, Simon Glass <sjg@chromium.org> wrote:

> Hi Mattijs,
>
> On 2026-10-20T09:25:49, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>> test/py: avb: Fix find() logic when matching response strings
>>
>> The AVB test suite runs some avb commands (such as "avb verify") and
>> then parses the console output for success/failure.
>>
>> It does so by using the string.find() function which returns:
>> * -1 when the subsystring does not exist
>
> Typo: 'subsystring' should be 'substring'

Will fix in v3.

>
>> * 0/a positive number when the substring exists
>>
>> For example, here is the output of:
>> $ ./test/py/test.py --bd sandbox --build -k test_avb_verify -s -v
>>
>> Hit any key to stop autoboot: 0
>> => =>  avb init 1
>> => => avb verify
>> get_partition: no MMC device at slot 1
>> get_partition: no MMC device at slot 1
>> avb_slot_verify.c:644: ERROR: boot: Error loading footer.
>> Verification failed, reason: I/O error occurred while trying to load data
>> => PASSED
>> [...]
>>
>>  test/py/tests/test_android/test_avb.py | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>
>> diff --git a/test/py/tests/test_android/test_avb.py b/test/py/tests/test_android/test_avb.py
>> @@ -64,7 +64,7 @@ def test_avb_verify(ubman, avb_device_id):
>> -    assert response.find(success_str)
>> +    assert response.find(success_str) != -1
>
> It would be worth using the 'in' operator here and elsewhere, e.g.
> 'assert success_str in response', since that is the usual Python
> idiom. It also helps with debugging: pytest's assertion rewriting then
> shows both the expected string and the console output on failure,
> whereas '!= -1' just gives '-1 != -1'. What do you think?

Indeed, that's a good suggestion, thank you. Will do in v3.

>
> Regards,
> Simon