Fix volatileness of variables

Quite often, variables that need to be read/written using volatile
accesses are only read using volatile accesses (with ACCESS macro). They
need to be written using volatile writes as well to guarantee correct
ordering with locking functions as they do not introduce a compiler
memory barrier.

All the following statement is valid if considering a CPU that does not
need runtime barriers (DMB, exclusive atomic instructions, etc.) to have
predictable ordering between reads and writes on different threads
(multicore configuration). On the ARM architecture, these runtime
barriers are needed in addition to what the C standard guarantees. These
runtime barriers are typically provided by locking functions, but this
is not sufficient when synchronizing more than just memory accesses (for
example system register access as it is the case in the TF and TFTF).

Locking functions are implemented either as volatile inline assembly, or
can be considered as complete "volatile" blackboxes by the compiler as it
is implemented in pure assembly.

Volatile access only gives ordering guarantees between volatile accesses
in a specific thread, and does not influence the ordering of the
non-volatile accesses. It also gives no guarantee about the ordering
between volatile accesses in different threads. Only a compiler memory
barrier can guarantee an ordering between all kind of access.

Even though GCC currently seems to be quite conservative in its memory
analysis, it could very well determine that a non-volatile variable
local to the compilation unit never sees its address taken and that no
return value of any function depends on this variable, and therefore
that the locking function has no way of seeing it. This allows the
compiler to move the non-volatile write to this variable accross a
function call like spin_lock(). All these behaviors have been tested and
GCC even completely removes the storage space used by the variable if no
volatile access is made or if the variable is not "exposed" in any way.

As these variables are usually only read and written once or in polling
loops, all there accesses are required to be volatile, and using an
ACCESS macro brings no optimization compared to making the variables
volatile themselves.

The Linux kernels' document
Documentation/volatile-considered-harmful.txt does makes the assumption
that all volatile variables should be accessed within locked sections,
and that lock functions act as compiler memory barrier. This is not the
case in our code-base so volatile is still needed.

This patch makes this variables volatile and remove ACCESS macro usage
that becomes useless.

Also, remove the ACCESS macro as it is not needed anymore.

Change-Id: I69c48e28116118e6ff8e089ae8e1fe22c484f3c0
Signed-off-by: Douglas Raillard <douglas.raillard@arm.com>

1 files changed, 2 insertions, 2 deletions

diff --git a/framework/main.c b/framework/main.c
index a74e67d..b5b0182 100644
--- a/framework/main.c
+++ b/framework/main.c
@@ -37,7 +37,7 @@ extern const char version_string[];
 unsigned int lead_cpu_mpid;
 
 /* Defined in hotplug.c */
-extern test_function_t test_entrypoint[PLATFORM_CORE_COUNT];
+extern volatile test_function_t test_entrypoint[PLATFORM_CORE_COUNT];
 
 /* Per-CPU results for the current test */
 static test_result_t test_results[PLATFORM_CORE_COUNT];
@@ -374,7 +374,7 @@ void __dead2 run_tests(void)
 		 * - For other CPUs, it has been populated by tftf_cpu_on() or
 		 *   tftf_try_cpu_on()
 		 */
-		while (ACCESS(test_entrypoint[core_pos]) == 0)
+		while (test_entrypoint[core_pos] == 0)
 			;
 
 		test_results[core_pos] = test_entrypoint[core_pos]();