From f8b30ca89b3e3a09d96019bf3998255a3f7a4c8a Mon Sep 17 00:00:00 2001 From: Antonio Nino Diaz Date: Wed, 31 Oct 2018 15:55:57 +0000 Subject: [PATCH] bakery: Fix MISRA defects Change-Id: I600bc13522ae977db355b6dc5a1695bce39ec130 Signed-off-by: Antonio Nino Diaz --- include/lib/bakery_lock.h | 40 ++++++++++----- include/lib/spinlock.h | 6 +-- lib/locks/bakery/bakery_lock_coherent.c | 23 ++++----- lib/locks/bakery/bakery_lock_normal.c | 66 ++++++++++++++----------- 4 files changed, 82 insertions(+), 53 deletions(-) diff --git a/include/lib/bakery_lock.h b/include/lib/bakery_lock.h index c80082e9b..2d1612e17 100644 --- a/include/lib/bakery_lock.h +++ b/include/lib/bakery_lock.h @@ -4,8 +4,8 @@ * SPDX-License-Identifier: BSD-3-Clause */ -#ifndef __BAKERY_LOCK_H__ -#define __BAKERY_LOCK_H__ +#ifndef BAKERY_LOCK_H +#define BAKERY_LOCK_H #include @@ -13,21 +13,39 @@ #ifndef __ASSEMBLY__ #include +#include #include +#include /***************************************************************************** - * Internal helper macros used by the bakery lock implementation. + * Internal helpers used by the bakery lock implementation. ****************************************************************************/ + /* Convert a ticket to priority */ -#define PRIORITY(t, pos) (((t) << 8) | (pos)) +static inline unsigned int bakery_get_priority(unsigned int t, unsigned int pos) +{ + return (t << 8) | pos; +} -#define CHOOSING_TICKET 0x1 -#define CHOSEN_TICKET 0x0 +#define CHOOSING_TICKET U(0x1) +#define CHOSEN_TICKET U(0x0) -#define bakery_is_choosing(info) (info & 0x1) -#define bakery_ticket_number(info) ((info >> 1) & 0x7FFF) -#define make_bakery_data(choosing, number) \ - (((choosing & 0x1) | (number << 1)) & 0xFFFF) +static inline bool bakery_is_choosing(unsigned int info) +{ + return (info & 1U) == CHOOSING_TICKET; +} + +static inline unsigned int bakery_ticket_number(unsigned int info) +{ + return (info >> 1) & 0x7FFFU; +} + +static inline uint16_t make_bakery_data(unsigned int choosing, unsigned int num) +{ + unsigned int val = (choosing & 0x1U) | (num << 1); + + return (uint16_t) val; +} /***************************************************************************** * External bakery lock interface. @@ -83,4 +101,4 @@ void bakery_lock_release(bakery_lock_t *bakery); #endif /* __ASSEMBLY__ */ -#endif /* __BAKERY_LOCK_H__ */ +#endif /* BAKERY_LOCK_H */ diff --git a/include/lib/spinlock.h b/include/lib/spinlock.h index 8aec70789..fcd36e856 100644 --- a/include/lib/spinlock.h +++ b/include/lib/spinlock.h @@ -4,8 +4,8 @@ * SPDX-License-Identifier: BSD-3-Clause */ -#ifndef __SPINLOCK_H__ -#define __SPINLOCK_H__ +#ifndef SPINLOCK_H +#define SPINLOCK_H #ifndef __ASSEMBLY__ @@ -26,4 +26,4 @@ void spin_unlock(spinlock_t *lock); #endif -#endif /* __SPINLOCK_H__ */ +#endif /* SPINLOCK_H */ diff --git a/lib/locks/bakery/bakery_lock_coherent.c b/lib/locks/bakery/bakery_lock_coherent.c index 03277c32f..8e86790ab 100644 --- a/lib/locks/bakery/bakery_lock_coherent.c +++ b/lib/locks/bakery/bakery_lock_coherent.c @@ -35,9 +35,9 @@ */ #define assert_bakery_entry_valid(_entry, _bakery) do { \ - assert(_bakery); \ - assert(_entry < BAKERY_LOCK_MAX_CPUS); \ -} while (0) + assert((_bakery) != NULL); \ + assert((_entry) < BAKERY_LOCK_MAX_CPUS); \ +} while (false) /* Obtain a ticket for a given CPU */ static unsigned int bakery_get_ticket(bakery_lock_t *bakery, unsigned int me) @@ -46,7 +46,7 @@ static unsigned int bakery_get_ticket(bakery_lock_t *bakery, unsigned int me) unsigned int they; /* Prevent recursive acquisition */ - assert(!bakery_ticket_number(bakery->lock_data[me])); + assert(bakery_ticket_number(bakery->lock_data[me]) == 0U); /* * Flag that we're busy getting our ticket. All CPUs are iterated in the @@ -58,9 +58,9 @@ static unsigned int bakery_get_ticket(bakery_lock_t *bakery, unsigned int me) * ticket value. That's OK as the lock is acquired based on the priority * value, not the ticket value alone. */ - my_ticket = 0; + my_ticket = 0U; bakery->lock_data[me] = make_bakery_data(CHOOSING_TICKET, my_ticket); - for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) { + for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) { their_ticket = bakery_ticket_number(bakery->lock_data[they]); if (their_ticket > my_ticket) my_ticket = their_ticket; @@ -105,8 +105,8 @@ void bakery_lock_get(bakery_lock_t *bakery) * Now that we got our ticket, compute our priority value, then compare * with that of others, and proceed to acquire the lock */ - my_prio = PRIORITY(my_ticket, me); - for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) { + my_prio = bakery_get_priority(my_ticket, me); + for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) { if (me == they) continue; @@ -120,7 +120,8 @@ void bakery_lock_get(bakery_lock_t *bakery) * (valid) ticket value. If they do, compare priorities */ their_ticket = bakery_ticket_number(their_bakery_data); - if (their_ticket && (PRIORITY(their_ticket, they) < my_prio)) { + if ((their_ticket != 0U) && + (bakery_get_priority(their_ticket, they) < my_prio)) { /* * They have higher priority (lower value). Wait for * their ticket value to change (either release the lock @@ -148,7 +149,7 @@ void bakery_lock_release(bakery_lock_t *bakery) unsigned int me = plat_my_core_pos(); assert_bakery_entry_valid(me, bakery); - assert(bakery_ticket_number(bakery->lock_data[me])); + assert(bakery_ticket_number(bakery->lock_data[me]) != 0U); /* * Ensure that other observers see any stores in the critical section @@ -156,7 +157,7 @@ void bakery_lock_release(bakery_lock_t *bakery) * Then signal other waiting contenders. */ dmbst(); - bakery->lock_data[me] = 0; + bakery->lock_data[me] = 0U; dsb(); sev(); } diff --git a/lib/locks/bakery/bakery_lock_normal.c b/lib/locks/bakery/bakery_lock_normal.c index b947da9c7..beae63c72 100644 --- a/lib/locks/bakery/bakery_lock_normal.c +++ b/lib/locks/bakery/bakery_lock_normal.c @@ -53,21 +53,31 @@ CASSERT((PLAT_PERCPU_BAKERY_LOCK_SIZE & (CACHE_WRITEBACK_GRANULE - 1)) == 0, \ IMPORT_SYM(uintptr_t, __PERCPU_BAKERY_LOCK_SIZE__, PERCPU_BAKERY_LOCK_SIZE); #endif -#define get_bakery_info(_cpu_ix, _lock) \ - (bakery_info_t *)((uintptr_t)_lock + _cpu_ix * PERCPU_BAKERY_LOCK_SIZE) +static inline bakery_lock_t *get_bakery_info(unsigned int cpu_ix, + bakery_lock_t *lock) +{ + return (bakery_info_t *)((uintptr_t)lock + + cpu_ix * PERCPU_BAKERY_LOCK_SIZE); +} -#define write_cache_op(_addr, _cached) \ - do { \ - (_cached ? dccvac((uintptr_t)_addr) :\ - dcivac((uintptr_t)_addr));\ - dsbish();\ - } while (0) +static inline void write_cache_op(uintptr_t addr, bool cached) +{ + if (cached) + dccvac(addr); + else + dcivac(addr); -#define read_cache_op(_addr, _cached) if (_cached) \ - dccivac((uintptr_t)_addr) + dsbish(); +} + +static inline void read_cache_op(uintptr_t addr, bool cached) +{ + if (cached) + dccivac(addr); +} /* Helper function to check if the lock is acquired */ -static inline int is_lock_acquired(const bakery_info_t *my_bakery_info, +static inline bool is_lock_acquired(const bakery_info_t *my_bakery_info, int is_cached) { /* @@ -78,8 +88,8 @@ static inline int is_lock_acquired(const bakery_info_t *my_bakery_info, * 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)); + read_cache_op((uintptr_t)my_bakery_info, is_cached); + return bakery_ticket_number(my_bakery_info->lock_data) != 0U; } static unsigned int bakery_get_ticket(bakery_lock_t *lock, @@ -94,7 +104,7 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock, * it is not NULL. */ my_bakery_info = get_bakery_info(me, lock); - assert(my_bakery_info); + assert(my_bakery_info != NULL); /* Prevent recursive acquisition.*/ assert(!is_lock_acquired(my_bakery_info, is_cached)); @@ -103,16 +113,16 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock, * Tell other contenders that we are through the bakery doorway i.e. * going to allocate a ticket for this cpu. */ - my_ticket = 0; + my_ticket = 0U; my_bakery_info->lock_data = make_bakery_data(CHOOSING_TICKET, my_ticket); - write_cache_op(my_bakery_info, is_cached); + write_cache_op((uintptr_t)my_bakery_info, is_cached); /* * Iterate through the bakery information of each contender to allocate * the highest ticket number for this cpu. */ - for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) { + for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) { if (me == they) continue; @@ -121,9 +131,9 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock, * ensure that a stale copy is not read. */ their_bakery_info = get_bakery_info(they, lock); - assert(their_bakery_info); + assert(their_bakery_info != NULL); - read_cache_op(their_bakery_info, is_cached); + read_cache_op((uintptr_t)their_bakery_info, is_cached); /* * Update this cpu's ticket number if a higher ticket number is @@ -141,7 +151,7 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock, ++my_ticket; my_bakery_info->lock_data = make_bakery_data(CHOSEN_TICKET, my_ticket); - write_cache_op(my_bakery_info, is_cached); + write_cache_op((uintptr_t)my_bakery_info, is_cached); return my_ticket; } @@ -167,8 +177,8 @@ void bakery_lock_get(bakery_lock_t *lock) * Now that we got our ticket, compute our priority value, then compare * with that of others, and proceed to acquire the lock */ - my_prio = PRIORITY(my_ticket, me); - for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) { + my_prio = bakery_get_priority(my_ticket, me); + for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) { if (me == they) continue; @@ -177,11 +187,11 @@ void bakery_lock_get(bakery_lock_t *lock) * ensure that a stale copy is not read. */ their_bakery_info = get_bakery_info(they, lock); - assert(their_bakery_info); + assert(their_bakery_info != NULL); /* Wait for the contender to get their ticket */ do { - read_cache_op(their_bakery_info, is_cached); + read_cache_op((uintptr_t)their_bakery_info, is_cached); their_bakery_data = their_bakery_info->lock_data; } while (bakery_is_choosing(their_bakery_data)); @@ -190,7 +200,7 @@ void bakery_lock_get(bakery_lock_t *lock) * (valid) ticket value. If they do, compare priorities */ their_ticket = bakery_ticket_number(their_bakery_data); - if (their_ticket && (PRIORITY(their_ticket, they) < my_prio)) { + if (their_ticket && (bakery_get_priority(their_ticket, they) < my_prio)) { /* * They have higher priority (lower value). Wait for * their ticket value to change (either release the lock @@ -199,7 +209,7 @@ void bakery_lock_get(bakery_lock_t *lock) */ do { wfe(); - read_cache_op(their_bakery_info, is_cached); + read_cache_op((uintptr_t)their_bakery_info, is_cached); } while (their_ticket == bakery_ticket_number(their_bakery_info->lock_data)); } @@ -231,7 +241,7 @@ void bakery_lock_release(bakery_lock_t *lock) * Then signal other waiting contenders. */ dmbst(); - my_bakery_info->lock_data = 0; - write_cache_op(my_bakery_info, is_cached); + my_bakery_info->lock_data = 0U; + write_cache_op((uintptr_t)my_bakery_info, is_cached); sev(); }