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

[Safae Ouajih]

On 27/01/2023 01:55, Simon Glass wrote:
> Hi Safae,
>
> On Thu, 26 Jan 2023 at 09:05, Safae Ouajih <souajih at 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 at baylibre.com>
>> ---
>>   test/py/tests/test_android/test_abootimg.py | 135 ++++++++++++++++++--
>>   1 file changed, 122 insertions(+), 13 deletions(-)
>>
> Reviewed-by: Simon Glass <sjg at 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

Hi Simon,

I think that it is possible to run these commands in the test. However,

this requires adding "mkbootimg" android tool with the last updates

to generate the new vendor boot image and boot image with header version 4.

I think this change may come as an update for the whole test_abootimg later.

>> +
>>   """
>>
>>   # 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
>> + at 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
>> + at 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'
>> +
>> + at pytest.mark.boardspec('sandbox')
>> + at pytest.mark.buildconfigspec('android_boot_image')
>> + at pytest.mark.buildconfigspec('cmd_abootimg')
>> + at pytest.mark.buildconfigspec('cmd_fdt')
>> + at pytest.mark.requiredtool('xxd')
>> + at 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?


I will try to add this change and all the remarks/fixes

you have pointed out earlier in a v3.


Thank you for the review,

--safae

>> +    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