Intel cpus: Fix deadlock on hyper-threading init
authorKyösti Mälkki <kyosti.malkki@gmail.com>
Fri, 9 Mar 2012 15:02:37 +0000 (17:02 +0200)
committerRudolf Marek <r.marek@assembler.cz>
Sun, 25 Mar 2012 18:33:28 +0000 (20:33 +0200)
Only the BSP CPU was able to start its hyper-threading CPU siblings.
When an AP CPU attempts this it calls start_cpu() within start_cpu(),
deadlocking the system with start_cpu_lock.

At the time intel_sibling_init() is run, the BSP CPU is still
walking the cpu_bus linked list in lapic_cpu_init: start_other_cpus().
A sibling CPU appended at the end of this list will get started.

Also fail compile with #error if SERIAL_CPU_INIT==0, as microcode
updates on hyper-threading sibling CPUs must be serialized.

Tested with HT-enabled P4 Xeons on dual-socket604 platform.

Change-Id: I0053f58f49ed604605ce0a55e826d3e1afdc90b6
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/775
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
Tested-by: build bot (Jenkins)
Reviewed-by: Rudolf Marek <r.marek@assembler.cz>
src/cpu/intel/hyperthreading/intel_sibling.c

index 823d77c3a35a6313269b6aa3db3a1188f72f4273..e988664ddaafdc28322bf459f4734d2e81be88a1 100644 (file)
@@ -5,6 +5,11 @@
 #include <device/device.h>
 #include <pc80/mc146818rtc.h>
 #include <smp/spinlock.h>
+#include <assert.h>
+
+#if CONFIG_SERIAL_CPU_INIT==0
+#error Intel hyper-threading requires serialized cpu init
+#endif
 
 static int first_time = 1;
 static int disable_siblings = !CONFIG_LOGICAL_CPUS;
@@ -30,11 +35,9 @@ void intel_sibling_init(device_t cpu)
                siblings = 1;
        }
 
-#if 1
        printk(BIOS_DEBUG, "CPU: %u %d siblings\n",
                cpu->path.apic.apic_id,
                siblings);
-#endif
 
        /* See if I am a sibling cpu */
        if (cpu->path.apic.apic_id & (siblings -1)) {
@@ -53,25 +56,18 @@ void intel_sibling_init(device_t cpu)
                cpu_path.apic.apic_id = cpu->path.apic.apic_id + i;
 
 
-               /* Allocate the new cpu device structure */
-               new = alloc_dev(cpu->bus, &cpu_path);
+               /* Allocate new cpu device structure iff sibling CPU
+                * was not in static device tree.
+                */
+               new = alloc_find_dev(cpu->bus, &cpu_path);
 
                if (!new) {
                        continue;
                }
 
-#if 1
                printk(BIOS_DEBUG, "CPU: %u has sibling %u\n",
                        cpu->path.apic.apic_id,
                        new->path.apic.apic_id);
-#endif
-               /* Start the new cpu */
-               if (!start_cpu(new)) {
-                       /* Record the error in cpu? */
-                       printk(BIOS_ERR, "CPU %u would not start!\n",
-                               new->path.apic.apic_id);
-               }
        }
-
 }