Hi Neil, On jeu., oct. 17, 2024 at 14:01, Neil Armstrong <neil.armstr...@linaro.org> wrote:
> On 17/10/2024 13:58, Mattijs Korpershoek wrote: >> Hi Neil, >> >> On jeu., oct. 17, 2024 at 13:33, Mattijs Korpershoek >> <mkorpersh...@baylibre.com> wrote: >> >>> Hi Neil, >>> >>> Thank you for the series. >>> >>> On mer., oct. 16, 2024 at 17:46, Neil Armstrong <neil.armstr...@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.armstr...@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. > > 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.armstr...@linaro.org>