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

[Tom Rini]

On Mon, May 30, 2016 at 11:37:47AM -0700, Steve Rae wrote:
> 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....

OK.  Yes, lets call it ARM_SOC_BOOT0_HOOK until/unless someone chimes in
with a better still name.

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

Right.  But it should become ENABLE_ARM_SOC_BOOT0_HOOK and both
platforms (and other future ones probably) would enable it.

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

No, you would need to make both S files include <asm/arch/boot0.h> but I
think that's a logical thing to do.  I would be:
#ifdef CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK
#include <asm/arch/boot0.h>

/*
 * Various SoCs need something special and SoC-specific up front in
 * order to boot, allow them to set that in their boot0.h file and then
 * use it here.
 */
aRM_SOC_BOOT0_HOOK
#endif

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160531/083c56fb/attachment.sig>