[U-Boot] [PATCH v2 3/8] sandbox: Add support for bootz

[Simon Glass]

Hi Sjoerd,

On 7 April 2015 at 01:56, Sjoerd Simons <sjoerd.simons at collabora.co.uk> wrote:
> On Mon, 2015-04-06 at 15:40 -0600, Stephen Warren wrote:
>> On 04/06/2015 03:02 PM, Sjoerd Simons wrote:
>> > Add dummy bootz_setup implementation allowing the u-boot sandbox to
>> > run bootz. This recognizes both ARM and x86 zImages to validate a
>> > valid zImage was loaded.
>>
>> > diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
>>
>> > +int bootz_setup(ulong image, ulong *start, ulong *end)
>>
>> > +   *start = 0xdead;
>> > +   *end = 0xbeef;
>> > +   return 0;
>>
>> Isn't that going to cause the rest of bootz to access or jump to some
>> bogus address and crash?
>
> A very good question. I hadn't actually double-checked what these values
> are used for as things just worked and i got distracted by fixing other
> bits & pieces.
>
> Looking through the code, these values are only used to add an LMB
> region directly after the kernel entry load address. As the sandbox
> architecture doesn't define either arch_lmb_reserve nor
> board_lmb_reserve these bogus values don't cause any issues (as they
> don't seem to make the generic lmb code blow-up thatis), but it's
> definitely not pretty.

In the future we may want to emulate this with sandbox. Do you think
instead of this we should read out the correct values from the image
file? It seems odd to use fake values.

In fact I wonder if this should use a common function with ARM, or
perhaps part of it should be common?

>
> I'll fix up that patch a bit to make those values more meaningful later
> this.
>
>> Aside from that, this series looks plausible, so,
>> Acked-by: Stephen Warren <swarren at nvidia.com>
>>
>> (I read through patch 6 most thoroughly, briefly skimmed patch 7, and
>> read the others fairly quickly, so take that as an ack primarily on
>> patch 6 although I didn't see anything obviously wrong elsewhere).
>
> Thanks for the review!
>
> --
> Sjoerd Simons <sjoerd.simons at collabora.co.uk>
> Collabora Ltd.

Regards,
Simon