Re: [PATCH v2 17/17] test/py: android: extend abootimg test

Simon Glass
2023-01-27T00:59:47Z

Hi Safae,

On Thu, 26 Jan 2023 at 09:05, Safae Ouajih <souajih@baylibre.com> wrote:
>
> test_abootimg is extended to include the testing of boot images
> version 4. For this, boot.img and vendor_boot.img have been
> generated using mkbootimg tool with setting the header
> version to 4.
>
> This tests:
> - Getting the header version using abootimg
> - Extracting the load address of the dtb
> - Extracting the dtb start address in RAM
>
> Running test:
> $ ./test/py/test.py --bd sandbox --build -k test_abootimg
>
> Signed-off-by: Safae Ouajih <souajih@baylibre.com>
> ---
> test/py/tests/test_android/test_abootimg.py | 135 ++++++++++++++++++--
> 1 file changed, 122 insertions(+), 13 deletions(-)
>

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

Question below

> diff --git a/test/py/tests/test_android/test_abootimg.py b/test/py/tests/test_android/test_abootimg.py
> index 43a7099c46..a5c734b9c5 100644
> --- a/test/py/tests/test_android/test_abootimg.py
> +++ b/test/py/tests/test_android/test_abootimg.py
> @@ -32,6 +32,23 @@ Now one can obtain original boot.img from this hex dump like this:
>
> $ xxd -r -p boot.img.gz.hex boot.img.gz
> $ gunzip -9 boot.img.gz
> +
> +For boot image header version 4, these tests rely on two images that are generated
> +using the same steps above :
> +
> +1- boot.img :
> + $ mkbootimg --kernel ./kernel --ramdisk ./ramdisk.img \
> + --cmdline "cmdline test" --dtb ./dtb.img \
> + --os_version R --os_patch_level 2019-06-05 \
> + --header_version 4 --output ./boot.img
> +
> +2- vendor_boot.img
> + $ mkbootimg --kernel ./kernel --ramdisk ./ramdisk.img \
> + --cmdline "cmdline test" --dtb ./dtb.img \
> + --os_version R --os_patch_level 2019-06-05 \
> + --pagesize 4096 --vendor_ramdisk ./ramdisk.img \
> + --header_version 4 --vendor_boot ./vboot.img \

Is it possible for us to run these commands in the test, to avoid
hassle when we want to change the disk image? See test_ut.py

> +
> """
>
> # boot.img.gz hex dump
> @@ -44,6 +61,24 @@ b7762ffff07d345446c1281805e8a0868d81e117a45e111c0d8dc101b253
> 9c03c41a0c90f17fe85400986d82452b6c3680198a192a0ce17c3610ae34
> d4a9820881a70f3873f35352731892f3730b124b32937252a96bb9119ae5
> 463a5546f82c1f05a360148c8251300a462e000085bf67f200200000"""
> +
> +# boot img v4 hex dump
> +boot_img_hex = """1f8b080827b0cd630203626f6f742e696d6700edd8bd0d82601885d1d7c4
> +58d8c808b88195bd098d8d246e40e42b083f1aa0717be99d003d277916b8
> +e5bddc8a7b792d8e8788c896ce9b88d32ebe6c971e7ddd3543cae734cd01
> +c0ffc84c0000b0766d1a87d4e5afeadd3dab7a6f10000000f84163d5d7cd
> +d43a000000000000000060c53e7544995700400000"""
> +
> +# vendor boot image v4 hex dump
> +vboot_img_hex = """1f8b0808baaecd63020376626f6f742e696d6700edd8310b824018c6f1b3
> +222a08f41b3436b4280dcdd19c11d16ee9109d18d59042d047ec8b04cd0d
> +d19d5a4345534bf6ffc173ef29272f38e93b1d0ec67dd79d548462aa1cd2
> +d5d20b0000f8438678f90c18d584b8a4bbb3a557991ecb2a0000f80d6b2f
> +f4179b656be5c532f2fc066f040000000080e23936af2755f62a3d918df1
> +db2a7ab67f9ffdeb7df7cda3465ecb79c4ce7e5c577562bb9364b74449a5
> +1e467e20c53c0a57de763193c1779b3b4fcd9d4ee27c6a0e00000000c0ff
> +309ffea7010000000040f1dc004129855400400000"""
> +
[..]

