[U-Boot-Users] arm linux images with load address == entry point

[U-Boot-Users] arm linux images with load address == entry point

[Uwe Kleine-König]

Hello,

the make target "uImage" in the linux kernel calls mkimage as follows:

	 $(MKIMAGE) -A arm -O linux -T kernel \
	 -C none -a $(ZRELADDR) -e $(ZRELADDR) \
	 -n 'Linux-$(KERNELRELEASE)' -d $< $@

(see http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/arm/boot/Makefile;h=ec9c400c7f82d171ba9ccf6e52cdb055bf1a50cb;hb=HEAD,
line 63)

i.e. sets the load address and the entry point to the same address.

This part is unchanged since it was introduced in 2003.


If I understand do_bootm and do_bootm_linux (for arm, in
lib_arm/armlinux.c) correctly, the first asserts that the image is
loaded to the load address[1] and the latter jumps to entry point.

For an image created with the above rule from the vanilla kernel, that
means, jump to the header (instead of the actual image@entry point +
0x40).

So I think the linux rule is wrong, or did I miss anything?

The U-Boot I have on my target here has somewhere after the crc check in
do_bootm the following:

        hdr->ih_load = htonl(addr);
        hdr->ih_ep = htonl(addr + 0x40);

(Actually it's still worse, load_addr instead of addr is used ...)
This works but is ugly.

I wonder how other people using U-Boot + Linux on ARM handle that.

Best regards
Uwe

PS: Linux uses entry point == load address == 0 for PowerPC.  Didn't
look in the PowerPC version of bootm_linux, but maybe it has the same
problem?

[1] For Linux this is not needed, because the decompressor (at least for
2.6) is position independant.  But that's another topic.
-- 
Uwe Kleine-K?nig

http://www.google.com/search?q=1+stone%3D

[U-Boot-Users] arm linux images with load address == entry point

[Wolfgang Denk]

In message <20070315155550.GA7760@lala> you wrote:
> 
> the make target "uImage" in the linux kernel calls mkimage as follows:
...
> i.e. sets the load address and the entry point to the same address.

This may, or may not be ok.

> If I understand do_bootm and do_bootm_linux (for arm, in
> lib_arm/armlinux.c) correctly, the first asserts that the image is
> loaded to the load address[1] and the latter jumps to entry point.

You download the image  to  an  arbitrary  address  which  should  be
sufficiently  out  of  the  way  of  the [load_addr,load_addr+size of
uncompressed kernel] area. The bootm will  (uncompress  &)  copy  the
image to the load_addr, and jumpt to the entry point.

> For an image created with the above rule from the vanilla kernel, that
> means, jump to the header (instead of the actual image at entry point +
> 0x40).

No. The header will not be copied by U-Boot's "bootm" command.

> So I think the linux rule is wrong, or did I miss anything?

Yes. You talk about two addresses, but there are three.

> PS: Linux uses entry point == load address == 0 for PowerPC.  Didn't
> look in the PowerPC version of bootm_linux, but maybe it has the same
> problem?

It works the same in this respect. Note that I write *it works*.

> [1] For Linux this is not needed, because the decompressor (at least for
> 2.6) is position independant.  But that's another topic.

Right - the decompressor  itself  is  not  needed,  as  it's  already
included  in U-Boot. Adding it to the Kernel image just wastes memroy
and time as it makes booting slower.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
When choosing between two evils, I always like to take the  one  I've
never tried before.                     -- Mae West, "Klondike Annie"

[U-Boot-Users] arm linux images with load address == entry point

[Uwe Kleine-König]

Hello Wolfgang,

> You download the image  to  an  arbitrary  address  which  should  be
> sufficiently  out  of  the  way  of  the [load_addr,load_addr+size of
> uncompressed kernel] area. The bootm will  (uncompress  &)  copy  the
> image to the load_addr, and jumpt to the entry point.
With the above command line (i.e. -C none) it will only copy it, not
uncompress.  But probably you meant that, as you put uncompress in
parenthesis.

> > For an image created with the above rule from the vanilla kernel, that
> > means, jump to the header (instead of the actual image at entry point +
> > 0x40).
> 
> No. The header will not be copied by U-Boot's "bootm" command.
Ah, OK, I see.  Then you may want to take the patch I'll send as a
follow up.
 
> > So I think the linux rule is wrong, or did I miss anything?
> 
> Yes. You talk about two addresses, but there are three.
> 
> > PS: Linux uses entry point == load address == 0 for PowerPC.  Didn't
> > look in the PowerPC version of bootm_linux, but maybe it has the same
> > problem?
> 
> It works the same in this respect. Note that I write *it works*.
> 
> > [1] For Linux this is not needed, because the decompressor (at least for
> > 2.6) is position independant.  But that's another topic.
> 
> Right - the decompressor  itself  is  not  needed,  as  it's  already
> included  in U-Boot. Adding it to the Kernel image just wastes memroy
> and time as it makes booting slower.
So you suggest to gzip the vmlinux image and use -C gzip?
 
Best regards,
Uwe

-- 
Uwe Kleine-K?nig

http://www.google.com/search?q=12+mol+in+dozen

[U-Boot-Users] arm linux images with load address == entry point

[Wolfgang Denk]

In message <20070315211340.GA27920@informatik.uni-freiburg.de> you wrote:
> 
> > uncompressed kernel] area. The bootm will  (uncompress  &)  copy  the
> > image to the load_addr, and jumpt to the entry point.
> With the above command line (i.e. -C none) it will only copy it, not
> uncompress.  But probably you meant that, as you put uncompress in
> parenthesis.

