| Message ID | 20241016-topic-fastboot-fixes-mkbootimg-v1-3-94fd9340722b@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | image: android: misc fixes when using on Qualcomm platforms | expand |
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. > + */ > 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; > > -- > 2.34.1
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;
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(-)