Re: [PATCH 3/3] image: android: handle ramdisk default address

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

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