[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Stefano Babic]

Booting a FIT image containing a ramdisk,
the ramdisk is loaded at address 0x0 that causes
bus errors for architectures that do not have
RAM starting at address zero.

Signed-off-by: Stefano Babic <sbabic@denx.de>

---
 common/image.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/image.c b/common/image.c
index 1be384f..08f712a 100644
--- a/common/image.c
+++ b/common/image.c
@@ -910,7 +910,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
 					fit_uname_config, arch,
 					IH_TYPE_RAMDISK,
 					BOOTSTAGE_ID_FIT_RD_START,
-					FIT_LOAD_REQUIRED, &rd_data, &rd_len);
+					FIT_LOAD_IGNORED, &rd_data, &rd_len);
 			if (rd_noffset < 0)
 				return 1;
 
-- 
1.7.9.5

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Albert ARIBAUD]

Hi Stefano,

On Fri, 12 Jul 2013 15:09:23 +0200, Stefano Babic <sbabic@denx.de>
wrote:

> Booting a FIT image containing a ramdisk,
> the ramdisk is loaded at address 0x0 that causes
> bus errors for architectures that do not have
> RAM starting at address zero.

Kind-of-minor nitpick: ARMs which have RAM at address 0 might well have
their exception tables there too (and will if they don't right now),
which makes it always bad to to load anything there anyway.

Properly major nitpick: the commit message explains what needed fixing,
but neither the commit message nor the code (to an admittedly completely
unFIT reader) explain *how* exactly it was fixed.

> Signed-off-by: Stefano Babic <sbabic@denx.de>
> 
> ---
>  common/image.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/image.c b/common/image.c
> index 1be384f..08f712a 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -910,7 +910,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
>  					fit_uname_config, arch,
>  					IH_TYPE_RAMDISK,
>  					BOOTSTAGE_ID_FIT_RD_START,
> -					FIT_LOAD_REQUIRED, &rd_data, &rd_len);
> +					FIT_LOAD_IGNORED, &rd_data, &rd_len);
>  			if (rd_noffset < 0)
>  				return 1;
>  


Amicalement,
-- 
Albert.

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Tom Rini]

On Fri, Jul 12, 2013 at 03:09:23PM +0200, Stefano Babic wrote:

> Booting a FIT image containing a ramdisk,
> the ramdisk is loaded at address 0x0 that causes
> bus errors for architectures that do not have
> RAM starting at address zero.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>

I've re-worded this as:
common/image.c: Fix regression with ramdisk load/entry points in FIT
    
A FIT image with a ramdisk that sets the entry or load points to 0x0
must be treated as meaning "leave in place" and NOT "relocate to 0x0".
This regression was introduced in a51ec63.

And applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130712/96f00303/attachment.pgp>

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Albert ARIBAUD]

Hi Tom,

> I've re-worded this as:
> common/image.c: Fix regression with ramdisk load/entry points in FIT
>     
> A FIT image with a ramdisk that sets the entry or load points to 0x0
> must be treated as meaning "leave in place" and NOT "relocate to 0x0".
> This regression was introduced in a51ec63.

Thanks! :)

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Stefano Babic]

Am 12/07/2013 23:24, schrieb Tom Rini:
> On Fri, Jul 12, 2013 at 03:09:23PM +0200, Stefano Babic wrote:
> 
>> Booting a FIT image containing a ramdisk,
>> the ramdisk is loaded at address 0x0 that causes
>> bus errors for architectures that do not have
>> RAM starting at address zero.
>>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
> 
> I've re-worded this as:
> common/image.c: Fix regression with ramdisk load/entry points in FIT
>     
> A FIT image with a ramdisk that sets the entry or load points to 0x0
> must be treated as meaning "leave in place" and NOT "relocate to 0x0".
> This regression was introduced in a51ec63.
> 

Great !

> And applied to u-boot/master, thanks!
> 
Thanks !

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Wolfgang Denk]

Dear Tom Rini,

In message <20130712212416.GV13531@bill-the-cat> you wrote:
> 
> A FIT image with a ramdisk that sets the entry or load points to 0x0
> must be treated as meaning "leave in place" and NOT "relocate to 0x0".

Why is this the case?  0x0 could be a valid address on some systems.
If we need a special address that "cannot exist", we should rather use
the last address in the addressable range (i. e. (void *)(~0)).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Imitation is the sincerest form of plagarism.

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/13/2013 07:21 AM, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <20130712212416.GV13531@bill-the-cat> you wrote:
>> 
>> A FIT image with a ramdisk that sets the entry or load points to
>> 0x0 must be treated as meaning "leave in place" and NOT "relocate
>> to 0x0".
> 
> Why is this the case?  0x0 could be a valid address on some
> systems. If we need a special address that "cannot exist", we
> should rather use the last address in the addressable range (i. e.
> (void *)(~0)).

