Add missing barriers to Bakery Locks
authorJeenu Viswambharan <jeenu.viswambharan@arm.com>
Wed, 8 Aug 2018 13:10:55 +0000 (14:10 +0100)
committerJeenu Viswambharan <jeenu.viswambharan@arm.com>
Wed, 5 Sep 2018 13:39:14 +0000 (14:39 +0100)
With the current implementation, it's possible for a contender to
observe accesses in the Critical Section before acquiring or releasing
the lock. Insert fencing in the locking and release codes to prevent any
reorder.

Fixes ARM-software/tf-issues#609

Change-Id: I773b82aa41dd544a2d3dbacb9a4b42c9eb767bbb
Signed-off-by: Jeenu Viswambharan <jeenu.viswambharan@arm.com>
lib/locks/bakery/bakery_lock_coherent.c
lib/locks/bakery/bakery_lock_normal.c

index 788ba981846ef8d3ba025c1a6ee5d46015e45d49..03277c32f7903d0d86484d398cd699210afeb766 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013-2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2013-2018, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -133,7 +133,12 @@ void bakery_lock_get(bakery_lock_t *bakery)
                                bakery_ticket_number(bakery->lock_data[they]));
                }
        }
-       /* Lock acquired */
+
+       /*
+        * Lock acquired. Ensure that any reads from a shared resource in the
+        * critical section read values after the lock is acquired.
+        */
+       dmbld();
 }
 
 
@@ -146,9 +151,11 @@ void bakery_lock_release(bakery_lock_t *bakery)
        assert(bakery_ticket_number(bakery->lock_data[me]));
 
        /*
-        * Release lock by resetting ticket. Then signal other
-        * waiting contenders
+        * Ensure that other observers see any stores in the critical section
+        * before releasing the lock. Release the lock by resetting ticket.
+        * Then signal other waiting contenders.
         */
+       dmbst();
        bakery->lock_data[me] = 0;
        dsb();
        sev();
index 630226ae2d514c334637492502bba9009d27ff26..b947da9c7912e758c5e14ed387421da5293c7e65 100644 (file)
@@ -204,7 +204,12 @@ void bakery_lock_get(bakery_lock_t *lock)
                                == bakery_ticket_number(their_bakery_info->lock_data));
                }
        }
-       /* Lock acquired */
+
+       /*
+        * Lock acquired. Ensure that any reads from a shared resource in the
+        * critical section read values after the lock is acquired.
+        */
+       dmbld();
 }
 
 void bakery_lock_release(bakery_lock_t *lock)
@@ -220,6 +225,12 @@ void bakery_lock_release(bakery_lock_t *lock)
 
        assert(is_lock_acquired(my_bakery_info, is_cached));
 
+       /*
+        * Ensure that other observers see any stores in the critical section
+        * before releasing the lock. Release the lock by resetting ticket.
+        * Then signal other waiting contenders.
+        */
+       dmbst();
        my_bakery_info->lock_data = 0;
        write_cache_op(my_bakery_info, is_cached);
        sev();