> -gtdi = None
> +gtdi1 = None
> @pytest.fixture(scope='function')
> def abootimg_disk_image(u_boot_console):
> """pytest fixture to provide a AbootimgTestDiskImage object to tests.
> @@ -109,10 +148,36 @@ def abootimg_disk_image(u_boot_console):
> function-scoped. However, we don't need to actually do any function-scope
> work, so this simply returns the same object over and over each time."""
>
> - global gtdi
> - if not gtdi:
> - gtdi = AbootimgTestDiskImage(u_boot_console)
> - return gtdi
> + global gtdi1
> + if not gtdi1:
> + gtdi1 = AbootimgTestDiskImage(u_boot_console, 'boot.img', img_hex)
> + return gtdi1
> +
> +gtdi2 = None
> +@pytest.fixture(scope='function')
> +def abootimgv4_disk_image_vboot(u_boot_console):
> + """pytest fixture to provide a AbootimgTestDiskImage object to tests.
> + This is function-scoped because it uses u_boot_console, which is also
> + function-scoped. However, we don't need to actually do any function-scope
> + work, so this simply returns the same object over and over each time."""
> +
> + global gtdi2
> + if not gtdi2:
> + gtdi2 = AbootimgTestDiskImage(u_boot_console, 'vendor_boot.img', vboot_img_hex)
> + return gtdi2
> +
> +gtdi3 = None
> +@pytest.fixture(scope='function')
> +def abootimgv4_disk_image_boot(u_boot_console):
> + """pytest fixture to provide a AbootimgTestDiskImage object to tests.

blank line here (please fix throughout)

> + This is function-scoped because it uses u_boot_console, which is also
> + function-scoped. However, we don't need to actually do any function-scope
> + work, so this simply returns the same object over and over each time."""
> +
> + global gtdi3
> + if not gtdi3:
> + gtdi3 = AbootimgTestDiskImage(u_boot_console, 'bootv4.img', boot_img_hex)
> + return gtdi3
>
> @pytest.mark.boardspec('sandbox')
> @pytest.mark.buildconfigspec('android_boot_image')
> @@ -157,3 +222,47 @@ def test_abootimg(abootimg_disk_image, u_boot_console):
> u_boot_console.run_command('fdt get value v / model')
> response = u_boot_console.run_command('env print v')
> assert response == 'v=x2'
> +
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('android_boot_image')
> +@pytest.mark.buildconfigspec('cmd_abootimg')
> +@pytest.mark.buildconfigspec('cmd_fdt')
> +@pytest.mark.requiredtool('xxd')
> +@pytest.mark.requiredtool('gunzip')
> +def test_abootimgv4(abootimgv4_disk_image_vboot, abootimgv4_disk_image_boot, u_boot_console):
> + """Test the 'abootimg' command with boot image header v4."""
> +
> + u_boot_console.log.action('Loading disk image to RAM...')

Can you put u_boot_console in a var like 'cons' to make it less verbose?

> + u_boot_console.run_command('setenv loadaddr 0x%x' % (loadaddr))
> + u_boot_console.run_command('setenv vloadaddr 0x%x' % (vloadaddr))
> + u_boot_console.run_command('host load hostfs - 0x%x %s' % (vloadaddr,
> + abootimgv4_disk_image_vboot.path))
> + u_boot_console.run_command('host load hostfs - 0x%x %s' % (loadaddr,
> + abootimgv4_disk_image_boot.path))
> + u_boot_console.run_command('abootimg addr 0x%x 0x%x' % (loadaddr, vloadaddr))
> + u_boot_console.log.action('Testing \'abootimg get ver\'...')
> + response = u_boot_console.run_command('abootimg get ver')
> + assert response == "4"
> + u_boot_console.run_command('abootimg get ver v')
> + response = u_boot_console.run_command('env print v')
> + assert response == 'v=4'
> +
> + u_boot_console.log.action('Testing \'abootimg get recovery_dtbo\'...')
> + response = u_boot_console.run_command('abootimg get recovery_dtbo a')
> + assert response == 'Error: header version must be >= 1 and <= 2 to get dtbo'
> +
> + u_boot_console.log.action('Testing \'abootimg get dtb_load_addr\'...')
> + u_boot_console.run_command('abootimg get dtb_load_addr a')
> + response = u_boot_console.run_command('env print a')
> + assert response == 'a=11f00000'
> +
> + u_boot_console.log.action('Testing \'abootimg get dtb --index\'...')
> + u_boot_console.run_command('abootimg get dtb --index=1 dtb2_start')
> + response = u_boot_console.run_command('env print dtb2_start')
> + correct_str = "dtb2_start=%x" % (dtb2_addr)
> + assert response == correct_str
> +
> + u_boot_console.run_command('fdt addr $dtb2_start')
> + u_boot_console.run_command('fdt get value v / model')
> + response = u_boot_console.run_command('env print v')
> + assert response == 'v=x2'
> --
> 2.34.1
>

Regards,
Simon