[PATCH 2/6] ARM: zImage: Allow the appending of a device tree binary

[dave.martin@linaro.org (Dave Martin)]

On Wed, Sep 14, 2011 at 10:04:28AM -0400, Nicolas Pitre wrote:
> On Wed, 14 Sep 2011, Dave Martin wrote:
> 
> > On Wed, Sep 14, 2011 at 01:41:42AM -0400, Nicolas Pitre wrote:
> > > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > 
> > > This patch provides the ability to boot using a device tree that is appended
> > > to the raw binary zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > > 
> > > Signed-off-by: John Bonesio <bones@secretlab.ca>
> > > [nico: adjusted to latest zImage changes plus additional cleanups]
> > > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > > Acked-by: Grant Likely <grant.likely@secretlab.ca>
> > > Acked-by: Tony Lindgren <tony@atomide.com>
> > > ---
> > >  arch/arm/Kconfig                |    8 ++++
> > >  arch/arm/boot/compressed/head.S |   70 +++++++++++++++++++++++++++++++++++++--
> > >  2 files changed, 75 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index 5ebc5d922e..83323c2b1f 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -1781,6 +1781,14 @@ config ZBOOT_ROM_SH_MOBILE_SDHI
> > >  
> > >  endchoice
> > >  
> > > +config ARM_APPENDED_DTB
> > > +	bool "Use appended device tree blob to zImage"
> > > +	depends on OF && !ZBOOT_ROM
> > > +	help
> > > +	  With this option, the boot code will look for a device tree binary
> > > +	  (dtb) appended to zImage
> > > +	  (e.g. cat zImage <filename>.dtb > zImage_w_dtb).
> > > +
> > >  config CMDLINE
> > >  	string "Default kernel command string"
> > >  	default ""
> > > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > > index e95a598960..3ce5738ddb 100644
> > > --- a/arch/arm/boot/compressed/head.S
> > > +++ b/arch/arm/boot/compressed/head.S
> > > @@ -216,6 +216,59 @@ restart:	adr	r0, LC0
> > >  		mov	r10, r6
> > >  #endif
> > >  
> > > +		mov	r5, #0			@ init dtb size to 0
> > > +#ifdef CONFIG_ARM_APPENDED_DTB
> > > +/*
> > > + *   r0  = delta
> > > + *   r2  = BSS start
> > > + *   r3  = BSS end
> > > + *   r4  = final kernel address
> > > + *   r5  = appended dtb size (still unknown)
> > > + *   r6  = _edata
> > > + *   r7  = architecture ID
> > > + *   r8  = atags/device tree pointer
> > > + *   r9  = size of decompressed image
> > > + *   r10 = end of this image, including  bss/stack/malloc space if non XIP
> > > + *   r11 = GOT start
> > > + *   r12 = GOT end
> > > + *   sp  = stack pointer
> > > + *
> > > + * if there are device trees (dtb) appended to zImage, advance r10 so that the
> > > + * dtb data will get relocated along with the kernel if necessary.
> > > + */
> > > +
> > > +		ldr	lr, [r6, #0]
> > > +#ifndef __ARMEB__
> > > +		ldr	r1, =0xedfe0dd0		@ sig is 0xd00dfeed big endian
> > > +#else
> > > +		ldr	r1, =0xd00dfeed
> > > +#endif
> > 
> > Do we worry that garbage in memory after the zImage might match this
> > magic number?
> > 
> > For example, if an ordinary userspace program allocates a huge number
> > of pages and fills them with bogus device tree headers, is there a chance
> > that the those headers could remain in memory across a reboot?
> 
> In theory this _could_ be possible.  However I don't expect this feature 
> to be enabled if you are not going to actually use it, especially in a 
> production setup.  If you are not appending a DTB to your kernel then 
> there is simply no point keeping this config option set (normally you 
> should use this option only because you have no other choices).

That seems reasonable.

Should we document this recommendation, in the Kconfig help or
Documentation/?


On the other hand, if there's any chance this might need to be fixed later,
it could make sense to fix it now, particularly given that the fix is simple
(based on the assumption that whatever is documented, people can and will
sometimes do the wrong thing when deploying a platform...)

Anyway, I don't have a strong feeling on this myself.

Cheers
---Dave