Indeed.

> > Right - the decompressor  itself  is  not  needed,  as  it's  already
> > included  in U-Boot. Adding it to the Kernel image just wastes memroy
> > and time as it makes booting slower.
> So you suggest to gzip the vmlinux image and use -C gzip?

Right. That's what we do here on all architectures, including ARM.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I may be synthetic, but I'm not stupid"  -  the  artificial  person,
from _Aliens_

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Uwe Kleine-König]

Before moving the image was skipped if the header started at the load address,
but it's the data that should be at this address.

Signed-off-by: Uwe Kleine-K?nig <ukleinek@informatik.uni-freiburg.de>
---
I didn't test that change yet but I saw it fail without that change.

Best regards
Uwe

 common/cmd_bootm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 7aae8a6..56cdac8 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -318,7 +318,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 
 	switch (hdr->ih_comp) {
 	case IH_COMP_NONE:
-		if(ntohl(hdr->ih_load) == addr) {
+		if(ntohl(hdr->ih_load) == data) {
 			printf ("   XIP %s ... ", name);
 		} else {
 #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
-- 
1.5.0.3

Uwe Kleine-K?nig wrote:
> Hello Wolfgang,
> 
> > You download the image  to  an  arbitrary  address  which  should  be
> > sufficiently  out  of  the  way  of  the [load_addr,load_addr+size of
> > uncompressed kernel] area. The bootm will  (uncompress  &)  copy  the
> > image to the load_addr, and jumpt to the entry point.
> With the above command line (i.e. -C none) it will only copy it, not
> uncompress.  But probably you meant that, as you put uncompress in
> parenthesis.
> 
> > > For an image created with the above rule from the vanilla kernel, that
> > > means, jump to the header (instead of the actual image at entry point +
> > > 0x40).
> > 
> > No. The header will not be copied by U-Boot's "bootm" command.
> Ah, OK, I see.  Then you may want to take the patch I'll send as a
> follow up.
>  
> > > So I think the linux rule is wrong, or did I miss anything?
> > 
> > Yes. You talk about two addresses, but there are three.
> > 
> > > PS: Linux uses entry point == load address == 0 for PowerPC.  Didn't
> > > look in the PowerPC version of bootm_linux, but maybe it has the same
> > > problem?
> > 
> > It works the same in this respect. Note that I write *it works*.
> > 
> > > [1] For Linux this is not needed, because the decompressor (at least for
> > > 2.6) is position independant.  But that's another topic.
> > 
> > Right - the decompressor  itself  is  not  needed,  as  it's  already
> > included  in U-Boot. Adding it to the Kernel image just wastes memroy
> > and time as it makes booting slower.
> So you suggest to gzip the vmlinux image and use -C gzip?
>  
> Best regards,
> Uwe
> 
> -- 
> Uwe Kleine-K?nig
> 
> http://www.google.com/search?q=12+mol+in+dozen

-- 
Uwe Kleine-K?nig

http://www.google.com/search?q=parsec%5E2*Joule%2FNewton+in+tablespoon

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Wolfgang Denk]

In message <20070315214229.GB27920@informatik.uni-freiburg.de> you wrote:
> Before moving the image was skipped if the header started at the load address,
> but it's the data that should be at this address.
> 
> Signed-off-by: Uwe Kleine-K?nig <ukleinek@informatik.uni-freiburg.de>
> ---
> I didn't test that change yet but I saw it fail without that change.

I reject your patch. It breaks booting XIP images on PowerPC.
See http://www.denx.de/wiki/view/DULG/ConfigureLinuxForXIP for
details.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"We learn from history that we learn nothing from history."
- George Bernard Shaw

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Uwe Kleine-König]

Hello Wolfgang,

Wolfgang Denk wrote:
> In message <20070315214229.GB27920@informatik.uni-freiburg.de> you wrote:
> > Before moving the image was skipped if the header started at the load address,
> > but it's the data that should be at this address.
> > 
> > Signed-off-by: Uwe Kleine-K?nig <ukleinek@informatik.uni-freiburg.de>
> > ---
> > I didn't test that change yet but I saw it fail without that change.
> 
> I reject your patch. It breaks booting XIP images on PowerPC.
> See http://www.denx.de/wiki/view/DULG/ConfigureLinuxForXIP for
> details.
Do you admit that the first branch of the if I changed asserts the
header to start at ih_load and the other asserts the data to start at
ih_load?  So if you have an XIP image in RAM starting at a wrong
address, booting will fail.

So in my eyes the correct fix is to take the patch I sent + fix Linux
not to skip the header.

BTW: I couldn't find this config item "Make a XIP (eXecute in Place)
kernel", neither in Linus nor in your tree:

	zeisberg at cassiopeia:~/gsrc/linux-2.6$ for tree in denx linus; do git describe remotes/$tree/master; git grep "Make a XIP" remotes/$tree/master; done
	v2.6.21-rc3-582-gb0dd6c9
	v2.6.21-rc3-276-gbaab108

And even

	git branch -r | grep denx | xargs git grep "Make a XIP"

didn't return anything.

Best regards
Uwe

-- 
Uwe Kleine-K?nig

primes where sieve (p:xs) = [ x | x<-xs, x `rem` p /= 0 ]; \
primes = map head (iterate sieve [2..])

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Wolfgang Denk]

