[PATCH 0/3] image: android: misc fixes when using on Qualcomm platforms
Mattijs Korpershoek
mkorpershoek at baylibre.com
Thu Oct 17 14:14:05 CEST 2024
On jeu., oct. 17, 2024 at 14:07, Mattijs Korpershoek <mkorpershoek at baylibre.com> wrote:
> Hi Neil,
>
> On jeu., oct. 17, 2024 at 14:01, Neil Armstrong <neil.armstrong at 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 at baylibre.com> wrote:
>>>
>>>> Hi Neil,
>>>>
>>>> Thank you for the series.
>>>>
>>>> On mer., oct. 16, 2024 at 17:46, Neil Armstrong <neil.armstrong at 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 at 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?
>
>>
>> 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 at linaro.org>
More information about the U-Boot
mailing list