LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Nicolas Pitre <nicolas.pitre@linaro.org>
To: Pavel Machek <pavel@ucw.cz>
Cc: "Tony Lindgren" <tony@atomide.com>,
	"Pali Rohár" <pali.rohar@gmail.com>,
	linux-omap <linux-omap@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Aaro Koskinen" <aaro.koskinen@iki.fi>,
	"Sebastian Reichel" <sre@kernel.org>,
	"Ivaylo Dimitrov" <ivo.g.dimitrov.75@gmail.com>,
	"Russell King" <linux@arm.linux.org.uk>,
	linux-arm-kernel@lists.infradead.org,
	"Laura Abbott" <lauraa@codeaurora.org>
Subject: Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken
Date: Mon, 26 Jan 2015 19:06:11 -0500 (EST)	[thread overview]
Message-ID: <alpine.LFD.2.11.1501261903130.1322@knanqh.ubzr> (raw)
In-Reply-To: <20150126214821.GA8884@amd>

On Mon, 26 Jan 2015, Pavel Machek wrote:

> Hi!
> 
> > > > $ du -b arch/arm/boot/dts/omap3-n900.dtb 
> > > > 70212   arch/arm/boot/dts/omap3-n900.dtb
> > > > 
> > > > $ echo $((0x10000))
> > > > 65536
> > > > 
> > > > I would say, problem is because omap3-n900 binary DT is too large
> > 
> > I agree.
> > 
> > > OK if that's the case, then your patch makes sense to me. It also
> > > seems we can have the temporary stack be larger than the initial
> > > stack just for atags_to_fdt.
> > 
> > The stack size isn't the issue, but rather its location.  We need to 
> > position it away from the DT data.  The DT size is known and we could 
> > use that, plus some room for the insertion of new data coming from the
> > ATAG conversion.
> > 
> > Something like the following would be a more robust solution:
> > 
> > diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> > index 68be901759..89718de9dd 100644
> > --- a/arch/arm/boot/compressed/head.S
> > +++ b/arch/arm/boot/compressed/head.S
> > @@ -263,16 +263,37 @@ restart:	adr	r0, LC0
> >  		 * OK... Let's do some funky business here.
> >  		 * If we do have a DTB appended to zImage, and we do have
> >  		 * an ATAG list around, we want the later to be translated
> > -		 * and folded into the former here.  To be on the safe side,
> > -		 * let's temporarily move  the stack away into the malloc
> > -		 * area.  No GOT fixup has occurred yet, but none of the
> > -		 * code we're about to call uses any global variable.
> > +		 * and folded into the former here. No GOT fixup has occurred
> > +		 * yet, but none of the code we're about to call uses any
> > +		 * global variable.
> >  		*/
> > -		add	sp, sp, #0x10000
> > +
> > +		/* Get the initial DTB size */
> > +		ldr	r5, [r6, #4]
> > +#ifndef __ARMEB__
> > +		/* convert to little endian */
> > +		eor	r1, r5, r5, ror #16
> > +		bic	r1, r1, #0x00ff0000
> > +		mov	r5, r5, ror #8
> > +		eor	r5, r5, r1, lsr #8
> > +#endif
> > +		/* 50% DTB growth should be good enough */
> > +		add	r5, r5, r5, lsr #1
> > +		/* preserve 64-bit alignment */
> > +		add	r5, r5, #7
> > +		bic	r5, r5, #7
> > +		/* clamp to 32KB min and 1MB max */
> > +		movs	r1, r5, lsr #15
> > +		moveq	r5, #(1 << 15)
> > +		movs	r1, r5, lsr #20
> > +		movne	r5, #(1 << 20)
> 
> Dunno. Would it be easier/simpler to just use 1MB, always? Do we
> support machines with <16MB RAM?

If people are used to put other things relatively close to the kernel 
image like, say, some initrd image, then I'd prefer to be more 
conservative and avoid spreading out too much.


Nicolas

  reply	other threads:[~2015-01-27  0:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-23 21:39 Pali Rohár
2015-01-23 22:36 ` Pali Rohár
2015-01-23 23:48   ` Pavel Machek
2015-01-24  9:04     ` Pali Rohár
2015-01-26 16:14   ` Tony Lindgren
2015-01-26 16:23     ` Pali Rohár
2015-01-26 17:03       ` Tony Lindgren
2015-01-26 18:57         ` Nicolas Pitre
2015-01-26 21:48           ` Pavel Machek
2015-01-27  0:06             ` Nicolas Pitre [this message]
2015-01-27 13:37           ` Pavel Machek
2015-01-27 15:16             ` Nicolas Pitre
2015-01-27 15:26               ` Pali Rohár
2015-01-27 15:26               ` Tony Lindgren
2015-01-27 17:53               ` Russell King - ARM Linux
2015-01-27 18:50                 ` Nicolas Pitre
2015-01-27 23:09                   ` Aaro Koskinen
2015-01-27 23:17                   ` Pavel Machek
2015-02-02 13:39               ` Pavel Machek
2015-02-02 13:56                 ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.11.1501261903130.1322@knanqh.ubzr \
    --to=nicolas.pitre@linaro.org \
    --cc=aaro.koskinen@iki.fi \
    --cc=ivo.g.dimitrov.75@gmail.com \
    --cc=lauraa@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=pali.rohar@gmail.com \
    --cc=pavel@ucw.cz \
    --cc=sre@kernel.org \
    --cc=tony@atomide.com \
    --subject='Re: N900 v3.19-rc5 arm atags_to_fdt.c is broken' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).