diff --git a/include/lib/bakery_lock.h b/include/lib/bakery_lock.h index c80082e..2d1612e 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 @@ #endif /* __ASSEMBLY__ */ -#endif /* __BAKERY_LOCK_H__ */ +#endif /* BAKERY_LOCK_H */ diff --git a/include/lib/spinlock.h b/include/lib/spinlock.h index 8aec707..fcd36e8 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 @@ #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 03277c3..8e86790 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 @@ 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 @@ * 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 @@ * 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 @@ * (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 @@ 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 @@ * 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 b947da9..beae63c 100644 --- a/lib/locks/bakery/bakery_lock_normal.c +++ b/lib/locks/bakery/bakery_lock_normal.c @@ -53,21 +53,31 @@ 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 @@ * 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 @@ * 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 @@ * 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 @@ * 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 @@ ++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 @@ * 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 @@ * 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 @@ * (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 @@ */ 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 @@ * 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(); }