[PATCH] fvp: Add support for loading Android boot images via semihosting

[Peter Collingbourne]

On Mon, Apr 6, 2020 at 10:40 AM Ryan Harkin <ryan.harkin at 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 at 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 at 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.

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
>>
>>