Re: [U-Boot-Users] [PATCH] Make MPC83xx one step closer to full relocation.
Hi Joakim,
On Saturday 29 March 2008, Joakim Tjernlund wrote:
Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
Remove a few absolute references to CFG_MONITOR_BASE for ppc/mpc83xx and use GOT relative reference.
cpu/mpc83xx/start.S | 11 +++++++---- lib_ppc/board.c | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-)
<snip>
btw, shouldn't it fix up the other ppc start.S files?
The other ppc's won't break and I only got a 83xx, hence I only did it for 83xx, hoping it would serve as a guide for the rest.
I'm afraid, but the "other ppc's" did break with this patch. At least 4xx does. With your patch applied relocation to SDRAM does not work anymore. Here's what I get:
CFG_MONITOR_BASE=fffa0000 (ulong)&_start + EXC_OFF_SYS_RESET=fffa2200 EXC_OFF_SYS_RESET=100
I haven't looked into it closer yet. Any idea on how to fix this?
Thanks.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
On Tue, 2008-04-08 at 10:58 +0200, Stefan Roese wrote:
Hi Joakim,
On Saturday 29 March 2008, Joakim Tjernlund wrote:
Joakim Tjernlund Joakim.Tjernlund@transmode.se wrote:
Remove a few absolute references to CFG_MONITOR_BASE for ppc/mpc83xx and use GOT relative reference.
cpu/mpc83xx/start.S | 11 +++++++---- lib_ppc/board.c | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-)
<snip>
btw, shouldn't it fix up the other ppc start.S files?
The other ppc's won't break and I only got a 83xx, hence I only did it for 83xx, hoping it would serve as a guide for the rest.
I'm afraid, but the "other ppc's" did break with this patch. At least 4xx does. With your patch applied relocation to SDRAM does not work anymore. Here's what I get:
CFG_MONITOR_BASE=fffa0000 (ulong)&_start + EXC_OFF_SYS_RESET=fffa2200 EXC_OFF_SYS_RESET=100
I haven't looked into it closer yet. Any idea on how to fix this?
Thanks.
Best regards, Stefan
Oops, didn't see that coming. Your _start symbol in ppc4xx/start.S isn't pointing to your real start of execution. Seems like _start_440 is your real start but I can't be sure. There are some strange code in there that I don't understand.
Jocke
On Tuesday 08 April 2008, Joakim Tjernlund wrote:
I'm afraid, but the "other ppc's" did break with this patch. At least 4xx does. With your patch applied relocation to SDRAM does not work anymore. Here's what I get:
CFG_MONITOR_BASE=fffa0000 (ulong)&_start + EXC_OFF_SYS_RESET=fffa2200 EXC_OFF_SYS_RESET=100
I haven't looked into it closer yet. Any idea on how to fix this?
Thanks.
Best regards, Stefan
Oops, didn't see that coming. Your _start symbol in ppc4xx/start.S isn't pointing to your real start of execution. Seems like _start_440 is your real start but I can't be sure.
No, unfortunately it's not. _start_440 is loaded into the last 4k of bootrom (via linker script), since 440 has a shadow TLB upon startup to only map 4k of address space. After looking at System.map it seems that _start_of_vectors seems to be the way to go for 4xx:
fffa0004 T version_string fffa0100 t CritcalInput fffa0100 T _start_of_vectors
But I don't want to introduce some #ifdef here. Perhaps it would be better to introduce a common (PPC) label that points to the beginning of the U-Boot image here.
BTW: I think this version:
len = (ulong)&_end - ((ulong)&_start - EXC_OFF_SYS_RESET);
instead of:
len = (ulong)&_end - (ulong)&_start + EXC_OFF_SYS_RESET;
is better. Makes the transition from CFG_MONITOR_BASE clearer, don't you think so?
There are some strange code in there that I don't understand.
Yes, it's quite complex because it supports all 4xx variants.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
On Tue, 2008-04-08 at 12:06 +0200, Stefan Roese wrote:
On Tuesday 08 April 2008, Joakim Tjernlund wrote:
I'm afraid, but the "other ppc's" did break with this patch. At least 4xx does. With your patch applied relocation to SDRAM does not work anymore. Here's what I get:
CFG_MONITOR_BASE=fffa0000 (ulong)&_start + EXC_OFF_SYS_RESET=fffa2200 EXC_OFF_SYS_RESET=100
I haven't looked into it closer yet. Any idea on how to fix this?
Thanks.
Best regards, Stefan
Oops, didn't see that coming. Your _start symbol in ppc4xx/start.S isn't pointing to your real start of execution. Seems like _start_440 is your real start but I can't be sure.
No, unfortunately it's not. _start_440 is loaded into the last 4k of bootrom (via linker script), since 440 has a shadow TLB upon startup to only map 4k of address space. After looking at System.map it seems that _start_of_vectors seems to be the way to go for 4xx:
fffa0004 T version_string fffa0100 t CritcalInput fffa0100 T _start_of_vectors
But I don't want to introduce some #ifdef here. Perhaps it would be better to introduce a common (PPC) label that points to the beginning of the U-Boot image here.
I first did a new symbol for this but changed it to use _start as I didn't want to introduce yet another symbol. I would hope it is possible to rework 4xx to move the _start symbol to were it belongs? Not having the _start symbol where it should be could bite you some other day too.
BTW: I think this version:
len = (ulong)&_end - ((ulong)&_start - EXC_OFF_SYS_RESET);
instead of:
len = (ulong)&_end - (ulong)&_start + EXC_OFF_SYS_RESET;
is better. Makes the transition from CFG_MONITOR_BASE clearer, don't you think so?
Sure, but I don't feel that strongly about it.
In message 1207651833.5826.21.camel@gentoo-jocke.transmode.se you wrote:
CFG_MONITOR_BASE=fffa0000
...
I first did a new symbol for this but changed it to use _start as I didn't want to introduce yet another symbol. I would hope it is possible to rework 4xx to move the _start symbol to were it belongs? Not having the _start symbol where it should be could bite you some other day too.
I think you got this wrong. _start *is* where it should be, where the execution of the code begins. Where you set the breakpoint in gdb.
The "new symbol" you mention corresponds to CFG_MONITOR_BASE, but note that it has no fixed address as it depends where you place your image in flash.
Best regards,
Wolfgang Denk
On Tue, 2008-04-08 at 13:58 +0200, Wolfgang Denk wrote:
In message 1207651833.5826.21.camel@gentoo-jocke.transmode.se you wrote:
CFG_MONITOR_BASE=fffa0000
...
I first did a new symbol for this but changed it to use _start as I didn't want to introduce yet another symbol. I would hope it is possible to rework 4xx to move the _start symbol to were it belongs? Not having the _start symbol where it should be could bite you some other day too.
I think you got this wrong. _start *is* where it should be, where the execution of the code begins. Where you set the breakpoint in gdb.
Ah, right. I *assumed* that _start was always the first symbol in the text segment too. On 4xx it isn't for some reason, but it should be possible to move it first.
Sidnote: Does execution really begin at _start for 4xx? I still looks like it begin at _start_440 and eventually end up at _start
The "new symbol" you mention corresponds to CFG_MONITOR_BASE, but note that it has no fixed address as it depends where you place your image in flash.
Yes, but the address is not important here, it is the difference _end - _start. I guess we could define a linker symbol too that calculates _end - _start for us and then just do len = _uboot_reloc_size + EXC_OFF_SYS_RESET;
or define a new symbol that is initialised to CFG_MONITOR_BASE or let the linker place it at the beginning of .text, hopefully there is already a name reserved for the symbol although I don't know of such a name. I either case I think one needs to add that symbol to the GOT list in start.S
Jocke
On Tuesday 08 April 2008, Joakim Tjernlund wrote:
I first did a new symbol for this but changed it to use _start as I didn't want to introduce yet another symbol. I would hope it is possible to rework 4xx to move the _start symbol to were it belongs? Not having the _start symbol where it should be could bite you some other day too.
I think you got this wrong. _start *is* where it should be, where the execution of the code begins. Where you set the breakpoint in gdb.
Ah, right. I *assumed* that _start was always the first symbol in the text segment too. On 4xx it isn't for some reason, but it should be possible to move it first.
Sidnote: Does execution really begin at _start for 4xx? I still looks like it begin at _start_440 and eventually end up at _start
On 4xx execution always starts at 0xfffffffc (last lword in 32bit address space). This location holds a jump to _start for 405 PPC's and to _start_440 for 440 PPC's. 440 PPC's need some extended initialization (TLB setup etc) and later jump to the 4xx common _start.
The "new symbol" you mention corresponds to CFG_MONITOR_BASE, but note that it has no fixed address as it depends where you place your image in flash.
Yes, but the address is not important here, it is the difference _end - _start. I guess we could define a linker symbol too that calculates _end - _start for us and then just do len = _uboot_reloc_size + EXC_OFF_SYS_RESET;
or define a new symbol that is initialised to CFG_MONITOR_BASE or let the linker place it at the beginning of .text, hopefully there is already a name reserved for the symbol although I don't know of such a name. I either case I think one needs to add that symbol to the GOT list in start.S
I'm an linker script dyslexic. So no idea if we can handle this solely in the linker script or if we need a new common symbol in the PPC start.S's.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
On Tue, 2008-04-08 at 15:25 +0200, Stefan Roese wrote:
On Tuesday 08 April 2008, Joakim Tjernlund wrote:
I first did a new symbol for this but changed it to use _start as I didn't want to introduce yet another symbol. I would hope it is possible to rework 4xx to move the _start symbol to were it belongs? Not having the _start symbol where it should be could bite you some other day too.
I think you got this wrong. _start *is* where it should be, where the execution of the code begins. Where you set the breakpoint in gdb.
Ah, right. I *assumed* that _start was always the first symbol in the text segment too. On 4xx it isn't for some reason, but it should be possible to move it first.
Sidnote: Does execution really begin at _start for 4xx? I still looks like it begin at _start_440 and eventually end up at _start
On 4xx execution always starts at 0xfffffffc (last lword in 32bit address space). This location holds a jump to _start for 405 PPC's and to _start_440 for 440 PPC's. 440 PPC's need some extended initialization (TLB setup etc) and later jump to the 4xx common _start.
OK, then it is like I suspected. What if you rename _start to _common_start. Make _start equal _common_start for 405 and rename _start_440 to _start, i.e make sure that _start is defined where you start executing after the jump.
The "new symbol" you mention corresponds to CFG_MONITOR_BASE, but note that it has no fixed address as it depends where you place your image in flash.
Yes, but the address is not important here, it is the difference _end - _start. I guess we could define a linker symbol too that calculates _end - _start for us and then just do len = _uboot_reloc_size + EXC_OFF_SYS_RESET;
or define a new symbol that is initialised to CFG_MONITOR_BASE or let the linker place it at the beginning of .text, hopefully there is already a name reserved for the symbol although I don't know of such a name. I either case I think one needs to add that symbol to the GOT list in start.S
I'm an linker script dyslexic. So no idea if we can handle this solely in the linker script or if we need a new common symbol in the PPC start.S's.
Both ways should be doable I think. A linker script would probably look something like(pseudo diff below): .text : { + _monitor_base = . ; + PROVIDE (_monitor_base = .); cpu/mpc83xx/start.o (.text)
And then add a GOT_ENTRY(_monitor_base) in start.S
On Tuesday 08 April 2008, Joakim Tjernlund wrote:
On 4xx execution always starts at 0xfffffffc (last lword in 32bit address space). This location holds a jump to _start for 405 PPC's and to _start_440 for 440 PPC's. 440 PPC's need some extended initialization (TLB setup etc) and later jump to the 4xx common _start.
OK, then it is like I suspected. What if you rename _start to _common_start. Make _start equal _common_start for 405 and rename _start_440 to _start, i.e make sure that _start is defined where you start executing after the jump.
As I mentioned earlier, _start_440 is mapped to 0xfffff000 via the linker script since the jump from 0xfffffffc can't be too long (because of the 4k shadow TLB entry). So renaming _start_440 to _start won't help here. It can be done, but frankly I don't have the time for it currently.
I'm an linker script dyslexic. So no idea if we can handle this solely in the linker script or if we need a new common symbol in the PPC start.S's.
Both ways should be doable I think. A linker script would probably look something like(pseudo diff below): .text : {
- _monitor_base = . ;
- PROVIDE (_monitor_base = .); cpu/mpc83xx/start.o (.text)
And then add a GOT_ENTRY(_monitor_base) in start.S
The disadvantage I see is that I need to change the linker scripts for all boards for such a solution. Doesn't sound like fun.
So for now, I would really like to see the old version with the ugly CFG_MONITOR_BASE back so that 4xx board can be used again.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================
-----Original Message----- From: u-boot-users-bounces@lists.sourceforge.net [mailto:u-boot-users-bounces@lists.sourceforge.net] On Behalf Of Stefan Roese Sent: den 8 april 2008 21:53 To: joakim.tjernlund@transmode.se Cc: u-boot-users@lists.sourceforge.net; 'Kim Phillips'; Wolfgang Denk Subject: Re: [U-Boot-Users] [PATCH] Make MPC83xx one step closer to full relocation.
On Tuesday 08 April 2008, Joakim Tjernlund wrote:
On 4xx execution always starts at 0xfffffffc (last lword in 32bit address space). This location holds a jump to _start for 405 PPC's and to _start_440 for 440 PPC's. 440 PPC's need some extended initialization (TLB setup etc) and later jump to the 4xx common _start.
OK, then it is like I suspected. What if you rename _start to _common_start. Make _start equal _common_start for 405 and rename _start_440 to _start, i.e make sure that _start is defined where you start executing after the jump.
As I mentioned earlier, _start_440 is mapped to 0xfffff000 via the linker script since the jump from 0xfffffffc can't be too long (because of the 4k shadow TLB entry). So renaming _start_440 to _start won't help here. It can be done, but frankly I don't have the time for it currently.
I see, suspected that we would not get away that easy :(
I'm an linker script dyslexic. So no idea if we can handle this solely in the linker script or if we need a new common symbol in the PPC start.S's.
Both ways should be doable I think. A linker script would probably look something like(pseudo diff below): .text : {
- _monitor_base = . ;
- PROVIDE (_monitor_base = .); cpu/mpc83xx/start.o (.text)
And then add a GOT_ENTRY(_monitor_base) in start.S
The disadvantage I see is that I need to change the linker scripts for all boards for such a solution. Doesn't sound like fun.
Yeah, probably easier to define the symbol in start.S and skip the linker version.
So for now, I would really like to see the old version with the ugly CFG_MONITOR_BASE back so that 4xx board can be used again.
Then I would ask you to do an #ifdef for ppc440(or similar). If the long term solution should be to move _start to its true start vector, you could just go back to the old way for ppc4xx. If the long term solution should be a new symbol you could just impl. that for ppc4xx and just use it for ppc4xx only, then the other archs can follow when the next merge window opens.
Jocke
Best regards, Stefan
In message 1207663498.5826.56.camel@gentoo-jocke.transmode.se you wrote:
Both ways should be doable I think. A linker script would probably look something like(pseudo diff below): .text : {
- _monitor_base = . ;
There is no guarantee that this is the same as the start address of .text
Best regards,
Wolfgang Denk
-----Original Message----- From: wd@denx.de [mailto:wd@denx.de] Sent: den 8 april 2008 22:42 To: joakim.tjernlund@transmode.se Cc: Stefan Roese; u-boot-users@lists.sourceforge.net; 'Kim Phillips' Subject: Re: [U-Boot-Users] [PATCH] Make MPC83xx one step closer to full relocation.
In message 1207663498.5826.56.camel@gentoo-jocke.transmode.se you wrote:
Both ways should be doable I think. A linker script would probably look something like(pseudo diff below): .text : {
- _monitor_base = . ;
There is no guarantee that this is the same as the start address of .text
I am no link script expert, but I cannot see why that would not be the start of the .text segment?
Anyhow, we can go with the other solution instead, define the symbol in start.S instead, if one is needed.
Jocke
In message 1207647084.5826.14.camel@gentoo-jocke.transmode.se you wrote:
Oops, didn't see that coming. Your _start symbol in ppc4xx/start.S isn't pointing to your real start of execution. Seems like _start_440 is your real start but I can't be sure. There are some strange code in there that I don't understand.
Of course. _start is where the execution of the code starts, i. e. the entry point for U-Boot. There is no reason to expect that this might be at the begin of any specific section.
Best regards,
Wolfgang Denk
participants (4)
-
Joakim Tjernlund -
Joakim Tjernlund
-
Stefan Roese -
Wolfgang Denk