From 42d89b657da16b2cf6eae627f294e4153e0f2ba2 Mon Sep 17 00:00:00 2001 From: Dan Crowell Date: Thu, 10 Jan 2019 13:40:32 -0600 Subject: [PATCH] Add sync to doorbell function to avoid weak consistency bug Due to the way POWER works, there is no guarantee that a memory location written by 1 thread is actually visible to another thread unless you explicitly assert a sync of some kind. In the code that runs to wake up new cores, there is a path where memory is written by 1 thread (usually on the master processor) but is immediately consumed by a new thread that just woke up. It is possible for the new thread to consume the memory and not see the contents that were written by the other thread. The fix is to add a sync command before we send the doorbell msgsnd operation. This will ensure the memory contents are visible to the thread as soon as it wakes up. Change-Id: I8a1483dd7bbda5af064ba6d004dc9e0a3a61ce78 CQ: SW453195 Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/70311 Reviewed-by: Dean Sanner Reviewed-by: Nicholas E. Bofferding Tested-by: Jenkins Server Tested-by: Jenkins OP Build CI Tested-by: Jenkins OP HW Reviewed-by: Daniel M. Crowell --- src/kernel/doorbell.C | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/kernel/doorbell.C b/src/kernel/doorbell.C index c24ee138413..dbfc91059b2 100644 --- a/src/kernel/doorbell.C +++ b/src/kernel/doorbell.C @@ -5,7 +5,7 @@ /* */ /* OpenPOWER HostBoot Project */ /* */ -/* Contributors Listed Below - COPYRIGHT 2015,2018 */ +/* Contributors Listed Below - COPYRIGHT 2015,2019 */ /* [+] International Business Machines Corp. */ /* */ /* */ @@ -75,6 +75,10 @@ void send_doorbell_wakeup(uint64_t i_pir) // execution KernelWorkItem* l_work = new CpuWakeupDoorbellWorkItem(); l_cpu->doorbell_actions.push(l_work); + //Put a barrier here to prevent a possible weak consistency + // issue with the l_work memory getting consumed incorrectly + // by the new thread that wakes up + sync(); //Send doorbell to wakeup core/thread doorbell_send(i_pir); } @@ -89,6 +93,10 @@ void send_doorbell_restore_tb(uint64_t i_pir, uint64_t i_tb) // execution KernelWorkItem* l_work = new CpuTbRestoreDoorbellWorkItem(); l_cpu->doorbell_actions.push(l_work); + //Put a barrier here to prevent a possible weak consistency + // issue with the l_work memory getting consumed incorrectly + // by the new thread that wakes up + sync(); //Send doorbell to wakeup core/thread doorbell_send(i_pir); }