On Mon, Apr 6, 2020 at 11:30 AM Ryan Harkin ryan.harkin@linaro.org wrote:

On Mon, 6 Apr 2020 at 19:25, Peter Collingbourne pcc@google.com wrote:

...

On Mon, Apr 6, 2020 at 10:40 AM Ryan Harkin ryan.harkin@linaro.org wrote:

...

Hi Peter,

This looks good to me, but I have a quick question below.

On Sat, 4 Apr 2020 at 03:58, Peter Collingbourne pcc@google.com wrote:

...

FVP now loads an Android boot image named boot.img if available, otherwise it falls back to the existing code path.

Signed-off-by: Peter Collingbourne pcc@google.com

configs/vexpress_aemv8a_semi_defconfig | 2 ++ include/configs/vexpress_aemv8a.h | 30 +++++++++++++++++--------- 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/configs/vexpress_aemv8a_semi_defconfig b/configs/vexpress_aemv8a_semi_defconfig index f31baab197..b52c761dee 100644 --- a/configs/vexpress_aemv8a_semi_defconfig +++ b/configs/vexpress_aemv8a_semi_defconfig @@ -14,6 +14,8 @@ CONFIG_BOOTARGS="console=ttyAMA0 earlycon=pl011,0x1c090000 debug user_debug=31 l # CONFIG_DISPLAY_CPUINFO is not set # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_SYS_PROMPT="VExpress64# " +CONFIG_ANDROID_BOOT_IMAGE=y +CONFIG_CMD_ABOOTIMG=y # CONFIG_CMD_CONSOLE is not set # CONFIG_CMD_XIMG is not set # CONFIG_CMD_EDITENV is not set diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h index 9a9cec414c..4f3a792f49 100644 --- a/include/configs/vexpress_aemv8a.h +++ b/include/configs/vexpress_aemv8a.h @@ -177,16 +177,26 @@ "initrd_addr=0x88000000\0" \ "fdtfile=devtree.dtb\0" \ "fdt_addr=0x83000000\0" \

•                           "fdt_high=0xffffffffffffffff\0" \
  

•                           "initrd_high=0xffffffffffffffff\0"
  
  

Why did you move these two inside the 'if' statement below? Is it because you explicitly don't want them set when booting Android?

Yes. We can't have these set when loading an Android boot image because they instruct the bootloader to use the device tree/initrd in place instead of copying them to another location, and since we're already using the kernel in place this may result in the kernel overwriting the device tree or initrd when it initializes its own BSS since they appear right after the kernel in the boot image format.

Ok, thanks for the clarification. That's fine by me.

Reviewed-by: Ryan Harkin ryan.harkin@linaro.org

Thanks for the review! Do you know what is the next step for getting this patch landed in master? I read https://www.denx.de/wiki/U-Boot/Patches but unfortunately I did not get a clear idea of what the next step is.

Peter

...

Peter

...

...

-#define CONFIG_BOOTCOMMAND "smhload ${kernel_name} ${kernel_addr}; " \

•                           "smhload ${fdtfile} ${fdt_addr}; " \
  

•                           "smhload ${initrd_name} ${initrd_addr}
  

"\

•                           "initrd_end; " \
  

•                           "fdt addr ${fdt_addr}; fdt resize; " \
  

•                           "fdt chosen ${initrd_addr}
  

${initrd_end}; " \

•                           "booti $kernel_addr - $fdt_addr"
  

•                           "boot_name=boot.img\0"          \
  

•                           "boot_addr=0x8007f800\0"
  

+#define CONFIG_BOOTCOMMAND "if smhload ${boot_name} ${boot_addr}; then " \

•                           "  set bootargs; " \
  

•                           "  abootimg addr ${boot_addr}; " \
  

•                           "  abootimg get dtb --index=0 fdt_addr;
  

" \

•                           "  bootm ${boot_addr} ${boot_addr} " \
  

•                           "  ${fdt_addr}; " \
  

•                           "else; " \
  

•                           "  set fdt_high 0xffffffffffffffff; " \
  

•                           "  set initrd_high 0xffffffffffffffff;
  

" \

•                           "  smhload ${kernel_name}
  

${kernel_addr}; " \

•                           "  smhload ${fdtfile} ${fdt_addr}; " \
  

•                           "  smhload ${initrd_name}
  

${initrd_addr} "\

•                           "  initrd_end; " \
  

•                           "  fdt addr ${fdt_addr}; fdt resize; " \
  

•                           "  fdt chosen ${initrd_addr}
  

${initrd_end}; " \

•                           "  booti $kernel_addr - $fdt_addr; " \
  

•                           "fi"
  
  
  

#endif

2.26.0.292.g33ef6b2f38-goog