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

[Peter Collingbourne]

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

>
> On Mon, 6 Apr 2020 at 19:25, Peter Collingbourne <pcc at google.com> wrote:
>
>> 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.
>>
>
> Ok, thanks for the clarification. That's fine by me.
>
> Reviewed-by: Ryan Harkin <ryan.harkin at 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
>>>>
>>>>