From cfc9593fa8814411e8fd7418a6a4291bd6318e11 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Thu, 7 Feb 2008 23:19:28 +0000 Subject: [PATCH] Fix possible though unlikely SMP issue reported by Robert Szeleney. One must not use a single static variable to synchronize CPUs at two points. In an environment where CPUs do not really run concurently (in emulation or with logical processors) it would be possible for CPUs to get trapped in the first synchronization while another CPU might just do its thing and change the sync variable again. These CPUs would then never leave the first loop as the exit condition has already passed again. The key is to use two different sync variables like it is done in early kernel initialization. As I didn't manage to trigger this code though I am not sure if this is gonna work. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@23926 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/system/kernel/arch/x86/arch_cpu.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/system/kernel/arch/x86/arch_cpu.c b/src/system/kernel/arch/x86/arch_cpu.c index 27d899cc0b..744ba21f3b 100644 --- a/src/system/kernel/arch/x86/arch_cpu.c +++ b/src/system/kernel/arch/x86/arch_cpu.c @@ -66,7 +66,8 @@ extern void reboot(void); void (*gX86SwapFPUFunc)(void *oldState, const void *newState); bool gHasSSE = false; -static vint32 sWaitAllCPUs; +static uint32 sCpuRendezvous; +static uint32 sCpuRendezvous2; segment_descriptor *gGDT = NULL; @@ -114,9 +115,7 @@ set_mtrr(void *_parameter, int cpu) struct set_mtrr_parameter *parameter = (struct set_mtrr_parameter *)_parameter; // wait until all CPUs have arrived here - atomic_add(&sWaitAllCPUs, 1); - while (sWaitAllCPUs != smp_get_num_cpus()) - asm volatile ("pause;"); + smp_cpu_rendezvous(&sCpuRendezvous, cpu); disable_caches(); @@ -126,9 +125,7 @@ set_mtrr(void *_parameter, int cpu) enable_caches(); // wait until all CPUs have arrived here - atomic_add(&sWaitAllCPUs, -1); - while (sWaitAllCPUs != 0) - asm volatile ("pause;"); + smp_cpu_rendezvous(&sCpuRendezvous2, cpu); } @@ -136,9 +133,7 @@ static void init_mtrrs(void *_unused, int cpu) { // wait until all CPUs have arrived here - atomic_add(&sWaitAllCPUs, 1); - while (sWaitAllCPUs != smp_get_num_cpus()) - asm volatile ("pause;"); + smp_cpu_rendezvous(&sCpuRendezvous, cpu); disable_caches(); @@ -147,9 +142,7 @@ init_mtrrs(void *_unused, int cpu) enable_caches(); // wait until all CPUs have arrived here - atomic_add(&sWaitAllCPUs, -1); - while (sWaitAllCPUs != 0) - asm volatile ("pause;"); + smp_cpu_rendezvous(&sCpuRendezvous2, cpu); } @@ -174,6 +167,7 @@ x86_set_mtrr(uint32 index, uint64 base, uint64 length, uint8 type) parameter.length = length; parameter.type = type; + sCpuRendezvous = sCpuRendezvous2 = 0; call_all_cpus(&set_mtrr, ¶meter); } @@ -549,8 +543,10 @@ arch_cpu_init_post_modules(kernel_args *args) close_module_list(cookie); // initialize MTRRs if available - if (x86_count_mtrrs() > 0) + if (x86_count_mtrrs() > 0) { + sCpuRendezvous = sCpuRendezvous2 = 0; call_all_cpus(&init_mtrrs, NULL); + } // get optimized functions from the CPU module if (sCpuModule != NULL && sCpuModule->get_optimized_functions != NULL) {