[U-Boot] [PATCH v2 5/5] bootm: Add the missing PREP stage to bootz and correct image handling

[Simon Glass]

Hi Tom,


On Tue, Jul 9, 2013 at 2:19 PM, Tom Rini <trini at ti.com> wrote:

> On Tue, Jul 09, 2013 at 01:04:58PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, Jul 9, 2013 at 12:05 PM, Tom Rini <trini at ti.com> wrote:
> >
> > > On Tue, Jul 09, 2013 at 12:00:00PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Tue, Jul 9, 2013 at 10:01 AM, Tom Rini <trini at ti.com> wrote:
> > > >
> > > > > On Mon, Jul 08, 2013 at 10:22:13AM -0400, Tom Rini wrote:
> > > > > > On Mon, Jul 08, 2013 at 09:17:10AM -0500, Robert Nelson wrote:
> > > > > >
> > > > > > > On Mon, Jul 8, 2013 at 9:13 AM, Tom Rini <trini at ti.com> wrote:
> > > > > > > > On Thu, Jul 04, 2013 at 01:26:11PM -0700, Simon Glass wrote:
> > > > > > > >> In the recent bootm refactor, the PREP stage was missing in
> the
> > > > > bootz
> > > > > > > >> command. This causes unpredictable behaviour.
> > > > > > > >>
> > > > > > > >> The use of a local variable means that the reset of
> cmd_bootm.c
> > > > > does not
> > > > > > > >> in fact use the same image structure, so remove this.
> > > > > > > >>
> > > > > > > >> Also manually set the OS type to Linux, since this is the
> only
> > > > > possibility
> > > > > > > >> at present, and we need to select the right boot function.
> > > > > > > >>
> > > > > > > >> Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > >
> > > > > > > > With the whole series applied, I still see a hang at:
> > > > > > > > Kernel image @ 0x80200000 [ 0x000000 - 0x3d44a0 ]
> > > > > > > >
> > > > > > > > Starting kernel ...
> > > > > > > >
> > > > > > > > Perhaps something to do with how my DDR is not at 0x0 ->
> 256MiB
> > > but
> > > > > > > > 0x80000000 -> 256MiB ?
> > > > > > >
> > > > > > >
> > > > > > > Tom, which board is that?
> > > > > > >
> > > > > > > These 5 patches just on top of v2013.07-rc2, the panda (non es)
> > > (board
> > > > > > > file) works, but Wand (device tree) is still locking up for
> me...
> > > > > > >
> > > > > > > Panda (Board file boot)
> > > > > > >
> > > > > > > load mmc ${mmcdev}:${mmcpart} ${loadaddr} zImage
> > > > > > > run mmcargs
> > > > > > > bootz ${loadaddr}
> > > > > >
> > > > > > Ah-ha!  It's an appended dtb vs not problem now.  I can boot my
> > > > > > beagelbone with with an appended dtb and bootz, but can't with
> > > separate.
> > > > >
> > > > > OK, the BOOTM_STATE_FINDOTHER state code isn't working since we
> don't
> > > > > have the rest of the header bits that the code checks for set.
>  I've
> > > > > taken a few stabs at reworking things, but it's not working yet.
> > >  Simon,
> > > > > do you have any ideas here?  I'm starting to wonder if we don't
> need to
> > > > > revert things afterall and sort this out post release.
> > > > >
> > > > >
> > > > Yes time is running short. I did post two reverts - I wonder if they
> are
> > > > effective for this problem?
> > > >
> > > > Is the appended dtb with bootz the only problem remaining as far as
> we
> > > > know? If so then perhaps we are close.
> > > >
> > > > I will see if I can get a Beaglebone today or failing that I should
> be
> > > able
> > > > to try bootz with appeneded dtb on another ARM board.
> > >
> > > I've got a fix locally now, and I'm working on cleaning it up further.
> > > The problem is that BOOTM_STATE_FINDOTHER would never work since we
> > > aren't the "right" image types, so passed ramdisk and/or dtb didn't
> > > work.  Another problem was that bootz was never consuming 'bootz', but
> > > this was OK before.  I'll post a patch shortly.
> >
> >
> > OK great, thanks for looking at this.  I wonder if we can collect a set
> of
> > different use cases for bootz as a basis for test cases?
>
> Well, what happens on sandbox when you try and boot a kernel? :)  bootz
> is any of:
> kernel, kernel+ramdisk, kernel+ramdisk+fdt, kernel+fdt.
>

The test currently checks that the kernel/ramdisk/fdt appear in the right
place, and that U-Boot reports that it is booting the kernel from the right
address. That actually should cover bugs where things don't end up in the
right place, or are corrupted. Of course there are limitations with
automated tests like this, but I still think it will make it easier for the
next person who takes on a clean-up in this area of U-Boot.


>
> > > My biggest worry right now is, what might show up broken next?  Anyone
> > > out there easily able to boot a netbsd kernel or something?
> >
> > Yes, I am not really comfortable with this. I will see if I can write
> some
> > sandbox tests for the other image types today and post my results. I
> guess
> > this bootm code has built up over a long time and it is hard to know all
> > the ways it is used.
>
> Important, but I really want to see real-world booting in a case or two.
> Unfortunately I don't have any ARM boards that work out of the box with
> NetBSD.
>

Yes, me too. But in the meantime I have tried to do what I can here - spent
many happy hours reviewing the code carefully and writing a 'sandbox' test
for the following OSes:

['netbsd', 'lynxos', 'rtems', 'ose', 'plan9', 'vxworks', 'qnx', 'integrity']

I found that most of them have the same problem fixed by Andreas/Tom's
commit 2cb0e55, so I will prepare a patch for this.

Additionally, the vxworks arguments are incorrect (argc is one less than
expected). I will prepare a patch for this also.

Other than that, everything looks correct so far as I can tell, up to the
point where U-Boot actually jumps to the OS.

I will tidy up my tests and submit these also at some point. Coverage now
includes FIT and legacy images, with bootm commands and subcommands. For
the record there is still a lot of test code missing:


Regards,
Simon