T589: Fix insufficient ordering guarantees in bakery lock

bakery_lock_get() uses DMB LD after lock acquisition and
bakery_lock_release() uses DMB ST before releasing the lock. This is
insufficient in both cases. With just DMB LD, stores in the critical
section can be reordered before the DMB LD which could mean writes in
the critical section completing before the lock has been acquired
successfully. Similarly, with just DMB ST, a load in the critical section
could be reordered after the the DMB ST. DMB is the least expensive
barrier that can provide the required ordering.

Signed-off-by: Raghu Krishnamurthy <raghu.ncstate@icloud.com>
Change-Id: Ieb74cbf5b76b09e1789331b71f37f7c660221b0e

2 files changed, 20 insertions, 12 deletions

diff --git a/lib/locks/bakery/bakery_lock_coherent.c b/lib/locks/bakery/bakery_lock_coherent.c
index 1634e3af6..748eeddf4 100644
--- a/lib/locks/bakery/bakery_lock_coherent.c
+++ b/lib/locks/bakery/bakery_lock_coherent.c
@@ -137,10 +137,11 @@ void bakery_lock_get(bakery_lock_t *bakery)
 	}
 
 	/*
-	 * Lock acquired. Ensure that any reads from a shared resource in the
-	 * critical section read values after the lock is acquired.
+	 * Lock acquired. Ensure that any reads and writes from a shared
+	 * resource in the critical section read/write values after the lock is
+	 * acquired.
 	 */
-	dmbld();
+	dmbish();
 }
 
 
@@ -154,11 +155,14 @@ void bakery_lock_release(bakery_lock_t *bakery)
 
 	/*
 	 * 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.
+	 * before releasing the lock. Also ensure all loads in the critical
+	 * section are complete before releasing the lock. Release the lock by
+	 * resetting ticket. Then signal other waiting contenders.
 	 */
-	dmbst();
+	dmbish();
 	bakery->lock_data[me] = 0U;
+
+	/* Required to ensure ordering of the following sev */
 	dsb();
 	sev();
 }
diff --git a/lib/locks/bakery/bakery_lock_normal.c b/lib/locks/bakery/bakery_lock_normal.c
index f906f51ea..caced8f46 100644
--- a/lib/locks/bakery/bakery_lock_normal.c
+++ b/lib/locks/bakery/bakery_lock_normal.c
@@ -219,10 +219,11 @@ void bakery_lock_get(bakery_lock_t *lock)
 	}
 
 	/*
-	 * Lock acquired. Ensure that any reads from a shared resource in the
-	 * critical section read values after the lock is acquired.
+	 * Lock acquired. Ensure that any reads and writes from a shared
+	 * resource in the critical section read/write values after the lock is
+	 * acquired.
 	 */
-	dmbld();
+	dmbish();
 }
 
 void bakery_lock_release(bakery_lock_t *lock)
@@ -240,11 +241,14 @@ void bakery_lock_release(bakery_lock_t *lock)
 
 	/*
 	 * 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.
+	 * before releasing the lock. Also ensure all loads in the critical
+	 * section are complete before releasing the lock. Release the lock by
+	 * resetting ticket. Then signal other waiting contenders.
 	 */
-	dmbst();
+	dmbish();
 	my_bakery_info->lock_data = 0U;
 	write_cache_op((uintptr_t)my_bakery_info, is_cached);
+
+	/* This sev is ordered by the dsbish in write_cahce_op */
 	sev();
 }