Re: [PATCH kvm-unit-tests] arm: fix crash when caches are off

[Paolo Bonzini <pbonzini@redhat.com>]

Il 16/09/2014 04:06, Andrew Jones ha scritto:
> We shouldn't try Load-Exclusive instructions unless we've enabled memory
> management, as these instructions depend on the data cache unit's
> coherency monitor. This patch adds a new setup boolean, initialized to false,
> that is used to guard Load-Exclusive instructions. Eventually we'll add more
> setup code that sets it true.
> 
> Note: This problem became visible on boards with Cortex-A7 processors. Testing
> with Cortex-A15 didn't expose it, as those may have an external coherency
> monitor that still allows the instruction to execute (on A7s we got data
> aborts). Although even on A15's it's not clear from the spec if the
> instructions will behave as expected while caches are off, so we no longer
> allow Load-Exclusive instructions on those processors without caches enabled
> either.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/arm/asm/setup.h |  2 ++
>  lib/arm/setup.c     |  1 +
>  lib/arm/spinlock.c  | 10 ++++++++++
>  3 files changed, 13 insertions(+)
> 
> diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
> index 21445ef2085fc..9c54c184e2866 100644
> --- a/lib/arm/asm/setup.h
> +++ b/lib/arm/asm/setup.h
> @@ -20,6 +20,8 @@ extern phys_addr_t __phys_offset, __phys_end;
>  #define PHYS_SIZE		(1ULL << PHYS_SHIFT)
>  #define PHYS_MASK		(PHYS_SIZE - 1ULL)
>  
> +extern bool mem_caches_enabled;
> +
>  #define L1_CACHE_SHIFT		6
>  #define L1_CACHE_BYTES		(1 << L1_CACHE_SHIFT)
>  #define SMP_CACHE_BYTES		L1_CACHE_BYTES
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 3941c9757dcb2..f7ed639c9d499 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -25,6 +25,7 @@ u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0UL) };
>  int nr_cpus;
>  
>  phys_addr_t __phys_offset, __phys_end;
> +bool mem_caches_enabled;
>  
>  static void cpu_set(int fdtnode __unused, u32 regval, void *info __unused)
>  {
> diff --git a/lib/arm/spinlock.c b/lib/arm/spinlock.c
> index d8a6d4c3383d6..43539c5e84062 100644
> --- a/lib/arm/spinlock.c
> +++ b/lib/arm/spinlock.c
> @@ -1,12 +1,22 @@
>  #include "libcflat.h"
>  #include "asm/spinlock.h"
>  #include "asm/barrier.h"
> +#include "asm/setup.h"
>  
>  void spin_lock(struct spinlock *lock)
>  {
>  	u32 val, fail;
>  
>  	dmb();
> +
> +	/*
> +	 * Without caches enabled Load-Exclusive instructions may fail.
> +	 * In that case we do nothing, and just hope the caller knows
> +	 * what they're doing.
> +	 */
> +	if (!mem_caches_enabled)
> +		return;
> +
>  	do {
>  		asm volatile(
>  		"1:	ldrex	%0, [%2]\n"
> 

Tested and pushed.  Thanks!

Paolo