ron minnich wrote: > comments welcome. Please see notes in the patch. Short version: Very nice!
> I am not as interested in comments on this specific code (it needs > cleanup) as I am in two questions: I'll mention a few things that I would like to be included in that cleanup. > 1. can artec please test the current svn to make sure there is > nothing I have broken Good point. Could be tested also on ALIX. > 2. Are the changes to lib/stage2.c ok I think so. > Anyway, take a look. With luck, we have SMP on the kontron within > the week; SMI follows, then ACPI, then maybe we can make v3 the > preferred kontron software base. It would be awesome! > +++ lib/stage2.c (working copy) .. > @@ -85,6 +104,11 @@ > dev_phase6(); > show_all_devs(BIOS_DEBUG, "After phase 6."); > > + /* final cleanup: do any remaining CPU setup. This can include memory > + * init, or not, depending on the CPU; it may have been done in phase > 1. > + */ > + cpu_phase2(is_coldboot(), sysinfo); > + The comment mentions phase 1 - but which phase 1 is that? Would help if it said cpu_phase1() or stage2_phase1() instead. > + movw $0x11, 0 Just curious, what do these movws to 0 do? > +++ arch/x86/intel/core2/init_cpus.c (working copy) .. > @@ -73,6 +72,7 @@ > int nodes, siblings; > result = cpuid(1); > /* See how many sibling cpus we have */ > + printk(BIOS_DEBUG, "cpuid(1) %x\n", result.ebx); Please make this debug output say ebx=%08x because even if that is redundant for the people looking at it right now, output like this can linger and anyone else may be confused. > @@ -377,18 +384,25 @@ > stackmem->stacks[index].post = 0; > stackmem->stacks[index].active_cpus = active_cpus; > stackmem->stacks[index].start_cpu_lock = start_cpu_lock; > + printk(BIOS_SPEW, "stack[index, apicid, post, active_cpus, > start_cpu_lock = [%lx, %x, %d, %p, %p]\n", index, apicid, 0, active_cpus, > start_cpu_lock); I guess this is missing ] after start_cpu_lock. Overall great improvements! //Peter -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
