ok, I'm on travel for the next 10 days, so this won't get done :-)
On Sat, Sep 20, 2008 at 3:59 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
On 20.09.2008 06:51, ron minnich wrote:
Thanks for the review Peter, here is try 2.
Also, yes, the order of some includes had to change.
The include thing is a difference of opinion, I sometimes get in Plan 9 habits. I'm not concerned too much, sure I'll include it in global_variables.h in 10 days.
While the new code is obviously what we wanted back then, please tell me why we pass BIST to stage1 if we totally ignore it. Should we die() early if bist!=0 or die even earlier in stage0?
Let's leave bist as a parameter. Just because we do not yet use it does not mean we never will.
it's a hard problem. if BIST is non zero things are bad. But we need to do a lot of work to even print a message.
Please leave that comment in. Once we have to run raminit from each node (family 10h), that code will be run by all cores. I'd say that your original patch for this was better because it made execution conditional.
v2 runs raminit on all nodes on some machines now. But they will never run this particular code. They will be given an IPI with the address of the AP code to be run for starting their ram. So checks for a non-zero core id are not needed here.
The most we should do:
stop_ap(); if (me.nodeid || me.coreid) die("Things look dark");
The k8 raminit is a very interesting piece of code, worth learning from. We've got some smart people out there :-)
If somebody wants to take this patch and improve it would be a big help. I think we know what we need.
thanks
ron