[coreboot] New patch to review for coreboot: bf36231 Intel cpus: Fix deadlock on hyper-threading init

Kyösti Mälkki (kyosti.malkki@gmail.com) gerrit at coreboot.org
Fri Mar 9 17:25:19 CET 2012


Kyösti Mälkki (kyosti.malkki at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/775

-gerrit

commit bf362311db9948925b5580a9922a58c4c8726c82
Author: Kyösti Mälkki <kyosti.malkki at gmail.com>
Date:   Fri Mar 9 17:02:37 2012 +0200

    Intel cpus: Fix deadlock on hyper-threading init
    
    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 at gmail.com>
---
 src/cpu/intel/hyperthreading/intel_sibling.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/cpu/intel/hyperthreading/intel_sibling.c b/src/cpu/intel/hyperthreading/intel_sibling.c
index 823d77c..e988664 100644
--- a/src/cpu/intel/hyperthreading/intel_sibling.c
+++ b/src/cpu/intel/hyperthreading/intel_sibling.c
@@ -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);
-		}
 	}
-
 }
 




More information about the coreboot mailing list