[U-Boot] ARM relocation, probably trivial mistake
Hello,
after enabling relocation I get the following output:
U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 16:57:43)
U-Boot code: 21F00000 -> 21F3C140 BSS: -> 21F7D700 CPU: AT91SAM9XE Crystal frequency: 18.432 MHz CPU clock : 198.656 MHz Master clock : 99.328 MHz I2C: ready monitor len: 0007D700 <<<< this must be wrong, the binary is short of 256k large ramsize: 04000000 <<<< correct Top of RAM usable for U-Boot at: 24000000 <<<< correct Reserving 501k for U-Boot at: 23f82000 Reserving 143k for malloc() at: 23f5e100 Reserving 24 Bytes for Board Info at: 23f5e0e8 Reserving 88 Bytes for Global Data at: 23f5e090 New Stack Pointer is: 23f5e088 RAM Configuration: Bank #0: 20000000 64 MiB <<< correct relocation Offset is: 02082000 <<<< could be right
<<< system hangs here >>>
Details: initial bootstrap loads u-boot to RAM at 21f00000 (1MiB short of 32 MiB which is the minimum RAM populated)
I have set the following values: #define CONFIG_SKIP_LOWLEVEL_INIT #define CONFIG_SKIP_RELOCATE_UBOOT /*#define CONFIG_SYS_ARM_WITHOUT_RELOC*/ #define CONFIG_SYS_NO_ICACHE #define CONFIG_SYS_NO_DCACHE
and
/* SDRAM: min 32, upto 128 MB */ #define CONFIG_NR_DRAM_BANKS 1 #define PHYS_SDRAM 0x20000000 #define PHYS_SDRAM_MAX_SIZE 0x08000000 #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM #define CONFIG_SYS_MEMTEST_END 0x21e00000 #define CONFIG_SYS_LOAD_ADDR (PHYS_SDRAM+0x01000000) /* 16 MB into RAM */ #define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x1000 - CONFIG_SYS_GBL_DATA_SIZE)
and in config.mk:
TEXT_BASE = 0x21f00000
and
int dram_init(void) { gd->ram_size = get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE); return 0; }
void dram_init_banksize(void) { gd->bd->bi_dram[0].start = PHYS_SDRAM; gd->bd->bi_dram[0].size = get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE); }
I do probably have a simple misunderstanding of the concept.
1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
2. TEXT_BASE must be the address where the u-boot is loaded by 1st loader and startet, right?
With Best Regards, Reinhard
Reinhard Meyer wrote:
Hello,
Hi Reinhard,
int dram_init(void) { gd->ram_size = get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE); return 0; }
void dram_init_banksize(void) { gd->bd->bi_dram[0].start = PHYS_SDRAM; gd->bd->bi_dram[0].size = get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE); }
Do we need both functions ?. I do not add dram_init_banksize() because I have as you only one bank, and the linker does not complain. However, which dram_init_banksize is linked in your image ? Yours or the function defined in arch/arm/lib/board.c ?
I do probably have a simple misunderstanding of the concept.
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
I had the same issue because I forget to remove it from my config file. After dropping it, I got the u-boot prompt again ;-)
Best regards, Stefano Babic
Dear Stefano Babic,
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
I had the same issue because I forget to remove it from my config file. After dropping it, I got the u-boot prompt again ;-)
Thank you, that (partially) helped:
U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 17:32:36)
U-Boot code: 21F00000 -> 21F3C180 BSS: -> 21F7D700 CPU: AT91SAM9XE Crystal frequency: 18.432 MHz CPU clock : 198.656 MHz Master clock : 99.328 MHz I2C: ready monitor len: 0007D700 ramsize: 04000000 Top of RAM usable for U-Boot at: 24000000 Reserving 501k for U-Boot at: 23f82000 Reserving 143k for malloc() at: 23f5e100 Reserving 24 Bytes for Board Info at: 23f5e0e8 Reserving 88 Bytes for Global Data at: 23f5e090 New Stack Pointer is: 23f5e088 RAM Configuration: Bank #0: 20000000 64 MiB relocation Offset is: 02082000 monitor flash len: 0003C180 Now running in RAM - U-Boot at: 23f82000 FLASH: 512 KiB NAND: raise: Signal # 8 caught raise: Signal # 8 caught raise: Signal # 8 caught 256 MiB *** Warning - bad CRC, using default environment
In: serial Out: serial Err: serial MMC: mci: 0 Net: macb0, enc1.0, enc1.1, enc1.2 Reset Ethernet PHY macb0: Starting autonegotiation... macb0: Autonegotiation timed out (status=0x7849) macb0: link down (status: 0x7849) enc1.0: timeout waiting for CLKRDY enc1.1: timeout waiting for CLKRDY enc1.2: timeout waiting for CLKRDY Press SPACE to abort autoboot in 1 seconds
Before the relocation: macb, enc and environment did work :) I have to look into that issue later...
Best Regards, Reinhard
Hello Reinhard,
Reinhard Meyer wrote:
Dear Stefano Babic,
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
I had the same issue because I forget to remove it from my config file. After dropping it, I got the u-boot prompt again ;-)
Thank you, that (partially) helped:
U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 17:32:36)
U-Boot code: 21F00000 -> 21F3C180 BSS: -> 21F7D700 CPU: AT91SAM9XE Crystal frequency: 18.432 MHz CPU clock : 198.656 MHz Master clock : 99.328 MHz I2C: ready monitor len: 0007D700 ramsize: 04000000 Top of RAM usable for U-Boot at: 24000000 Reserving 501k for U-Boot at: 23f82000 Reserving 143k for malloc() at: 23f5e100 Reserving 24 Bytes for Board Info at: 23f5e0e8 Reserving 88 Bytes for Global Data at: 23f5e090 New Stack Pointer is: 23f5e088 RAM Configuration: Bank #0: 20000000 64 MiB relocation Offset is: 02082000 monitor flash len: 0003C180 Now running in RAM - U-Boot at: 23f82000 FLASH: 512 KiB NAND: raise: Signal # 8 caught raise: Signal # 8 caught raise: Signal # 8 caught 256 MiB *** Warning - bad CRC, using default environment
In: serial Out: serial Err: serial MMC: mci: 0 Net: macb0, enc1.0, enc1.1, enc1.2 Reset Ethernet PHY macb0: Starting autonegotiation... macb0: Autonegotiation timed out (status=0x7849) macb0: link down (status: 0x7849) enc1.0: timeout waiting for CLKRDY enc1.1: timeout waiting for CLKRDY enc1.2: timeout waiting for CLKRDY Press SPACE to abort autoboot in 1 seconds
Before the relocation: macb, enc and environment did work :) I have to look into that issue later...
No spontaneous idea ... maybe something with gpio pins?
bye, Heiko
Dear Heiko Schocher,
Before the relocation: macb, enc and environment did work :) I have to look into that issue later...
No spontaneous idea ... maybe something with gpio pins?
The relocation should ideally not affect that ;)
I suspect the initiallization now does not call all functions or in a different order...
I'll have to look deeper into that.
Reinhard
Hello Reinhard,
Reinhard Meyer wrote:
Hello,
after enabling relocation I get the following output:
U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 16:57:43)
U-Boot code: 21F00000 -> 21F3C140 BSS: -> 21F7D700 CPU: AT91SAM9XE Crystal frequency: 18.432 MHz CPU clock : 198.656 MHz Master clock : 99.328 MHz I2C: ready monitor len: 0007D700 <<<< this must be wrong, the binary is short of 256k large ramsize: 04000000 <<<< correct Top of RAM usable for U-Boot at: 24000000 <<<< correct Reserving 501k for U-Boot at: 23f82000 Reserving 143k for malloc() at: 23f5e100 Reserving 24 Bytes for Board Info at: 23f5e0e8 Reserving 88 Bytes for Global Data at: 23f5e090 New Stack Pointer is: 23f5e088 RAM Configuration: Bank #0: 20000000 64 MiB <<< correct relocation Offset is: 02082000 <<<< could be right
<<< system hangs here >>>
Details: initial bootstrap loads u-boot to RAM at 21f00000 (1MiB short of 32 MiB which is the minimum RAM populated)
I have set the following values: #define CONFIG_SKIP_LOWLEVEL_INIT #define CONFIG_SKIP_RELOCATE_UBOOT
With this, your TEXT_BASE must be == relocation address, Probably this is not the case ...
/*#define CONFIG_SYS_ARM_WITHOUT_RELOC*/ #define CONFIG_SYS_NO_ICACHE #define CONFIG_SYS_NO_DCACHE
and
/* SDRAM: min 32, upto 128 MB */ #define CONFIG_NR_DRAM_BANKS 1 #define PHYS_SDRAM 0x20000000 #define PHYS_SDRAM_MAX_SIZE 0x08000000 #define CONFIG_SYS_MEMTEST_START PHYS_SDRAM #define CONFIG_SYS_MEMTEST_END 0x21e00000 #define CONFIG_SYS_LOAD_ADDR (PHYS_SDRAM+0x01000000) /* 16 MB into RAM */ #define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x1000 - CONFIG_SYS_GBL_DATA_SIZE)
and in config.mk:
TEXT_BASE = 0x21f00000
and
int dram_init(void) { gd->ram_size = get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE); return 0; }
void dram_init_banksize(void) { gd->bd->bi_dram[0].start = PHYS_SDRAM; gd->bd->bi_dram[0].size = get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE); }
This function is not necessary, because it is equal to the default in arch/arm/lib/board.c
I do probably have a simple misunderstanding of the concept.
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
If TEXT_BASE == relocation address it could be defined, never tested.
- TEXT_BASE must be the address where the u-boot is loaded
by 1st loader and startet, right?
Yep.
bye, Heiko
Dear Heiko Schocher,
In message 4CA4AEFF.3050101@denx.de you wrote:
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
If TEXT_BASE == relocation address it could be defined, never tested.
We should point out that this is only intended (and allowed) for boards where U-Boot gets loaded into RAM by some other means (like a first stage boot loader, say when booting from NAND or SDCard).
When booting from NOR or some other persistent memory TEXT_BASE will point to that address range, and then it is obviously impossible to use the same value as relocation address.
Best regards,
Wolfgang Denk
Hello Wolfgang,
Wolfgang Denk wrote:
Dear Heiko Schocher,
In message 4CA4AEFF.3050101@denx.de you wrote:
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
If TEXT_BASE == relocation address it could be defined, never tested.
We should point out that this is only intended (and allowed) for boards where U-Boot gets loaded into RAM by some other means (like a first stage boot loader, say when booting from NAND or SDCard).
When booting from NOR or some other persistent memory TEXT_BASE will point to that address range, and then it is obviously impossible to use the same value as relocation address.
Yes, you are right, that just work on boards, which use a preloader ...
bye, Heiko
Le 01/10/2010 07:25, Heiko Schocher a écrit :
Hello Wolfgang,
Wolfgang Denk wrote:
Dear Heiko Schocher,
In message4CA4AEFF.3050101@denx.de you wrote:
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
If TEXT_BASE == relocation address it could be defined, never tested.
We should point out that this is only intended (and allowed) for boards where U-Boot gets loaded into RAM by some other means (like a first stage boot loader, say when booting from NAND or SDCard).
When booting from NOR or some other persistent memory TEXT_BASE will point to that address range, and then it is obviously impossible to use the same value as relocation address.
Yes, you are right, that just work on boards, which use a preloader ...
bye, Heiko
Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all boards which use it are made to support relocation, as the code will always check if it runs at its intended location and relocate only if needed.
Of course, code which is loaded in RAM at a location different from its intended one (e.g., during a debugging session) will relocate -- which raises the interesting question of overlapping source and destination areas. That might of course be completely solved, if there is room enough in IRAM (or cache memory) by putting the relocating code there alongside initial stack, jumping to it from FLASH, and having it jump back to the RAM location of the rest of the code. But is it worth it?
Amicalement,
Hello Albert,
Albert ARIBAUD wrote:
Le 01/10/2010 07:25, Heiko Schocher a écrit :
Hello Wolfgang,
Wolfgang Denk wrote:
Dear Heiko Schocher,
In message4CA4AEFF.3050101@denx.de you wrote:
- should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
If TEXT_BASE == relocation address it could be defined, never tested.
We should point out that this is only intended (and allowed) for boards where U-Boot gets loaded into RAM by some other means (like a first stage boot loader, say when booting from NAND or SDCard).
When booting from NOR or some other persistent memory TEXT_BASE will point to that address range, and then it is obviously impossible to use the same value as relocation address.
Yes, you are right, that just work on boards, which use a preloader ...
bye, Heiko
Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all boards which use it are made to support relocation, as the code will always check if it runs at its intended location and relocate only if needed.
Yes, maybe we come to this solution.
Of course, code which is loaded in RAM at a location different from its intended one (e.g., during a debugging session) will relocate -- which raises the interesting question of overlapping source and destination areas. That might of course be completely solved, if there is room enough in IRAM (or cache memory) by putting the relocating code there alongside initial stack, jumping to it from FLASH, and having it jump back to the RAM location of the rest of the code. But is it worth it?
When starting from Flash, this is no problem, because relocation code runs from Flash, and copies u-boot to the relocation address in ram and jumps to it. So no need to copy relocation code.
If u-boot starts in RAM, then it is the task from the preloader where it copies u-boot code, and if there is somewhere in IRAM enough room for it, this would be an option. Otherwise it is a problem if relocation results in overlapping source and destination areas ... but you will fast detect this problem, when you see, that u-boot no longer works ;-)
bye, Heiko
Dear Heiko Schocher,
In message4CA4AEFF.3050101@denx.de you wrote:
If TEXT_BASE == relocation address it could be defined, never tested.
If I think right, relocation address would vary with u-boot size, so there seems no really reliable way to ensure above "if" is true. Even worse, when it happens to become untrue because the size changed, it will try to relocate overlapping areas.
Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all boards which use it are made to support relocation, as the code will always check if it runs at its intended location and relocate only if needed.
"intended location" == "relocation address based on ram size" ??
If u-boot starts in RAM, then it is the task from the preloader where it copies u-boot code, and if there is somewhere in IRAM enough room for it, this would be an option. Otherwise it is a problem if relocation results in overlapping source and destination areas ... but you will fast detect this problem, when you see, that u-boot no longer works ;-)
I think it would be safe that a preloader loads u-boot to the very bottom of SDRAM. If the SDRAM is not at least double the size of u-boot, u-boot needs a diet ;)
Reinhard
Hello Reinhard,
Reinhard Meyer wrote:
Dear Heiko Schocher,
In message4CA4AEFF.3050101@denx.de you wrote:
If TEXT_BASE == relocation address it could be defined, never tested.
If I think right, relocation address would vary with u-boot size, so there seems no really reliable way to ensure above "if" is true. Even worse, when it happens to become untrue because the size changed, it will try to relocate overlapping areas.
Yes, true, but you can with each release try your u-boot, and eventually fix it ...
Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all boards which use it are made to support relocation, as the code will always check if it runs at its intended location and relocate only if needed.
"intended location" == "relocation address based on ram size" ??
admitted, just a good idea for boards where ramsize is always fix ...
If u-boot starts in RAM, then it is the task from the preloader where it copies u-boot code, and if there is somewhere in IRAM enough room for it, this would be an option. Otherwise it is a problem if relocation results in overlapping source and destination areas ... but you will fast detect this problem, when you see, that u-boot no longer works ;-)
I think it would be safe that a preloader loads u-boot to the very bottom of SDRAM. If the SDRAM is not at least double the size of u-boot, u-boot needs a diet ;)
Yep. Idea behind my "intended location == relocation address" is to save one copy of u-boot (one from the preloader and one from relocation) ... but this is not a need for all boards, this is something a board maintainer can decide and use (for example, if speed is a critical point) ...
bye, Heiko
Dear Heiko Schocher,
In message 4CA58665.8080309@denx.de you wrote:
"intended location" == "relocation address based on ram size" ??
admitted, just a good idea for boards where ramsize is always fix ...
This is _not_ sufficient.
Assume PRAM support.
Best regards,
Wolfgang Denk
Le 01/10/2010 08:39, Reinhard Meyer a écrit :
Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all boards which use it are made to support relocation, as the code will always check if it runs at its intended location and relocate only if needed.
"intended location" == "relocation address based on ram size" ??
Correct for those boards which support relocation. There are still a few which dont, and which still have a fixed "intended location" -- that should become less and less prevalent in the coming months.
If u-boot starts in RAM, then it is the task from the preloader where it copies u-boot code, and if there is somewhere in IRAM enough room for it, this would be an option. Otherwise it is a problem if relocation results in overlapping source and destination areas ... but you will fast detect this problem, when you see, that u-boot no longer works ;-)
I think it would be safe that a preloader loads u-boot to the very bottom of SDRAM. If the SDRAM is not at least double the size of u-boot, u-boot needs a diet ;)
If you make a requirement that the preloader put u-boot in the lowest possible RAM location, and if you rewrite the relocation code to move from end to start of the source and target locations, rather than from start to end, then it will work even for RAM sizes only slightly bigger than the u-boot size :) (and it will still work in boot-from-NOR cases).
Amicalement,
Hello,
it seems, that with relocation enabled, some data does not seem to get initialized properly:
w/o relocation:
mmci mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 Device: mci Manufacturer ID: 89 OEM: 303 Name: NCard Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 2006974464 Bus Width: 4-bit
with relocation:
mmci mci: setting clock 0 Hz, block size 512 mci: setting clock 0 Hz, block size 512 mci: setting clock 0 Hz, block size 512 mci: setting clock 0 Hz, block size 512 mci: setting clock 0 Hz, block size 512 Device: mci Manufacturer ID: 89 OEM: 303 Name: NCard Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 2006974464 Bus Width: 4-bit
Do I understand right what the required changes are:
1. change dram_init 2. make sure TEXT_BASE is correct as to where u-boot is loaded by a preloader and is NOT pointing near top of RAM. 3. I do not have a board specific .lds - should I now have one?
Reinhard
Hello Reinhard,
Reinhard Meyer wrote:
it seems, that with relocation enabled, some data does not seem to get initialized properly:
w/o relocation:
mmci mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 194000 Hz, block size 512 mci: setting clock 24832000 Hz, block size 512 Device: mci Manufacturer ID: 89 OEM: 303 Name: NCard Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 2006974464 Bus Width: 4-bit
with relocation:
mmci mci: setting clock 0 Hz, block size 512
Hmm.. mabe something with at91_clock_init()
This is called in arch_cpu_init(), and at the end, clocks are stored in
arch/arm/cpu/arm926ejs/at91/clock.c
in "static unsigned long" vars ... as this code runs before relocation, this seems to me as it could be the reason for your problems ... but I can;t try it here ... can you check this?
mci: setting clock 0 Hz, block size 512 mci: setting clock 0 Hz, block size 512 mci: setting clock 0 Hz, block size 512 mci: setting clock 0 Hz, block size 512 Device: mci Manufacturer ID: 89 OEM: 303 Name: NCard Tran Speed: 25000000 Rd Block Len: 512 SD version 2.0 High Capacity: No Capacity: 2006974464 Bus Width: 4-bit
Do I understand right what the required changes are:
- change dram_init
Yep.
- make sure TEXT_BASE is correct as to where u-boot is loaded
by a preloader and is NOT pointing near top of RAM.
Yep.
- I do not have a board specific .lds - should I now have one?
No.
bye, Heiko
Dear Heiko Schocher,
Hmm.. mabe something with at91_clock_init()
This is called in arch_cpu_init(), and at the end, clocks are stored in
arch/arm/cpu/arm926ejs/at91/clock.c
in "static unsigned long" vars ... as this code runs before relocation, this seems to me as it could be the reason for your problems ... but I can;t try it here ... can you check this?
Indeed, that is called before relocation, because the clock frequencies are needed by almost all drivers, including the timer.
I am unsure now how to fix this. I see two options:
1. calculate afresh every time a get_*_clk_rate() is called - thus not needing static storage
2. store the values in bd->something (if it's permissible to expand bd)
Reinhard
Dear Reinhard Meyer,
In message 4CA5BB7A.8050304@emk-elektronik.de you wrote:
in "static unsigned long" vars ... as this code runs before relocation, this seems to me as it could be the reason for your problems ... but I can;t try it here ... can you check this?
Indeed, that is called before relocation, because the clock frequencies are needed by almost all drivers, including the timer.
...which is, and always has been, illegal.
I am unsure now how to fix this. I see two options:
- calculate afresh every time a get_*_clk_rate() is
called - thus not needing static storage
- store the values in bd->something (if it's permissible
to expand bd)
Not bd-> but gd-> which was made for exactly that purpose.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
...which is, and always has been, illegal.
<nitpick> it became illegal once u-boot for AT91 became required to be relocated </nitpick>
Not bd-> but gd-> which was made for exactly that purpose.
typedef struct global_data...
I will try that. And fix the whitespace error as well...:)
Thanks, Reinhard
Dear Reinhard Meyer,
In message 4CA5BFEF.3090208@emk-elektronik.de you wrote:
<nitpick> it became illegal once u-boot for AT91 became required to be relocated </nitpick>
No, it has always been illegal. You might thave been lucky that in your case the erros did not show up erarlier, but this does not change anything.
Even very early versions of PPCBoot had this warning in the README:
Only after this relocation will you have a "normal" C environment; until that you are restricted in several ways...
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
<nitpick> it became illegal once u-boot for AT91 became required to be relocated </nitpick>
No, it has always been illegal. You might thave been lucky that in your case the erros did not show up erarlier, but this does not change anything.
Sorry, before recently there was no relocation whatsoever used in u-boot for AT91 architectures. So there was no before/after relocation.
The only complaint that can be put up is that someone introduced functions using static data called from code that in other architectures runs before relocation ;)
Whatever, I am not an advocate for Atmel, neither am I getting paid by them, I am just a "user" that has to fix problems I did not expect to be there.
For the fix, I see an ugly multiline #if defined(AT91SAM9260) || defined(AT91SAM9G20) || ... coming into arch/arm/asm/global_data.h.
There is no common defined value for all AT91 SoCs that could be used.
Will that be ok?
Reinhard
Dear Reinhard Meyer,
In message 4CA5C7DE.6010300@emk-elektronik.de you wrote:
For the fix, I see an ugly multiline #if defined(AT91SAM9260) || defined(AT91SAM9G20) || ... coming into arch/arm/asm/global_data.h.
There is no common defined value for all AT91 SoCs that could be used.
If this is really for all AT91 SoCs, then please feel free to introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually you can clean up some other such #if's on the way.
Probably there is a header file common to all AT91 SoCs when then can use a single such construct to #define the new variable so you don;t have to touch all the many board config files.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
For the fix, I see an ugly multiline #if defined(AT91SAM9260) || defined(AT91SAM9G20) || ... coming into arch/arm/asm/global_data.h.
There is no common defined value for all AT91 SoCs that could be used.
If this is really for all AT91 SoCs, then please feel free to introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually you can clean up some other such #if's on the way.
That would have to be set either in each board.h file or in each at91*.h file.
Probably there is a header file common to all AT91 SoCs when then can use a single such construct to #define the new variable so you don;t have to touch all the many board config files.
The only files I can see included in each instance are those of the kind hardware.h, memory_map.h and similiar. The define does not really belong in any of those.
I did the changes of adding the clock values to gd, and it became somewhat better, but there are still issues pending:
1. NAND accesses cause "raise: Signal # 8 caught" but still work, kernel boots normally. 2. environment is still invalid - when I boot the "CONFIG_SYS_ARM_WITHOUT_RELOC" version the ENV is ok. 3. saveenv goes way to fast to do anything and i2c md 57 1000.2 100 shows no change 4. env default -f hangs the system
Q: after relocation, "gd->" values are still available, right?
I am searching for further "violations"...
U-Boot 2010.09-00106-g6e135b9-dirty (Oct 01 2010 - 15:13:14)
U-Boot code: 21F00000 -> 21F3C070 BSS: -> 21F7D600 CPU: AT91SAM9XE Crystal frequency: 18.432 MHz CPU clock : 198.656 MHz Master clock : 99.328 MHz I2C: ready monitor len: 0007D600 ramsize: 04000000 Top of RAM usable for U-Boot at: 24000000 Reserving 501k for U-Boot at: 23f82000 Reserving 143k for malloc() at: 23f5e100 Reserving 24 Bytes for Board Info at: 23f5e0e8 Reserving 112 Bytes for Global Data at: 23f5e078 (112 bytes is exactly correct with the expanded struct) New Stack Pointer is: 23f5e070 RAM Configuration: Bank #0: 20000000 64 MiB relocation Offset is: 02082000 monitor flash len: 0003C070 Now running in RAM - U-Boot at: 23f82000 FLASH: 512 KiB NAND: raise: Signal # 8 caught raise: Signal # 8 caught raise: Signal # 8 caught 256 MiB *** Warning - bad CRC, using default environment
In: serial Out: serial Err: serial MMC: mci: 0 Net: macb0, enc1.0, enc1.1, enc1.2 Reset Ethernet PHY macb0: Starting autonegotiation... macb0: Autonegotiation complete macb0: link up, 100Mbps full-duplex (lpa: 0x45e1) Press SPACE to abort autoboot in 1 seconds TOP9000> env env - environment handling commands
Usage: env ask name [message] [size] - ask for environment variable env default -f - reset default environment env edit name - edit environment variable env export [-t | -b | -c] addr [size] - export environmnt env import [-d] [-t | -b | -c] addr [size] - import environmnt env print [name ...] - print environment env run var [...] - run commands in an environment variable env save - save environment env set [-f] name [arg ...]
TOP9000> env default -f (hangs here)
I did the changes of adding the clock values to gd, and it became somewhat better, but there are still issues pending:
- NAND accesses cause "raise: Signal # 8 caught"
but still work, kernel boots normally. 2. environment is still invalid - when I boot the "CONFIG_SYS_ARM_WITHOUT_RELOC" version the ENV is ok. 3. saveenv goes way to fast to do anything and i2c md 57 1000.2 100 shows no change 4. env default -f hangs the system
Q: after relocation, "gd->" values are still available, right?
I am searching for further "violations"...
Found them in timer.c, fixed them, now the NAND issue is gone.
The environment issues still persist. I am at a loss there now.
Observation: the old style commands "setenv", "printenv", etc. work, but any "env" command except for "env" alone crashes.
Hints where to look into are welcome, I will investige more later or tomorrow.
Reinhard
Dear Reinhard Meyer,
In message 4CA5D857.5010009@emk-elektronik.de you wrote:
The environment issues still persist. I am at a loss there now.
Observation: the old style commands "setenv", "printenv", etc. work, but any "env" command except for "env" alone crashes.
OK. If "printenv" works and "env print" fails then it has nothing to do with the environment code itself, as both call the same function.
It must have something to do with the implementation of subcommands then. See do_env() in "common/cmd_nvedit.c"; check if the command table address for find_cmd_tbl() is OK.
Eventually other commands with subcommands fail as well (i2c ?) ?
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
The environment issues still persist. I am at a loss there now.
Observation: the old style commands "setenv", "printenv", etc. work, but any "env" command except for "env" alone crashes.
OK. If "printenv" works and "env print" fails then it has nothing to do with the environment code itself, as both call the same function.
It must have something to do with the implementation of subcommands then. See do_env() in "common/cmd_nvedit.c"; check if the command table address for find_cmd_tbl() is OK.
Eventually other commands with subcommands fail as well (i2c ?) ?
Nope, other commands with subcommands work fine. I did not test ALL but mmc, i2c, nand behave normally.
I suspect something to do with a 256k boundary maybe, u-boot.bin is short of 256k. I changed the initial bootstrap to load 496k yesterday (before that the unrelocated version did fail in some commands).
I'll keep investigating.
But the bad CRC issue must be something unrelated, or maybe not?
Best Regards, Reinhard
Hello Reinhard,
Reinhard Meyer wrote:
Dear Wolfgang Denk,
The environment issues still persist. I am at a loss there now.
Observation: the old style commands "setenv", "printenv", etc. work, but any "env" command except for "env" alone crashes.
OK. If "printenv" works and "env print" fails then it has nothing to do with the environment code itself, as both call the same function.
It must have something to do with the implementation of subcommands then. See do_env() in "common/cmd_nvedit.c"; check if the command table address for find_cmd_tbl() is OK.
Eventually other commands with subcommands fail as well (i2c ?) ?
Nope, other commands with subcommands work fine. I did not test ALL but mmc, i2c, nand behave normally.
nand and mmc don;t use a command subtable, so they work. i2c do the needed fixups for subcommandtables ...
And I see in common/cmd_nvedit.c there is a "static cmd_tbl_t cmd_env_sub" definition, so this must be fixed too (not only for arm, it should also don;t work on avr32, mips, m68k and sparc, because this plattforms also need relocation fixups because CONFIG_RELOC_FIXUP_WORKS is not defined for them ...
I suspect something to do with a 256k boundary maybe, u-boot.bin is short of 256k. I changed the initial bootstrap to load 496k yesterday (before that the unrelocated version did fail in some commands).
I'll keep investigating.
Please try my above idea ...
Thanks!
bye, Heiko
On Fri, Oct 1, 2010 at 5:55 AM, Wolfgang Denk wd@denx.de wrote:
Dear Reinhard Meyer,
In message 4CA5D857.5010009@emk-elektronik.de you wrote:
The environment issues still persist. I am at a loss there now.
Observation: the old style commands "setenv", "printenv", etc. work, but any "env" command except for "env" alone crashes.
OK. If "printenv" works and "env print" fails then it has nothing to do with the environment code itself, as both call the same function.
I am seeing the same thing on Overo with current top of tree plus my relocation patch. Things like "env print" and "env save" just hang. Perhaps it is a more general problem . . .
Steve
Hello Heiko,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Le 02/10/2010 09:15, Reinhard Meyer a écrit :
Hello Heiko,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Hi Reinhart,
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the tee-shirts I got :)
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
Amicalement,
Dear Albert ARIBAUD,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Hi Reinhart,
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the tee-shirts I got :)
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
That means that things like this cannot work (with relocation), unless adding the relocation offset before using the pointer:
const struct { const u8 shift; const u8 idcode; struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode); } flashes[] = { #ifdef CONFIG_SPI_FLASH_SPANSION { 0, 0x01, spi_flash_probe_spansion, }, #endif #ifdef CONFIG_SPI_FLASH_ATMEL { 0, 0x1F, spi_flash_probe_atmel, }, #endif #ifdef CONFIG_SPI_FLASH_MACRONIX { 0, 0xc2, spi_flash_probe_macronix, }, #endif #ifdef CONFIG_SPI_FLASH_WINBOND { 0, 0xef, spi_flash_probe_winbond, }, #endif #ifdef CONFIG_SPI_FLASH_STMICRO { 0, 0x20, spi_flash_probe_stmicro, }, { 0, 0xff, spi_flash_probe_stmicro, }, #endif #ifdef CONFIG_SPI_FLASH_SST { 0, 0xBF, spi_flash_probe_sst, }, #endif #ifdef CONFIG_SPI_FRAM_RAMTRON { 6, 0xc2, spi_fram_probe_ramtron, }, # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC { 0, 0xff, spi_fram_probe_ramtron, }, # endif # undef IDBUF_LEN # define IDBUF_LEN 9 /* we need to read 6+3 bytes */ #endif };
And I think there are more places of this type in u-boot...
Best Regards, Reinhard
Le 02/10/2010 10:10, Reinhard Meyer a écrit :
Dear Albert ARIBAUD,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Hi Reinhart,
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the tee-shirts I got :)
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
That means that things like this cannot work (with relocation), unless adding the relocation offset before using the pointer:
const struct { const u8 shift; const u8 idcode; struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode); } flashes[] = { #ifdef CONFIG_SPI_FLASH_SPANSION { 0, 0x01, spi_flash_probe_spansion, }, #endif #ifdef CONFIG_SPI_FLASH_ATMEL { 0, 0x1F, spi_flash_probe_atmel, }, #endif #ifdef CONFIG_SPI_FLASH_MACRONIX { 0, 0xc2, spi_flash_probe_macronix, }, #endif #ifdef CONFIG_SPI_FLASH_WINBOND { 0, 0xef, spi_flash_probe_winbond, }, #endif #ifdef CONFIG_SPI_FLASH_STMICRO { 0, 0x20, spi_flash_probe_stmicro, }, { 0, 0xff, spi_flash_probe_stmicro, }, #endif #ifdef CONFIG_SPI_FLASH_SST { 0, 0xBF, spi_flash_probe_sst, }, #endif #ifdef CONFIG_SPI_FRAM_RAMTRON { 6, 0xc2, spi_fram_probe_ramtron, }, # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC { 0, 0xff, spi_fram_probe_ramtron, }, # endif # undef IDBUF_LEN # define IDBUF_LEN 9 /* we need to read 6+3 bytes */ #endif };
And I think there are more places of this type in u-boot...
Best Regards, Reinhard
If this code is intended to execute after relocation [1] then no, it will not work.
There are two ways to fix that:
The first one is to make the variable non-const and, after relocation but before use, run a fixup loop specifically for this variable. Then you can call the (now fixed) functions.
The second one is to fix on-the-fly: provide a field in gd which contains the relocation offset in gd (if not done already); in the code which calls function pointers, DECLARE_GLOBAL_DATA_PTR and call the function through a global macro (defined in some general u-boot header), e.g. FIX_RELOCATED_FUNCTION_POINTER(fp), which would offset fp to its correct location.
Thus in the code, instead of x = fp(args) you'd have x = FIX_RELOCATED_FUNCTION_POINTER(fp)(args).
[1] or, in my case, before relocation but not from the location specified at link time. This is a slightly different issue, which the first solution fails to address but the second does.
Amicalement,
Dear Albert ARIBAUD,
In message 4CA6EC99.5080204@free.fr you wrote:
There are two ways to fix that:
...
We should find the third way, which is that the tools actually incude these pointers into the GOT so they get relocated automatically.
Best regards,
Wolfgang Denk
Hello Reinhard,
Reinhard Meyer wrote:
Dear Albert ARIBAUD,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Hi Reinhart,
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the
The init_sequence should not called anymore after relocation, as it is the init_sequence ... or?
tee-shirts I got :)
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
That means that things like this cannot work (with relocation), unless adding the relocation offset before using the pointer:
Yep, you have to fix these pointers after relocation ...
const struct { const u8 shift; const u8 idcode; struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode); } flashes[] = { #ifdef CONFIG_SPI_FLASH_SPANSION { 0, 0x01, spi_flash_probe_spansion, }, #endif
[...]
#ifdef CONFIG_SPI_FRAM_RAMTRON { 6, 0xc2, spi_fram_probe_ramtron, }, # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC { 0, 0xff, spi_fram_probe_ramtron, }, # endif # undef IDBUF_LEN # define IDBUF_LEN 9 /* we need to read 6+3 bytes */ #endif };
And I think there are more places of this type in u-boot...
Yes, maybe. But relocation as I did for arm, also works on m68k, sparc, mips, avr32 and they must do also this fixups, so for common functions (except the new env handling, which I think got never tested on this architectures?) should work ...
As I just searching in code: there is a env_relocate() function (which get called from arch/arm/lib/board.c board_init_r()), but it did not the necessary work for subcommandtable fixup... I think this should be the right place to do this ... or?
bye, Heiko
Le 02/10/2010 11:08, Heiko Schocher a écrit :
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the
The init_sequence should not called anymore after relocation, as it is the init_sequence ... or?
... or you may want to have an u-boot binary which is truly position-independent. I'd like to have that, but the init_sequence table issue makes it difficult.
Amicalement,
Dear Albert ARIBAUD,
In message 4CA6FB7E.3070009@free.fr you wrote:
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the
The init_sequence should not called anymore after relocation, as it is the init_sequence ... or?
... or you may want to have an u-boot binary which is truly = position-independent. I'd like to have that, but the init_sequence table = issue makes it difficult.
See previous thread by Pter Tyser.
Best regards,
Wolfgang Denk
Hello Reinhard,
Reinhard Meyer wrote:
Dear Albert ARIBAUD,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Hi Reinhart,
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the
The init_sequence should not called anymore after relocation, as it is the init_sequence ... or?
tee-shirts I got :)
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
That means that things like this cannot work (with relocation), unless adding the relocation offset before using the pointer:
Yep, you have to fix these pointers after relocation ...
const struct { const u8 shift; const u8 idcode; struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode); } flashes[] = { #ifdef CONFIG_SPI_FLASH_SPANSION { 0, 0x01, spi_flash_probe_spansion, }, #endif
[...]
#ifdef CONFIG_SPI_FRAM_RAMTRON { 6, 0xc2, spi_fram_probe_ramtron, }, # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC { 0, 0xff, spi_fram_probe_ramtron, }, # endif # undef IDBUF_LEN # define IDBUF_LEN 9 /* we need to read 6+3 bytes */ #endif };
And I think there are more places of this type in u-boot...
Yes, maybe. But relocation as I did for arm, also works on m68k, sparc, mips, avr32 and they must do also this fixups, so for common functions (except the new env handling, which I think got never tested on this architectures?) should work ...
This pointer problem is solved with the fixup relocs on ppc and should work without manual relocation. I think this is a ppc only extension but I might be wrong. I believe that the other alternative is to do it as x86 does which I think is the general way which should work on any arch. Graem Russ would know better.
Jocke
On 10/2/2010 3:17 AM, Joakim Tjernlund wrote:
Hello Reinhard,
Reinhard Meyer wrote:
Dear Albert ARIBAUD,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Hi Reinhart,
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the
The init_sequence should not called anymore after relocation, as it is the init_sequence ... or?
tee-shirts I got :)
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
That means that things like this cannot work (with relocation), unless adding the relocation offset before using the pointer:
Yep, you have to fix these pointers after relocation ...
const struct { const u8 shift; const u8 idcode; struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode); } flashes[] = { #ifdef CONFIG_SPI_FLASH_SPANSION { 0, 0x01, spi_flash_probe_spansion, }, #endif
[...]
#ifdef CONFIG_SPI_FRAM_RAMTRON { 6, 0xc2, spi_fram_probe_ramtron, }, # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC { 0, 0xff, spi_fram_probe_ramtron, }, # endif # undef IDBUF_LEN # define IDBUF_LEN 9 /* we need to read 6+3 bytes */ #endif };
And I think there are more places of this type in u-boot...
Yes, maybe. But relocation as I did for arm, also works on m68k, sparc, mips, avr32 and they must do also this fixups, so for common functions (except the new env handling, which I think got never tested on this architectures?) should work ...
This pointer problem is solved with the fixup relocs on ppc and should work without manual relocation. I think this is a ppc only extension but I might be wrong.
Hi All, You are correct that this is a ppc only extension. As such, it is not a good candidate for "general" use.
I believe that the other alternative is to do it as x86 does which I think is the general way which should work on any arch. Graem Russ would know better.
Almost exactly a year ago, this was all pretty much presented by Graeme in the threads Relocation size penalty calculation (October 14, 2009) i386 Relocation (November 24, 2009)
Using the full relocation scheme eliminates the need for all these "fixups" in u-boot C code. I think this is a very desirable result. It is also not clear to me that hard coding in the relocation as several C routines will produce a u-boot that is "smaller" than the one produced by using normal ELF relocation. However, using full relocation creates an environment that is true "C" and does not rely on people remembering that they may have to fix up some parts of their code. It is hard to see much downside in using the full relocation capability provided by Graeme's code. FWIW, the relocation code and data does not have to be moved into ram if space is at a premium.
Best Regards, Bill Campbell
Jocke
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Dear all,
thanks for all the info.
My AT91 boards will not use relocation for the time being, and if relocation is god-like enforced I will find a way not to use it. I don't need to spend 10% more code for all that trouble.
Reinhard
Dear Reinhard Meyer,
In message 4CA77AFA.2090909@emk-elektronik.de you wrote:
My AT91 boards will not use relocation for the time being, and if relocation is god-like enforced I will find a way not to use it. I don't need to spend 10% more code for all that trouble.
Please see http://article.gmane.org/gmane.comp.boot-loaders.u-boot/85186
Best regards,
Wolfgang Denk
Dear J. William Campbell,
On 10/2/2010 3:17 AM, Joakim Tjernlund wrote:
Hello Reinhard,
Reinhard Meyer wrote:
Dear Albert ARIBAUD,
I try to understand how the relocation process could handle pointers (to functions or other data) in const or data sections. Your code cannot know what is data and what is a pointer that needs adjustment?
Best Regards, Reinhard
Hi Reinhart,
Short answer - the relocation process does not handle pointers inside data structures.
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the
The init_sequence should not called anymore after relocation, as it is the init_sequence ... or?
tee-shirts I got :)
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
That means that things like this cannot work (with relocation), unless adding the relocation offset before using the pointer:
Yep, you have to fix these pointers after relocation ...
const struct { const u8 shift; const u8 idcode; struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode); } flashes[] = { #ifdef CONFIG_SPI_FLASH_SPANSION { 0, 0x01, spi_flash_probe_spansion, }, #endif
[...]
#ifdef CONFIG_SPI_FRAM_RAMTRON { 6, 0xc2, spi_fram_probe_ramtron, }, # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC { 0, 0xff, spi_fram_probe_ramtron, }, # endif # undef IDBUF_LEN # define IDBUF_LEN 9 /* we need to read 6+3 bytes */ #endif };
And I think there are more places of this type in u-boot...
Yes, maybe. But relocation as I did for arm, also works on m68k, sparc, mips, avr32 and they must do also this fixups, so for common functions (except the new env handling, which I think got never tested on this architectures?) should work ...
This pointer problem is solved with the fixup relocs on ppc and should work without manual relocation. I think this is a ppc only extension but I might be wrong.
Hi All, You are correct that this is a ppc only extension. As such, it is not a good candidate for "general" use.
I believe that the other alternative is to do it as x86 does which I think is the general way which should work on any arch. Graem Russ would know better.
Almost exactly a year ago, this was all pretty much presented by Graeme in the threads Relocation size penalty calculation (October 14, 2009) i386 Relocation (November 24, 2009)
Using the full relocation scheme eliminates the need for all these "fixups" in u-boot C code. I think this is a very desirable result. It is also not clear to me that hard coding in the relocation as several C routines will produce a u-boot that is "smaller" than the one produced by using normal ELF relocation. However, using full relocation creates an environment that is true "C" and does not rely on people remembering that they may have to fix up some parts of their code. It is hard to see much downside in using the full relocation capability provided by Graeme's code. FWIW, the relocation code and data does not have to be moved into ram if space is at a premium.
I agree here. _If_ relocation, it should work without hand-adding fixup stuff to all functions using initialized data with pointers. Even Wolfgang forgot to fixup his 2nd level command table in cmd_nvedit.c ;)
And, for space concerns in flash, relocation should always be an option on a board by board basis...
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
Best Regards, Reinhard
Le 02/10/2010 22:39, Reinhard Meyer a écrit :
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
IIU Bill C, yes the linker can emit the information and the startup code could use this information instead of relying on hand-provided info; the linker file probably needs to be modified in order to provide such info. I intend to look into this, but feel free to do too.
Amicalement,
On 03/10/10 08:09, Albert ARIBAUD wrote:
Le 02/10/2010 22:39, Reinhard Meyer a écrit :
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
IIU Bill C, yes the linker can emit the information and the startup code could use this information instead of relying on hand-provided info; the linker file probably needs to be modified in order to provide such info. I intend to look into this, but feel free to do too.
As mentioned previously, I have already done this for x86. The linker flags used are -pic and --emit-relocs. The linker produces a section named rel.dyn which needs to be processed but not loaded into RAM. rel.dyn contains a simple list of address (within .text, .data, .rodata etc) each of which need a simple adjustment equal to the relocation offset.
The size increase of the code + data loaded into RAM is 104012 bytes to 104296 bytes which is only 284 bytes or a mere 0.3% (which is negligible) with an additional 22424 bytes in rel.dyn (22%) not loaded into RAM
The additional bonus is that .got is not referenced during run-time, so there is no run-time performance penalty. However, the penalty of processing 2803 relocation records at startup may not be wholly recovered during a typical u-boot run-time session.
All this is for x86, and may not apply so neatly to other arches
Regards,
Graeme
Le 03/10/2010 01:07, Graeme Russ a écrit :
On 03/10/10 08:09, Albert ARIBAUD wrote:
Le 02/10/2010 22:39, Reinhard Meyer a écrit :
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
IIU Bill C, yes the linker can emit the information and the startup code could use this information instead of relying on hand-provided info; the linker file probably needs to be modified in order to provide such info. I intend to look into this, but feel free to do too.
As mentioned previously, I have already done this for x86. The linker flags used are -pic and --emit-relocs. The linker produces a section named rel.dyn which needs to be processed but not loaded into RAM. rel.dyn contains a simple list of address (within .text, .data, .rodata etc) each of which need a simple adjustment equal to the relocation offset.
Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary for relocation. You seem to imply it actually is... In my experience, -fPIC and-fPIE do increase code by adding GOT relocation to symbols that need fixing, so they would indeed be redundant to any other relocation mechanism -- I just did some test with basic code and this seems to confirm, no -fPIx is needed to get relocation the way you do on ARM.
The size increase of the code + data loaded into RAM is 104012 bytes to 104296 bytes which is only 284 bytes or a mere 0.3% (which is negligible) with an additional 22424 bytes in rel.dyn (22%) not loaded into RAM
The additional bonus is that .got is not referenced during run-time, so there is no run-time performance penalty. However, the penalty of processing 2803 relocation records at startup may not be wholly recovered during a typical u-boot run-time session.
All this is for x86, and may not apply so neatly to other arches
Of course. :)
Amicalement,
On 03/10/10 18:10, Albert ARIBAUD wrote:
Le 03/10/2010 01:07, Graeme Russ a écrit :
On 03/10/10 08:09, Albert ARIBAUD wrote:
Le 02/10/2010 22:39, Reinhard Meyer a écrit :
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
IIU Bill C, yes the linker can emit the information and the startup code could use this information instead of relying on hand-provided info; the linker file probably needs to be modified in order to provide such info. I intend to look into this, but feel free to do too.
As mentioned previously, I have already done this for x86. The linker flags used are -pic and --emit-relocs. The linker produces a section named rel.dyn which needs to be processed but not loaded into RAM. rel.dyn contains a simple list of address (within .text, .data, .rodata etc) each of which need a simple adjustment equal to the relocation offset.
Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary for relocation. You seem to imply it actually is... In my experience, -fPIC and-fPIE do increase code by adding GOT relocation to symbols that need fixing, so they would indeed be redundant to any other relocation mechanism -- I just did some test with basic code and this seems to confirm, no -fPIx is needed to get relocation the way you do on ARM.
Just to clarify -fpic is a compiler option, -pic is a linker option. x86 has no compile time relocation options (therefore no referencing .got etc). Using the link time pic option produces the relocation data table (.rel.dyn) which must be pre-processed before execution can begin at the relocated address
Cheers,
Graeme
Le 03/10/2010 10:44, Graeme Russ a écrit :
Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary for relocation. You seem to imply it actually is... In my experience, -fPIC and-fPIE do increase code by adding GOT relocation to symbols that need fixing, so they would indeed be redundant to any other relocation mechanism -- I just did some test with basic code and this seems to confirm, no -fPIx is needed to get relocation the way you do on ARM.
Just to clarify -fpic is a compiler option, -pic is a linker option. x86 has no compile time relocation options (therefore no referencing .got etc). Using the link time pic option produces the relocation data table (.rel.dyn) which must be pre-processed before execution can begin at the relocated address
Thanks for clarifying, Graeme.
This is consistent with the ARM compile-time options -fPIC/-fPIE vs link-time option -pie. So there may be at least an interest in investigating ELF-style relocation on ARM and comparing it to GOT-based relocation in terms of FLASH and RAM sizes and code speed.
Cheers,
Graeme
Amicalement,
On 10/3/2010 1:58 AM, Albert ARIBAUD wrote:
Le 03/10/2010 10:44, Graeme Russ a écrit :
Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary for relocation. You seem to imply it actually is... In my experience, -fPIC and-fPIE do increase code by adding GOT relocation to symbols that need fixing, so they would indeed be redundant to any other relocation mechanism -- I just did some test with basic code and this seems to confirm, no -fPIx is needed to get relocation the way you do on ARM.
Just to clarify -fpic is a compiler option, -pic is a linker option. x86 has no compile time relocation options (therefore no referencing .got etc). Using the link time pic option produces the relocation data table (.rel.dyn) which must be pre-processed before execution can begin at the relocated address
Thanks for clarifying, Graeme.
This is consistent with the ARM compile-time options -fPIC/-fPIE vs link-time option -pie. So there may be at least an interest in investigating ELF-style relocation on ARM and comparing it to GOT-based relocation in terms of FLASH and RAM sizes and code speed.
Hi All, It is for sure that -fPIC/-fPIE programs will contain more executable instructions than programs compiled without these options. The program will also contain more data space for the got. If -fPIC actually produced a fully position-independent executable, the extra overhead would perhaps be tolerable. However, since it does not do this, (problems with initialized data etc.) there is really no advantage in using these compile-time options. The executable code and required data space for the program without these switches will "always" be smaller and faster than with them. In order to fix the remaining issues even when using -fPIC, a relocation loop must exist in the u-boot code, either one global one or a bunch of user written specific ones. Also, the -pie switch will be needed anyway at link time to build the relocation table for the remaining relocation requirements. Programs compiled without -fPIC will have a larger .rel.dyn table than those compiled with -fPIC. However, the table entries in the relocation table occupy about the same storage as the code generated by the compiler to relocate a reference to the symbol at run time. So this is probably a almost a wash. Also, the dynamic relocation data need not be copied into the run-time object, as it is no longer needed. So the likely outcome is that the "flash" image is about the same size/slightly larger than the one compiled by -fPIC, and that the ram footprint after relocation is slightly smaller. If one is REALLY pressed for space, the size of the dynamic relocation area can be reduced by a post-processor program that would re-format the relocation entries. This re-formatting is possible because 1) ELF is a very general format and we only need a small subset of it, and 2) u-boot code will never occupy say 16 MB of space, so each relocation can probably be compressed into a 32 bit word. I doubt anyone is that desperate, but it IS possible. It will be interesting to see what the results of this comparison are. For me, the no user awareness of relocation is worth a lot, and the fact that the difference/overhead of relocation will all be in exactly one place is very appealing.
Best Regards, Bill Campbell
Cheers,
Graeme
Amicalement,
Le 03/10/2010 17:36, J. William Campbell a écrit :
Hi All, It is for sure that -fPIC/-fPIE programs will contain more executable instructions than programs compiled without these options. The program will also contain more data space for the got. If -fPIC actually produced a fully position-independent executable, the extra overhead would perhaps be tolerable. However, since it does not do this, (problems with initialized data etc.) there is really no advantage in using these compile-time options. The executable code and required data space for the program without these switches will "always" be smaller and faster than with them. In order to fix the remaining issues even when using -fPIC, a relocation loop must exist in the u-boot code, either one global one or a bunch of user written specific ones. Also, the -pie switch will be needed anyway at link time to build the relocation table for the remaining relocation requirements. Programs compiled without -fPIC will have a larger .rel.dyn table than those compiled with -fPIC. However, the table entries in the relocation table occupy about the same storage as the code generated by the compiler to relocate a reference to the symbol at run time. So this is probably a almost a wash. Also, the dynamic relocation data need not be copied into the run-time object, as it is no longer needed. So the likely outcome is that the "flash" image is about the same size/slightly larger than the one compiled by -fPIC, and that the ram footprint after relocation is slightly smaller. If one is REALLY pressed for space, the size of the dynamic relocation area can be reduced by a post-processor program that would re-format the relocation entries. This re-formatting is possible because 1) ELF is a very general format and we only need a small subset of it, and 2) u-boot code will never occupy say 16 MB of space, so each relocation can probably be compressed into a 32 bit word. I doubt anyone is that desperate, but it IS possible. It will be interesting to see what the results of this comparison are. For me, the no user awareness of relocation is worth a lot, and the fact that the difference/overhead of relocation will all be in exactly one place is very appealing.
Best Regards, Bill Campbell
Hi Bill,
Thanks for the explanations. I am experimenting with ELF relocation right now, replacing -fPIe with -pie, and this generates .rel.dyn, but also many other sections. I'm trying to get rid of them; apparently /DISCARD/ing them in the linker file seems to reduce this to a minimum (I still have a .got.plt section which seems useless but I cannot remove it lest the linker segfaults).
But the .rel.dyn generated by the linker section does not provide symbols to mark its start and end, and I have found no documentation in binutils ld which would describe how to rewrite the .rel.dyn section and add these symbols myself.
How did you manage that for i386? I did not see a linker file in the i386 part of u-boot.
Amicalement,
Le 03/10/2010 18:47, Albert ARIBAUD a écrit :
But the .rel.dyn generated by the linker section does not provide symbols to mark its start and end, and I have found no documentation in binutils ld which would describe how to rewrite the .rel.dyn section and add these symbols myself.
How did you manage that for i386? I did not see a linker file in the i386 part of u-boot.
Edit: found the linker, not in the arch part but in board eNET. Now trying to do same on ARM.
Amicalement,
Dear "J. William Campbell",
In message 4CA8A2E0.7090407@comcast.net you wrote:
executable instructions than programs compiled without these options. The program will also contain more data space for the got. If -fPIC actually produced a fully position-independent executable, the extra overhead would perhaps be tolerable. However, since it does not do this, (problems with initialized data etc.) there is really no advantage in using these compile-time options. The executable code and required data space for the program without these switches will "always" be smaller and faster than with them. In order to fix the remaining issues even when using -fPIC, a relocation loop must exist in the u-boot code, either one global one or a bunch of user written specific ones. Also,
If needed, we should have a global one only.
It will be interesting to see what the results of this comparisonare. For me, the no user awareness of relocation is worth a lot, and the fact that the difference/overhead of relocation will all be in exactly one place is very appealing.
Agreed.
Best regards,
Wolfgang Denk
Hello Albert,
Albert ARIBAUD wrote:
Le 02/10/2010 22:39, Reinhard Meyer a écrit :
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
IIU Bill C, yes the linker can emit the information and the startup code could use this information instead of relying on hand-provided info; the linker file probably needs to be modified in order to provide such info. I intend to look into this, but feel free to do too.
I thought therefore is the GOT. But if there is another way, to get rid of this hand fixing, it would be a good thing.
bye, Heiko
Dear Reinhard Meyer,
In message 4CA79896.2010606@emk-elektronik.de you wrote:
I agree here. _If_ relocation, it should work without hand-adding fixup stuff to all functions using initialized data with pointers. Even Wolfgang forgot to fixup his 2nd level command table in cmd_nvedit.c ;)
I didn't forget it - at least not in the sensse that I think this is something that needs to be done.
This works fine on PPC with relocation, and we should make it work the same on other arches.
And, for space concerns in flash, relocation should always be an option on a board by board basis...
NAK.
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
It does. That's the GOT.
Best regards,
Wolfgang Denk
On 10/3/2010 11:29 AM, Wolfgang Denk wrote:
Dear Reinhard Meyer,
In message4CA79896.2010606@emk-elektronik.de you wrote:
I agree here. _If_ relocation, it should work without hand-adding fixup stuff to all functions using initialized data with pointers. Even Wolfgang forgot to fixup his 2nd level command table in cmd_nvedit.c ;)
I didn't forget it - at least not in the sensse that I think this is something that needs to be done.
This works fine on PPC with relocation, and we should make it work the same on other arches.
And, for space concerns in flash, relocation should always be an option on a board by board basis...
NAK.
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
It does. That's the GOT.
I think this is actually a misunderstanding. The purpose of the GOT, at least from GCC's point of view, is to hold the absolute addresses of private data referenced by shared library code. That is what it was invented to do. This is similar to, but not identical with, relocating all data references. Initialized data in the library must have a copy created (and relocated as necessary if it contains pointers) by the runtime linker when the library is initialized in the address space of the process using the library. The code in the shared library is -fPIC, but it still needs the runtime linker to allocate a copy of the GOT for the current user AND to allocate and relocate any data that is required for the library that is private to the user. It is that second step where we have trouble.
Best Regards, Bill Campbell
Best regards,
Wolfgang Denk
Hello Wolfgang,
Wolfgang Denk wrote:
Dear Reinhard Meyer,
In message 4CA79896.2010606@emk-elektronik.de you wrote:
I agree here. _If_ relocation, it should work without hand-adding fixup stuff to all functions using initialized data with pointers. Even Wolfgang forgot to fixup his 2nd level command table in cmd_nvedit.c ;)
I didn't forget it - at least not in the sensse that I think this is something that needs to be done.
This works fine on PPC with relocation, and we should make it work the same on other arches.
If we find a way for this on ARM, Ack!
And, for space concerns in flash, relocation should always be an option on a board by board basis...
NAK.
And as an idea, if position independent code is used, only pointers in initialized data need adjustment. Cannot the linker emit a table of addresses that need fixing?
It does. That's the GOT.
So I thought too, and I made a fixup in relocate_code() for the GOT entries. But this don;t work with for example the commandtables.
bye, Heiko
Dear "J. William Campbell",
In message 4CA75BFB.5030208@comcast.net you wrote:
And I think there are more places of this type in u-boot...
Yes, maybe. But relocation as I did for arm, also works on m68k, sparc, mips, avr32 and they must do also this fixups, so for common functions (except the new env handling, which I think got never tested on this architectures?) should work ...
This pointer problem is solved with the fixup relocs on ppc and should work without manual relocation. I think this is a ppc only extension but I might be wrong.
You are correct that this is a ppc only extension. As such, it isnot a good candidate for "general" use.
On contrary.
If it works for PPC, then there should be ways to do the same on other architectures.
Using the full relocation scheme eliminates the need for all these "fixups" in u-boot C code. I think this is a very desirable result. It is also not clear to me that hard coding in the relocation as several C routines will produce a u-boot that is "smaller" than the one produced by using normal ELF relocation. However, using full relocation creates an environment that is true "C" and does not rely on people remembering that they may have to fix up some parts of their code. It is hard to see much downside in using the full relocation capability provided by Graeme's code.
Agreed. But if we take this path, we need to find an implementation that looks clean and readable.
Best regards,
Wolfgang Denk
On 10/3/2010 11:14 AM, Wolfgang Denk wrote:
Dear "J. William Campbell",
In message4CA75BFB.5030208@comcast.net you wrote:
And I think there are more places of this type in u-boot...
Yes, maybe. But relocation as I did for arm, also works on m68k, sparc, mips, avr32 and they must do also this fixups, so for common functions (except the new env handling, which I think got never tested on this architectures?) should work ...
This pointer problem is solved with the fixup relocs on ppc and should work without manual relocation. I think this is a ppc only extension but I might be wrong.
You are correct that this is a ppc only extension. As such, it isnot a good candidate for "general" use.
On contrary.
If it works for PPC, then there should be ways to do the same on other architectures.
Well, maybe so, but GCC won't do it now, and there has been no move by other architectures to adopt this capability. I suspect that it is extremely unlikley that this capability will ever be ported to other architectures since it has been available for so long on PPC without any movement to other systems.
Using the full relocation scheme eliminates the need for all these "fixups" in u-boot C code. I think this is a very desirable result. It is also not clear to me that hard coding in the relocation as several C routines will produce a u-boot that is "smaller" than the one produced by using normal ELF relocation. However, using full relocation creates an environment that is true "C" and does not rely on people remembering that they may have to fix up some parts of their code. It is hard to see much downside in using the full relocation capability provided by Graeme's code.
Agreed. But if we take this path, we need to find an implementation that looks clean and readable.
Agreed. This should be possible to do now that there is a better understanding of the ELF format by the u-boot community. Perhaps the place to start would be trying to port what Graeme has done to ARM or perhaps better yet, PPC. Since lots of people on this list are PPC folks, we should have a lot of leverage there.
Best Regards, Bill Campbell
Best regards,
Wolfgang Denk
Le 03/10/2010 20:54, J. William Campbell a écrit :
Agreed. This should be possible to do now that there is a better understanding of the ELF format by the u-boot community. Perhaps the place to start would be trying to port what Graeme has done to ARM or perhaps better yet, PPC. Since lots of people on this list are PPC folks, we should have a lot of leverage there.
I am currently looking into ELF relocation on ARM.
Amicalement,
Dear Reinhard Meyer,
In message 4CA6E8E5.2090605@emk-elektronik.de you wrote:
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the tee-shirts I got :)
It should work.
Eventually we need to find out which sort of tweaking of compiler and/or linker options is needed on ARM.
ATM I have not found a way to fix this, except making the code which uses the pointers aware that the are location-sensitive and fix them when using them.
That means that things like this cannot work (with relocation), unless adding the relocation offset before using the pointer:
const struct { const u8 shift; const u8 idcode; struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode); } flashes[] = { #ifdef CONFIG_SPI_FLASH_SPANSION { 0, 0x01, spi_flash_probe_spansion, }, #endif #ifdef CONFIG_SPI_FLASH_ATMEL { 0, 0x1F, spi_flash_probe_atmel, }, #endif #ifdef CONFIG_SPI_FLASH_MACRONIX { 0, 0xc2, spi_flash_probe_macronix, }, #endif #ifdef CONFIG_SPI_FLASH_WINBOND { 0, 0xef, spi_flash_probe_winbond, }, #endif #ifdef CONFIG_SPI_FLASH_STMICRO { 0, 0x20, spi_flash_probe_stmicro, }, { 0, 0xff, spi_flash_probe_stmicro, }, #endif #ifdef CONFIG_SPI_FLASH_SST { 0, 0xBF, spi_flash_probe_sst, }, #endif #ifdef CONFIG_SPI_FRAM_RAMTRON { 6, 0xc2, spi_fram_probe_ramtron, }, # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC { 0, 0xff, spi_fram_probe_ramtron, }, # endif # undef IDBUF_LEN # define IDBUF_LEN 9 /* we need to read 6+3 bytes */ #endif };
Well, please keep in mind that all this code is working find on PowerPC, which has been using relocation right from the beginning.
It is my understanding that we don't suffer from this issue any more on PPC - Peter Tyser posted relocation fixup patches for PPC about a year ago or so.
I have to admit that I cannot remeber the final result of this discussion (there were tool chain dependencies?), but IIRC this has been solved for PPC.
We should do the same for AMR now.
Peter, could you please fill in the details of that old story?
Best regards,
Wolfgang Denk
Le 03/10/2010 20:03, Wolfgang Denk a écrit :
Dear Reinhard Meyer,
In message4CA6E8E5.2090605@emk-elektronik.de you wrote:
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the tee-shirts I got :)
It should work.
Eventually we need to find out which sort of tweaking of compiler and/or linker options is needed on ARM.
While looking for enhancements to Heiko's arm relocation patches, I have gone through all relocation related compiler and linker options, and then some. -fPIC / -fPIE will not generate GOT fixups for data containing pointers, and I have found no additional option that will.
OTOH, -pie will generate this kind of fixup (and all others needed) in the form of .rel.dyn relocations, as Bill describes.
Well, please keep in mind that all this code is working find on PowerPC, which has been using relocation right from the beginning.
I don't mean to say that PPC relocation does not work; I mean to say that a mechanism intended for one architecture may not be the optimal one for another, and if two mechanisms exist which provide relocation and one of them does not require specific tricks in the code, then I would choose this one.
Best regards,
Wolfgang Denk
Amicalement,
Dear Albert ARIBAUD,
In message 4CA8CCC1.2010309@free.fr you wrote:
Well, please keep in mind that all this code is working find on PowerPC, which has been using relocation right from the beginning.
I don't mean to say that PPC relocation does not work; I mean to say that a mechanism intended for one architecture may not be the optimal one for another, and if two mechanisms exist which provide relocation and one of them does not require specific tricks in the code, then I would choose this one.
Agreed. But then, I did not mean to suggest that the situaltion on PPC was perfect. If you find something that works even better for AMR, then we should try and get the same improvment for PPC (and other arches) as well.
Best regards,
Wolfgang Denk
Hello Albert,
Albert ARIBAUD wrote:
Le 03/10/2010 20:03, Wolfgang Denk a écrit :
Dear Reinhard Meyer,
In message4CA6E8E5.2090605@emk-elektronik.de you wrote:
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the tee-shirts I got :)
It should work.
Eventually we need to find out which sort of tweaking of compiler and/or linker options is needed on ARM.
While looking for enhancements to Heiko's arm relocation patches, I have gone through all relocation related compiler and linker options, and then some. -fPIC / -fPIE will not generate GOT fixups for data containing pointers, and I have found no additional option that will.
Yep, that was also my problem ...
OTOH, -pie will generate this kind of fixup (and all others needed) in the form of .rel.dyn relocations, as Bill describes.
Ah! Have to look in this, maybe thats the way we have to go ...
bye, Heiko
Le 04/10/2010 08:08, Heiko Schocher a écrit :
Hello Albert,
Albert ARIBAUD wrote:
Le 03/10/2010 20:03, Wolfgang Denk a écrit :
Dear Reinhard Meyer,
In message4CA6E8E5.2090605@emk-elektronik.de you wrote:
And yes, this means the content arrays of pointers such as init_sequence is not relocated. Been there, done that, can give you one of the tee-shirts I got :)
It should work.
Eventually we need to find out which sort of tweaking of compiler and/or linker options is needed on ARM.
While looking for enhancements to Heiko's arm relocation patches, I have gone through all relocation related compiler and linker options, and then some. -fPIC / -fPIE will not generate GOT fixups for data containing pointers, and I have found no additional option that will.
Yep, that was also my problem ...
OTOH, -pie will generate this kind of fixup (and all others needed) in the form of .rel.dyn relocations, as Bill describes.
Ah! Have to look in this, maybe thats the way we have to go ...
Right now I can build (not run, mind you) u-boot for edminiv2 without -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all of the .rel.dyn fixup entries are type 23, that is, relative to the base address, which is good. However, here are about ten at the end which are type 2 -- symbol-relative -- and I am studying them in order to see if they are needed.
If type 23 relocations are all that is needed, then a first ARM ELF relocation implementation should 'simply' trade GOT vs .rel.dyn relocation in start.S (I am almost there) and remove fixups in board_init_r. Start.S would apply type 23 fixups only and ignore the rest. Later on we could add a build stage to rewrite the .rel.dyn section as suggested, by filtering out non-type-23 relocs and keeping only the address part of type-23 ones, reducing the .rel.dyn table roughly by half.
The good news is, I can spare a couple more hours today on this. I'll let you all know how this fares!
bye, Heiko
Amicalement,
Dear Albert ARIBAUD,
Right now I can build (not run, mind you) u-boot for edminiv2 without -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all of the .rel.dyn fixup entries are type 23, that is, relative to the base address, which is good. However, here are about ten at the end which are type 2 -- symbol-relative -- and I am studying them in order to see if they are needed.
If type 23 relocations are all that is needed, then a first ARM ELF relocation implementation should 'simply' trade GOT vs .rel.dyn relocation in start.S (I am almost there) and remove fixups in board_init_r. Start.S would apply type 23 fixups only and ignore the rest. Later on we could add a build stage to rewrite the .rel.dyn section as suggested, by filtering out non-type-23 relocs and keeping only the address part of type-23 ones, reducing the .rel.dyn table roughly by half.
The good news is, I can spare a couple more hours today on this. I'll let you all know how this fares!
Thats good news! How much did the image size increase with this table?
And I am willing to test your efforts on AT91 here, maybe you can send me the changes to .lds and start.S beforehand so I can see what type of relocation info gets produced here.
A rather wild, but quite arch independant additional "build stage" for relocation would be to link u-boot for two different TEXT_BASE values e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff" tool should find the 32 bit places where relocation is required and add a table to the end of u-boot.bin... (Just a rough idea)
Best Regards, Reinhard
Le 04/10/2010 09:27, Reinhard Meyer a écrit :
Dear Albert ARIBAUD,
Right now I can build (not run, mind you) u-boot for edminiv2 without -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all of the .rel.dyn fixup entries are type 23, that is, relative to the base address, which is good. However, here are about ten at the end which are type 2 -- symbol-relative -- and I am studying them in order to see if they are needed.
If type 23 relocations are all that is needed, then a first ARM ELF relocation implementation should 'simply' trade GOT vs .rel.dyn relocation in start.S (I am almost there) and remove fixups in board_init_r. Start.S would apply type 23 fixups only and ignore the rest. Later on we could add a build stage to rewrite the .rel.dyn section as suggested, by filtering out non-type-23 relocs and keeping only the address part of type-23 ones, reducing the .rel.dyn table roughly by half.
The good news is, I can spare a couple more hours today on this. I'll let you all know how this fares!
Thats good news! How much did the image size increase with this table?
./MAKEALL edminiv2 results:
text data bss dec hex filename 141376 4388 16640 162404 27a64 ./u-boot (for GOT reloc) 150160 3819 16640 170619 29a7b ./u-boot (for ELF reloc)
u-boot.bin size in bytes:
145764 (for GOT reloc) 153976 (for ELF reloc)
The .rel.dyn table is 18472 bytes, and should eventually shrink by half, losing about 9 KB. That would bring the u-boot.bin size down to 145 KB, roughly the same size as GOT reloc -- plus we'd save a few code bytes since reloc fixup functions in board_init_r would not be needed any more.
And I am willing to test your efforts on AT91 here, maybe you can send me the changes to .lds and start.S beforehand so I can see what type of relocation info gets produced here.
I'll post an RFC patch within one or two hours.
A rather wild, but quite arch independant additional "build stage" for relocation would be to link u-boot for two different TEXT_BASE values e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff" tool should find the 32 bit places where relocation is required and add a table to the end of u-boot.bin... (Just a rough idea)
That would be the simplest option even though it's obviously not optimal. Feel free to start this if you want, and let's meet at the bridge. :)
Best Regards, Reinhard
Amicalement,
Hello Albert,
Albert ARIBAUD wrote:
Le 04/10/2010 09:27, Reinhard Meyer a écrit :
Dear Albert ARIBAUD,
Right now I can build (not run, mind you) u-boot for edminiv2 without -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all of the .rel.dyn fixup entries are type 23, that is, relative to the base address, which is good. However, here are about ten at the end which are type 2 -- symbol-relative -- and I am studying them in order to see if they are needed.
If type 23 relocations are all that is needed, then a first ARM ELF relocation implementation should 'simply' trade GOT vs .rel.dyn relocation in start.S (I am almost there) and remove fixups in board_init_r. Start.S would apply type 23 fixups only and ignore the rest. Later on we could add a build stage to rewrite the .rel.dyn section as suggested, by filtering out non-type-23 relocs and keeping only the address part of type-23 ones, reducing the .rel.dyn table roughly by half.
The good news is, I can spare a couple more hours today on this. I'll let you all know how this fares!
Thats good news! How much did the image size increase with this table?
./MAKEALL edminiv2 results:
text data bss dec hex filename 141376 4388 16640 162404 27a64 ./u-boot (for GOT reloc) 150160 3819 16640 170619 29a7b ./u-boot (for ELF reloc)
u-boot.bin size in bytes:
145764 (for GOT reloc) 153976 (for ELF reloc)
Huh...
The .rel.dyn table is 18472 bytes, and should eventually shrink by half, losing about 9 KB. That would bring the u-boot.bin size down to 145 KB, roughly the same size as GOT reloc -- plus we'd save a few code
... puuh ;-)
bytes since reloc fixup functions in board_init_r would not be needed any more.
Sounds good. And we can easy test this, by defining CONFIG_RELOC_FIXUP_WORKS for all arm boards ...
And I am willing to test your efforts on AT91 here, maybe you can send me the changes to .lds and start.S beforehand so I can see what type of relocation info gets produced here.
I'll post an RFC patch within one or two hours.
Thanks! I am wating for it, and try your patches too.
bye, Heiko
Le 04/10/2010 10:57, Heiko Schocher a écrit :
./MAKEALL edminiv2 results:
text data bss dec hex filename141376 4388 16640 162404 27a64 ./u-boot (for GOT reloc) 150160 3819 16640 170619 29a7b ./u-boot (for ELF reloc)
u-boot.bin size in bytes:
145764 (for GOT reloc) 153976 (for ELF reloc)
Huh...
The .rel.dyn table is 18472 bytes, and should eventually shrink by half, losing about 9 KB. That would bring the u-boot.bin size down to 145 KB, roughly the same size as GOT reloc -- plus we'd save a few code
... puuh ;-)
:)
Think also that if I'm not mistaken, the GOT has to move to RAM while the .rel.dyn and .dynsym tables will not be necessary once relocated (unless you want u-boot to be able to move around in RAM), so RAM footprint would be smaller.
bytes since reloc fixup functions in board_init_r would not be needed any more.
Sounds good. And we can easy test this, by defining CONFIG_RELOC_FIXUP_WORKS for all arm boards ...
I'll check that.
And I am willing to test your efforts on AT91 here, maybe you can send me the changes to .lds and start.S beforehand so I can see what type of relocation info gets produced here.
I'll post an RFC patch within one or two hours.
Thanks! I am wating for it, and try your patches too.
Testing on the board right now.
bye, Heiko
Amicalement,
Le 04/10/2010 10:57, Heiko Schocher a écrit :
./MAKEALL edminiv2 results:
text data bss dec hex filename141376 4388 16640 162404 27a64 ./u-boot (for GOT reloc) 150160 3819 16640 170619 29a7b ./u-boot (for ELF reloc)
u-boot.bin size in bytes:
145764 (for GOT reloc) 153976 (for ELF reloc)
Huh...
The .rel.dyn table is 18472 bytes, and should eventually shrink by half, losing about 9 KB. That would bring the u-boot.bin size down to 145 KB, roughly the same size as GOT reloc -- plus we'd save a few code
... puuh ;-)
:)
Think also that if I'm not mistaken, the GOT has to move to RAM while the .rel.dyn and .dynsym tables will not be necessary once relocated (unless you want u-boot to be able to move around in RAM), so RAM footprint would be smaller.
You might get away with less relocs using -msdata -G 9999. Not sure how that works on arm.
Jocke
On 04/10/10 19:28, Albert ARIBAUD wrote:
Le 04/10/2010 09:27, Reinhard Meyer a écrit :
Dear Albert ARIBAUD,
Right now I can build (not run, mind you) u-boot for edminiv2 without -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all
Any reason to do it in assembler? Have a look at arch/i386/lib/board.c board_init_f() (especially if you apply my latest patch series)
of the .rel.dyn fixup entries are type 23, that is, relative to the base address, which is good. However, here are about ten at the end which are type 2 -- symbol-relative -- and I am studying them in order to see if they are needed.
Hmm, for x86 they are all type 8 (R_386_RELATIVE) which are a simple Base + Addend (B + A) entries
Type 23 is R_ARM_RELATIVE which are also B + A (although they can also by S + A whatever that means)
If type 23 relocations are all that is needed, then a first ARM ELF relocation implementation should 'simply' trade GOT vs .rel.dyn relocation in start.S (I am almost there) and remove fixups in
Removing fixups - sweet, oh so sweet ;)
board_init_r. Start.S would apply type 23 fixups only and ignore the rest. Later on we could add a build stage to rewrite the .rel.dyn section as suggested, by filtering out non-type-23 relocs and keeping only the address part of type-23 ones, reducing the .rel.dyn table roughly by half.
Also non type-8 for x86 - If all arches reduce down to a single relocation type in .rel.dyn then we can ignore the type and simply strip all the 'type' fields.
[snip]
A rather wild, but quite arch independant additional "build stage" for relocation would be to link u-boot for two different TEXT_BASE values e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff" tool should find the 32 bit places where relocation is required and add a table to the end of u-boot.bin... (Just a rough idea)
I don't think we need to - everything should be handled by .rel.dyn. I wrote a diff tool to do as you suggest, but I have no need for it now
Looks like we could be onto a winner :)
Regards,
Graeme
Le 04/10/2010 11:58, Graeme Russ a écrit :
On 04/10/10 19:28, Albert ARIBAUD wrote:
Le 04/10/2010 09:27, Reinhard Meyer a écrit :
Dear Albert ARIBAUD,
Right now I can build (not run, mind you) u-boot for edminiv2 without -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all
Any reason to do it in assembler? Have a look at arch/i386/lib/board.c board_init_f() (especially if you apply my latest patch series)
The main reasons are that start.S is historically responsible for setting up the C environment, and that only in assembly language can you ensure that no nasty relocation fixup is going to be required by the code that is precisely supposed to do the fixing up. Now it may be possible to do the fixups in C on ARM; that'll be a second step IMO.
of the .rel.dyn fixup entries are type 23, that is, relative to the base address, which is good. However, here are about ten at the end which are type 2 -- symbol-relative -- and I am studying them in order to see if they are needed.
Hmm, for x86 they are all type 8 (R_386_RELATIVE) which are a simple Base + Addend (B + A) entries
Type 23 is R_ARM_RELATIVE which are also B + A (although they can also by S
- A whatever that means)
23 is program base relative -- basically, subtract link-time image base address, add run-time image base address and you're set.
If type 23 relocations are all that is needed, then a first ARM ELF relocation implementation should 'simply' trade GOT vs .rel.dyn relocation in start.S (I am almost there) and remove fixups in
Removing fixups - sweet, oh so sweet ;)
Apparently sweetness is not far away, see below. :)
board_init_r. Start.S would apply type 23 fixups only and ignore the rest. Later on we could add a build stage to rewrite the .rel.dyn section as suggested, by filtering out non-type-23 relocs and keeping only the address part of type-23 ones, reducing the .rel.dyn table roughly by half.
Also non type-8 for x86 - If all arches reduce down to a single relocation type in .rel.dyn then we can ignore the type and simply strip all the 'type' fields.
Can't reduce to a single relocation type as produced by the linker, because references to linker-file-generated symbols seem to always be symbol-relative, not program-relative, even under -pie. This may be something to ask on the binutils mailing list, though.
[snip]
A rather wild, but quite arch independant additional "build stage" for relocation would be to link u-boot for two different TEXT_BASE values e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff" tool should find the 32 bit places where relocation is required and add a table to the end of u-boot.bin... (Just a rough idea)
I don't think we need to - everything should be handled by .rel.dyn. I wrote a diff tool to do as you suggest, but I have no need for it now
Looks like we could be onto a winner :)
Regards,
Graeme
At this point I have an ARM926, ELF-relocating, u-boot reaching prompt.
Environment is correctly read and can be modified (did not try saving though).
Flash operations work (flinfo, erase, cp.b).
Ethernet does not work, however -- ping or tftp just wait without me being able to ^C it. I'll look into that as soon as some domestic chores are done. :)
Amicalement,
On 2010/10/04 4:17 PM, Albert ARIBAUD wrote:
At this point I have an ARM926, ELF-relocating, u-boot reaching prompt.
Environment is correctly read and can be modified (did not try saving though).
Flash operations work (flinfo, erase, cp.b).
Ethernet does not work, however -- ping or tftp just wait without me being able to ^C it. I'll look into that as soon as some domestic chores are done. :)
Excellent work! Congratulations!
Rogan
Le 04/10/2010 16:25, Rogan Dawes a écrit :
On 2010/10/04 4:17 PM, Albert ARIBAUD wrote:
At this point I have an ARM926, ELF-relocating, u-boot reaching prompt.
Environment is correctly read and can be modified (did not try saving though).
Flash operations work (flinfo, erase, cp.b).
Ethernet does not work, however -- ping or tftp just wait without me being able to ^C it. I'll look into that as soon as some domestic chores are done. :)
Excellent work! Congratulations!
Thanks, but I'd suggest to wait for congrats until ethernet works -- tough I've got a clue, I think; it may have to do with the core activating its caches and the driver doing DMA. :)
Amicalement,
On Monday 04 October 2010 17:24:09 Albert ARIBAUD wrote:
Excellent work! Congratulations!
Thanks, but I'd suggest to wait for congrats until ethernet works -- tough I've got a clue, I think; it may have to do with the core activating its caches and the driver doing DMA. :)
Yes. Very likely a aching problem and not an relocation issue. With D-cache enabled some IO drivers might have some problems. This will also be a problem with for example USB support.
Cheers, 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
Le 04/10/2010 18:31, Stefan Roese a écrit :
On Monday 04 October 2010 17:24:09 Albert ARIBAUD wrote:
Excellent work! Congratulations!
Thanks, but I'd suggest to wait for congrats until ethernet works -- tough I've got a clue, I think; it may have to do with the core activating its caches and the driver doing DMA. :)
Yes. Very likely a aching problem and not an relocation issue. With D-cache enabled some IO drivers might have some problems. This will also be a problem with for example USB support.
... and that is a new proof that assumption is the mother of all screw-ups.
It was not a question of caching; it was a question of trashing the address of gd passed from board_init_f() through relocate_code() to board_init_r(), and all code before the marvell ethernet driver not relying on gd anought to break.
Patches to follow right away as an RFC.
Amicalement,
Le 04/10/2010 08:40, Albert ARIBAUD a écrit :
Right now I can build (not run, mind you) u-boot for edminiv2 without -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all of the .rel.dyn fixup entries are type 23, that is, relative to the base address, which is good. However, here are about ten at the end which are type 2 -- symbol-relative -- and I am studying them in order to see if they are needed.
Those type 2 relocations correspond to symbols which are defined in the linker file (e.g. __got_start and __got_end) and thus are absolute, not relative.
If type 23 relocations are all that is needed, then a first ARM ELF relocation implementation should 'simply' trade GOT vs .rel.dyn relocation in start.S (I am almost there) and remove fixups in board_init_r. Start.S would apply type 23 fixups only and ignore the rest. Later on we could add a build stage to rewrite the .rel.dyn section as suggested, by filtering out non-type-23 relocs and keeping only the address part of type-23 ones, reducing the .rel.dyn table roughly by half.
Al right, so type 2 are needed too, and of course they need a different processing than type 23, but the good news is, one should easily make the linker provide relative values for type 2 fixups by telling it to base the executable at offset 0. The interest of doing so is that all fixups in .rel.dyn could be processed homogeneously by adding the actual base address of the code to each fixup location.
Amicalement,
Hi Wolfgang,
<snip>
Well, please keep in mind that all this code is working find on PowerPC, which has been using relocation right from the beginning.
It is my understanding that we don't suffer from this issue any more on PPC - Peter Tyser posted relocation fixup patches for PPC about a year ago or so.
I have to admit that I cannot remeber the final result of this discussion (there were tool chain dependencies?), but IIRC this has been solved for PPC.
We should do the same for AMR now.
Peter, could you please fill in the details of that old story?
I haven't been following the ARM relocation thread very closely, but a summary of the PPC relocation is: - Prior to commit 858290178f222d998b6425d85cf06822467918f3 PPC supported basic relocation, but didn't relocate static pointers, eg pointers in a structure, such as the strings in the cmd_pca953x[] table. So we had to use the hokey "struct->field += gd->reloc_off" fixups that we still have for some arches.
- Adding "-mrelocatable" to the gcc's flags would add additional relocation info into the ".fixup" section that allowed us to properly fixup static pointers and get rid of the "+= gd->reloc_off" fixups. The additional fixup code was relatively small, in the 1-2% ballpark I think.
- Unfortunately "-mrelocatable" is PPC specific. I remember dabbling with other, more general relocation flags like -fPIC, pie, etc, but went with -mrelocatable mainly because it was a smaller, easier change. We could share the already-used relocation fixup code in many PPC arch's start.S, so it wasn't too hard to get working unlike the other relocation schemes.
- Graeme Russ was working on relocation for x86 near the same time. He started this thread which may provide useful info: http://www.mail-archive.com/u-boot@lists.denx.de/msg23347.html He discusses the impact of other compile flags, most of which were generic so could apply to this ARM discussion hopefully.
Regards, Peter
Dear Peter Tyser,
In message 1286167382.22760.19.camel@ptyser-laptop you wrote:
Peter, could you please fill in the details of that old story?
I haven't been following the ARM relocation thread very closely, but a summary of the PPC relocation is:
- Prior to commit 858290178f222d998b6425d85cf06822467918f3 PPC supported
basic relocation, but didn't relocate static pointers, eg pointers in a structure, such as the strings in the cmd_pca953x[] table. So we had to use the hokey "struct->field += gd->reloc_off" fixups that we still have for some arches.
- Adding "-mrelocatable" to the gcc's flags would add additional
relocation info into the ".fixup" section that allowed us to properly fixup static pointers and get rid of the "+= gd->reloc_off" fixups. The additional fixup code was relatively small, in the 1-2% ballpark I think.
- Unfortunately "-mrelocatable" is PPC specific. I remember dabbling
with other, more general relocation flags like -fPIC, pie, etc, but went with -mrelocatable mainly because it was a smaller, easier change. We could share the already-used relocation fixup code in many PPC arch's start.S, so it wasn't too hard to get working unlike the other relocation schemes.
- Graeme Russ was working on relocation for x86 near the same time. He
started this thread which may provide useful info: http://www.mail-archive.com/u-boot@lists.denx.de/msg23347.html He discusses the impact of other compile flags, most of which were generic so could apply to this ARM discussion hopefully.
Thanks alot for the summary.
Best regards,
Wolfgang Denk
Hi Wolfgang,
<snip>
Well, please keep in mind that all this code is working find on PowerPC, which has been using relocation right from the beginning.
It is my understanding that we don't suffer from this issue any more on PPC - Peter Tyser posted relocation fixup patches for PPC about a year ago or so.
I have to admit that I cannot remeber the final result of this discussion (there were tool chain dependencies?), but IIRC this has been solved for PPC.
We should do the same for AMR now.
Peter, could you please fill in the details of that old story?
I haven't been following the ARM relocation thread very closely, but a summary of the PPC relocation is:
- Prior to commit 858290178f222d998b6425d85cf06822467918f3 PPC supported
basic relocation, but didn't relocate static pointers, eg pointers in a structure, such as the strings in the cmd_pca953x[] table. So we had to use the hokey "struct->field += gd->reloc_off" fixups that we still have for some arches.
- Adding "-mrelocatable" to the gcc's flags would add additional
relocation info into the ".fixup" section that allowed us to properly fixup static pointers and get rid of the "+= gd->reloc_off" fixups. The additional fixup code was relatively small, in the 1-2% ballpark I think.
- Unfortunately "-mrelocatable" is PPC specific. I remember dabbling
with other, more general relocation flags like -fPIC, pie, etc, but went with -mrelocatable mainly because it was a smaller, easier change. We could share the already-used relocation fixup code in many PPC arch's start.S, so it wasn't too hard to get working unlike the other relocation schemes.
- Graeme Russ was working on relocation for x86 near the same time. He
started this thread which may provide useful info: http://www.mail-archive.com/u-boot@lists.denx.de/msg23347.html He discusses the impact of other compile flags, most of which were generic so could apply to this ARM discussion hopefully.
However, I think we will loose the possibility to add "link once, burn and run anywhere" feature I impl. once(but it was at the time deemed to intrusive) if we skip -fPIC and go for the linker -pie relocation.
I would suggest to leave ppc as is for the time being and see how -pie works out on the other archs.
Jocke
Le 04/10/2010 09:36, Joakim Tjernlund a écrit :
However, I think we will loose the possibility to add "link once, burn and run anywhere" feature I impl. once(but it was at the time deemed to intrusive) if we skip -fPIC and go for the linker -pie relocation.
On ARM at least, I don't think so. From what I see, the -pie ld option without the -fPIC/-fPIE compiler option alone builds a .rel.syn table that contains all necessary fixes to mve the code anywhere.
However:
I would suggest to leave ppc as is for the time being and see how -pie works out on the other archs.
I second that. More precisely, I'd suggest to wait for ELF relocation to succeed on arm296ejs before considering any other ARM, then any other arch.
Amicalement,
Le 04/10/2010 09:36, Joakim Tjernlund a écrit :
However, I think we will loose the possibility to add "link once, burn and
run anywhere"
feature I impl. once(but it was at the time deemed to intrusive) if we skip -fPIC and go for the linker -pie relocation.
On ARM at least, I don't think so. From what I see, the -pie ld option without the -fPIC/-fPIE compiler option alone builds a .rel.syn table that contains all necessary fixes to mve the code anywhere.
hmm, maybe my memory fails me but doesn't these relocs change the code to relocate accesses? While in flash you can't do that. Perhaps this is fixable too with the LINK_OFF method I impl. but memory fails me as it was quite some time ago.
Jocke
Le 04/10/2010 10:28, Joakim Tjernlund a écrit :
Le 04/10/2010 09:36, Joakim Tjernlund a écrit :
However, I think we will loose the possibility to add "link once, burn and
run anywhere"
feature I impl. once(but it was at the time deemed to intrusive) if we skip -fPIC and go for the linker -pie relocation.
On ARM at least, I don't think so. From what I see, the -pie ld option without the -fPIC/-fPIE compiler option alone builds a .rel.syn table that contains all necessary fixes to mve the code anywhere.
hmm, maybe my memory fails me but doesn't these relocs change the code to relocate accesses? While in flash you can't do that. Perhaps this is fixable too with the LINK_OFF method I impl. but memory fails me as it was quite some time ago.
Jocke
Actually the principle is to link with TEXT_BASE equal to the NOR FLASH location of the image [1] so that relocation is not needed there. Only when you move the code to RAM do you need relocation.
[1] which voids my idea of setting TEXT_BASE to 0, btw, but that does not matter much anyway.
Amicalement,
Albert ARIBAUD albert.aribaud@free.fr wrote on 2010/10/04 10:33:05:
Le 04/10/2010 10:28, Joakim Tjernlund a écrit :
Le 04/10/2010 09:36, Joakim Tjernlund a écrit :
However, I think we will loose the possibility to add "link once, burn and
run anywhere"
feature I impl. once(but it was at the time deemed to intrusive) if we skip -fPIC and go for the linker -pie relocation.
On ARM at least, I don't think so. From what I see, the -pie ld option without the -fPIC/-fPIE compiler option alone builds a .rel.syn table that contains all necessary fixes to mve the code anywhere.
hmm, maybe my memory fails me but doesn't these relocs change the code to relocate accesses? While in flash you can't do that. Perhaps this is fixable too with the LINK_OFF method I impl. but memory fails me as it was quite some time ago.
JockeActually the principle is to link with TEXT_BASE equal to the NOR FLASH location of the image [1] so that relocation is not needed there. Only when you move the code to RAM do you need relocation.
Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0) but burn and run into another address. I impl. this quite some time ago for PPC(search for LINK_OFF)
Jocke
Le 04/10/2010 10:52, Joakim Tjernlund a écrit :
Albert ARIBAUDalbert.aribaud@free.fr wrote on 2010/10/04 10:33:05:
Le 04/10/2010 10:28, Joakim Tjernlund a écrit :
Le 04/10/2010 09:36, Joakim Tjernlund a écrit :
However, I think we will loose the possibility to add "link once, burn and
run anywhere"
feature I impl. once(but it was at the time deemed to intrusive) if we skip -fPIC and go for the linker -pie relocation.
On ARM at least, I don't think so. From what I see, the -pie ld option without the -fPIC/-fPIE compiler option alone builds a .rel.syn table that contains all necessary fixes to mve the code anywhere.
hmm, maybe my memory fails me but doesn't these relocs change the code to relocate accesses? While in flash you can't do that. Perhaps this is fixable too with the LINK_OFF method I impl. but memory fails me as it was quite some time ago.
JockeActually the principle is to link with TEXT_BASE equal to the NOR FLASH location of the image [1] so that relocation is not needed there. Only when you move the code to RAM do you need relocation.
Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0) but burn and run into another address. I impl. this quite some time ago for PPC(search for LINK_OFF)
I am ultimately looking for same here on ARM.
Note however that linking for base address 0 is not mandatory for achieving true position independence. What is required is that the code which runs from power-up until relocation be able to run anywhere, i.e., this code should not require any relocation fixup. That can be achieved on ARM by using only relative branches and accessing data only relative to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
Amicalement,
Dear Albert ARIBAUD,
In message 4CA999EE.5030309@free.fr you wrote:
Note however that linking for base address 0 is not mandatory for achieving true position independence. What is required is that the code which runs from power-up until relocation be able to run anywhere, i.e., this code should not require any relocation fixup. That can be achieved on ARM by using only relative branches and accessing data only relative to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
That means you need to build all of U-Boot that way, because significant parts of the code already run before relocation (including all clocks and timers setup, console setup, printf and all routines these pull in).
Best regards,
Wolfgang Denk
On 10/4/2010 3:13 AM, Wolfgang Denk wrote:
Dear Albert ARIBAUD,
In message4CA999EE.5030309@free.fr you wrote:
Note however that linking for base address 0 is not mandatory for achieving true position independence. What is required is that the code which runs from power-up until relocation be able to run anywhere, i.e., this code should not require any relocation fixup. That can be achieved on ARM by using only relative branches and accessing data only relative to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
That means you need to build all of U-Boot that way, because significant parts of the code already run before relocation (including all clocks and timers setup, console setup, printf and all routines these pull in).
Yes, I think Wolfgang is correct. This is not going to be easy to do in general. To run anywhere, the code must be true Position Independent code. If you intend to use any C code in the initialization, this will result in needing -fPIC for at least that code. I am not sure you can mix -fPIC and non -fPIC code in the same link, but I expect not. I am a bit surprised that it is possible to get even the initialization code to be Position Independent, but it appears that on at least some PPC it is possible/has been done. On a related topic, I did find some information on the -mrelocatable history. Take a look at http://www.mail-archive.com/gcc@gcc.gnu.org/msg02528.html. If you read both thread entries, it explains -mrelocatable as more or less the post-processor that re-formats the ELF relocation information into a smaller format and puts it in the text as another segment. What Albert is doing now, and Graeme did before, is the first option, creating a loader that understands ELF. This has the advantage that it will work on all architectures. However, once this understanding is in place, it would be easy to write a small post-processing program that would reduce the size of the relocation entries, much like -mrelocatable does. This may or may not be necessary, but it is certainly possible.
Best Regards, Bill Campbell
Best regards,
Wolfgang Denk
Le 04/10/2010 17:28, J. William Campbell a écrit :
On 10/4/2010 3:13 AM, Wolfgang Denk wrote:
Dear Albert ARIBAUD,
In message4CA999EE.5030309@free.fr you wrote:
Note however that linking for base address 0 is not mandatory for achieving true position independence. What is required is that the code which runs from power-up until relocation be able to run anywhere, i.e., this code should not require any relocation fixup. That can be achieved on ARM by using only relative branches and accessing data only relative to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
That means you need to build all of U-Boot that way, because significant parts of the code already run before relocation (including all clocks and timers setup, console setup, printf and all routines these pull in).
Yes, I think Wolfgang is correct. This is not going to be easy to do in general. To run anywhere, the code must be true Position Independent code. If you intend to use any C code in the initialization, this will result in needing -fPIC for at least that code. I am not sure you can mix -fPIC and non -fPIC code in the same link, but I expect not. I am a bit surprised that it is possible to get even the initialization code to be Position Independent, but it appears that on at least some PPC it is possible/has been done.
I'm not entirely sure about -fPIC, but it is possible indeed that true position independence might need it. For the moment, I'll settle for ELF relocatable. :)
On a related topic, I did find some information on the -mrelocatable history. Take a look at http://www.mail-archive.com/gcc@gcc.gnu.org/msg02528.html. If you read both thread entries, it explains -mrelocatable as more or less the post-processor that re-formats the ELF relocation information into a smaller format and puts it in the text as another segment. What Albert is doing now, and Graeme did before, is the first option, creating a loader that understands ELF. This has the advantage that it will work on all architectures. However, once this understanding is in place, it would be easy to write a small post-processing program that would reduce the size of the relocation entries, much like -mrelocatable does. This may or may not be necessary, but it is certainly possible.
Best Regards, Bill Campbell
Thanks Bill. I'll look into it once I get the current issues resolved; however it seems GOT-related, and -pie is not GOT-based -- and unlike GOT, -pie handles pointers in data, for instance, removing the need for manual fixups.
Amicalement,
Dear "J. William Campbell",
In message 4CA9F294.8080007@comcast.net you wrote:
Yes, I think Wolfgang is correct. This is not going to be easy to do in general. To run anywhere, the code must be true Position Independent code. If you intend to use any C code in the initialization, this will result in needing -fPIC for at least that code. I am not sure you can mix -fPIC and non -fPIC code in the same link, but I expect not. I am a bit surprised that it is possible to get even the initialization code to be Position Independent, but it appears that on at least some PPC it is possible/has been done.
Not really. On PowerPC, only the first 20 or 30 lines of assembler code in start.S are position independent; then we compute the link (resp. execution) address and branch to it. From then, we run from the very address range we were linked for (starting at TEXT_BASE).
Albert is doing now, and Graeme did before, is the first option, creating a loader that understands ELF. This has the advantage that it will work on all architectures. However, once this understanding is in place, it would be easy to write a small post-processing program that would reduce the size of the relocation entries, much like -mrelocatable does. This may or may not be necessary, but it is certainly possible.
Eventually we might even add -mrelocatable support for the other architectures to the tool chain.
Best regards,
Wolfgang Denk
On 10/4/2010 10:06 AM, Wolfgang Denk wrote:
Dear "J. William Campbell",
In message4CA9F294.8080007@comcast.net you wrote:
Yes, I think Wolfgang is correct. This is not going to be easy to do in general. To run anywhere, the code must be true Position Independent code. If you intend to use any C code in the initialization, this will result in needing -fPIC for at least that code. I am not sure you can mix -fPIC and non -fPIC code in the same link, but I expect not. I am a bit surprised that it is possible to get even the initialization code to be Position Independent, but it appears that on at least some PPC it is possible/has been done.
Not really. On PowerPC, only the first 20 or 30 lines of assembler code in start.S are position independent; then we compute the link (resp. execution) address and branch to it. From then, we run from the very address range we were linked for (starting at TEXT_BASE).
Hi Wolfgang, You are of course correct. I was referring more to Jocke's (joakim.tjernlund@transmode.se) statements regarding:
Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0) but burn and run into another address. I impl. this quite some time ago for PPC(search for LINK_OFF)
I understand from his comment that he had achieved total PIC for the initialization, that would run at any location regardless of TEXT_BASE. I think this code was not accepted into mainline, so it is not a problem at present. However, any relocation code added would have to be modified by Jocke if he wished to preserve that capability. I am amazed that he was able to get the rest of u-boot to work under the constraints you pointed out. It must have been quite tedious.
I also wish to support Graeme's desire that the added relocation code at the end of the day be written in C. The routine to do the relocation does not require .bss and is not real long. The obvious advantage of this approach is that all architectures can use it. The ELF relocation codes will have to be changed to the architecture equivalents, and in some casesarchitecture specific relocation code processing added, but the theory will always be the same. This approach will make using relocation much easier/trivial for new architecture ports, thereby reducing resistance to doing it!
Best Regards, Bill Campbell
Albert is doing now, and Graeme did before, is the first option, creating a loader that understands ELF. This has the advantage that it will work on all architectures. However, once this understanding is in place, it would be easy to write a small post-processing program that would reduce the size of the relocation entries, much like -mrelocatable does. This may or may not be necessary, but it is certainly possible.
Eventually we might even add -mrelocatable support for the other architectures to the tool chain.
Best regards,
Wolfgang Denk
"J. William Campbell" jwilliamcampbell@comcast.net wrote on 2010/10/04 19:59:47:
On 10/4/2010 10:06 AM, Wolfgang Denk wrote: Dear "J. William Campbell",
In message 4CA9F294.8080007@comcast.net you wrote:
Yes, I think Wolfgang is correct. This is not going to be easy to do in general. To run anywhere, the code must be true Position Independent code. If you intend to use any C code in the initialization, this will result in needing -fPIC for at least that code. I am not sure you can mix -fPIC and non -fPIC code in the same link, but I expect not. I am a bit surprised that it is possible to get even the initialization code to be Position Independent, but it appears that on at least some PPC it is possible/has been done.
Not really. On PowerPC, only the first 20 or 30 lines of assembler code in start.S are position independent; then we compute the link (resp. execution) address and branch to it. From then, we run from the very address range we were linked for (starting at TEXT_BASE).
Hi Wolfgang, You are of course correct. I was referring more to Jocke's ( joakim.tjernlund@transmode.se) statements regarding: Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0) but burn and run into another address. I impl. this quite some time ago for PPC(search for LINK_OFF)
I understand from his comment that he had achieved total PIC for theinitialization, that would run at any location regardless of TEXT_BASE. I think this code was not accepted into mainline, so it is not a problem at present. However, any relocation code added would have to be modified by Jocke if he wished to preserve that capability. I am amazed that he was able to get the rest of u-boot to work under the constraints you pointed out. It must have been quite tedious.
:), actually it wasn't that bad. Wolfgang nearly accepted the code even :) Mainly, I had to wrap code that accessed global data with a LINK_OFF() function that calculated the offset and only in code that executed before relocation. And fix a few things in start.S to be PIC.
Jocke
Dear Joakim Tjernlund,
In message OFF06E784F.A10A5A15-ONC12577B2.0065FB3C-C12577B2.0066D69F@transmode.se you wrote:
:), actually it wasn't that bad. Wolfgang nearly accepted the code even :)
Yes, I was really tempted because I do appreciate the value of such a feature.
Mainly, I had to wrap code that accessed global data with a LINK_OFF() function that calculated the offset and only in code that executed before relocation.
That was the part that gave me the creeps. It looked too much unreadable and error prone to me, especially as this is not only a one-time conversion but has to be kept in mind for all changes to related code.
I'm still undecided, to be honest.
Best regards,
Wolfgang Denk
Wolfgang Denk wd@denx.de wrote on 2010/10/04 23:10:31:
Dear Joakim Tjernlund,
In message <OFF06E784F.A10A5A15-ONC12577B2.0065FB3C-C12577B2. 0066D69F@transmode.se> you wrote:
:), actually it wasn't that bad. Wolfgang nearly accepted the code even :)
Yes, I was really tempted because I do appreciate the value of such a feature.
Mainly, I had to wrap code that accessed global data with a LINK_OFF() function that calculated the offset and only in code that executed before relocation.
That was the part that gave me the creeps. It looked too much unreadable and error prone to me, especially as this is not only a one-time conversion but has to be kept in mind for all changes to related code.
Yes, that is a bummer. If one could convince gcc to do %pc relative addressing on strings/constant data the we would be in business I think. Anyone got gcc connections?
I'm still undecided, to be honest.
On Monday, October 4, 2010, Wolfgang Denk wd@denx.de wrote:
Dear Albert ARIBAUD,
In message 4CA999EE.5030309@free.fr you wrote:
Note however that linking for base address 0 is not mandatory for achieving true position independence. What is required is that the code which runs from power-up until relocation be able to run anywhere, i.e., this code should not require any relocation fixup. That can be achieved on ARM by using only relative branches and accessing data only relative to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
That means you need to build all of U-Boot that way, because significant parts of the code already run before relocation (including all clocks and timers setup, console setup, printf and all routines these pull in).
Have a look at x86 - Relocation is performed at the first possible moment
This made full relocation for x86 was relatively trivial :)
Regards,
Graeme
Dear Graeme Russ,
In message AANLkTikNKFjUQ6Dmw3Ey=0qiEkiM716E=1+3nP3jG_ss@mail.gmail.com you wrote:
That means you need to build all of U-Boot that way, because significant parts of the code already run before relocation (including all clocks and timers setup, console setup, printf and all routines these pull in).
Have a look at x86 - Relocation is performed at the first possible moment
This made full relocation for x86 was relatively trivial :)
Well, U-Boot is not only a fancy boot loader, but also a hardware bringup tool. It was designed to make it as easy for the software guy to bring up code on new hardware. That means, that one of the very first things we always try to do is get a (usually serial) console port working, so we can use printf() to get some helpful information out. This happens especially before doing anythign that is known to be complicated and error prone, like especially the initialization of both the memory controller and the RAM system on the board.
I am aware that there are systems out there which perform the RAM initialization either in hardware or for example table-driven by some built-in ROM boot loader code. Here RAM initalization is obviously not such an issue, but nevertheless there is a LOT of code running before we relocate the code to RAM.
Best regards,
Wolfgang Denk
Dear Joakim Tjernlund,
In message OFE6153B0A.80735DD8-ONC12577B2.002963A5-C12577B2.0029D57C@transmode.se you wrote:
However, I think we will loose the possibility to add "link once, burn and run anywhere" feature I impl. once(but it was at the time deemed to intrusive) if we skip -fPIC and go for the linker -pie relocation.
I would suggest to leave ppc as is for the time being and see how -pie works out on the other archs.
I see two tasks here:
Prio 1: fix the current problems on ARM
Prio 2: make architectures as similar as possible.
My dream would be to have all that work in the same way (even if eventually differen mechanisms need ti be deployed) on ARM, x86 and PPC (and MIPS? and others?).
Best regards,
Wolfgang Denk
Hello Wolfgang, Reinhard,
Wolfgang Denk wrote:
Dear Reinhard Meyer,
In message 4CA5D857.5010009@emk-elektronik.de you wrote:
The environment issues still persist. I am at a loss there now.
Observation: the old style commands "setenv", "printenv", etc. work, but any "env" command except for "env" alone crashes.
OK. If "printenv" works and "env print" fails then it has nothing to do with the environment code itself, as both call the same function.
It must have something to do with the implementation of subcommands then. See do_env() in "common/cmd_nvedit.c"; check if the command table address for find_cmd_tbl() is OK.
Eventually other commands with subcommands fail as well (i2c ?) ?
Yep, I think thats the direction. i2c should work, because the subcommands get fixed in board_init_r() through i2c_reloc()
For fixing commandtable (and subcommands) I made a common function fixup_cmdtable() in common/command.c see also commit 620f1f6a64095ed558e68d37f1965d015cd49b02
Note: For powerpc this fixups are not needed, so CONFIG_RELOC_FIXUP_WORKS is defined for powerpc ... don;t sure, how exactly this work on powerpc ...
bye, Heiko
Dear Reinhard Meyer,
In message 4CA5D26D.2090505@emk-elektronik.de you wrote:
If this is really for all AT91 SoCs, then please feel free to introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually you can clean up some other such #if's on the way.
That would have to be set either in each board.h file or in each at91*.h file.
Isn't there a central place?
Probably there is a header file common to all AT91 SoCs when then can use a single such construct to #define the new variable so you don;t have to touch all the many board config files.
The only files I can see included in each instance are those of the kind hardware.h, memory_map.h and similiar. The define does not really belong in any of those.
If no better place is found, we can even add this to <common.h> (we do similar things there already for CONFIG_MPC866_FAMILY, CONFIG_MPC86x, CONFIG_MPC8272_FAMILY, CONFIG_TQM8xxM, CONFIG_TQM8xxL, etc.
Q: after relocation, "gd->" values are still available, right?
Yes, it gets relocated to RAM, too, and then remains available.
I am searching for further "violations"...
Good luck!
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
In message 4CA5D26D.2090505@emk-elektronik.de you wrote:
If this is really for all AT91 SoCs, then please feel free to introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually you can clean up some other such #if's on the way.
That would have to be set either in each board.h file or in each at91*.h file.
Isn't there a central place?
Probably there is a header file common to all AT91 SoCs when then can use a single such construct to #define the new variable so you don;t have to touch all the many board config files.
The only files I can see included in each instance are those of the kind hardware.h, memory_map.h and similiar. The define does not really belong in any of those.
I tried arch-at91/hardware.h. Apparently it gets included after global_data.h. Same would be true for all files included inside hardware.h. Without reordering the includes (which I don't want to do) the remaining place would be each board's config file. Since all AT91 boards are broken anyway right now; adding it could be left to the respective maintainers :)
If no better place is found, we can even add this to <common.h> (we do similar things there already for CONFIG_MPC866_FAMILY, CONFIG_MPC86x, CONFIG_MPC8272_FAMILY, CONFIG_TQM8xxM, CONFIG_TQM8xxL, etc.
Into this chain? #if defined(CONFIG_MPC852) || defined(CONFIG_MPC852T) || \ defined(CONFIG_MPC859) || defined(CONFIG_MPC859T) || \ defined(CONFIG_MPC859DSL) || \ defined(CONFIG_MPC866) || defined(CONFIG_MPC866T) || \ defined(CONFIG_MPC866P) # define CONFIG_MPC866_FAMILY 1 #elif defined(CONFIG_MPC870) \
Ugly, but possible :) Just hoping that everytime a new AT91 comes out, it will not be forgotten to be added there ;)
Reinhard
I wrote:
Dear Wolfgang Denk,
In message 4CA5D26D.2090505@emk-elektronik.de you wrote:
If this is really for all AT91 SoCs, then please feel free to introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually you can clean up some other such #if's on the way.
That would have to be set either in each board.h file or in each at91*.h file.
Isn't there a central place?
Probably there is a header file common to all AT91 SoCs when then can use a single such construct to #define the new variable so you don;t have to touch all the many board config files.
The only files I can see included in each instance are those of the kind hardware.h, memory_map.h and similiar. The define does not really belong in any of those.
I tried arch-at91/hardware.h. Apparently it gets included after global_data.h. Same would be true for all files included inside hardware.h. Without reordering the includes (which I don't want to do) the remaining place would be each board's config file. Since all AT91 boards are broken anyway right now; adding it could be left to the respective maintainers :)
If no better place is found, we can even add this to <common.h> (we do similar things there already for CONFIG_MPC866_FAMILY, CONFIG_MPC86x, CONFIG_MPC8272_FAMILY, CONFIG_TQM8xxM, CONFIG_TQM8xxL, etc.
Into this chain? #if defined(CONFIG_MPC852) || defined(CONFIG_MPC852T) || \ defined(CONFIG_MPC859) || defined(CONFIG_MPC859T) || \ defined(CONFIG_MPC859DSL) || \ defined(CONFIG_MPC866) || defined(CONFIG_MPC866T) || \ defined(CONFIG_MPC866P) # define CONFIG_MPC866_FAMILY 1 #elif defined(CONFIG_MPC870) \
Ugly, but possible :) Just hoping that everytime a new AT91 comes out, it will not be forgotten to be added there ;)
I have added required variables to global_data.h, it looks like this now:
... #ifdef CONFIG_FSL_ESDHC unsigned long sdhc_clk; #endif #if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9XE) /* "static data" needed by at91's clock.c */ unsigned long cpu_clk_rate_hz; unsigned long main_clk_rate_hz; unsigned long mck_rate_hz; unsigned long plla_rate_hz; unsigned long pllb_rate_hz; unsigned long at91_pllb_usb_init; /* "static data" needed by at91's timer.c */ unsigned long timer_rate_hz; unsigned long tbl; unsigned long tbu; unsigned long long timer_reset_value; #endif #if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC) unsigned long relocaddr; /* Start address of U-Boot in RAM */ ...
At the same time I cleaned up timer.c to really operate using 64 bit values like on powerpc (hence tbu, tbl)
The Qs that remain now are:
1. currently I see about 8 CONFIG_AT91SAMxxx defines required (not only 2 like above). The method is still undecided how to handle this. Add "CONFIG_AT91SAM9_FAMILY" to "common.h"? Or put all 8 defined() into "global_data.h" (would give 4 lines there)?
2. How many patches would this change need, and who would collect them? It affects at worst 3 areas: common.h, arm/global_data.h, at91/clock.c+timer.c --- Or is it ok to put that into one patch and add it later to my atmel tree?
Reinhard
Dear Reinhard Meyer,
In message 4CA985DC.4000208@emk-elektronik.de you wrote:
The Qs that remain now are:
- currently I see about 8 CONFIG_AT91SAMxxx defines required
(not only 2 like above). The method is still undecided how to handle this. Add "CONFIG_AT91SAM9_FAMILY" to "common.h"? Or put all 8 defined() into "global_data.h" (would give 4 lines there)?
You wille ventually need this in other locations as well, so please add a single 4-line-#if to some appropriate header file; if there is no better file for AT91 I will accept this for common.h
- How many patches would this change need, and who would collect them?
It affects at worst 3 areas: common.h, arm/global_data.h, at91/clock.c+timer.c --- Or is it ok to put that into one patch and add it later to my atmel tree?
I see two patches:
Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY instead.
Patch 2 will fix at91/clock.c+timer.c
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
The Qs that remain now are:
- currently I see about 8 CONFIG_AT91SAMxxx defines required
(not only 2 like above). The method is still undecided how to handle this. Add "CONFIG_AT91SAM9_FAMILY" to "common.h"? Or put all 8 defined() into "global_data.h" (would give 4 lines there)?
You wille ventually need this in other locations as well, so please add a single 4-line-#if to some appropriate header file; if there is no better file for AT91 I will accept this for common.h
There is no at91 common header file that gets included BEFORE global_data.h, unfortunately.
- How many patches would this change need, and who would collect them?
It affects at worst 3 areas: common.h, arm/global_data.h, at91/clock.c+timer.c --- Or is it ok to put that into one patch and add it later to my atmel tree?
I see two patches:
Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY instead.
Patch 2 will fix at91/clock.c+timer.c
Maybe there is a misunderstanding here. I see three patches
1. introduce CONFIG_AT91SAM9_FAMILY to common.h
2. add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the required variables
3. change clock and timer.
2+3 combined in one patch? OK
And I bet, this must be a patch series ;)
Reinhard
Dear Reinhard Meyer,
In message 4CA99373.7020704@emk-elektronik.de you wrote:
Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY instead.
Patch 2 will fix at91/clock.c+timer.c
Maybe there is a misunderstanding here. I see three patches
introduce CONFIG_AT91SAM9_FAMILY to common.h
add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the
required variables
- change clock and timer.
2+3 combined in one patch? OK
No. 1 + 2 belong together; they deal with the same topic and should be applied as a single commit.
3 is a completely different story and goes in a separate patch.
And I bet, this must be a patch series ;)
Not necessarily. The 1+2 combo is independent from 3 (but needed as a prerequisite, so posting this in a series is fine as well).
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY instead.
Patch 2 will fix at91/clock.c+timer.c
Maybe there is a misunderstanding here. I see three patches
introduce CONFIG_AT91SAM9_FAMILY to common.h
add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the
required variables
- change clock and timer.
2+3 combined in one patch? OK
No. 1 + 2 belong together; they deal with the same topic and should be applied as a single commit.
Ok, so introducing (at this point still) unreferenced variables here is OK?
3 is a completely different story and goes in a separate patch.
And I bet, this must be a patch series ;)
Not necessarily. The 1+2 combo is independent from 3 (but needed as a prerequisite, so posting this in a series is fine as well).
Ok, I rather post it independant, if thats OK, patch series are still more trouble ;)
Best Regards,
Reinhard
Dear Reinhard Meyer,
In message 4CA995D7.5010008@emk-elektronik.de you wrote:
Maybe there is a misunderstanding here. I see three patches
introduce CONFIG_AT91SAM9_FAMILY to common.h
add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the
required variables
- change clock and timer.
2+3 combined in one patch? OK
No. 1 + 2 belong together; they deal with the same topic and should be applied as a single commit.
Ok, so introducing (at this point still) unreferenced variables here is OK?
It will not be unreferenced.
It will be needed to replace the "#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9XE)" in global_data.h and a ton of similar ocurrences like these:
arch/arm/cpu/arm926ejs/at91/clock.c:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) arch/arm/cpu/arm926ejs/at91/clock.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) arch/arm/cpu/arm926ejs/at91/lowlevel_init.S:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9263) arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9260) arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45) arch/arm/include/asm/arch-at91/at91_pio.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \ arch/arm/include/asm/arch-at91/at91_pio.h: defined(CONFIG_AT91SAM9G10) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_pio.h:#elif defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45) || \ arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9G10) arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/i2c/soft_i2c.c: defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \ drivers/net/macb.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \ drivers/net/macb.c: defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/net/macb.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \ drivers/net/macb.c: defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/usb/host/ohci-at91.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \ drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/usb/host/ohci-at91.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) include/i2c.h: defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
I see the misunderstanding here:
It will be needed to replace the "#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9XE)" in global_data.h and a ton of similar ocurrences like these:
That does not exist yet (its only in my local tree so far!)
arch/arm/cpu/arm926ejs/at91/clock.c:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) arch/arm/cpu/arm926ejs/at91/clock.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) arch/arm/cpu/arm926ejs/at91/lowlevel_init.S:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9263) arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9260) arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \ arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45) arch/arm/include/asm/arch-at91/at91_pio.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \ arch/arm/include/asm/arch-at91/at91_pio.h: defined(CONFIG_AT91SAM9G10) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91_pio.h:#elif defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45) || \ arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9G10) arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/i2c/soft_i2c.c: defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \ drivers/net/macb.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \ drivers/net/macb.c: defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/net/macb.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \ drivers/net/macb.c: defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/usb/host/ohci-at91.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \ drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) drivers/usb/host/ohci-at91.c: defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) include/i2c.h: defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \
Careful here, the ones here are distinguishing between different "family" members! See the abundance of #elif's there!
Reinhard
Dear Wolfgang Denk,
I see the misunderstanding here:
It will be needed to replace the "#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9XE)" in global_data.h and a ton of similar ocurrences like these:
That does not exist yet (its only in my local tree so far!)
arch/arm/cpu/arm926ejs/at91/clock.c:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) arch/arm/cpu/arm926ejs/at91/clock.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
Careful here, the ones here are distinguishing between different "family" members! See the abundance of #elif's there!
The actual example code: #if defined(CONFIG_AT91RM9200) /* mdiv */ gd->mck_rate_hz = freq / (1 + ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 8)); #elif defined(CONFIG_AT91SAM9G20) /* mdiv ; (x >> 7) = ((x >> 8) * 2) */ gd->mck_rate_hz = (mckr & AT91_PMC_MCKR_MDIV_MASK) ? freq / ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 7) : freq; if (mckr & AT91_PMC_MCKR_MDIV_MASK) freq /= 2; /* processor clock division */ #elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) gd->mck_rate_hz = (mckr & AT91_PMC_MCKR_MDIV_MASK) == (AT91_PMC_MCKR_MDIV_2 | AT91_PMC_MCKR_MDIV_4) ? freq / 3 : freq / (1 << ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 8)); #else gd->mck_rate_hz = freq / (1 << ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 8)); #endif
Note: I did not write that code, and I am sure it could be made to look less obfuscated. But that's not my problem right now.
I looked at common.h which is already overcrowded by arch and even board specifics. I don't want to add anything there. In my tree I have solved the issue as follows:
1. The board's config file defines "CONFIG_AT91FAMILY" like this: /* SoC */ #define CONFIG_ARM926EJS 1 /* ARM926EJS Core */ #define CONFIG_AT91FAMILY 1 /* it's a member of AT91 */ #define CONFIG_AT91SAM9260 1 /* Atmel AT91SAM9260 based SoC */ #define CONFIG_AT91SAM9XE 1 /* more specific: AT91SAM9XE */
2. both at91 clock.c and timer.c contain the statements: #if !defined(CONFIG_AT91FAMILY) # error You need to define CONFIG_AT91FAMILY in your board config! #endif This will catch all boards that are affected. Since all ARM/AT91 boards are broken right now anyway and need fixing their config file that should work fine.
3. arm/global_data.h has now: ... #endif #ifdef CONFIG_AT91FAMILY /* "static data" needed by at91's clock.c */ unsigned long cpu_clk_rate_hz; unsigned long main_clk_rate_hz; unsigned long mck_rate_hz; unsigned long plla_rate_hz; unsigned long pllb_rate_hz; unsigned long at91_pllb_usb_init; /* "static data" needed by at91's timer.c */ unsigned long timer_rate_hz; unsigned long tbl; unsigned long tbu; unsigned long long timer_reset_value; #endif #if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC) ...
I can now post this as a patch.
Best Regards Reinhard
Dear Reinhard Meyer,
In message 4CA9EB88.5000203@emk-elektronik.de you wrote:
- The board's config file defines "CONFIG_AT91FAMILY" like this:
/* SoC */ #define CONFIG_ARM926EJS 1 /* ARM926EJS Core */ #define CONFIG_AT91FAMILY 1 /* it's a member of AT91 */ #define CONFIG_AT91SAM9260 1 /* Atmel AT91SAM9260 based SoC */ #define CONFIG_AT91SAM9XE 1 /* more specific: AT91SAM9XE */
Out of principle: please omit the "1" in all such cases unless you really want to pass or test for such a numeric value anywhere. As far as I can tell all relevant code uses only "#ifdef" or "defined(...)", so not to confuse people better omit the irrelevant value.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
In message4CA9EB88.5000203@emk-elektronik.de you wrote:
- The board's config file defines "CONFIG_AT91FAMILY" like this:
/* SoC */ #define CONFIG_ARM926EJS 1 /* ARM926EJS Core */ #define CONFIG_AT91FAMILY 1 /* it's a member of AT91 */ #define CONFIG_AT91SAM9260 1 /* Atmel AT91SAM9260 based SoC */ #define CONFIG_AT91SAM9XE 1 /* more specific: AT91SAM9XE */
Out of principle: please omit the "1" in all such cases unless you really want to pass or test for such a numeric value anywhere. As far as I can tell all relevant code uses only "#ifdef" or "defined(...)", so not to confuse people better omit the irrelevant value.
I agree here, and will change that whenever I encounter it in files I touch (most is copied from the atmel EK config file). I also see the tab vs. space issue once its quoted. I think a space after the #define is better.
The general question, however was, if that approach is OK with you.
Best Regards, Reinhard
Dear Reinhard Meyer,
In message 4CAA0BB1.7010608@emk-elektronik.de you wrote:
I agree here, and will change that whenever I encounter it in files I touch (most is copied from the atmel EK config file).
Thanks.
The general question, however was, if that approach is OK with you.
Well, I still prefer to have this added to a single, central location instead to a large number of board config files.
However, if you have to touch all these board config files anyway (and accept the need to re-test on all affected boards) then I don't insist on any specific implementation.
Please don't forget to document the new CONFOG_ variable (in the README).
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
The general question, however was, if that approach is OK with you.
Well, I still prefer to have this added to a single, central location instead to a large number of board config files.
AT91 does not have a really large number of config files.
However, if you have to touch all these board config files anyway (and accept the need to re-test on all affected boards) then I don't insist on any specific implementation.
No, I can't re-test on other AT91 boards. With the relocation patch none of them builds right now, and without the clock/timer cleanup they would not work, even if they would build. Since the respective maintainers are required to fix for relocation they can also add the define required to make the relocated code work...
Currently, a "./MAKEALL at91" is pointless.
Please don't forget to document the new CONFOG_ variable (in the README).
In which README and where in it? So far in no README are such variables listed/explained (of type CONFIG_<arch>,<SoC>,<Family>)
Best regards,
Reinhard
Dear Reinhard Meyer,
In message 4CA590E6.6070701@emk-elektronik.de you wrote:
it seems, that with relocation enabled, some data does not seem to get initialized properly:
I rather suspect you have code running that violates the pre-relocation restrictions (no bss segment available, read-only data segment).
Do I understand right what the required changes are:
- change dram_init
- make sure TEXT_BASE is correct as to where u-boot is loaded
by a preloader and is NOT pointing near top of RAM.
...or identical to the final position in RAM.
- I do not have a board specific .lds - should I now have one?
No need for that.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Dear Reinhard Meyer,
In message 4CA590E6.6070701@emk-elektronik.de you wrote:
it seems, that with relocation enabled, some data does not seem to get initialized properly:
I rather suspect you have code running that violates the pre-relocation restrictions (no bss segment available, read-only data segment).
Definitely, since AT91 ALWAYS uses a preloader and therefore until now u-boot was loaded to its final location with working bss, nobody ever before observed any problems therefore.
There has never been a real need for relocation before.
That means quite some common AT91 code has to be checked and adjusted because this upcoming "enforcement" of relocation.
...or identical to the final position in RAM.
Which never is a good idea.
And Wolfgang, very begin of SDRAM is meant seeing through "AT91 glasses", where SDRAM never holds any vectors. One has to be watchful about the initial stack, which currently is at the begin of SDRAM, too. But that could be put inside the SRAM.
So, for AT91, I think I will put stack into the 4/32k SRAM, U-Boot at the very beginning. Unfortunately that involves modifying the preloader as well.
I suspect Atmel will not modify the source of their evaluation boards, so those will fall out of u-boot soon.
Best Regards, Reinhard
Dear Reinhard Meyer,
In message 4CA5821E.3070108@emk-elektronik.de you wrote:
I think it would be safe that a preloader loads u-boot to the very bottom of SDRAM. If the SDRAM is not at least double the size of u-boot, u-boot needs a diet ;)
This may or may not be safe. Some systems (for example, PowerPC) uses low RAM for exception vectors, and you definitely don;t want to mess with these.
Best regards,
Wolfgang Denk
Dear Heiko Schocher,
In message 4CA57762.3000201@denx.de you wrote:
If u-boot starts in RAM, then it is the task from the preloader where it copies u-boot code, and if there is somewhere in IRAM enough room for it, this would be an option. Otherwise it is a problem if relocation results in overlapping source and destination areas ... but you will fast detect this problem, when you see, that u-boot no longer works ;-)
I think we have to be careful here.
Please keep in mind that the with the new setup the relocation address is not a constant, not even for systems that come with a fixed memroy configuration.
If you enable for example the protected RAM feature, the relocation address will be shifted down by the amount needed for the reserved PRAM area - which is variable, as it can be set through an environment variable.
So assume you have a system with a preloader, and you optimize your configuration to load U-Bot to the "final" position close to the end of the RAM. Not the user defines "setenv pram 128" and reboots. Now U-Boot will try to relocate itself 128 kB down, which pretty reliably causes an overlap.
Eventually we have to detect such situations and relocate twice then?
Best regards,
Wolfgang Denk
Hello Wolfgang,
Wolfgang Denk wrote:
Dear Heiko Schocher,
In message 4CA57762.3000201@denx.de you wrote:
If u-boot starts in RAM, then it is the task from the preloader where it copies u-boot code, and if there is somewhere in IRAM enough room for it, this would be an option. Otherwise it is a problem if relocation results in overlapping source and destination areas ... but you will fast detect this problem, when you see, that u-boot no longer works ;-)
I think we have to be careful here.
Please keep in mind that the with the new setup the relocation address is not a constant, not even for systems that come with a fixed memroy configuration.
If you enable for example the protected RAM feature, the relocation address will be shifted down by the amount needed for the reserved PRAM area - which is variable, as it can be set through an environment variable.
So assume you have a system with a preloader, and you optimize your configuration to load U-Bot to the "final" position close to the end of the RAM. Not the user defines "setenv pram 128" and reboots. Now U-Boot will try to relocate itself 128 kB down, which pretty reliably causes an overlap.
Good point ...
Eventually we have to detect such situations and relocate twice then?
Hmm.. just an idea:
We calculate the relocation address in arch/arm/lib/board.c board_init_f()
So, it should be possible to add a check if
relocation_address + u-boot_len < TEXT_BASE
If this is not the case, we can try to relocate twice ... but this will result in one more copy of u-boot ...
... easier would be, not to use such a setting (relocate address == TEXT_BASE) on a board which uses such features ...
bye, Heiko
participants (12)
-
Albert ARIBAUD -
Graeme Russ -
Heiko Schocher -
J. William Campbell -
Joakim Tjernlund -
Peter Tyser -
Reinhard Meyer -
Rogan Dawes -
Stefan Roese -
Stefano Babic -
Steve Sakoman -
Wolfgang Denk