[PATCH 0/3] image: android: misc fixes when using on Qualcomm platforms
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
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,
When booting with platforms having > 4GiB of memory, the kernel physical address can be more than 32bits.
Use ulong like all the other addresses, and fix the print to show the > 32bits address numbers.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- boot/image-android.c | 4 ++-- include/android_image.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/image-android.c b/boot/image-android.c index e74dd498a30..bb5f4f84487 100644 --- a/boot/image-android.c +++ b/boot/image-android.c @@ -256,7 +256,7 @@ int android_image_get_kernel(const void *hdr, ulong *os_data, ulong *os_len) { struct andr_image_data img_data = {0}; - u32 kernel_addr; + ulong kernel_addr; const struct legacy_img_hdr *ihdr;
if (!android_image_get_data(hdr, vendor_boot_img, &img_data)) @@ -275,7 +275,7 @@ int android_image_get_kernel(const void *hdr, if (strlen(andr_tmp_str)) printf("Android's image name: %s\n", andr_tmp_str);
- printf("Kernel load addr 0x%08x size %u KiB\n", + printf("Kernel load addr 0x%08lx size %u KiB\n", kernel_addr, DIV_ROUND_UP(img_data.kernel_size, 1024));
int len = 0; diff --git a/include/android_image.h b/include/android_image.h index d503c980b23..96820709b42 100644 --- a/include/android_image.h +++ b/include/android_image.h @@ -348,7 +348,7 @@ struct andr_image_data { ulong bootconfig_addr; /* bootconfig image address */ ulong bootconfig_size; /* bootconfig image size */
- u32 kernel_addr; /* physical load addr */ + ulong kernel_addr; /* physical load addr */ ulong ramdisk_addr; /* physical load addr */ ulong ramdisk_ptr; /* ramdisk address */ ulong dtb_load_addr; /* physical load address for DTB image */
When trying to boot an android boot image with a compressed kernel, if the kernel is used in-place because it was created with mkbootimg, the space will be too small to properly uncompress.
Take in account the compressed state, and if compressed use the kernel_addr_r which should be big enough.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- boot/image-android.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/boot/image-android.c b/boot/image-android.c index bb5f4f84487..3adcc69a392 100644 --- a/boot/image-android.c +++ b/boot/image-android.c @@ -208,7 +208,8 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr, return true; }
-static ulong android_image_get_kernel_addr(struct andr_image_data *img_data) +static ulong android_image_get_kernel_addr(struct andr_image_data *img_data, + ulong comp) { /* * All the Android tools that generate a boot.img use this @@ -221,8 +222,11 @@ static ulong android_image_get_kernel_addr(struct andr_image_data *img_data) * * Otherwise, we will return the actual value set by the user. */ - if (img_data->kernel_addr == ANDROID_IMAGE_DEFAULT_KERNEL_ADDR) - return img_data->kernel_ptr; + if (img_data->kernel_addr == ANDROID_IMAGE_DEFAULT_KERNEL_ADDR) { + if (comp == IH_COMP_NONE) + return img_data->kernel_ptr; + return env_get_ulong("kernel_addr_r", 16, 0); + }
/* * abootimg creates images where all load addresses are 0 @@ -258,11 +262,14 @@ int android_image_get_kernel(const void *hdr, struct andr_image_data img_data = {0}; ulong kernel_addr; const struct legacy_img_hdr *ihdr; + ulong comp;
if (!android_image_get_data(hdr, vendor_boot_img, &img_data)) return -EINVAL;
- kernel_addr = android_image_get_kernel_addr(&img_data); + comp = android_image_get_kcomp(hdr, vendor_boot_img); + + kernel_addr = android_image_get_kernel_addr(&img_data, comp); ihdr = (const struct legacy_img_hdr *)img_data.kernel_ptr;
/* @@ -359,11 +366,14 @@ ulong android_image_get_kload(const void *hdr, const void *vendor_boot_img) { struct andr_image_data img_data; + ulong comp;
if (!android_image_get_data(hdr, vendor_boot_img, &img_data)) return -EINVAL;
- return android_image_get_kernel_addr(&img_data); + comp = android_image_get_kcomp(hdr, vendor_boot_img); + + return android_image_get_kernel_addr(&img_data, comp); }
ulong android_image_get_kcomp(const void *hdr,
The two tools that create android boot images, mkbootimg and the fastboot client, set the kernel address by default to 0x11008000.
U-boot always honors this field, and will try to copy the ramdisk to whatever value is set in the header, which won't be mapped to the actual RAM on most platforms, resulting in the kernel obviously not booting.
All the targets in U-Boot right now will download the android boot image to CONFIG_SYS_LOAD_ADDR, which means that it will already have been downloaded to some location that is suitable to use the ramdisk in-place for header version 0 to 2. For header version 3 and later, the ramdisk can't be used in-place to use ramdisk_addr_r in this case.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- boot/image-android.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/boot/image-android.c b/boot/image-android.c index 3adcc69a392..a261bb63999 100644 --- a/boot/image-android.c +++ b/boot/image-android.c @@ -14,6 +14,7 @@ #include <linux/libfdt.h>
#define ANDROID_IMAGE_DEFAULT_KERNEL_ADDR 0x10008000 +#define ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR 0x11000000
static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
@@ -405,9 +406,24 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
if (!img_data.ramdisk_size) return -ENOENT; - + /* + * Android tools can generate a boot.img with default load address + * or 0, even though it doesn't really make a lot of sense, and it + * might be valid on some platforms, we treat that address as + * the default value for this field, and try to pass ramdisk + * in place if possible. + */ if (img_data.header_version > 2) { - ramdisk_ptr = img_data.ramdisk_addr; + /* 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; @@ -420,15 +436,21 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img, img_data.bootconfig_size); } } else { - ramdisk_ptr = img_data.ramdisk_addr; - memcpy((void *)(ramdisk_ptr), (void *)img_data.ramdisk_ptr, - img_data.ramdisk_size); + /* Ramdisk can be used in-place, use current ptr */ + if (img_data.ramdisk_addr == 0 || + img_data.ramdisk_addr == ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR) { + ramdisk_ptr = img_data.ramdisk_ptr; + } else { + ramdisk_ptr = img_data.ramdisk_addr; + memcpy((void *)(ramdisk_ptr), (void *)img_data.ramdisk_ptr, + img_data.ramdisk_size); + } }
printf("RAM disk load addr 0x%08lx size %u KiB\n", - img_data.ramdisk_addr, DIV_ROUND_UP(img_data.ramdisk_size, 1024)); + ramdisk_ptr, DIV_ROUND_UP(img_data.ramdisk_size, 1024));
- *rd_data = img_data.ramdisk_addr; + *rd_data = ramdisk_ptr;
*rd_len = img_data.ramdisk_size; return 0;
Hi Neil,
Thank you for the patch.
On mer., oct. 16, 2024 at 17:46, Neil Armstrong neil.armstrong@linaro.org wrote:
The two tools that create android boot images, mkbootimg and the fastboot client, set the kernel address by default to 0x11008000.
U-boot always honors this field, and will try to copy the ramdisk to whatever value is set in the header, which won't be mapped to the actual RAM on most platforms, resulting in the kernel obviously not booting.
All the targets in U-Boot right now will download the android boot image to CONFIG_SYS_LOAD_ADDR, which means that it will already have been downloaded to some location that is suitable to use the ramdisk in-place for header version 0 to 2. For header version 3 and later, the ramdisk can't be used in-place to use ramdisk_addr_r in this case.
Is said in [1] I found some boot issues on Beagle Play with boot image v4.
If a new series need to be send, please also adapt the commit message a bit as it fails checkpatch:
$ ~/work/amlogic/u-boot/ neil/android-fixes ./scripts/checkpatch.pl --strict --u-boot --git HEAD^..HEAD WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #10: whatever value is set in the header, which won't be mapped to the actual RAM on
total: 0 errors, 1 warnings, 0 checks, 59 lines checked
NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace.
Commit 19a75a41096f ("image: android: handle ramdisk default address") has style problems, please review.
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO ENOSYS MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE
NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
[1] https://lore.kernel.org/r/all/87ed4f2ccc.fsf@baylibre.com/
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org
boot/image-android.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/boot/image-android.c b/boot/image-android.c index 3adcc69a392..a261bb63999 100644 --- a/boot/image-android.c +++ b/boot/image-android.c @@ -14,6 +14,7 @@ #include <linux/libfdt.h>
#define ANDROID_IMAGE_DEFAULT_KERNEL_ADDR 0x10008000 +#define ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR 0x11000000
static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
@@ -405,9 +406,24 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
if (!img_data.ramdisk_size) return -ENOENT;
- /*
* Android tools can generate a boot.img with default load address* or 0, even though it doesn't really make a lot of sense, and it* might be valid on some platforms, we treat that address as* the default value for this field, and try to pass ramdisk* in place if possible.*/
ramdisk_ptr = img_data.ramdisk_addr;
/* 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;}@@ -420,15 +436,21 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img, img_data.bootconfig_size); } } else {
ramdisk_ptr = img_data.ramdisk_addr;memcpy((void *)(ramdisk_ptr), (void *)img_data.ramdisk_ptr,img_data.ramdisk_size);
/* Ramdisk can be used in-place, use current ptr */if (img_data.ramdisk_addr == 0 ||img_data.ramdisk_addr == ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR) {ramdisk_ptr = img_data.ramdisk_ptr;} else {ramdisk_ptr = img_data.ramdisk_addr;memcpy((void *)(ramdisk_ptr), (void *)img_data.ramdisk_ptr,img_data.ramdisk_size);}}
printf("RAM disk load addr 0x%08lx size %u KiB\n",
img_data.ramdisk_addr, DIV_ROUND_UP(img_data.ramdisk_size, 1024));
ramdisk_ptr, DIV_ROUND_UP(img_data.ramdisk_size, 1024));
- *rd_data = img_data.ramdisk_addr;
*rd_data = ramdisk_ptr;
*rd_len = img_data.ramdisk_size; return 0;
-- 2.34.1
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.
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
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
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.
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
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.
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
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.
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
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?
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
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
participants (3)
-
Mattijs Korpershoek -
Neil Armstrong -
neil.armstrong＠linaro.org