Re: [PATCH 0/3] image: android: misc fixes when using on Qualcomm platforms

17 Oct 2024

On 17/10/2024 14:14, Mattijs Korpershoek wrote:
...
On jeu., oct. 17, 2024 at 14:07, Mattijs Korpershoek mkorpershoek@baylibre.com wrote:
...
Hi Neil,
On jeu., oct. 17, 2024 at 14:01, Neil Armstrong neil.armstrong@linaro.org wrote:
...
On 17/10/2024 13:58, Mattijs Korpershoek wrote:
...
Hi Neil,
On jeu., oct. 17, 2024 at 13:33, Mattijs Korpershoek mkorpershoek@baylibre.com wrote:
...
Hi Neil,
Thank you for the series.
On mer., oct. 16, 2024 at 17:46, Neil Armstrong neil.armstrong@linaro.org wrote:
...
When trying to use the Android boot image with header version 2
on recent Qualcomm platforms, we get into some troubles.
First the kernel in-place address can be > 32bit, then since
we use the Android mkbootimg, it uses the default load address
which isn't big enough to uncompress the kernel.
Finally, the ramdisk also uses a default load address, and
it should be taken in account like for the kernel address.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org
Neil Armstrong (3):
        image: android: use ulong for kernel address
        boot: image-android: do not boot XIP when kernel is compressed
        image: android: handle ramdisk default address
I have boot tested aosp/main on Khadas VIM3 using
khadas_vim3_android_defconfig
This ensures that boot image v2 still works.
I also tried to boot test the Beagle Play board (which runs Android 14
with boot image v4).
Unfortunetly, that does not boot. The kernel starts but then I see:
[    0.434360][    T1] /dev/root: Can't open blockdev
[    0.439587][    T1] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
Full boot logs:
https://paste.debian.net/1332547/
Full boot logs on master:
https://paste.debian.net/1332548/
It seems that somehow, the bootconfig section is no longer present.
I'll try to identify the offending patch and help debug this.
Offending patch is
    [PATCH 3/3] image: android: handle ramdisk default address
Thanks for looking
...
The following (invalid) diff "fixes it"
modified   boot/image-android.c
@@ -448,9 +448,9 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
   	}
printf("RAM disk load addr 0x%08lx size %u KiB\n",


      ramdisk_ptr, DIV_ROUND_UP(img_data.ramdisk_size, 1024));




      img_data.ramdisk_addr, DIV_ROUND_UP(img_data.ramdisk_size, 1024));





*rd_data = ramdisk_ptr;


*rd_data = img_data.ramdisk_addr;
*rd_len = img_data.ramdisk_size;
return 0;


I'll debug a bit more.
OK so this basically reverts the patch, so it means on Beagle Play
the 0x11000000 is valid and can't use the randisk in-place.
img_data.ramdisk_ptr is the "real" address the data has been loaded to,
and img_data.ramdisk_addr is the address passed to mkbootimg, where it
should be loaded.
Beagle Play uses boot image v4, therefore, we go through the following
code path:
if (img_data.header_version > 2) {
   	/* Ramdisk can't be used in-place, copy it to ramdisk_addr_r */
   	if (img_data.ramdisk_addr == ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR) {
   		ramdisk_ptr = env_get_ulong("ramdisk_addr_r", 16, 0);
   		if (!ramdisk_ptr) {
   			printf("Invalid ramdisk_addr_r to copy ramdisk into\n");
   			return -EINVAL;
   		}
   	} else {
   		ramdisk_ptr = img_data.ramdisk_addr;
   	}
   	memcpy((void *)(ramdisk_ptr), (void *)img_data.vendor_ramdisk_ptr,
   	       img_data.vendor_ramdisk_size);
   	ramdisk_ptr += img_data.vendor_ramdisk_size;
   	memcpy((void *)(ramdisk_ptr), (void *)img_data.ramdisk_ptr,
   	       img_data.boot_ramdisk_size);
   	ramdisk_ptr += img_data.boot_ramdisk_size;
   	if (img_data.bootconfig_size) {
   		memcpy((void *)
   		       (ramdisk_ptr), (void *)img_data.bootconfig_addr,
   		       img_data.bootconfig_size);
   	}
We can see here, that we **increment** ramdisk_ptr.
Therefore, the following line is invalid:
 *rd_data = ramdisk_ptr;


Because ramdisk_ptr is not at the beginning of the ramdisk, but at the
beginning of bootconfig.
I think saving ramdisk_ptr in the above block should fix the issues I see.
The following diff fixes the issue I see on Beagle Play with boot image
v4:

diff --git a/boot/image-android.c b/boot/image-android.c
index a261bb639990..e9d898e003f6 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -424,6 +424,7 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
                 } else {
                         ramdisk_ptr = img_data.ramdisk_addr;
                 }

          ulong ramdisk_begin_ptr = ramdisk_ptr;
           memcpy((void *)(ramdisk_ptr), (void *)img_data.vendor_ramdisk_ptr,
                  img_data.vendor_ramdisk_size);
           ramdisk_ptr += img_data.vendor_ramdisk_size;



@@ -435,6 +436,11 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
                                (ramdisk_ptr), (void *)img_data.bootconfig_addr,
                                img_data.bootconfig_size);
                 }

          /*


           * Since we moved ramdisk_ptr, restore it back to the beginning


           * of the ramdisk


           */


          ramdisk_ptr = ramdisk_begin_ptr;
   } else {
           /* Ramdisk can be used in-place, use current ptr */
           if (img_data.ramdisk_addr == 0 ||




(it's not super clean, but the general idea should work)
Can you add something similar for v2?
Neat, I'll try to make it cleaner but I get the idea :-)
Thanks!
Neil
...
...
...
Neil
...
...
...
boot/image-android.c    | 60 +++++++++++++++++++++++++++++++++++++------------
   include/android_image.h |  2 +-
   2 files changed, 47 insertions(+), 15 deletions(-)

base-commit: d5cab0d6adc26ec1bbd45c2fed101184d04454ae
change-id: 20241016-topic-fastboot-fixes-mkbootimg-8d73ab93db3d
Best regards,
Neil Armstrong neil.armstrong@linaro.org

    

Re: [PATCH 0/3] image: android: misc fixes when using on Qualcomm platforms

neil.armstrong＠linaro.org

Signed-off-by: Neil Armstrong neil.armstrong@linaro.org

Best regards,