bakery: Fix MISRA defects

Change-Id: I600bc13522ae977db355b6dc5a1695bce39ec130
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
This commit is contained in:
Antonio Nino Diaz 2018-10-31 15:55:57 +00:00
parent b8a02d53c0
commit f8b30ca89b
4 changed files with 82 additions and 53 deletions

View File

@ -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 <platform_def.h>
@ -13,21 +13,39 @@
#ifndef __ASSEMBLY__
#include <cdefs.h>
#include <stdbool.h>
#include <stdint.h>
#include <utils_def.h>
/*****************************************************************************
* 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 */

View File

@ -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 */

View File

@ -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();
}

View File

@ -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();
}