| Message ID | 20230205235021.355410-19-souajih@baylibre.com |
|---|---|
| State | Accepted |
| Commit | 83d48a3c8bfbbbc2a8ad951b211081f72fc268b6 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Support android boot image v3/v4 | expand |
On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih 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> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > test/py/tests/test_android/test_abootimg.py | 136 ++++++++++++++++++-- > 1 file changed, 123 insertions(+), 13 deletions(-) Alright, so I don't know where the failure starts, exactly. And to make testing this easier, there's currently the trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can use to replicate my problem. The problem is that while this test passes in CI, with GCC, with Clang it fails, consistently: https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 and I'm not quite sure why. I hope that building sandbox with clang and also just trying these features out interactively will fail too, and so debugging this will be less of a problem.
On 07/02/2023 20:02, Tom Rini wrote: > On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih 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> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> --- >> test/py/tests/test_android/test_abootimg.py | 136 ++++++++++++++++++-- >> 1 file changed, 123 insertions(+), 13 deletions(-) > Alright, so I don't know where the failure starts, exactly. And to make > testing this easier, there's currently the > trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can > use to replicate my problem. The problem is that while this test passes > in CI, with GCC, with Clang it fails, consistently: > https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 > and I'm not quite sure why. I hope that building sandbox with clang and > also just trying these features out interactively will fail too, and so > debugging this will be less of a problem. > Hi Tom, I will try to reproduce this fail and fix it on a v4. Thank you for reporting, BRs, Safae
On 07/02/2023 20:02, Tom Rini wrote: > On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih 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> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> --- >> test/py/tests/test_android/test_abootimg.py | 136 ++++++++++++++++++-- >> 1 file changed, 123 insertions(+), 13 deletions(-) > Alright, so I don't know where the failure starts, exactly. And to make > testing this easier, there's currently the > trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can > use to replicate my problem. The problem is that while this test passes > in CI, with GCC, with Clang it fails, consistently: > https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 > and I'm not quite sure why. I hope that building sandbox with clang and > also just trying these features out interactively will fail too, and so > debugging this will be less of a problem. > Hello Tom, I have investigated this issue, clang has a strange behavior in: * abootimg_get_dtb_load_addr() : cmd/abootimg.c * android_image_get_dtb_by_index() : boot/image-android.c That is probably linked to some sort of optimization clang does. However, The fail is not reproducible using clang-15 and clang-16 and also not reproducible when turning off clang optimizations. I suggest using clang-15 to run the test or I can remove all optimizations on the related functions if clang-14 is used. Best regards, Safae
On Mon, Feb 27, 2023 at 03:15:31PM +0100, Safae Ouajih wrote: > > On 07/02/2023 20:02, Tom Rini wrote: > > On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih 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> > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > --- > > > test/py/tests/test_android/test_abootimg.py | 136 ++++++++++++++++++-- > > > 1 file changed, 123 insertions(+), 13 deletions(-) > > Alright, so I don't know where the failure starts, exactly. And to make > > testing this easier, there's currently the > > trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can > > use to replicate my problem. The problem is that while this test passes > > in CI, with GCC, with Clang it fails, consistently: > > https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 > > and I'm not quite sure why. I hope that building sandbox with clang and > > also just trying these features out interactively will fail too, and so > > debugging this will be less of a problem. > > > Hello Tom, > > I have investigated this issue, clang has a strange behavior in: > > * abootimg_get_dtb_load_addr() : cmd/abootimg.c > * android_image_get_dtb_by_index() : boot/image-android.c > > That is probably linked to some sort of optimization clang does. > > However, The fail is not reproducible using clang-15 and clang-16 and also > not reproducible when turning off clang optimizations. > > I suggest using clang-15 to run the test or I can remove all optimizations > > on the related functions if clang-14 is used. Thanks for investigating. I see that 15 is now considered stable, so I'll update the next branch for that, then re-take this series.
On 27/02/2023 15:18, Tom Rini wrote: > On Mon, Feb 27, 2023 at 03:15:31PM +0100, Safae Ouajih wrote: >> On 07/02/2023 20:02, Tom Rini wrote: >>> On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih 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> >>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>> --- >>>> test/py/tests/test_android/test_abootimg.py | 136 ++++++++++++++++++-- >>>> 1 file changed, 123 insertions(+), 13 deletions(-) >>> Alright, so I don't know where the failure starts, exactly. And to make >>> testing this easier, there's currently the >>> trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can >>> use to replicate my problem. The problem is that while this test passes >>> in CI, with GCC, with Clang it fails, consistently: >>> https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 >>> and I'm not quite sure why. I hope that building sandbox with clang and >>> also just trying these features out interactively will fail too, and so >>> debugging this will be less of a problem. >>> >> Hello Tom, >> >> I have investigated this issue, clang has a strange behavior in: >> >> * abootimg_get_dtb_load_addr() : cmd/abootimg.c >> * android_image_get_dtb_by_index() : boot/image-android.c >> >> That is probably linked to some sort of optimization clang does. >> >> However, The fail is not reproducible using clang-15 and clang-16 and also >> not reproducible when turning off clang optimizations. >> >> I suggest using clang-15 to run the test or I can remove all optimizations >> >> on the related functions if clang-14 is used. > Thanks for investigating. I see that 15 is now considered stable, so > I'll update the next branch for that, then re-take this series. > Hello Tom, Thank you. I am a bit confused, do you mean that you will apply this series after clang-15 update? Best regards, Safae
On Mon, Mar 06, 2023 at 08:49:02PM +0100, Safae Ouajih wrote: > > On 27/02/2023 15:18, Tom Rini wrote: > > On Mon, Feb 27, 2023 at 03:15:31PM +0100, Safae Ouajih wrote: > > > On 07/02/2023 20:02, Tom Rini wrote: > > > > On Mon, Feb 06, 2023 at 12:50:20AM +0100, Safae Ouajih 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> > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > --- > > > > > test/py/tests/test_android/test_abootimg.py | 136 ++++++++++++++++++-- > > > > > 1 file changed, 123 insertions(+), 13 deletions(-) > > > > Alright, so I don't know where the failure starts, exactly. And to make > > > > testing this easier, there's currently the > > > > trini/u-boot-gitlab-ci-runner:jammy-20230126-07Feb2023 container you can > > > > use to replicate my problem. The problem is that while this test passes > > > > in CI, with GCC, with Clang it fails, consistently: > > > > https://source.denx.de/u-boot/u-boot/-/jobs/572239#L284 > > > > and I'm not quite sure why. I hope that building sandbox with clang and > > > > also just trying these features out interactively will fail too, and so > > > > debugging this will be less of a problem. > > > > > > > Hello Tom, > > > > > > I have investigated this issue, clang has a strange behavior in: > > > > > > * abootimg_get_dtb_load_addr() : cmd/abootimg.c > > > * android_image_get_dtb_by_index() : boot/image-android.c > > > > > > That is probably linked to some sort of optimization clang does. > > > > > > However, The fail is not reproducible using clang-15 and clang-16 and also > > > not reproducible when turning off clang optimizations. > > > > > > I suggest using clang-15 to run the test or I can remove all optimizations > > > > > > on the related functions if clang-14 is used. > > Thanks for investigating. I see that 15 is now considered stable, so > > I'll update the next branch for that, then re-take this series. > > > > Hello Tom, > > Thank you. > > I am a bit confused, do you mean that you will apply this series after > clang-15 update? Yes, and the series to move CI to clang-15 is currently stuck on some pytest (real) failures that need to be resolved. But I would expect this to all be resolved in time for this series here to be included in v2023.07.
diff --git a/test/py/tests/test_android/test_abootimg.py b/test/py/tests/test_android/test_abootimg.py index 43a7099c46..6a8ff34538 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 \ + """ # 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""" + # Expected response for "abootimg dtb_dump" command dtb_dump_resp="""## DTB area contents (concat format): - DTB #0: @@ -56,15 +91,21 @@ dtb_dump_resp="""## DTB area contents (concat format): (DTB)compatible = y2,z2""" # Address in RAM where to load the boot image ('abootimg' looks in $loadaddr) loadaddr = 0x1000 +# Address in RAM where to load the vendor boot image ('abootimg' looks in $vloadaddr) +vloadaddr= 0x10000 # Expected DTB #1 offset from the boot image start address dtb1_offset = 0x187d +# Expected DTB offset from the vendor boot image start address +dtb2_offset = 0x207d # DTB #1 start address in RAM dtb1_addr = loadaddr + dtb1_offset +# DTB #2 start address in RAM +dtb2_addr = vloadaddr + dtb2_offset class AbootimgTestDiskImage(object): """Disk image used by abootimg tests.""" - def __init__(self, u_boot_console): + def __init__(self, u_boot_console, image_name, hex_img): """Initialize a new AbootimgDiskImage object. Args: @@ -74,13 +115,13 @@ class AbootimgTestDiskImage(object): Nothing. """ - gz_hex = u_boot_console.config.persistent_data_dir + '/boot.img.gz.hex' - gz = u_boot_console.config.persistent_data_dir + '/boot.img.gz' + gz_hex = u_boot_console.config.persistent_data_dir + '/' + image_name + '.gz.hex' + gz = u_boot_console.config.persistent_data_dir + '/' + image_name + '.gz' - filename = 'boot.img' + filename = image_name persistent = u_boot_console.config.persistent_data_dir + '/' + filename self.path = u_boot_console.config.result_dir + '/' + filename - + u_boot_console.log.action('persistent is ' + persistent) with u_boot_utils.persistent_file_helper(u_boot_console.log, persistent): if os.path.exists(persistent): u_boot_console.log.action('Disk image file ' + persistent + @@ -89,19 +130,17 @@ class AbootimgTestDiskImage(object): u_boot_console.log.action('Generating ' + persistent) f = open(gz_hex, "w") - f.write(img_hex) + f.write(hex_img) f.close() - cmd = ('xxd', '-r', '-p', gz_hex, gz) u_boot_utils.run_and_log(u_boot_console, cmd) - cmd = ('gunzip', '-9', gz) u_boot_utils.run_and_log(u_boot_console, cmd) cmd = ('cp', persistent, self.path) u_boot_utils.run_and_log(u_boot_console, cmd) -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. + 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,48 @@ 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.""" + + cons = u_boot_console + cons.log.action('Loading disk image to RAM...') + cons.run_command('setenv loadaddr 0x%x' % (loadaddr)) + cons.run_command('setenv vloadaddr 0x%x' % (vloadaddr)) + cons.run_command('host load hostfs - 0x%x %s' % (vloadaddr, + abootimgv4_disk_image_vboot.path)) + cons.run_command('host load hostfs - 0x%x %s' % (loadaddr, + abootimgv4_disk_image_boot.path)) + cons.run_command('abootimg addr 0x%x 0x%x' % (loadaddr, vloadaddr)) + cons.log.action('Testing \'abootimg get ver\'...') + response = cons.run_command('abootimg get ver') + assert response == "4" + cons.run_command('abootimg get ver v') + response = cons.run_command('env print v') + assert response == 'v=4' + + cons.log.action('Testing \'abootimg get recovery_dtbo\'...') + response = cons.run_command('abootimg get recovery_dtbo a') + assert response == 'Error: header version must be >= 1 and <= 2 to get dtbo' + + cons.log.action('Testing \'abootimg get dtb_load_addr\'...') + cons.run_command('abootimg get dtb_load_addr a') + response = cons.run_command('env print a') + assert response == 'a=11f00000' + + cons.log.action('Testing \'abootimg get dtb --index\'...') + cons.run_command('abootimg get dtb --index=1 dtb2_start') + response = cons.run_command('env print dtb2_start') + correct_str = "dtb2_start=%x" % (dtb2_addr) + assert response == correct_str + + cons.run_command('fdt addr $dtb2_start') + cons.run_command('fdt get value v / model') + response = cons.run_command('env print v') + assert response == 'v=x2'