In message <20070315223514.GA51@informatik.uni-freiburg.de> you wrote:
> 
> Do you admit that the first branch of the if I changed asserts the
> header to start at ih_load and the other asserts the data to start at
> ih_load?  So if you have an XIP image in RAM starting at a wrong
> address, booting will fail.

XIP = eXecute In Place, i. e. the image is run directly from flash /
ROM, without loading it to RAM at all.

> BTW: I couldn't find this config item "Make a XIP (eXecute in Place)
> kernel", neither in Linus nor in your tree:

If you had read the page I referred you to, you should have seen the
link at the end, which indicates pretty clearly which tree was used.

> And even
> 	git branch -r | grep denx | xargs git grep "Make a XIP"
> didn't return anything.

Wrong tree... We did not even import that tree to git yet...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
At the source of every error which is blamed on the computer you will
find at least two human errors, including the error of blaming it  on
the computer.

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Uwe Kleine-König]

Hello Wolfgang,

Wolfgang Denk wrote:
> In message <20070315223514.GA51@informatik.uni-freiburg.de> you wrote:
> > 
> > Do you admit that the first branch of the if I changed asserts the
> > header to start at ih_load and the other asserts the data to start at
> > ih_load?  So if you have an XIP image in RAM starting at a wrong
> > address, booting will fail.
> 
> XIP = eXecute In Place, i. e. the image is run directly from flash /
> ROM, without loading it to RAM at all.
That's clear, yes.  I recognize that my example is notional.  But the
other way round makes more sense.  If you have a non-XIP image then it
is not moved if it was loaded to ih_load or (ih_load - 0x40).