True enough, but we're not back to the behaviour of the previous
release.  In fact, this makes me wonder if there's not a solution to
the problem I had before of wanting to just leave everything in-place
(so that we could have a FIT image with an ARM multi-platform kernel
that would be usable on multiple platforms without multiple copies of
the zImage).

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJR4bJiAAoJENk4IS6UOR1WMbgP/jNjjMJbAPNBLTlyPGPZxK92
K2tBEXezsaWV53MGVTaHt6ZcrpOhSo7j2DP7lO0/j9nH5TM9mJ6tWDKzZxaopMoT
chyyDEN38IaY1LiklwqwyHj8tOCIbCGLtaG0vplHeodE8UFgmBU4QMpBqVvwBids
vtNAjQy0MHI5LXDTY0OpVNzVIQD2Rm2y3xyPnRsuUWoY5BtGmcxs1X5KyoCKbQ3J
qtG0Eb2nP2fuADSgOOTtA1CQRKrKoSk7k2r8fVPzZ85S+8goro8n+a7AgWhuvU+C
WrGKxrlejbUejh7JZt0HGpNr1iIos+7+V3G5PP8pel4JOkaD/jdBgf4UgN2sADXM
JKvkd65jiIRvW4Axw+tZ8omHzXRYPYNe6wgH9CKDtUbSWnRVdvXvtF78XEIMi1XX
iDOAZVca+3qsrx0hJTc34++C2rBsONa5Yd88GwsNwb9BZufqs+uyhD0N19xBqjFC
pd9cJwKuL1P7T7IEHP5N1anerJPNfUCtvZv5RnWKzZBGpvjfZUeZ2PcS2/nuXEHZ
rj3jX9PvzGO1L/mgYCMQ39/6zH5Wge70ojbMkn/Lh+cWWZb4Y7LuUfGINH/XTjrb
H+/OQ2RuDPje04X6e3ws1vAXwy8NF8mltcLwnusKkcqf3VhhTHoOOm/OHyjTLw5a
tf4c94PHN023tdpUtsR1
=G4tN
-----END PGP SIGNATURE-----

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Stephen Warren]

On 07/13/2013 05:21 AM, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <20130712212416.GV13531@bill-the-cat> you wrote:
>>
>> A FIT image with a ramdisk that sets the entry or load points to 0x0
>> must be treated as meaning "leave in place" and NOT "relocate to 0x0".
> 
> Why is this the case?  0x0 could be a valid address on some systems.
> If we need a special address that "cannot exist", we should rather use
> the last address in the addressable range (i. e. (void *)(~0)).

For the kernel, we created a special image type for "no relocation";
IH_TYPE_KERNEL_NOLOAD. Is an equivalent needed to the initrd?

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/13/2013 10:26 PM, Stephen Warren wrote:
> On 07/13/2013 05:21 AM, Wolfgang Denk wrote:
>> Dear Tom Rini,
>> 
>> In message <20130712212416.GV13531@bill-the-cat> you wrote:
>>> 
>>> A FIT image with a ramdisk that sets the entry or load points
>>> to 0x0 must be treated as meaning "leave in place" and NOT
>>> "relocate to 0x0".
>> 
>> Why is this the case?  0x0 could be a valid address on some
>> systems. If we need a special address that "cannot exist", we
>> should rather use the last address in the addressable range (i.
>> e. (void *)(~0)).
> 
> For the kernel, we created a special image type for "no
> relocation"; IH_TYPE_KERNEL_NOLOAD. Is an equivalent needed to the
> initrd?

No, because what we have today is insufficient for the kernel, you
still have to specify the load/entry point, in FIT at least, even on
NOLOAD.  I'd have sworn at least, I couldn't find a way to get around
this problem before...

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJR4hM4AAoJENk4IS6UOR1WKOUP/1IVfyWujtl1d5EIqLWRbaRe
BxkIafbsWE8MgmGHtDLhy/snJqHvi57Bs/egkV+mrhfOYmQIjYWOylKCyRhIXW/7
s+b0fLqo5y5fMjdtANdHIU0Jp6j7tkT+dIkU5SPWsGp38uHTDhP9TuDtwHtF4Brm
uMhdxCT72x+VzyzwsUXRj0BY5LhJJULmOEHTm0t40Gs3wNCKx/SQKKOrNGwLH9n9
VzLlPZGX8ABKHdEcSjGExEJ1vXV5ulvKFKhUkSURoM0R9qG3/Esvw4nsnkLiobEK
Bh1Ja6jX9uGR2afUS8JSgem6SYl/8wnBvNGMjWHR5gPNeY4Ujhv/Q4fc2qYhIJTa
qHIZb+lMSlQCe7qBD3JaTeGfuPIQfVgLDQqqrLNKLLr47me8Vw8gMDf3PK9lKuwd
8H7zTlMvdUM3he+QrI4capLSmfYzNkgmfK2HC/V3aswJ38eJUrw/jIRRz4py/biK
E1gz0dCAbVvlh7waN72szapFsZXT6/2bqLg1HQQM6H/Hsf18fA8XxkVCO7C29HJo
1e90TV0DYJ5qENWY6BFyI7B+f6wtWx0JP23tlcmEciag/nwd6TkSaf03X8mDKlhY
yRs56ejWpogg/T5PIjj/L332yVwPuzsvZTWGTtAsV+C2V2xu7oo3esgvLzDbBmXO
rXuYrHDKoRQ5+jnBE1aa
=dxLu
-----END PGP SIGNATURE-----

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Simon Glass]

Hi,

On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <trini@ti.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/13/2013 10:26 PM, Stephen Warren wrote:
> > On 07/13/2013 05:21 AM, Wolfgang Denk wrote:
> >> Dear Tom Rini,
> >>
> >> In message <20130712212416.GV13531@bill-the-cat> you wrote:
> >>>
> >>> A FIT image with a ramdisk that sets the entry or load points
> >>> to 0x0 must be treated as meaning "leave in place" and NOT
> >>> "relocate to 0x0".
> >>
> >> Why is this the case?  0x0 could be a valid address on some
> >> systems. If we need a special address that "cannot exist", we
> >> should rather use the last address in the addressable range (i.
> >> e. (void *)(~0)).
>

I'm not entirely sure what is happening with ramdisk loading. Prior to the
bootm factor it seemed to me that a ramdisk had to have a load address. It
was carefully extracted from the FIT node and then ignored. So my code
for FIT_LOAD_REQUIRED should in fact have done the same! I somehow assumed
that the address would be used. Was there ever any code in U-Boot which
actually loads the ramdisk to an address (other than boot_ramdisk_high()
which doesn't actually use the FIT node load address? If so I can't find it.


> >
> > For the kernel, we created a special image type for "no
> > relocation"; IH_TYPE_KERNEL_NOLOAD. Is an equivalent needed to the
> > initrd?
>
> No, because what we have today is insufficient for the kernel, you
> still have to specify the load/entry point, in FIT at least, even on
> NOLOAD.  I'd have sworn at least, I couldn't find a way to get around
> this problem before...
>

NOLOAD does work provided that the kernel in the FIT is a zImage.
Personally I think that is an odd thing to do, since U-Boot is perfectly
capable of decompressing a kernel, and the decompression shim requires ugly
hacks for caches and low-level serial access.

But it does have one advantage - there is no need to specify a load address
in the kernel FIT node. It would be great if there were an environment
variable which specifies the kernel load address when none is given by the
FIT (I don't mean loadaddr which is used as the address of the FIT). Then
we could leave out the load address from the FIT node and boards could do
the right thing. Bonus points if the address could be an offset from the
start of DRAM, since then many platforms could probably have the same value.

Thoughts?

Regards,
Simon

<http://lists.denx.de/mailman/listinfo/u-boot>

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Tom Rini]

On Sat, Jul 20, 2013 at 04:06:27PM -0600, Simon Glass wrote:
> Hi,
> 
> On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <trini@ti.com> wrote:
[snip]
> > No, because what we have today is insufficient for the kernel, you
> > still have to specify the load/entry point, in FIT at least, even on
> > NOLOAD.  I'd have sworn at least, I couldn't find a way to get around
> > this problem before...
> >
> 
> NOLOAD does work provided that the kernel in the FIT is a zImage.
> Personally I think that is an odd thing to do, since U-Boot is perfectly
> capable of decompressing a kernel, and the decompression shim requires ugly
> hacks for caches and low-level serial access.

I haven't seen a .its with a NOLOAD kernel that doesn't specify a
load/entry property for it.  If you've got one, and it works on a
Beaglebone (which doesn't have DDR starting at 0x0 ...), I'd love to see
the .its :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130720/efbc147e/attachment.pgp>

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Simon Glass]

+Stephen

Hi Tom,

On Sat, Jul 20, 2013 at 4:38 PM, Tom Rini <trini@ti.com> wrote:

> On Sat, Jul 20, 2013 at 04:06:27PM -0600, Simon Glass wrote:
> > Hi,
> >
> > On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <trini@ti.com> wrote:
> [snip]
> > > No, because what we have today is insufficient for the kernel, you
> > > still have to specify the load/entry point, in FIT at least, even on
> > > NOLOAD.  I'd have sworn at least, I couldn't find a way to get around
> > > this problem before...
> > >
> >
> > NOLOAD does work provided that the kernel in the FIT is a zImage.
> > Personally I think that is an odd thing to do, since U-Boot is perfectly
> > capable of decompressing a kernel, and the decompression shim requires
> ugly
> > hacks for caches and low-level serial access.
>
> I haven't seen a .its with a NOLOAD kernel that doesn't specify a
> load/entry property for it.  If you've got one, and it works on a
> Beaglebone (which doesn't have DDR starting at 0x0 ...), I'd love to see
> the .its :)
>

We use one with load/exec addresses of 0 (on a platform with no RAM there).
With NOLOAD the address seems to be ignored anyway.

And another thing, perhaps instead of the NOLOAD image type, we should
generalise a bit and have a property in the FDT node to mean that loading
should be skipped, like 'no-load'. Then we could apply it to ramdisks and
FDT also, rather than relying on address 0 having a special meaning.
Perhaps I should have suggested this at the time.

Regards,
Simon

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Tom Rini]

