[U-Boot-Users] Why are some global vars part of the image, and some not?
I have the following code in one of my files:
#ifdef CFG_SPD_BUS_NUM static volatile unsigned int i2c_bus_num = CFG_SPD_BUS_NUM; #else static volatile unsigned int i2c_bus_num = 0; #endif
static volatile struct fsl_i2c *i2c_dev[2] = { (struct fsl_i2c *) (CFG_IMMR + CFG_I2C_OFFSET), #ifdef CFG_I2C2_OFFSET (struct fsl_i2c *) (CFG_IMMR + CFG_I2C2_OFFSET) #endif };
As you can see, I defined two static volatile global vars: i2c_bus_num and i2c_dev.
My U-Boot image is 0x294E0 bytes in size, and it's located at address 0xFE000000. When I run U-Boot, the address of i2c_bus_num is 0xfe02a114, and the address of i2c_dev[] is 0xfe028124.
In other words, i2c_dev[] is part of the actual U-Boot image, but i2c_bus_num, which is defined right next to it, isn't. This means that i2c_dev[] is properly initialized, but i2c_bus_num is set to 0xFFFFFFFF (the value that erased flash has).
How is this possible? How can two adjacent global variables be located is completely different memory segments?
Timur Tabi wrote:
#ifdef CFG_SPD_BUS_NUM static volatile unsigned int i2c_bus_num = CFG_SPD_BUS_NUM; #else static volatile unsigned int i2c_bus_num = 0; #endif
Ok, I fixed my problem by changing the above line to:
static volatile unsigned int i2c_bus_num __attribute__ ((section ("data"))) = 0;
Is "data" the right section to use? Here's my lds file:
OUTPUT_ARCH(powerpc) SECTIONS { /* Read-only sections, merged into text segment: */ . = + SIZEOF_HEADERS; .interp : { *(.interp) } .hash : { *(.hash) } .dynsym : { *(.dynsym) } .dynstr : { *(.dynstr) } .rel.text : { *(.rel.text) } .rela.text : { *(.rela.text) } .rel.data : { *(.rel.data) } .rela.data : { *(.rela.data) } .rel.rodata : { *(.rel.rodata) } .rela.rodata : { *(.rela.rodata) } .rel.got : { *(.rel.got) } .rela.got : { *(.rela.got) } .rel.ctors : { *(.rel.ctors) } .rela.ctors : { *(.rela.ctors) } .rel.dtors : { *(.rel.dtors) } .rela.dtors : { *(.rela.dtors) } .rel.bss : { *(.rel.bss) } .rela.bss : { *(.rela.bss) } .rel.plt : { *(.rel.plt) } .rela.plt : { *(.rela.plt) } .init : { *(.init) } .plt : { *(.plt) } .text : { cpu/mpc83xx/start.o (.text) *(.text) *(.fixup) *(.got1) . = ALIGN(16); *(.rodata) *(.rodata1) *(.rodata.str1.4) *(.eh_frame) } .fini : { *(.fini) } =0 .ctors : { *(.ctors) } .dtors : { *(.dtors) }
/* Read-write section, merged into data segment: */ . = (. + 0x0FFF) & 0xFFFFF000; _erotext = .; PROVIDE (erotext = .); .reloc : { *(.got) _GOT2_TABLE_ = .; *(.got2) _FIXUP_TABLE_ = .; *(.fixup) } __got2_entries = (_FIXUP_TABLE_ - _GOT2_TABLE_) >> 2; __fixup_entries = (. - _FIXUP_TABLE_) >> 2;
.data : { *(.data) *(.data1) *(.sdata) *(.sdata2) *(.dynamic) CONSTRUCTORS } _edata = .; PROVIDE (edata = .);
. = .; __u_boot_cmd_start = .; .u_boot_cmd : { *(.u_boot_cmd) } __u_boot_cmd_end = .;
. = .; __start___ex_table = .; __ex_table : { *(__ex_table) } __stop___ex_table = .;
. = ALIGN(4096); __init_begin = .; .text.init : { *(.text.init) } .data.init : { *(.data.init) } . = ALIGN(4096); __init_end = .;
__bss_start = .; .bss : { *(.sbss) *(.scommon) *(.dynbss) *(.bss) *(COMMON) } _end = . ; PROVIDE (end = .); } ENTRY(_start)
In message 454BC07C.4090704@freescale.com you wrote:
#ifdef CFG_SPD_BUS_NUM static volatile unsigned int i2c_bus_num = CFG_SPD_BUS_NUM; #else static volatile unsigned int i2c_bus_num = 0; #endif
Ok, I fixed my problem by changing the above line to:
I don't see any problem that needed fixing.
static volatile unsigned int i2c_bus_num __attribute__ ((section ("data"))) = 0;
Is "data" the right section to use? Here's my lds file:
If you have to ask this question, then the answer is no, and better don't mess with things you don't understand.
What exactly is the "problem" you are experiencing? Did you read the documentation, especially section "Initial Stack, Global Data" in the README? Please let me know if there is anything in this text which is not clear enough.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 454BC07C.4090704@freescale.com you wrote:
#ifdef CFG_SPD_BUS_NUM static volatile unsigned int i2c_bus_num = CFG_SPD_BUS_NUM; #else static volatile unsigned int i2c_bus_num = 0; #endif
Ok, I fixed my problem by changing the above line to:
I don't see any problem that needed fixing.
The problem is that without specifying the section attribute, when I ran my code, i2c_bus_num was equal to 0xFFFFFFFF.
static volatile unsigned int i2c_bus_num __attribute__ ((section ("data"))) = 0;
Is "data" the right section to use? Here's my lds file:
If you have to ask this question, then the answer is no, and better don't mess with things you don't understand.
What exactly is the "problem" you are experiencing? Did you read the documentation, especially section "Initial Stack, Global Data" in the README? Please let me know if there is anything in this text which is not clear enough.
That section doesn't address issues with global (static or otherwise) variables that need to be read prior to relocation.
In message 454BDA40.5090806@freescale.com you wrote:
I don't see any problem that needed fixing.
The problem is that without specifying the section attribute, when I ran my code, i2c_bus_num was equal to 0xFFFFFFFF.
Yes, this to be expected. And explained in the README:
... and BSS is not initialized as zero.
What exactly is the "problem" you are experiencing? Did you read the documentation, especially section "Initial Stack, Global Data" in the README? Please let me know if there is anything in this text which is not clear enough.
That section doesn't address issues with global (static or otherwise) variables that need to be read prior to relocation.
Yes, it does:
This means that we don't have writable Data or BSS segments, and BSS is not initialized as zero.
Best regards,
Wolfgang Denk
Dear Timur,
in message 454BBB85.5040507@freescale.com you wrote:
I have the following code in one of my files:
#ifdef CFG_SPD_BUS_NUM static volatile unsigned int i2c_bus_num = CFG_SPD_BUS_NUM; #else static volatile unsigned int i2c_bus_num = 0; #endif
static volatile struct fsl_i2c *i2c_dev[2] = { (struct fsl_i2c *) (CFG_IMMR + CFG_I2C_OFFSET), #ifdef CFG_I2C2_OFFSET (struct fsl_i2c *) (CFG_IMMR + CFG_I2C2_OFFSET) #endif };
I'm not exactly sure what you mean by the "global vars" in the Subject line. The declaration you show here says "static". This may or may not be what you mean.
As you can see, I defined two static volatile global vars: i2c_bus_num and i2c_dev.
Yes, and one of these is initialized with non-zero values (so it will go to the data segment), while the other is not (so it will go to bss).
In other words, i2c_dev[] is part of the actual U-Boot image, but i2c_bus_num, which is defined right next to it, isn't. This means that i2c_dev[] is properly initialized, but i2c_bus_num is set to 0xFFFFFFFF (the value that erased flash has).
THis is perfectly OK. Just check the variables after U-Boot has been relocated to RAM and BSS has been initialized.
How is this possible? How can two adjacent global variables be located is completely different memory segments?
You need to understand the difference between data and bss segments, or, between initialized and uninitialized (resp. initialized as zero) data.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
I'm not exactly sure what you mean by the "global vars" in the Subject line. The declaration you show here says "static". This may or may not be what you mean.
To me, a variable defined outside of a function is a global variable, regardless as to whether it's "static" or not. Technically, these variables are static globals, but I don't think removing the "static" would change what I'm seeing. Would it?
As you can see, I defined two static volatile global vars: i2c_bus_num and i2c_dev.
Yes, and one of these is initialized with non-zero values (so it will go to the data segment), while the other is not (so it will go to bss).
I realized that about two minutes after I posted my email, which is why I posted a follow-up.
THis is perfectly OK. Just check the variables after U-Boot has been relocated to RAM and BSS has been initialized.
I can't. This code is used to initialize the I2C bus, which is used to initialize SPD on DDR, which is obviously done before U-Boot is relocated.
These variables contain the I2C bus number. On some boards, SPD is on bus 0, on some they're on bus 1. So the compile-time initializer needs to be the right bus.
(An alternative would be to change i2c_read() and i2c_write() to take a bus number as a parameter, but that would change hundreds of files).
In message 454BD9AB.1000303@freescale.com you wrote:
To me, a variable defined outside of a function is a global variable,
May I suggest we stick with ANSI C standard terminology instead of private interpretations?
Since you complained about a problem where I could not see any (at least none that was not clearly documented) I was not sure if your expectation of "global" was correct, i. e. if you were aware of the differences between "global" on file level versus "global" in the meaning of "external" to the linker.
THis is perfectly OK. Just check the variables after U-Boot has been relocated to RAM and BSS has been initialized.
I can't. This code is used to initialize the I2C bus, which is used to initialize SPD on DDR, which is obviously done before U-Boot is relocated.
These variables contain the I2C bus number. On some boards, SPD is on bus 0, on some they're on bus 1. So the compile-time initializer needs to be the right bus.
Then you must make sure that even zero-initialized data don't get placed in bss, and your approach was correct.
[It would have been good if you had mentioned this in the opriginal posting.]
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Since you complained about a problem where I could not see any (at least none that was not clearly documented) I was not sure if your expectation of "global" was correct, i. e. if you were aware of the differences between "global" on file level versus "global" in the meaning of "external" to the linker.
I was aware, I just didn't think it mattered for the issue that I was seeing.
Then you must make sure that even zero-initialized data don't get placed in bss, and your approach was correct.
What about modifying the linker script so that all of bss is merged into the image, i.e. treat it like 'data'?
In message 454BEF23.1020909@freescale.com you wrote:
What about modifying the linker script so that all of bss is merged into the image, i.e. treat it like 'data'?
That would mean that you have to allocate flash memory for all this stuff, which is typically in the 100...150 kB range, i. e. you would double the memory footprint of U-Boot in flash memory.,
No, this is NOT acceptable. Don't even think of it.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 454BEF23.1020909@freescale.com you wrote:
What about modifying the linker script so that all of bss is merged into the image, i.e. treat it like 'data'?
That would mean that you have to allocate flash memory for all this stuff, which is typically in the 100...150 kB range, i. e. you would double the memory footprint of U-Boot in flash memory.,
That seems rather ridiculously large... perhaps some of the larger chunks of that could be dynamically allocated, rather than leaving a giant welcome mat for bugs by executing C code before the BSS is cleared (especially if, as it appears to be in this case, the compiler and/or linker is putting things in the BSS even when explicitly initialized to zero, rather than just left uninitialized)?
BTW, I'm only seeing about 19kB of BSS on an 8349EMDS build from the current git head.
No, this is NOT acceptable. Don't even think of it.
I don't think refusing to think about alternatives is conducive to good software engineering.
-Scott
Scott Wood wrote:
Wolfgang Denk wrote:
In message 454BEF23.1020909@freescale.com you wrote:
What about modifying the linker script so that all of bss is merged into the image, i.e. treat it like 'data'?
That would mean that you have to allocate flash memory for all this stuff, which is typically in the 100...150 kB range, i. e. you would double the memory footprint of U-Boot in flash memory.,
That seems rather ridiculously large... perhaps some of the larger chunks of that could be dynamically allocated, rather than leaving a giant welcome mat for bugs by executing C code before the BSS is cleared (especially if, as it appears to be in this case, the compiler and/or linker is putting things in the BSS even when explicitly initialized to zero, rather than just left uninitialized)?
BTW, I'm only seeing about 19kB of BSS on an 8349EMDS build from the current git head.
No, this is NOT acceptable. Don't even think of it.
I don't think refusing to think about alternatives is conducive to good software engineering.
-Scott
You don't understand, u-boot _is_ what zeros bss so, until u-boot is running enough to zero bss, bss will be random garbage. The stuff you are doing is very early initialization - using I2C to read SPD, which is before you even _have_ a normal bss because you don't even have your SDRAM active at this point.
Welcome to the Wacky World of Embedded where you must do everything yourself, and the world is populated with very sharp objects waiting to nail you if you do anything wrong.
gvb
Jerry Van Baren wrote:
You don't understand, u-boot _is_ what zeros bss so, until u-boot is running enough to zero bss, bss will be random garbage.
If BSS were merged into the DATA segment, then the compiler would be the one that zeros BSS. Basically, BSS would go away, and all global variables (static or otherwise) would be placed into DATA. They would be initialized to zeros by the compiler, and when the image is burned into flash, that image would have zeros in those memory locations.
This would also allow us to delete the BSS-initialization code in U-Boot.
I'm not advocating that we should implement this idea. I just wanted to clarify things.
Timur Tabi wrote:
Jerry Van Baren wrote:
You don't understand, u-boot _is_ what zeros bss so, until u-boot is running enough to zero bss, bss will be random garbage.
If BSS were merged into the DATA segment, then the compiler would be the one that zeros BSS. Basically, BSS would go away, and all global variables (static or otherwise) would be placed into DATA. They would be initialized to zeros by the compiler, and when the image is burned into flash, that image would have zeros in those memory locations.
This would also allow us to delete the BSS-initialization code in U-Boot.
I'm not advocating that we should implement this idea. I just wanted to clarify things.
In theory, yes. In practice, I have my doubts that it is workable, let alone a better way. Writing "then the compiler would be the one that zeros BSS" is easy, actually making it work may be tricky. IMHO, there are more profitable windmills to tilt. :-) http://en.wikipedia.org/wiki/Don_Quixote
gvb
Jerry Van Baren wrote:
In theory, yes. In practice, I have my doubts that it is workable, let alone a better way. Writing "then the compiler would be the one that zeros BSS" is easy, actually making it work may be tricky.
It's a pretty simple linker script change, actually. Just move all of the lines within .bss {} into .data {}. Where do you see the difficulty coming from?
-Scott
Scott Wood wrote:
Jerry Van Baren wrote:
In theory, yes. In practice, I have my doubts that it is workable, let alone a better way. Writing "then the compiler would be the one that zeros BSS" is easy, actually making it work may be tricky.
It's a pretty simple linker script change, actually. Just move all of the lines within .bss {} into .data {}. Where do you see the difficulty coming from?
-Scott
Does the linker create proper zero initialization? If you initialize a variable to zero, the compiler puts it in bss and does not initialize it, secure in the knowledge that the runtime zeroes bss. If you subsequently put it in the data section via the linker script, does it get zeroed?
Does the bss/data end up in the right place? On start up your stack is in cache or dual-port RAM until SDRAM is initialized. When does the initialized portion of the data section get initialized and where? Before it is in SDRAM or after SDRAM is initialized?
Simple enough things to experiment with, but it all takes time and the benefit is questionable. Feel free to prove us curmudgeons wrong. ;-)
gvb
Jerry Van Baren wrote:
Does the linker create proper zero initialization?
Yes.
Does the bss/data end up in the right place?
It ends up where nonzero-initialized data currently is (i.e. in flash before relocation, in RAM after).
On start up your stack is in cache or dual-port RAM until SDRAM is initialized. When does the initialized portion of the data section get initialized and where? Before it is in SDRAM or after SDRAM is initialized?
I don't follow what, specifically, you mean by "initialized portion". The data section contains data which is statically initiallized at compile/link time; the only difference is that it would now include data whose initial value is zero. Some pieces of data will be further "initialized" at runtime by U-Boot code; this, of course, must happen after SDRAM is initialized (and U-Boot has relocated to it), but that has nothing to do with moving the BSS into the data segment.
The BSS is purely an optimization; besides increasing flash footprint somewhat, removing it should not change anything except for code that was already broken.
Simple enough things to experiment with, but it all takes time and the benefit is questionable. Feel free to prove us curmudgeons wrong. ;-)
The main things that would take effort are changing all the board linker scripts, and finding large BSS allocations and changing them to be dynamically allocated so they don't take up space in flash.
The benefit is that bss-related bugs in the pre-RAM code go away and stay away, and that ugliness such as having to specify __attribute__ ((section ("data"))) whenever the value is zero also goes away.
-Scott
Scott Wood wrote:
The benefit is that bss-related bugs in the pre-RAM code go away and stay away, and that ugliness such as having to specify __attribute__ ((section ("data"))) whenever the value is zero also goes away.
For the record, the only use of "section (data)" is in the I2C code I just wrote. And this code can be eliminated if we change i2c_read() and i2c_write() to take the bus number as a parameter. Although I should we should implement that change, doing so would require changing 91 files.
Scott Wood wrote:
Jerry Van Baren wrote:
Does the linker create proper zero initialization?
Yes.
[snipped the rest of the "yes" answers - good to know this, thank you]
The main things that would take effort are changing all the board linker scripts, and finding large BSS allocations and changing them to be dynamically allocated so they don't take up space in flash.
The benefit is that bss-related bugs in the pre-RAM code go away and stay away, and that ugliness such as having to specify __attribute__ ((section ("data"))) whenever the value is zero also goes away.
-Scott
OK, but you are trading one fixed bss-related bug in the pre-RAM code for a larger u-boot image and more complexity.
Pre-RAM code is short and sweet and thus has a pretty limited opportunity for bugs. On the other hand, changing all the board linker scripts takes effort and regression testing by all the board maintainers. Finding large bss allocations and changing them to be dynamically allocated sounds like a job that is initially large and ultimately never ending. http://en.wikipedia.org/wiki/Sisyphus I'm afraid the audience here isn't very sympathetic.
Best regards, gvb
Jerry Van Baren wrote:
Scott Wood wrote:
Jerry Van Baren wrote:
Does the linker create proper zero initialization?
Yes.
[snipped the rest of the "yes" answers - good to know this, thank you]
The main things that would take effort are changing all the board linker scripts, and finding large BSS allocations and changing them to be dynamically allocated so they don't take up space in flash.
The benefit is that bss-related bugs in the pre-RAM code go away and stay away, and that ugliness such as having to specify __attribute__ ((section ("data"))) whenever the value is zero also goes away.
-Scott
OK, but you are trading one fixed bss-related bug in the pre-RAM code for a larger u-boot image and more complexity.
No, I don't think that's accurate.
Advantages:
1) Eliminates the one (there may be more in the future) BSS-related bug 2) Eliminates the BSS initialization code 3) It's LESS complex. I don't know why you think it's more complex. 4) Eliminates the obscure bug that could occur if you expect "int X = 0" to actually be 0 instead of 0xFFFFFFFF.
Disadvantes:
1) Larger U-Boot image file. 2) Larger flash storage requirements.
Pre-RAM code is short and sweet and thus has a pretty limited opportunity for bugs. On the other hand, changing all the board linker scripts takes effort and regression testing by all the board maintainers.
I agree that making the change globally would require regression testing. For instance, there could be buggy code out there that depends on the BSS section being initialized to FF before the relocation.
Finding large bss allocations and changing them to be dynamically allocated sounds like a job that is initially large and ultimately never ending.
It could be implemented as a compile-time option that would be disabled by default.
In message 454F992D.7010402@freescale.com you wrote:
Advantages:
- Eliminates the one (there may be more in the future) BSS-related bug
Let's keep this straight. The bug is not in U-Boot, but in newly introduced code, which ignores the documented way to write code that is running in the restrictred environment before relocation.
- Eliminates the BSS initialization code
Yes, what a win. We save 50 bytes of code and add - wait a second:
-> size u-boot text data bss dec hex filename 328632 30584 314076 673292 a460c u-boot
...and add 307 KiB of data. You lose by one to several thousands.
- It's LESS complex. I don't know why you think it's more complex.
- Eliminates the obscure bug that could occur if you expect "int X = 0" to
actually be 0 instead of 0xFFFFFFFF.
You don't understand. No matter what your're trying to do, pre- relocation code runs in a severly restricted environment. All your attapts to "fix" this are void, especially since the whole data segment remains read-only. Solving the "int X=0;" case does not solve the "X=1;" five lines later.
You have to be aware of this situation, and trying to hide it by providong a "working solution" for a single special case is making things more obscure.
I recommend just to keep in mind *all* the restrictions when writing pre-reloc code, and to minimize the amount of such code in your systems.
Anything else is probably worse than what we have now.
I agree that making the change globally would require regression testing. For instance, there could be buggy code out there that depends on the BSS section being initialized to FF before the relocation.
You speculations are ... ummm... interesting. You find funny ways to defend a situatioin where the problem is all your's only.
It could be implemented as a compile-time option that would be disabled by default.
No. I will not accept any of such code. Period.
Best regards,
Wolfgang Denk
Scott Wood wrote:
The BSS is purely an optimization; besides increasing flash footprint somewhat, removing it should not change anything except for code that was already broken.
But that increase in size can be important. Besides after relocation from ROM to RAM the area will not be used. I personally would rather not increase the size of image to handle a limited number of special cases where this initialization is needed prior to relocation to RAM. You can handle such special cases by instructing the variable to be placed on "data" section (like Timur did). Perhaps a macro could be defined to make it more readable and documentation/faq would take care of education.
Tolunay
In message 454F7A30.9030100@freescale.com you wrote:
If BSS were merged into the DATA segment, then the compiler would be the one that zeros BSS. Basically, BSS would go away, and all global variables (static or otherwise) would be placed into DATA. They would be initialized to
... and U-Boot would stop being usable on a significan percentage of boards simply because it does not fit any more in the available flash memory.
There are many other ways to solve your problem with much less impact on other boards.
I reject your suggestion. And this is a final statement.
Best regards,
Wolfgang Denk
In message 454F7440.5030709@freescale.com you wrote:
No, this is NOT acceptable. Don't even think of it.
I don't think refusing to think about alternatives is conducive to good software engineering.
The concept of BSS for using uninitialized data (meaning: implicitely initialized to zero) is as old as C and Unix, actualy even older than that (from IBM 7090 times, IIRC). It is simple, efficient, and powerful.
I see zero reason to drop it.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
The concept of BSS for using uninitialized data (meaning: implicitely initialized to zero)
However, with the current u-boot linker scripts (some of them, at least; I'm not going to make a sweeping statement about all 270), data *explicitly* initialized to zero is also going in the bss. Combined with the fact that some C code is executed before the bss is cleared, that is simply evil.
is as old as C and Unix, actualy even older than that (from IBM 7090 times, IIRC). It is simple, efficient, and powerful.
Your point being? I'm not attacking the concept of a bss, just the use of it in a codebase where C code is executed prior to being able to clear the bss.
I see zero reason to drop it.
I have stated the reason. If you prefer an environment that invites bugs and ugly workarounds thereto, then suit yourself.
-Scott
In message 454F9FB2.8090009@freescale.com you wrote:
However, with the current u-boot linker scripts (some of them, at least; I'm not going to make a sweeping statement about all 270), data *explicitly* initialized to zero is also going in the bss. Combined
Yes, and this is perfectly fine from the ANSI C standard point of view. The situation is that we don;t have a standard C environment yet, so you must not insist on being able to use standard C.
with the fact that some C code is executed before the bss is cleared, that is simply evil.
It is one of several restrictions. There are many more. For example, you cannot write any data in the data segment either. And you cannot put big data structures on the stack. And you have to keep in mind that you must not nest function calls too deeply because thestack size is very limited. And... and...
It's IMHO nensense to figt this situation, you just make it worse. Insted, please focus on moving all complex code *after* relocation, where all these problems go away automagically.
I have stated the reason. If you prefer an environment that invites bugs and ugly workarounds thereto, then suit yourself.
You don't fix anything but one special case (the initial assignment of a zero). Any later assignments to the same variable will still fail. If this deems OK to you, then please just use "#define foo 0" instead of "int foo=0;"
Best regards,
Wolfgang Denk
participants (5)
-
Jerry Van Baren -
Scott Wood -
Timur Tabi -
Tolunay Orkun -
Wolfgang Denk