Fix normal memory bakery lock implementation
This patch fixes an issue in the normal memory bakery lock
implementation. During assertion of lock status, there is a possibility
that the assertion could fail. This is because the previous update done
to the lock status by the owning CPU when not participating in cache
coherency could result in stale data in the cache due to cache maintenance
operations not propagating to all the caches. This patch fixes this issue
by doing an extra read cache maintenance operation prior to the assertion.
Fixes ARM-software/tf-issues#402
Change-Id: I0f38a7c52476a4f58e17ebe0141d256d198be88d
Signed-off-by: Soby Mathew <soby.mathew@arm.com>
diff --git a/lib/locks/bakery/bakery_lock_normal.c b/lib/locks/bakery/bakery_lock_normal.c
index efc1b57..5a2fb07 100644
--- a/lib/locks/bakery/bakery_lock_normal.c
+++ b/lib/locks/bakery/bakery_lock_normal.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -90,6 +90,22 @@
#define read_cache_op(addr, cached) if (cached) \
dccivac((uintptr_t)addr)
+/* Helper function to check if the lock is acquired */
+static inline int is_lock_acquired(const bakery_info_t *my_bakery_info,
+ int is_cached)
+{
+ /*
+ * Even though lock data is updated only by the owning cpu and
+ * appropriate cache maintenance operations are performed,
+ * if the previous update was done when the cpu was not participating
+ * in coherency, then there is a chance that cache maintenance
+ * operations were not propagated to all the caches in the system.
+ * Hence do a `read_cache_op()` prior to read.
+ */
+ read_cache_op(my_bakery_info, is_cached);
+ return !!(bakery_ticket_number(my_bakery_info->lock_data));
+}
+
static unsigned int bakery_get_ticket(bakery_lock_t *lock,
unsigned int me, int is_cached)
{
@@ -104,12 +120,8 @@
my_bakery_info = get_bakery_info(me, lock);
assert(my_bakery_info);
- /*
- * Prevent recursive acquisition.
- * Since lock data is written to and cleaned by the owning cpu, it
- * doesn't require any cache operations prior to reading the lock data.
- */
- assert(!bakery_ticket_number(my_bakery_info->lock_data));
+ /* Prevent recursive acquisition.*/
+ assert(!is_lock_acquired(my_bakery_info, is_cached));
/*
* Tell other contenders that we are through the bakery doorway i.e.
@@ -222,7 +234,8 @@
unsigned int is_cached = read_sctlr_el3() & SCTLR_C_BIT;
my_bakery_info = get_bakery_info(plat_my_core_pos(), lock);
- assert(bakery_ticket_number(my_bakery_info->lock_data));
+
+ assert(is_lock_acquired(my_bakery_info, is_cached));
my_bakery_info->lock_data = 0;
write_cache_op(my_bakery_info, is_cached);
