[coreboot] patch: make node id/core id available on all arch; really implement stop_ap

ron minnich rminnich at gmail.com
Sat Sep 20 16:36:23 CEST 2008


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 at 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




More information about the coreboot mailing list