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

[Tom Rini]

On Wed, Jun 13, 2018 at 12:39:25PM +0200, Marek Vasut wrote:
> 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.

And, we have also overloaded the term "noload", historically.  It does
mean "execute the contents where they are".  But it also means (to
humans) "You don't need to specify a load address, it will just work".
So it is important to fix this so that a fitImage for aarch64 can work
on multiple SoCs so long as of course it has the dtbs for each of those
SoCs.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180613/152ffff1/attachment.sig>