[U-Boot] [PATCH v3] arm64: arm: implement a boot header capability

[Steve Rae]

Hi Tom,

On Mon, May 30, 2016 at 11:14 AM, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, May 30, 2016 at 09:51:22AM -0700, Steve Rae wrote:
>
> > From: Andre Przywara <andre.przywara at arm.com>
> >
> > Some SPL loaders (like Allwinner's boot0, and Broadcom's boot0)
> > require a header before the actual U-Boot binary to both check its
> > validity and to find other data to load. Sometimes this header may
> > only be a few bytes of information, and sometimes this might simply
> > be space that needs to be reserved for a post-processing tool.
> >
> > Introduce a config option to allow assembler preprocessor commands
> > to be inserted into the code at the appropriate location; typical
> > preprocessor commands might be:
> >   .space 1000
> >   .word 0x12345678
> > etc.
> > Please note that the current code:
> >   start.S (arm64) and
> >   vectors.S (arm)
> > already jumps over some portion of data already, so this option basically
> > just increases the size of this region (and the resulting binary).
> >
> > For use with Allwinner's boot0 blob there is a tool called boot0img[1],
> > which fills the header to allow booting A64 based boards.
> > For the Pine64 we need a 1536 byte header (including the branch
> > instruction) at the moment, so we add this to the defconfig.
> >
> > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > Signed-off-by: Steve Rae <srae at broadcom.com>
>
> I think this is a step in the right direction.
Thanks....

>
> [snip]
> > diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> > index e933021..9202889 100644
> > --- a/arch/arm/cpu/armv8/start.S
> > +++ b/arch/arm/cpu/armv8/start.S
> > @@ -21,6 +21,10 @@
> >  _start:
> >       b       reset
> >
> > +#ifdef CONFIG_BOOT0_CODE
> > +CONFIG_BOOT0_CODE
> > +#endif
> > +
> >       .align 3
>
> I don't like adding things to the CONFIG name space that we can't
> control the contents of via Kconfig.  So I think we need to change the
> BOOT0_CODE portion to something like ARMV8_SOC_BOOT0_HOOK.  And if
> there's some part of the ARMv8 docs we can reference that explains why
> it's boot0 and at least 2 different SoCs have done this, we can use a
> better name here.
>
OK, I follow your logic -- but this is not ARMV8 specific...
In this commit:
- Allwinner is arm64, but
- Broadcom is arm (armv7l)
so maybe just:
SOC_BOOT0_HOOK ?
And I don't know where the "BOOT0" terminology started - maybe it is
in ARM/ARMV8 documentation; I just don't know....

> Then..
> > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> > index c1ae6f5..d2678c9 100644
> > --- a/board/sunxi/Kconfig
> > +++ b/board/sunxi/Kconfig
> > @@ -15,6 +15,13 @@ config SUNXI_GEN_SUN6I
> >       separate ahb reset control registers, custom pmic bus, new style
> >       watchdog, etc.
> >
> > +config SUNXI_BOOT0
> > +     bool "prepare for boot0 header"
> > +     ---help---
> > +     If U-Boot is loaded from the Allwinner provided boot0 blob, it
> > +     expects a header area filled with magic values.
> > +     This option will add some space at the beginning of the image to
> > +     let a tool later on fill in this header with sensible data.
>
> This becomes something like ENABLE_ARMV8_SOC_BOOT0_HOOK and generic the
> text a bit more here and mention the Allwinner and Broadcom examples.
OK for the Allwinner...
But I don't think the Broadcom boards are going to define SUNXI_BOOT0 ?!?!?!

>
> > diff --git a/include/configs/bcm28155_ap.h b/include/configs/bcm28155_ap.h
> > index 889e5db..515552b 100644
> > --- a/include/configs/bcm28155_ap.h
> > +++ b/include/configs/bcm28155_ap.h
> > @@ -137,4 +137,8 @@
> >  #define CONFIG_USB_GADGET_BCM_UDC_OTG_PHY
> >  #define CONFIG_USBID_ADDR            0x34052c46
> >
> > +#define CONFIG_BOOT0_CODE    \
> > +     .word   0xbabeface;     \
> > +     .word   _end - _start
> > +
>
> Then this goes into arch/arm/include/asm/arch-bcm281xx/boot0.h or
> similar?
OK - I get that it shouldn't be in the (deprecated) include/configs file(s)....
However, I didn't really want to add more #include lines to:
arch/arm/cpu/armv8/start.S  or
arch/arm/lib/vectors.S
So, if I add your suggested filename, will it get "picked up" through
the existing
#include <config.h>
?

Thanks, Steve

>
> >  #endif /* __BCM28155_AP_H */
> > diff --git a/include/configs/sun50i.h b/include/configs/sun50i.h
> > index 0fdb4c7..fe9624b 100644
> > --- a/include/configs/sun50i.h
> > +++ b/include/configs/sun50i.h
> > @@ -17,6 +17,11 @@
> >  #define GICD_BASE            0x1c81000
> >  #define GICC_BASE            0x1c82000
> >
> > +#ifdef CONFIG_SUNXI_BOOT0
> > +#define CONFIG_BOOT0_CODE    \
> > +     .space  1532
> > +#endif
>
> and .../arch-sunxi/boot0.h
>
> --
> Tom