Re: [PATCH 3/5] ARM: cache-armv7: start invalidation from outer levels

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 4/23/19 7:18 PM, Ahmad Fatoum wrote:
> The ARM Cortex-A Series Programmer's Guide notes[1]:
>> Some care is required with cache maintenance activity in multi-core

Unfortunately, the only error scenario I could find in the ARM docs
was this one here, which presumes a multi-core system.

Lucas, do you have a better example in context of barebox on how things
could go awry without this patch?


>> systems that include a L2C-310 L2 cache (or similar). Cleaning or
>> invalidating the L1 cache and L2 cache will not be a single atomic
>> operation. A core might therefore perform cache maintenance on
>> a particular address in both L1 and L2 caches only as two discrete
>> steps. If another core were to access the affected address between
>> those two actions, a coherency problem can occur. Such problems can
>> be avoided by following two simple rules.
>>
>> * When cleaning, always clean the innermost (L1) cache first and then
>>   clean the outer cache(s).
>> * When invalidating, always invalidate the outermost cache first and
>>   the L1 cache last.
> 
> The current code correctly iterates from inner to outer cache levels
> when flushing/cleaning (r8 == 0), invalidation (r8 == 1) occurs in the
> same direction though. Adjust the invalidation iteration order to start
> from the outermost cache instead.
> 
> Equivalent C-Code:
> 
> 	enum cache_op { CACHE_FLUSH = 0, CACHE_INVAL = 1 };
> 	register enum cache_op operation asm("r8");
> 	register int i asm("r12");
> 	register int limit asm("r3") = max_cache_level << 1; // e.g. 4 with L2 max
> 
> 	+if (operation == CACHE_FLUSH) {
> 	 	i = 0;
> 	+} else {
> 	+	i = limit - 2;
> 	+}
> 
> 	 bool loop_again;
> 	 do {
> 		/* [snip] */
> 	+	if (operation == CACHE_FLUSH) {
> 			i += 2;
> 			loop_again = limit > i;
> 	+	} else {
> 	+		loop_again = i > 0;
> 	+		i -= 2;
> 	+	}
> 	} while (loop_again);
> 
> [1]: 18.6 "TLB and cache maintenance broadcast", Version 4.0
> 
> Suggested-by: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/cpu/cache-armv7.S | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/cache-armv7.S b/arch/arm/cpu/cache-armv7.S
> index 0eb0ecfee756..b2d70bece7b9 100644
> --- a/arch/arm/cpu/cache-armv7.S
> +++ b/arch/arm/cpu/cache-armv7.S
> @@ -83,7 +83,10 @@ hierarchical:
>  		ands	r3, r0, #0x7000000	@ extract loc from clidr
>  		mov	r3, r3, lsr #23		@ left align loc bit field
>  		beq	finished		@ if loc is 0, then no need to clean
> -		mov	r12, #0			@ start clean at cache level 0
> +		cmp	r8, #0
> +THUMB(		ite	eq			)
> +		moveq	r12, #0
> +		subne	r12, r3, #2		@ start invalidate at outmost cache level
>  loop1:
>  		add	r2, r12, r12, lsr #1	@ work out 3x current cache level
>  		mov	r1, r0, lsr r2		@ extract cache type bits from clidr
> @@ -118,8 +121,15 @@ THUMB(		ite	eq			)
>  		subs	r7, r7, #1		@ decrement the index
>  		bge	loop2
>  skip:
> +		cmp	r8, #0
> +		bne	inval_check
>  		add	r12, r12, #2		@ increment cache number
>  		cmp	r3, r12
> +		b	loop_end_check
> +inval_check:
> +		cmp	r12, #0
> +		sub	r12, r12, #2		@ decrement cache number
> +loop_end_check:
>  #ifdef CONFIG_ARM_ERRATA_814220
>  		dsb
>  #endif
> 


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox