Skip to contentSkip to conversation

tools: fix FIT image with ramdisk

PATCH New Stefano Babic 6 participants 14 messages Jul 12, 2013 – Aug 13, 2013

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

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.

---
 common/image.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/image.c +1 -1
index 1be384f..08f712a 100644
@@ -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;
 
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

Hi Stefano,

Quoted 4 lines · 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.

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.

Quoted 11 lines
---
 common/image.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/image.c +0 -0
index 1be384f..08f712a 100644

common/image.c int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, · line 916

Quoted 8 lines
common/image.c +1 -1
@@ -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,

Signature · 1 line
Albert.
#2 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

In reply to Stefano Babic
Quoted 6 lines · 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.

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!

Signature · 8 lines
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>
#3 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

Hi Tom,

Quoted 6 lines

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,

Signature · 1 line
Albert.
#4 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

In reply to Tom Rini

Am 12/07/2013 23:24, schrieb Tom Rini:

Quoted 7 lines · Stefano Babic
Quoted 6 lines

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.

Quoted 9 lines
Quoted 1 line

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 !

Quoted 2 lines

And applied to u-boot/master, thanks!

Thanks !

Stefano

Signature · 5 lines
=====================================================================
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
=====================================================================
#5 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

In reply to Tom Rini

Dear Tom Rini,

Quoted 3 lines · 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

Signature · 4 lines
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.
#6 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

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

Quoted 4 lines · Wolfgang Denk

Dear Tom Rini,

Quoted 1 line · In message <20130712212416.GV13531@bill-the-cat> you wrote:
Quoted 8 lines
Quoted 3 lines

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-----

#7 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

In reply to Wolfgang Denk
Quoted 10 lines · Wolfgang Denk

Dear Tom Rini,

Quoted 3 lines · 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?

#8 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

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

Quoted 9 lines · Stephen Warren
Quoted 8 lines · Wolfgang Denk

Dear Tom Rini,

Quoted 4 lines · 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".

Quoted 8 lines
Quoted 4 lines

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-----

#9 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

Hi,

Quoted 18 lines · Tom Rini

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

Quoted 13 lines · Stephen Warren
Quoted 12 lines · Wolfgang Denk

Dear Tom Rini,

Quoted 4 lines · 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.

Quoted 10 lines
Quoted 4 lines

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>

#10 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

Quoted 3 lines · Simon Glass

Hi,

On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <[email protected]> wrote:

[snip]

Quoted 10 lines
Quoted 5 lines

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 :)

Signature · 8 lines
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>
#11 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

+Stephen

Hi Tom,

Quoted 14 lines · Tom Rini
Quoted 3 lines · Simon Glass

Hi,

On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <[email protected]> wrote:

[snip]

Quoted 9 lines
Quoted 5 lines

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

Quoted 8 lines

ugly

Quoted 1 line

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

#12 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

Quoted 23 lines · Simon Glass

+Stephen

Hi Tom,

Quoted 17 lines · Tom Rini
Quoted 3 lines · Simon Glass

Hi,

On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <[email protected]> wrote:

[snip]

Quoted 9 lines
Quoted 5 lines

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

Quoted 1 line

hacks for caches and low-level serial access.

Quoted 8 lines
Quoted 5 lines

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.

Signature · 8 lines
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>
#13 lore raw

PATCH 1/1 tools: fix FIT image with ramdisk

Quoted 30 lines · Tom Rini
Quoted 29 lines · Simon Glass

+Stephen

Hi Tom,

Quoted 22 lines · Tom Rini
Quoted 3 lines · Simon Glass

Hi,

On Sat, Jul 13, 2013 at 8:55 PM, Tom Rini <[email protected]> wrote:

[snip]

Quoted 9 lines
Quoted 5 lines

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

Quoted 1 line

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 :)

Quoted 5 lines
Quoted 2 lines

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.

Signature · 8 lines
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>