On Mon, 6 Apr 2020 at 19:25, Peter Collingbourne <p...@google.com> wrote:
> On Mon, Apr 6, 2020 at 10:40 AM Ryan Harkin <ryan.har...@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 <p...@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 <p...@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.har...@linaro.org> > > 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 >>> >>>