[U-Boot] [PATCH V4 2/2] bootm: Handle kernel_noload on arm64

[Marek Vasut]

On 06/13/2018 09:51 AM, Bin Chen wrote:
> 
> 
> On 13 June 2018 at 14:13, Marek Vasut <marek.vasut at gmail.com
> <mailto:marek.vasut at gmail.com>> wrote:
> 
>     The ARM64 has 2 MiB alignment requirement for the kernel. When using
>     fitImage, this requirement may by violated, the kernel will thus be
>     executed from unaligned address and fail to boot. Do what booti does
>     and run booti_setup() for kernel_noload images on arm64 to obtain a
>     suitable aligned address to which the image shall be relocated.
> 
>     Signed-off-by: Marek Vasut <marek.vasut+renesas at gmail.com
>     <mailto:marek.vasut%2Brenesas at gmail.com>>
>     Cc: Bin Chen <bin.chen at linaro.org <mailto:bin.chen at linaro.org>>
>     Cc: Masahiro Yamada <yamada.masahiro at socionext.com
>     <mailto:yamada.masahiro at socionext.com>>
>     Cc: Tom Rini <trini at konsulko.com <mailto:trini at konsulko.com>>
>     ---
>     V2: Protect the ARM64 booti bit with if IS_ENABLED(CMD_BOOTI)
>     V3: Use if() instead of #ifdef
>     V4: Switch force_reloc to bool
>     ---
>      common/bootm.c | 19 +++++++++++++++++--
>      1 file changed, 17 insertions(+), 2 deletions(-)
> 
>     diff --git a/common/bootm.c b/common/bootm.c
>     index e789f6818a..e517d9f118 100644
>     --- a/common/bootm.c
>     +++ b/common/bootm.c
>     @@ -202,8 +202,23 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int
>     flag, int argc,
>             }
> 
>             if (images.os.type == IH_TYPE_KERNEL_NOLOAD) {
>     -               images.os.load = images.os.image_start;
>     -               images.ep += images.os.load;
>     +               if (CONFIG_IS_ENABLED(CMD_BOOTI) &&
>     +                   images.os.arch == IH_ARCH_ARM64) {
>     +                       ulong image_addr;
>     +                       ulong image_size;
>     +
>     +                       ret = booti_setup(images.os.image_start,
>     &image_addr,
>     +                                         &image_size, true);
> 
> 
> Is it guaranteed to be conflict free (with other images) by always
> moving the kernel to the start of the RAM? 

Yes

> 
>     +                       if (ret != 0)
>     +                               return 1;
> 
> 
> Do you think it helps to add some debug/error message here as in other
> path that returns 1?

No, what for ?

>     +
>     +                       images.os.type = IH_TYPE_KERNEL;
>     +                       images.os.load = image_addr;
>     +                       images.ep = image_addr;
>     +               } else {
>     +                       images.os.load = images.os.image_start;
>     +                       images.ep += images.os.image_start;
> 
> 
> I know this is same the orinigal code. I'm not famaliar the
> history/purpse of type 
> IH_TYPE_KERNEL_NOLOAD (compared with IH_TYPE_KERNEL), so just for my
> understanding, 
> why in this case we have to increment the images.ep by images.os.load,
> not just set images.ep to 
> images.os.load.
The ep is incremented by the offset from the start of the image. That's
also the purpose of kernel noload -- don't copy the kernel to the load
address and just execute it in place. Problem with arm64 is that it does
not work so because the kernel needs to be at 2 MiB offset, which is
hard to achieve with fitImage. Thus we work around that by placing the
kernel at the start of RAM. It's overloading the kernel noload, but at
least it works.

[...]

-- 
Best regards,
Marek Vasut