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.
- can artec please test the current svn to make sure there is
nothing I have broken
Good point. Could be tested also on ALIX.
- 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