This implies that do_bootm_linux is called either with the data
starting at addr or addr + sizeof(image_header_t).  Then the ugly fix is
to check if (image_header_t)addr->ih_magic == IH_MAGIC and if so move
the image there.  In my eyes this is unneeded code duplication.

Would you think the fix being correct if that xip patch were not relying
on that behaviour?  If so, not taking the patch because of this IMHO is
a poor policy.

> > BTW: I couldn't find this config item "Make a XIP (eXecute in Place)
> > kernel", neither in Linus nor in your tree:
> 
> If you had read the page I referred you to, you should have seen the
> link at the end, which indicates pretty clearly which tree was used.
caught guilty, sorry.

Best regards
Uwe


-- 
Uwe Kleine-K?nig

http://www.google.com/search?q=Planck%27s+constant%3D

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Wolfgang Denk]

In message <20070316090213.GA4940@informatik.uni-freiburg.de> you wrote:
> 
> > XIP = eXecute In Place, i. e. the image is run directly from flash /
> > ROM, without loading it to RAM at all.
> That's clear, yes.  I recognize that my example is notional.  But the
> other way round makes more sense.  If you have a non-XIP image then it
> is not moved if it was loaded to ih_load or (ih_load - 0x40).

If you have a non-XIP image you willnot execute that part of the code.
It was in an if-branch ...

> Would you think the fix being correct if that xip patch were not relying
> on that behaviour?  If so, not taking the patch because of this IMHO is
> a poor policy.

Your "fix" was made to a XIP specific part of the code, where it
breaks existing code.

For non-XIP images I don't see a problem - that works well on all
architectures.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It's all Klatchian to me.
        - Terry Pratchett & Stephen Briggs, _The Discworld Companion_

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Uwe Kleine-König]

Hello Wolfgang,

Wolfgang Denk wrote:
> In message <20070316090213.GA4940@informatik.uni-freiburg.de> you wrote:
> > 
> > > XIP = eXecute In Place, i. e. the image is run directly from flash /
> > > ROM, without loading it to RAM at all.
> > That's clear, yes.  I recognize that my example is notional.  But the
> > other way round makes more sense.  If you have a non-XIP image then it
> > is not moved if it was loaded to ih_load or (ih_load - 0x40).
> 
> If you have a non-XIP image you willnot execute that part of the code.
> It was in an if-branch ...
The only condition I see is:

	hdr->ih_comp == IH_COMP_NONE

Could you please be more verbose and point out which if-branch you mean?

Best regards
Uwe

-- 
Uwe Kleine-K?nig

exit vi, lesson I:
: q ! <CR>

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Wolfgang Denk]

In message <20070321203315.GD16180@informatik.uni-freiburg.de> you wrote:
>
> > If you have a non-XIP image you willnot execute that part of the code.
> > It was in an if-branch ...
> The only condition I see is:
> 
> 	hdr->ih_comp == IH_COMP_NONE
> 
> Could you please be more verbose and point out which if-branch you mean?

The one you suggested to change:

--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -318,7 +318,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 
 	switch (hdr->ih_comp) {
 	case IH_COMP_NONE:
-		if(ntohl(hdr->ih_load) == addr) {
+		if(ntohl(hdr->ih_load) == data) {
 			printf ("   XIP %s ... ", name);
 		} else {

That's exactly the test that is used to decide  if  we  have  an  XIP
image:  if  the  load  address  is identical to the address where the
image is currently stored, it will not be  copied,  but  executed  in
place.  Later,  there is an additional check that entry point address
must be immediately following the header.

And, as mentioned before, your modification will  break  this  logic,
this breaking booting XIP images on PowerPC.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Computers are not intelligent. They only think they are.

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Uwe Kleine-König]

Hello Wolfgang,

Wolfgang Denk wrote:
> In message <20070321203315.GD16180@informatik.uni-freiburg.de> you wrote:
> >
> > > If you have a non-XIP image you willnot execute that part of the code.
> > > It was in an if-branch ...
> > The only condition I see is:
> > 
> > 	hdr->ih_comp == IH_COMP_NONE
> > 
> > Could you please be more verbose and point out which if-branch you mean?
> 
> The one you suggested to change:
> 
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -318,7 +318,7 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  
>  	switch (hdr->ih_comp) {
>  	case IH_COMP_NONE:
> -		if(ntohl(hdr->ih_load) == addr) {
> +		if(ntohl(hdr->ih_load) == data) {
>  			printf ("   XIP %s ... ", name);
>  		} else {
???
I thought you wanted to say that the if is not reached in the non-XIP
case.  Maybe I didn't get what code you mean?

> That's exactly the test that is used to decide  if  we  have  an  XIP
> image:  if  the  load  address  is identical to the address where the
> image is currently stored, it will not be  copied,  but  executed  in
> place.
In my eyes it would be more clear if an XIP image had 

	ih_load = xip_address + sizeof(header)

because then the load address would have the same meaning for both XIP
and non-XIP.  Now it's somehow artifical that I cannot load an non-XIP
image (with header) to it's load address.  

Yes, it is stupid to do so but not more stupid than any other address
different from (ih_load - sizeof(header)).

So the current state is that the load address specifies the start of
the complete image (i.e. with header) for the XIP case and the start of
data (i.e. without header) in the non-XIP case.

> Later,  there is an additional check that entry point address
> must be immediately following the header.
I don't see why this matters here nor the check itself.  

> And, as mentioned before, your modification will  break  this  logic,
> this breaking booting XIP images on PowerPC.
I got that.  As XIP booting on PowerPC relies on an---in my
eyes---broken feature it has to be fixed of course after that feature is
fixed.

Best regards
Uwe

-- 
Uwe Kleine-K?nig

exit vi, lesson II:
: w q ! <CR>

NB: write the current file

[U-Boot-Users] [PATCH] Fix the condition to detect if an image is already in its place

[Wolfgang Denk]

In message <20070321211706.GA17632@informatik.uni-freiburg.de> you wrote:
> 
> > That's exactly the test that is used to decide  if  we  have  an  XIP
> > image:  if  the  load  address  is identical to the address where the
> > image is currently stored, it will not be  copied,  but  executed  in
> > place.
> In my eyes it would be more clear if an XIP image had 
> 
> 	ih_load = xip_address + sizeof(header)
> 
> because then the load address would have the same meaning for both XIP
> and non-XIP.  Now it's somehow artifical that I cannot load an non-XIP
> image (with header) to it's load address.  

That's not the way how it's been implemented more than 5 years ago.

> > And, as mentioned before, your modification will  break  this  logic,
> > this breaking booting XIP images on PowerPC.
> I got that.  As XIP booting on PowerPC relies on an---in my
> eyes---broken feature it has to be fixed of course after that feature is
> fixed.

You are welcome to submit patches  to  support XIP  with  ARCH=powerpc
Linux kernels! [And yes, I will then even happily drop backwards
compatibility with old 2.4.4 kernels - which is something I don't do
light-heartedly ;-)  ]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Landru! Guide us!
	-- A Beta 3-oid, "The Return of the Archons", stardate 3157.4

[U-Boot-Users] arm linux images with load address == entry point

[Uwe Kleine-König]

Hello Wolfgang,

Wolfgang Denk wrote:
> > the make target "uImage" in the linux kernel calls mkimage as follows:
> >	$(MKIMAGE) -A arm -O linux -T kernel \
> >	-C none -a $(ZRELADDR) -e $(ZRELADDR) \
> >	-n 'Linux-$(KERNELRELEASE)' -d $< $@
>
> ...
> 
> You download the image  to  an  arbitrary  address  which  should  be
> sufficiently  out  of  the  way  of  the [load_addr,load_addr+size of
> uncompressed kernel] area. The bootm will  (uncompress  &)  copy  the
> image to the load_addr, and jumpt to the entry point.
So choosing ZRELADDR is not sensible.  This is the address the
decompressed kernel should be placed to.  So the (in-kernel)
decompressor has to move the image for sure.  So worst case I loaded it
to address X, U-Boot moved it to ZRELADDR and the decompressor moves it
a 2nd time because it wants to decompress to ZRELADDR.

Best regards
Uwe

-- 
Uwe Kleine-K?nig

http://www.google.com/search?q=Planck%27s+constant%3D