On 28.08.2008 01:48, ron minnich wrote:
Sorry I messed it up, if you can hit me with a patch for my broken patch I'll ack it.
(Patch is also attached.)
Hardest part first: BIST handling. Unless I'm mistaken, we already die() in stage1_main() if processor BIST is nonzero. Checking it in initram makes no sense. Having it as global variable is unnecessary as well. Link BIST is an entirely different animal.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: corebootv3-bist_sanitization/include/globalvars.h =================================================================== --- corebootv3-bist_sanitization/include/globalvars.h (Revision 833) +++ corebootv3-bist_sanitization/include/globalvars.h (Arbeitskopie) @@ -47,7 +47,6 @@ #endif unsigned int loglevel; /* these two values are of interest in many stages */ - u32 bist; u32 init_detected; struct sys_info sys_info; }; Index: corebootv3-bist_sanitization/mainboard/amd/serengeti/initram.c =================================================================== --- corebootv3-bist_sanitization/mainboard/amd/serengeti/initram.c (Revision 833) +++ corebootv3-bist_sanitization/mainboard/amd/serengeti/initram.c (Arbeitskopie) @@ -86,12 +86,11 @@
/** * main for initram for the AMD Serengeti - * @param bist Built In Self Test, which is used to indicate status of self test * @param init_detected Used to indicate that we have been started via init * @returns 0 on success * The purpose of this code is to not only get ram going, but get any other cpus/cores going. * The two activities are very tightly connected and not really seperable. - * The BSP (boot strap processor? ) Core 0 is responsible for all training or all sockets. Note that + * The BSP (boot strap processor) Core 0 (BSC) is responsible for all training or all sockets. Note that * this need not be socket 0; one great strength of coreboot, as opposed to other BIOSes, is that it could * always boot with with a CPU in any socket, and even with empty sockets (as opposed to, e.g., the BIOS * that came installed on the Sun Ultra 40, which would freeze if one CPU were not installed). @@ -100,9 +99,6 @@ * */ /* - * bist is defined by the CPU hardware and is present in EAX on first instruction of coreboot. - * Its value is implementation defined. - * * init_detected is used to determine if we did a soft reset as required by a reprogramming of the * hypertransport links. If we did this kind of reset, bit 11 will be set in the MTRRdefType_MSR MSR. * That may seem crazy, but there are not lots of places to hide a bit when the CPU does a reset. @@ -111,7 +107,7 @@ int main(void) { void enable_smbus(void); - u32 bist, u32 init_detected; + u32 init_detected; static const u16 spd_addr[] = { //first node RC0|DIMM0, RC0|DIMM2, 0, 0, @@ -139,17 +135,11 @@ post_code(POST_START_OF_MAIN); sysinfo = &(global_vars()->sys_info);
- bist = sysinfo->bist; init_detected = sysinfo->init_detected; - /* We start out by looking at bist. Where was bist set? */ /* well, here we are. For starters, we need to know if this is cpu0 core0. * cpu0 core 0 will do all the DRAM setup. */ - if (bist) { - printk(BIOS_EMERG, "Bist 0x%x\n", bist); - die("bist failure"); - } else - bsp_apicid = init_cpus(init_detected, sysinfo); + bsp_apicid = init_cpus(init_detected, sysinfo);
// dump_mem(DCACHE_RAM_BASE+DCACHE_RAM_SIZE-0x200, DCACHE_RAM_BASE+DCACHE_RAM_SIZE);
Index: corebootv3-bist_sanitization/arch/x86/stage1.c =================================================================== --- corebootv3-bist_sanitization/arch/x86/stage1.c (Revision 833) +++ corebootv3-bist_sanitization/arch/x86/stage1.c (Arbeitskopie) @@ -143,7 +143,9 @@ * This function is called from assembler code with its argument on the * stack. Force the compiler to generate always correct code for this case. * We have cache as ram running and can start executing code in C. - * @param bist Built In Self Test value + * @param bist Built In Self Test, which is used to indicate status of self test. + * bist is defined by the CPU hardware and is present in EAX on first instruction of coreboot. + * Its value is implementation defined. * @param init_detected This (optionally set) value is used on some platforms (e.g. k8) to indicate * that we are restarting after some sort of reconfiguration. Note that we could use it on geode but * do not at present. @@ -174,6 +176,7 @@
// before we do anything, we want to stop if we dont run // on the bootstrap processor. +#warning We do not want to check BIST here, we want to check whether we are BSC! if (bist==0) { // stop secondaries stop_ap(); @@ -183,7 +186,6 @@ * NEVER run this on an AP! */ global_vars_init(&globvars); - globvars.bist = bist; globvars.init_detected = init_detected;
hardware_stage1();