Re: [coreboot] r1026 - in coreboot-v3: mainboard/kontron/986lcd-m southbridge/intel/i82801gx
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
and /home/rminnich/coreboot-v3/southbridge/intel/i82801gx/smbus.c:34: error: conflicting types for ?\226?\128?\152smbus_read_byte?\226?\128?\153 include/device/smbus.h:56: error: previous declaration of ?\226?\128?\152smbus_read_byte?\226?\128?\153 was here
we are working these. The second is much harder than it seems. It concerns whether we put i2c devices (i.e. DRAM spd SEEPROMS) in the dts.
I have a 8 month old patch to fix this, but the patch was vetoed back then. I could repost.
Regards, Carl-Daniel
On Fri, Nov 14, 2008 at 9:28 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
I have a 8 month old patch to fix this, but the patch was vetoed back then. I could repost.
sure, let's see it again
ron
On 14.11.2008 18:28, Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
IFF (if and only if) these pointers point to places in the boot block or other non relocatable locations, you might be able to tell gcc that you want the pointer array to end up in section .rodata and the section checker won't complain anymore.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
ron
On Fri, Nov 14, 2008 at 9:56 AM, ron minnich rminnich@gmail.com wrote:
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
for now I'm fixing it with a section attribute.
ron
On 14.11.2008 19:04, ron minnich wrote:
On Fri, Nov 14, 2008 at 9:56 AM, ron minnich rminnich@gmail.com wrote:
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
for now I'm fixing it with a section attribute.
Compilation may work, but RAMinit will fail during runtime if you do that.
Regards, Carl-Daniel
On Fri, Nov 14, 2008 at 10:37 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
for now I'm fixing it with a section attribute.
Compilation may work, but RAMinit will fail during runtime if you do that.
ok, why would that be? It's being placed into that section ... what will fail?
second question, this is failling: STAGE2_CHIPSET_SRC += \ $(src)/southbridge/intel/i82801gx/smmrelocate.S \
note the .S: I get this error:
LD build/coreboot.stage2 /home/rminnich/coreboot-v3/southbridge/intel/i82801gx/smmrelocate.S: file not recognized: File format not recognized
how do we do assembly?
ron
On 14.11.2008 19:40, ron minnich wrote:
On Fri, Nov 14, 2008 at 10:37 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
for now I'm fixing it with a section attribute.
Compilation may work, but RAMinit will fail during runtime if you do that.
ok, why would that be? It's being placed into that section ... what will fail?
The section is not the problem. Let's look at the code to make this a bit clearer (edited for clarity):
static const u32 dq2030[] = { 0x08070706, 0x0a090908, 0x0d0c0b0a, 0x12100f0e }; static const u32 nc[] = { 0x00000000, 0x00000000, 0x00000000, 0x00000000 }; static const u32 const * const dual_channel_slew_group_lookup[] = { dq2030, nc };
dq2030 is const and will be placed in section ".rodata". Same for nc. The addresses of dq2030 and nc need to be calculated during runtime because their location is not known during early link time (this is PIC, remember?). That works fine if dq2030 and nc are accessed from code because code can calculate the addresses. dual_channel_slew_group_lookup is a completely different beast. You claim the pointers are const, but they are unknown until we know the location of initram inside the ROM and that's during runtime. So we need to modify the pointers stored in dual_channel_slew_group_lookup during runtime. That's _not_ const by definition.
I think I can propose a fixup, though.
second question, this is failling: STAGE2_CHIPSET_SRC += \ $(src)/southbridge/intel/i82801gx/smmrelocate.S \
note the .S: I get this error:
LD build/coreboot.stage2 /home/rminnich/coreboot-v3/southbridge/intel/i82801gx/smmrelocate.S: file not recognized: File format not recognized
how do we do assembly?
You need to run "as" on ssmrelocate.S to get an object file. The linker won't handle assembly directly.
Regards, Carl-Daniel
On Fri, Nov 14, 2008 at 10:58 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
You need to run "as" on ssmrelocate.S to get an object file. The linker won't handle assembly directly.
let me say it differently: there appear to be no rules for convering the .S to .o. So I give a name, e.g., smi.S, and make passes it to the linker instead of assembling it.
I think we need a .S.o rule somewhere.
ron
ron minnich wrote:
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
We could try and use indices instead of pointers. That's the easiest rewrite i can think of to get the problem done without wasting the code.
This is clearly a gcc weakness. This stuff should not happen when compiling PIC. That's what PIC is for.
On 14.11.2008 19:06, Stefan Reinauer wrote:
ron minnich wrote:
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
We could try and use indices instead of pointers. That's the easiest rewrite i can think of to get the problem done without wasting the code.
This is clearly a gcc weakness. This stuff should not happen when compiling PIC. That's what PIC is for.
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known. That means a linker would have to pass over the final LAR archive. Right now, we circumvent that requirement by using early linker tricks and by prohibiting the use of arrays of pointers.
If we insist on using arrays of pointers, we must either run a linker over the final LAR archive or abandon LAR completely and go for v2 linker magic.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known.
That sounds absurd as it would totally defeat the purpose of PIC.
It's position independent code exactly _because_ you can't know the position at link time.
Which is why the compiler would always block a register for house keeping, when compiling -fPIC (at least it used to be that way). This hurts more on x86 than on other platforms because x86 is basically a stack machine with an unclean design (pun intended), but we wouldn't care.
Stefan
On Fri, Nov 14, 2008 at 10:52 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known.
That sounds absurd as it would totally defeat the purpose of PIC.
It's position independent code exactly _because_ you can't know the position at link time.
actually, the GCC definiton of PIC is odd to say the least, as compared to what I used to call PIC.
But I am afraid carl-daniel is right. PIC in the gcc sense really means "shared libraries" from what I can see, and does require a linker post-pass.
Possibly on machines such as core 2, we should copy the (tiny) initram to CARBASE and run it there, and link it for CARBASE.
ron
On 14.11.2008 19:54, ron minnich wrote:
On Fri, Nov 14, 2008 at 10:52 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known.
That sounds absurd as it would totally defeat the purpose of PIC.
It's position independent code exactly _because_ you can't know the position at link time.
actually, the GCC definiton of PIC is odd to say the least, as compared to what I used to call PIC.
Remember what Segher said: We're (ab)using gcc on x86 in a way that was never envisioned.
But I am afraid carl-daniel is right. PIC in the gcc sense really means "shared libraries" from what I can see, and does require a linker post-pass.
Possibly on machines such as core 2, we should copy the (tiny) initram to CARBASE and run it there, and link it for CARBASE.
At least on AMD Fam10h processors, this is documented to fail. I believe we can trick the processor into doing that anyway, but that would be so highly complicated and processor dependent that the author of that code would become a critical resource ;-)
I'd like to have the initram in CAR option available for emergency situations, though.
Regards, Carl-Daniel
OK, another idea then. rework ROM layout.
top 32k: stage 1. top-1 32k: fallback initram top-2 32k: normal initram The rest: the rest of lar.
waste memory? yes. But this would let us dump PIC.
ron
On 15.11.2008 04:32, ron minnich wrote:
OK, another idea then. rework ROM layout.
top 32k: stage 1. top-1 32k: fallback initram top-2 32k: normal initram The rest: the rest of lar.
waste memory? yes. But this would let us dump PIC.
I'd like to postpone the discussion about dropping PIC until it becomes really necessary. Why? Because there are lots of benefits in freely movable initram files inside LAR. For one, the ability to unpack fallback/initram and repackage it as normal/initram (or vice versa) would disappear. I couldn't justify that as a deliberate design decision in any discussion. Especially the fact that there is no way to recognize from a bare initram file whether it is fallback or normal is a killer for the desire to be able to unpack and repack a LAR.
Regards, Carl-Daniel
On Fri, Nov 14, 2008 at 11:00 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
On 15.11.2008 04:32, ron minnich wrote:
OK, another idea then. rework ROM layout.
top 32k: stage 1. top-1 32k: fallback initram top-2 32k: normal initram The rest: the rest of lar.
waste memory? yes. But this would let us dump PIC.
I'd like to postpone the discussion about dropping PIC until it becomes really necessary. Why? Because there are lots of benefits in freely movable initram files inside LAR. For one, the ability to unpack fallback/initram and repackage it as normal/initram (or vice versa) would disappear. I couldn't justify that as a deliberate design decision in any discussion. Especially the fact that there is no way to recognize from a bare initram file whether it is fallback or normal is a killer for the desire to be able to unpack and repack a LAR.
Should we just succumb to the way a factory BIOS does it? Initialize a minimal amount of ram with the default chipset timings in initram, then do a full blown ram init once we've got some real memory to work off of? Or would that not solve the problem?
-Corey
On Fri, Nov 14, 2008 at 8:40 PM, Corey Osgood corey.osgood@gmail.com wrote:
Should we just succumb to the way a factory BIOS does it?
Initialize a minimal amount of ram with the default chipset timings in initram, then do a full blown ram init once we've got some real memory to work off of? Or would that not solve the problem?
That has not been a workable dram path for some time. For ddr and later, there are no safe default timings, or so I am told.
ron
On Sat, Nov 15, 2008 at 12:20 AM, ron minnich rminnich@gmail.com wrote:
On Fri, Nov 14, 2008 at 8:40 PM, Corey Osgood corey.osgood@gmail.com wrote:
Should we just succumb to the way a factory BIOS does it?
Initialize a minimal amount of ram with the default chipset timings in initram, then
do a
full blown ram init once we've got some real memory to work off of? Or
would
that not solve the problem?
That has not been a workable dram path for some time. For ddr and later, there are no safe default timings, or so I am told.
ron
Sure there are, CN700 in v2 uses a safe (read: slowest possible) set of timings that seems to work on all the memory I've thrown at it. AFAIK DDR2 doesn't like running too much slower then spec, but it can handle quite a bit.
-Corey
On Fri, Nov 14, 2008 at 10:02 PM, Corey Osgood corey.osgood@gmail.com wrote:
Sure there are, CN700 in v2 uses a safe (read: slowest possible) set of timings that seems to work on all the memory I've thrown at it. AFAIK DDR2 doesn't like running too much slower then spec, but it can handle quite a bit.
I used to try to use this approach and had only mixed luck with it. I am glad it works on the VIA but I don't think we can count it working in the general case.
Also, I learned the hard way that some DRAM controllers reset completely when you change ANY timing registers and the result is you lose all contents of DRAM.
It's very hard to get a general solution to DRAM that will work on all chipsets.But picking safe initial timing is without a doubt the one of the hardest problems.
thanks
ron
Corey Osgood wrote:
Should we just succumb to the way a factory BIOS does it? Initialize a minimal amount of ram with the default chipset timings in initram, then do a full blown ram init once we've got some real memory to work off of? Or would that not solve the problem?
I have not encountered that behavior in any factory BIOS ... Which platforms do that?
Carl-Daniel Hailfinger wrote:
On 14.11.2008 19:54, ron minnich wrote:
On Fri, Nov 14, 2008 at 10:52 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known.
That sounds absurd as it would totally defeat the purpose of PIC.
It's position independent code exactly _because_ you can't know the position at link time.
actually, the GCC definiton of PIC is odd to say the least, as compared to what I used to call PIC.
Remember what Segher said: We're (ab)using gcc on x86 in a way that was never envisioned.
... when we were jumping to fixed non-pic compiled code outside of our pic scope.
I appreciate your scaremongering, but we should try not to confuse apples with pears when comparing apples and oranges.
Stefan
Carl-Daniel Hailfinger wrote:
On 14.11.2008 19:06, Stefan Reinauer wrote:
ron minnich wrote:
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
Author: rminnich New Revision: 1026
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 single_channel_slew_group_lookup.3243
Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
We could try and use indices instead of pointers. That's the easiest rewrite i can think of to get the problem done without wasting the code.
This is clearly a gcc weakness. This stuff should not happen when compiling PIC. That's what PIC is for.
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known. That means a linker would have to pass over the final LAR archive. Right now, we circumvent that requirement by using early linker tricks and by prohibiting the use of arrays of pointers.
If we insist on using arrays of pointers, we must either run a linker over the final LAR archive or abandon LAR completely and go for v2 linker magic.
Fully agreed. If we run a linker over the final LAR archive, then we will need a way to run the linker every single time the LAR has changed (including, mind you, on target, if we are going to go for the "replace stages on the fly" thing). Does not sound appetizing at all. I also don't like abandoning LAR - it is useful for payloads and such. But I don't think thats what you meant - I think you meant to say that we should "abandon the stages concept". And you know what - with all due respect to Ron, I tend to be in favor of that option. Stages continue to be rather painful. Just one guy's opinion.
Jordan
On Fri, Nov 14, 2008 at 10:54 AM, Jordan Crouse jordan@cosmicpenguin.net wrote:
And you know what - with all due respect to Ron, I tend to be in favor of that option. Stages continue to be rather painful. Just one guy's opinion.
actually, stages just define what we do in v2 now. We have the ROM part, the CAR part, and the RAM part. These are pretty much stage0/1, initram, and stage2. We gave them names in hamburg because the v2 names (ram_rom for example) are confusing.
What would you do differently? How would you restructure it?
ron
On 14.11.2008, at 19:57, "ron minnich" rminnich@gmail.com wrote:
On Fri, Nov 14, 2008 at 10:54 AM, Jordan Crouse jordan@cosmicpenguin.net wrote:
And you know what - with all due respect to Ron, I tend to be in favor of that option. Stages continue to be rather painful. Just one guy's opinion.
actually, stages just define what we do in v2 now. We have the ROM part, the CAR part, and the RAM part.
With the big difference that we always know where those stages lived in rom at link time in v2.
If we go back to that, and we want fallback, we will likely be wasting many Kb of rom space again as we usually do in v2
These are pretty much stage0/1, initram, and stage2. We gave them names in hamburg because the v2 names (ram_rom for example) are confusing.
What would you do differently? How would you restructure it?
ron
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
On 14.11.2008 19:54, Jordan Crouse wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 19:06, Stefan Reinauer wrote:
ron minnich wrote:
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 18:14, svn@coreboot.org wrote:
> Author: rminnich > New Revision: 1026 > > /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: > section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 > single_channel_slew_group_lookup.3243 > Basic rule: If you want to have arrays of pointers in initram, you lose. Pointers are not relocatable by definition. const is not going to help you there.
Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
We could try and use indices instead of pointers. That's the easiest rewrite i can think of to get the problem done without wasting the code.
This is clearly a gcc weakness. This stuff should not happen when compiling PIC. That's what PIC is for.
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known. That means a linker would have to pass over the final LAR archive. Right now, we circumvent that requirement by using early linker tricks and by prohibiting the use of arrays of pointers.
If we insist on using arrays of pointers, we must either run a linker over the final LAR archive or abandon LAR completely and go for v2 linker magic.
Fully agreed. If we run a linker over the final LAR archive, then we will need a way to run the linker every single time the LAR has changed (including, mind you, on target, if we are going to go for the "replace stages on the fly" thing). Does not sound appetizing at all. I also don't like abandoning LAR - it is useful for payloads and such.
Yes, I'd like to keep LAR and I'd also like to avoid linker runs on the final LAR.
But I don't think thats what you meant - I think you meant to say that we should "abandon the stages concept".
Abandoning stages won't fix this, unfortunately. The problem is PIC in a LAR. (Okay, if we merged initram into the boot block, it would work, but that would kill the ability to have fallback/normal initram).
And you know what - with all due respect to Ron, I tend to be in favor of that option. Stages continue to be rather painful. Just one guy's opinion.
I found stages rather useful, but that's only my personal opinion.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
On 14.11.2008 19:54, Jordan Crouse wrote:
Carl-Daniel Hailfinger wrote:
On 14.11.2008 19:06, Stefan Reinauer wrote:
ron minnich wrote:
On Fri, Nov 14, 2008 at 9:53 AM, Stefan Reinauer stepan@coresystems.de wrote:
Carl-Daniel Hailfinger wrote:
> On 14.11.2008 18:14, svn@coreboot.org wrote: > > >> Author: rminnich >> New Revision: 1026 >> >> /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o: >> section .data.rel.ro.local: dual_channel_slew_group_lookup.3242 >> single_channel_slew_group_lookup.3243 >> > Basic rule: If you want to have arrays of pointers in initram, > you lose. > Pointers are not relocatable by definition. const is not going to > help > you there. > Hm. This is bad. 'nother regression in v3 that wasn't in v2.
it's not acceptable as a rule. We have to fix it one way or another. Why gcc is marking it as writeable is a puzzle but points to a bug somewhere.
We could try and use indices instead of pointers. That's the easiest rewrite i can think of to get the problem done without wasting the code.
This is clearly a gcc weakness. This stuff should not happen when compiling PIC. That's what PIC is for.
We're missing one crucial piece which is necessary to get PIC to work: The linker. PIC code must be linked _after_ its location is known. That means a linker would have to pass over the final LAR archive. Right now, we circumvent that requirement by using early linker tricks and by prohibiting the use of arrays of pointers.
If we insist on using arrays of pointers, we must either run a linker over the final LAR archive or abandon LAR completely and go for v2 linker magic.
Fully agreed. If we run a linker over the final LAR archive, then we will need a way to run the linker every single time the LAR has changed (including, mind you, on target, if we are going to go for the "replace stages on the fly" thing). Does not sound appetizing at all. I also don't like abandoning LAR - it is useful for payloads and such.
Yes, I'd like to keep LAR and I'd also like to avoid linker runs on the final LAR.
But I don't think thats what you meant - I think you meant to say that we should "abandon the stages concept".
Abandoning stages won't fix this, unfortunately. The problem is PIC in a LAR. (Okay, if we merged initram into the boot block, it would work, but that would kill the ability to have fallback/normal initram).
Yep, it sure would.
And you know what - with all due respect to Ron, I tend to be in favor of that option. Stages continue to be rather painful. Just one guy's opinion.
I found stages rather useful, but that's only my personal opinion.
There is a difference between theoretically useful and technologically feasible. As it stands, we are starting to have to make serious compromises between the code we want to write, and the design we want to keep. And even more unfortunately, we are starting to dip in to the issues that we cannot easily protect against until the linker dies with an error of questionable usefulness.
The problem is that our good intentions are revealing an ugly side - kind of like the mercury in a florescent light bulb. Everything we have been taught about embedded programming says that we need to share as much code as possible. And there is code that we want to share between stages. And we want to use standard C constructs such as pointers throughout the code. All this with the pesky constraint that we have "memory" but we don't actually have memory.
Everything we know about the coreboot design in v2 is that we want stages. And everything we know about the v2 design says that we don't like linker scripts, and we hate home grown compiler tools. We want to pull everything off with just a $(CC) call, but it is clear we are quickly exposing the limitations of the compiler.
So we are in a no-mans land. We want to share code and data between stages, but we want stages to be completely separate. And we want to do this without reinventing any wheels from POSIX and adding many K of infrastructure to the ROM to do it. Eventually, something is going to give. There will be a sacrifice. I advocate removing stages all together and instituting a single binary loader to rule them all, but I am biased since I have not yet personally had a need for a fallback kernel nor am I at all interested in updating individual stages on the fly. Certainly, the HPC folks have a dramatically different opinion - these are debates that can be held assuming the community universally agrees that a compromise is needed.
Jordan
I see two possible compromises:
1. copy initram to the start of CAR. I think this is easy and would work just fine. Then we can link initram to CARBASE 2. link initram into stage 0. The issue here is that we lose fallback. Speaking as the only person here who ever reflashed 1023 nodes with a defective bios, I have to say I am fond of fallback. Having fallback saved about 5 days of cracking nodes open and replacing lpc parts. It may have saved my life; I think the guys would have killed me had they had to do the flash replacements. That said, I don't think most people care about fallback, but that said, that's cause we're all hackers. What about end users?
I don't see post-pass links on the lar as a realistic option.
As for ldscipts.. I was asked by an OLPC guy early in the game to tell him how the jump from fallback to normal worked -- where did the symbol get created in the process of building a fallback image? I found the answer. Now I dare anyone on this list to go find it. Not just "it's this linker script". But find the actual place the computation is done and the symbol assigned. Then come back and tell me you like the linker script approach :-)
I vote for (1), myself.
But the fact is, for as long as we unpack a compressed coreboot to RAM, we're going to have at least two stages, whether we call it that or not.
ron
-
Carl-Daniel Hailfinger -
Corey Osgood -
Jordan Crouse -
ron minnich -
Stefan Reinauer