From e43422b734269d23f336e5ca3c05dedd8fd52901 Mon Sep 17 00:00:00 2001 From: Jeenu Viswambharan Date: Wed, 5 Sep 2018 14:23:27 +0100 Subject: [PATCH 1/2] ARMv7: Alias dmbld() to dmb() 'dmb ld' is not a recognized instruction for ARMv7. Since generic code may use 'dmb ld', alias it to 'dmb' when building for ARMv7. Change-Id: I502f360cb6412897ca9580b725d9f79469a7612e Signed-off-by: Jeenu Viswambharan --- include/lib/aarch32/arch_helpers.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/lib/aarch32/arch_helpers.h b/include/lib/aarch32/arch_helpers.h index 6369a5d78..5d9c1c151 100644 --- a/include/lib/aarch32/arch_helpers.h +++ b/include/lib/aarch32/arch_helpers.h @@ -210,7 +210,12 @@ DEFINE_SYSOP_FUNC(sev) DEFINE_SYSOP_TYPE_FUNC(dsb, sy) DEFINE_SYSOP_TYPE_FUNC(dmb, sy) DEFINE_SYSOP_TYPE_FUNC(dmb, st) + +/* dmb ld is not valid for armv7/thumb machines */ +#if ARM_ARCH_MAJOR != 7 DEFINE_SYSOP_TYPE_FUNC(dmb, ld) +#endif + DEFINE_SYSOP_TYPE_FUNC(dsb, ish) DEFINE_SYSOP_TYPE_FUNC(dsb, ishst) DEFINE_SYSOP_TYPE_FUNC(dmb, ish) @@ -323,6 +328,11 @@ DEFINE_DCOP_PARAM_FUNC(cvac, DCCMVAC) #define dsb() dsbsy() #define dmb() dmbsy() +/* dmb ld is not valid for armv7/thumb machines, so alias it to dmb */ +#if ARM_ARCH_MAJOR == 7 +#define dmbld() dmb() +#endif + #define IS_IN_SECURE() \ (GET_NS_BIT(read_scr()) == 0) From 24dc97091532751ea810e99bd5a9be33514decf6 Mon Sep 17 00:00:00 2001 From: Jeenu Viswambharan Date: Wed, 8 Aug 2018 14:10:55 +0100 Subject: [PATCH 2/2] Add missing barriers to Bakery Locks 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 --- lib/locks/bakery/bakery_lock_coherent.c | 15 +++++++++++---- lib/locks/bakery/bakery_lock_normal.c | 13 ++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/locks/bakery/bakery_lock_coherent.c b/lib/locks/bakery/bakery_lock_coherent.c index 788ba9818..03277c32f 100644 --- a/lib/locks/bakery/bakery_lock_coherent.c +++ b/lib/locks/bakery/bakery_lock_coherent.c @@ -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(); diff --git a/lib/locks/bakery/bakery_lock_normal.c b/lib/locks/bakery/bakery_lock_normal.c index 630226ae2..b947da9c7 100644 --- a/lib/locks/bakery/bakery_lock_normal.c +++ b/lib/locks/bakery/bakery_lock_normal.c @@ -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();