On Sat, Jul 20, 2013 at 05:29:19PM -0600, Simon Glass wrote:
> +Stephen
> 
> Hi Tom,
> 
> On Sat, Jul 20, 2013 at 4:38 PM, Tom Rini <trini@ti.com> wrote:
> 
> > On Sat, Jul 20, 2013 at 04:06:27PM -0600, Simon Glass wrote:
> > > Hi,
> > >
> > > On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <trini@ti.com> wrote:
> > [snip]
> > > > No, because what we have today is insufficient for the kernel, you
> > > > still have to specify the load/entry point, in FIT at least, even on
> > > > NOLOAD.  I'd have sworn at least, I couldn't find a way to get around
> > > > this problem before...
> > > >
> > >
> > > NOLOAD does work provided that the kernel in the FIT is a zImage.
> > > Personally I think that is an odd thing to do, since U-Boot is perfectly
> > > capable of decompressing a kernel, and the decompression shim requires
> > ugly
> > > hacks for caches and low-level serial access.
> >
> > I haven't seen a .its with a NOLOAD kernel that doesn't specify a
> > load/entry property for it.  If you've got one, and it works on a
> > Beaglebone (which doesn't have DDR starting at 0x0 ...), I'd love to see
> > the .its :)
> >
> 
> We use one with load/exec addresses of 0 (on a platform with no RAM there).
> With NOLOAD the address seems to be ignored anyway.

OK, I shall play around again now that I've got a I swear working
FIT image.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130720/3b27e356/attachment.pgp>

[U-Boot] [PATCH] tools: fix FIT image with ramdisk

[Tom Rini]

On Sat, Jul 20, 2013 at 07:36:12PM -0400, Tom Rini wrote:
> On Sat, Jul 20, 2013 at 05:29:19PM -0600, Simon Glass wrote:
> > +Stephen
> > 
> > Hi Tom,
> > 
> > On Sat, Jul 20, 2013 at 4:38 PM, Tom Rini <trini@ti.com> wrote:
> > 
> > > On Sat, Jul 20, 2013 at 04:06:27PM -0600, Simon Glass wrote:
> > > > Hi,
> > > >
> > > > On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <trini@ti.com> wrote:
> > > [snip]
> > > > > No, because what we have today is insufficient for the kernel, you
> > > > > still have to specify the load/entry point, in FIT at least, even on
> > > > > NOLOAD.  I'd have sworn at least, I couldn't find a way to get around
> > > > > this problem before...
> > > > >
> > > >
> > > > NOLOAD does work provided that the kernel in the FIT is a zImage.
> > > > Personally I think that is an odd thing to do, since U-Boot is perfectly
> > > > capable of decompressing a kernel, and the decompression shim requires
> > > ugly
> > > > hacks for caches and low-level serial access.
> > >
> > > I haven't seen a .its with a NOLOAD kernel that doesn't specify a
> > > load/entry property for it.  If you've got one, and it works on a
> > > Beaglebone (which doesn't have DDR starting at 0x0 ...), I'd love to see
> > > the .its :)
> > >
> > 
> > We use one with load/exec addresses of 0 (on a platform with no RAM there).
> > With NOLOAD the address seems to be ignored anyway.
> 
> OK, I shall play around again now that I've got a I swear working
> FIT image.

Following up to myself, yes, a load/exec address of 0 (which I found to
be counter-intuitave) means that kernel and ramdisk are both used
in-place and not relocated.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130813/e194ea57/attachment.pgp>