[LinuxBIOS] RESEND: v3 patch for console functions, and mods to lib/lar.c to support a void * parameter for running functions.
attached. I need an ack, we have discussed these already.
ron
* ron minnich rminnich@gmail.com [080103 18:50]:
+void banner(int level, char *s) +{
- printk(level, "===========================%s===========================\n", s);
+}
I don't like this function all too much.. I'd prefer something where the width of the banner stays constant.
void die(const char *str) { printk(BIOS_EMERG, str); while (1)
hlt();
console_tx_byte(0, (void *)0);}
Will this not lead to the CPU consuming cycles and becoming/staying really hot?
Can't we clear the FIFO any other way and go to CPU hlt afterwards?
Stefan
On Jan 3, 2008 10:03 AM, Stefan Reinauer stepan@coresystems.de wrote:
Will this not lead to the CPU consuming cycles and becoming/staying really hot?
yeah but, hey, it should never happen. CPUs don't melt any more when you push them that hard anyway, usually they slow themselves down or in some cases reset. I don't think it's a real issue.
Also, recall that it will be doing a lot of IO, not computing pi :-)
Can't we clear the FIFO any other way and go to CPU hlt afterwards?
No, because of JTAG. I've had really terrible problems with JTAG debuggers when CPUs halt. Even a tight loop (while (1);) can be hard to break into on some CPUs. But if the CPU is in a call/return-with-IO type mode, it is pretty easy to grab it with JTAG, stop it, and debug it.
thanks
ron
On Jan 3, 2008 10:03 AM, Stefan Reinauer stepan@coresystems.de wrote:
- ron minnich rminnich@gmail.com [080103 18:50]:
+void banner(int level, char *s) +{
printk(level, "===========================%s===========================\n", s);+}
I don't like this function all too much.. I'd prefer something where the width of the banner stays constant.
hmm. OK, that means a strlen call but I guess we can do it.
ron
On 03.01.2008 18:50, ron minnich wrote:
Add a void * parameter to the LAR run functions; needed for some cases, in particular CAR.
I still do not like that because it works around a bug in our code. It should work just fine without that parameter.
Regards, Carl-Daniel
On Jan 3, 2008 10:20 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
On 03.01.2008 18:50, ron minnich wrote:
Add a void * parameter to the LAR run functions; needed for some cases, in particular CAR.
I still do not like that because it works around a bug in our code. It should work just fine without that parameter.
I don't agree. There are limits on where the stack ends up in CAR. I would be more comfortable having the option of starting with a new function with a new stack once CAR is turned off.
That said, I will hold off and let others fix CAR. Let me know when it is ready and I will test on Alix.
However, please don't fix the problem by moving the stack. You can't relocate the stack in a general way due to things like pointers to auto.
Thanks
ron
On 03.01.2008 19:26, ron minnich wrote:
On Jan 3, 2008 10:20 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
On 03.01.2008 18:50, ron minnich wrote:
Add a void * parameter to the LAR run functions; needed for some cases, in particular CAR.
I still do not like that because it works around a bug in our code. It should work just fine without that parameter.
I don't agree. There are limits on where the stack ends up in CAR. I would be more comfortable having the option of starting with a new function with a new stack once CAR is turned off.
A new stack would be OK, but not as a workaround for a bug. Being able to drop parts of the stack when CAR is disabled is an interesting feature, but I fear it will lead to a lot of headaches later on.
I still have some code pending which uses part of the stack as a printk buffer right from the beginning of stage1, making it available for inspection by later stages or even a payload. Sorry, can't find that code right now. I'll try to come up with a concept that works for architectures where we must move the stack as well as architectures where we don't want to move the stack.
That said, I will hold off and let others fix CAR. Let me know when it is ready and I will test on Alix.
Unfortunately, I do not have any Geode LX hardware, so I can't really test, but I can deliver patches. IIRC your problem was that the wbinvd only invalidated the CAR area without writing contents back to RAM, right?
However, please don't fix the problem by moving the stack. You can't relocate the stack in a general way due to things like pointers to auto.
I will find the root cause of the problem, and no, the fix will not involve moving the stack during runtime unless that is a hardware requirement.
Regards, Carl-Daniel
I am restarting the CAR discussion a bit. I want to get a resync.
One part of the discussion involves the idea that I am trying to avoid return from disable_car because of some "bug" in our code. It is not that simple. The issue is that, when the code enters disable_car,the CPU has been using cache as ram, and using "memory" at non-memory addresses. In disable_car, the CPU is supposed to disable cache as ram, and continue -- should it return? The return is on the stack. The function that called disable_car has its own stack variables. They're actually in cache, not ram. So if the code is going to return, it will need a working stack. It will need to restore the stack from cache to ram. But where is the stack? at c000 in one case. Where is ram? Somewhere else. So it is possible that variables on stack are wrong, if they are pointers to variables on the stack, and there is the problem of changing esp, ss, and then fixing up the stack for the RET.
Here's a useful comment: x86/car/cache_as_ram_post.c: /* FIXME: I hope we don't need to change esp and ebp value here, so we can restore value from mmx sse back But the problem is the range is some io related, So don't go back */
If you look at the cache as ram for the amd and x86, you will find something interesting: it does not go back. x86: leal _iseg, %edi jmp *%edi
For AMD, at the end of the mainboard cache as ram main, it calls post_cache_as_ram(void). At the end of that function is this: /*copy and execute linuxbios_ram */ copy_and_run(); /* We will not return */
in other words, once CAR is disabled, we chain to copy_and_run, and never return. We've also set up a new ESP.
To put it simply, the most current CAR code *does not return* --because getting that right, for all CPUs, is tricky and not always possible.
So, expecting a return from disable car is not really in line with what we do now. In fact, what we do now on most of v2 -- certainly on opteron -- is merely what I'm proposing we do on V3.
I'm pasting in some old mail now, with my comments in[[[]]]
================================================================= Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
Dumb question: We know the location of the stack when we enter disable_car(). We also know that all memory belongs to us. Can we copy CAR stack contents to some safe location and restore them after wbinvd()?
This shouldn't be needed. If the stack needed to be moved you would want to do it before the wbinvd so you were copying from cache to memory. Much faster than memory to memory.
Marc
and I am too smart for my own good.....
Here is the deal. Look in cpu/amd/model_lx/cache_as_ram.inc. /* If you wanted to maintain the stack in memory you would need to set the tags as dirty [[[ Good point here! For the wbinvd to push the stack out, we need to set cache tags! This is going to be tough on some CPUs -- how do we do it on LX? ]]] so the wbinvd would push out the old stack contents to memory */ /* Clear the cache, the following code from crt0.S.lb will setup a new stack*/ wbinvd
For LX we didn't need to maintain the stack or any variables beyond CAR. [[[[ But on v3 we do .... ]]]] That meant that we didn't need to do any stack copy like the K8 post_cache_as_ram()(that never returns). The LX cache_as_ram_main returns and the LB copy function is back in cache_as_ram.inc. Since there is nothing on the stack and everything is running out of the ROM a new stack is easily created.
[[[ Please note the comment: "that never returns"]]]
[[[ and, note: if you look at that code, round about line 313, you'll notice that it does not return either]]]
Sooo, if you want to maintain the stack in v3... I left a comment in the v2 code. Recall that they cache is disabled so the way doesn't get reallocated and thus the tags are never marked dirty and written back. Since the tags are never marked dirty the wbind won't wb but it will invd.
[[[meaning: stack cache lines will be invalidated but NOT written back ...]]]
There are a couple ways to address this. 1. copy the stack to a new location. 2. Set the tags dirty with by writing the way MSRs. [[[ This is how you set the tags ]]] 3. enable the cache and copy the stack back on it's self to dirty the tags. [[[This might work too to set the tags BUT -- the stack really has to be running at the right location. We would have to change this.]]]
I am least sure about number three.
I think that sums it up. I still think that it is best/fastest if the code returned to the CAR function and it setup a new stack in memory and then launched the next stage.
[[[ Which is pretty much what I am proposing]]] ============================================================= I hope this additional discussion is helpful.
Now, getting back to where we were: I am still arguing for what Marc argues in the last paragraph, stating "I still think that it is best/fastest if the code returned to the CAR function and it setup a new stack in memory and then launched the next stage.".
Carl-Daniel, I believe you are hoping we can do the return from disable_car, but as Marc points out in options 1-3 above, it's really not simple. This is not a "bug" per se. Fixing it will be complex. It is possible that you will need different solutions for each CPU *implementation* in some cases, not just each CPU type -- for example, the Intel CAR had to be changed for AMD, and different types of Opteron may well need different CAR return code.
Thanks
ron
On 04.01.2008 02:43, ron minnich wrote:
I am restarting the CAR discussion a bit. I want to get a resync.
Thanks.
One part of the discussion involves the idea that I am trying to avoid return from disable_car because of some "bug" in our code. It is not that simple. The issue is that, when the code enters disable_car,the CPU has been using cache as ram, and using "memory" at non-memory addresses. In disable_car, the CPU is supposed to disable cache as ram, and continue -- should it return? The return is on the stack. The function that called disable_car has its own stack variables. They're actually in cache, not ram. So if the code is going to return, it will need a working stack. It will need to restore the stack from cache to ram. But where is the stack? at c000 in one case. Where is ram? Somewhere else.
That's something I don't understand. Are you saying that there might be no RAM at address 0xC000? (Or at 0xC0000, IIRC.) That's well below 1MB and on my local system 0xC0000-0xCFFFF is claimed by the Video ROM of the embedded ATI graphics. Then again, I seem to remember that this area can be RAM as well.
So it is possible that variables on stack are wrong, if they are pointers to variables on the stack, and there is the problem of changing esp, ss, and then fixing up the stack for the RET.
Here's a useful comment: x86/car/cache_as_ram_post.c: /* FIXME: I hope we don't need to change esp and ebp value here, so we can restore value from mmx sse back But the problem is the range is some io related, So don't go back */
If you look at the cache as ram for the amd and x86, you will find something interesting: it does not go back. x86: leal _iseg, %edi jmp *%edi
For AMD, at the end of the mainboard cache as ram main, it calls post_cache_as_ram(void). At the end of that function is this: /*copy and execute linuxbios_ram */ copy_and_run(); /* We will not return */
in other words, once CAR is disabled, we chain to copy_and_run, and never return. We've also set up a new ESP.
To put it simply, the most current CAR code *does not return* --because getting that right, for all CPUs, is tricky and not always possible.
OK.
So, expecting a return from disable car is not really in line with what we do now. In fact, what we do now on most of v2 -- certainly on opteron -- is merely what I'm proposing we do on V3.
I'm pasting in some old mail now, with my comments in[[[]]]
Thanks for that. It seems I missed the essence of that mail back then.
================================================================= Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
Dumb question: We know the location of the stack when we enter disable_car(). We also know that all memory belongs to us. Can we copy CAR stack contents to some safe location and restore them after wbinvd()?
This shouldn't be needed. If the stack needed to be moved you would want to do it before the wbinvd so you were copying from cache to memory. Much faster than memory to memory.
Marc
and I am too smart for my own good.....
Here is the deal. Look in cpu/amd/model_lx/cache_as_ram.inc. /* If you wanted to maintain the stack in memory you would need to set the tags as dirty [[[ Good point here! For the wbinvd to push the stack out, we need to set cache tags! This is going to be tough on some CPUs -- how do we do it on LX? ]]]
I assumed from one of the mails in that old thread that setting the cache tags would be easy.
so the wbinvd would push out the old stack contents to memory *//* Clear the cache, the following code from crt0.S.lb will setup a new stack*/ wbinvd
For LX we didn't need to maintain the stack or any variables beyond CAR. [[[[ But on v3 we do .... ]]]] That meant that we didn't need to do any stack copy like the K8 post_cache_as_ram()(that never returns). The LX cache_as_ram_main returns and the LB copy function is back in cache_as_ram.inc. Since there is nothing on the stack and everything is running out of the ROM a new stack is easily created.
[[[ Please note the comment: "that never returns"]]]
OK.
[[[ and, note: if you look at that code, round about line 313, you'll notice that it does not return either]]]
Sooo, if you want to maintain the stack in v3... I left a comment in the v2 code. Recall that they cache is disabled so the way doesn't get reallocated and thus the tags are never marked dirty and written back. Since the tags are never marked dirty the wbind won't wb but it will invd.
[[[meaning: stack cache lines will be invalidated but NOT written back ...]]]
And that means the void * parameter to the running functions will be invalidated as well because it is passed on the stack. BOOM. You might be lucky enough that gcc generates code which loads the parameter into a register before the wbinvd and does not try to reload that parameter from your now-defunct old stack, but depending on luck is not an option here. If we kill our stack by making the wbinvd essentially an invd (because tags are not dirty), no function arguments nor any local variables will be available directly after the wbinvd. The only way out is to directly call a function (not by pointer) whose address is known during link time. Something like the call to copy_and_run() from v2 is needed here as well.
There are a couple ways to address this.
- copy the stack to a new location.
- Set the tags dirty with by writing the way MSRs.
[[[ This is how you set the tags ]]]
That's the way I'd prefer to handle things.
- enable the cache and copy the stack back on it's self to dirty the tags.
[[[This might work too to set the tags BUT -- the stack really has to be running at the right location. We would have to change this.]]]
I am least sure about number three.
I think that sums it up. I still think that it is best/fastest if the code returned to the CAR function and it setup a new stack in memory and then launched the next stage.
[[[ Which is pretty much what I am proposing]]]
I hope this additional discussion is helpful.
Indeed, thanks!
Now, getting back to where we were: I am still arguing for what Marc argues in the last paragraph, stating "I still think that it is best/fastest if the code returned to the CAR function and it setup a new stack in memory and then launched the next stage.".
As long as I can salvage the old stack contents in some way, that would be acceptable for me as well.
Carl-Daniel, I believe you are hoping we can do the return from disable_car, but as Marc points out in options 1-3 above, it's really not simple. This is not a "bug" per se. Fixing it will be complex. It is possible that you will need different solutions for each CPU *implementation* in some cases, not just each CPU type -- for example, the Intel CAR had to be changed for AMD, and different types of Opteron may well need different CAR return code.
An explosion of code variants for CAR disabling would seriously compromise maintainability of our code.
Regards, Carl-Daniel
Carl-Daniel, I admire your determination and hence I am willing to try making return work.
I would recommend we try to figure out how to get the stack to live where real memory lives, then copy the stack to itself, then wbinvd.
regards.
ron
-
Carl-Daniel Hailfinger -
ron minnich -
Stefan Reinauer