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

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Neil,

Thank you for the patch.

On jeu., oct. 17, 2024 at 16:44, 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.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  boot/image-android.c | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index 3adcc69a392f74ae64f3fbcf1b85204f60ac9aff..cd01278f211d63262f2bdad7aa1176e2c1bbfedd 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,25 @@ 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;
> +		}
> +		*rd_data = ramdisk_ptr;
>  		memcpy((void *)(ramdisk_ptr), (void *)img_data.vendor_ramdisk_ptr,
>  		       img_data.vendor_ramdisk_size);
>  		ramdisk_ptr += img_data.vendor_ramdisk_size;
> @@ -420,15 +437,20 @@ 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) {
> +			*rd_data = img_data.ramdisk_ptr;
> +		} else {
> +			ramdisk_ptr = img_data.ramdisk_addr;
> +			*rd_data = ramdisk_ptr;
> +			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));
> -
> -	*rd_data = img_data.ramdisk_addr;
> +	       *rd_data, DIV_ROUND_UP(img_data.ramdisk_size, 1024));
>  
>  	*rd_len = img_data.ramdisk_size;
>  	return 0;
>
> -- 
> 2.34.1