Re: [PATCH v2 1/3] arm/cpu/lowlevel: invalidate i-cache before enabling

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

Hello,

On Thu, Dec 11, 2014 at 09:51:31PM +0100, Uwe Kleine-König wrote:
> Architecturally the cache contents are undefined so it might well
> contain stale data at reset. So better be save than sorry.
> 
> I verifyed that the added instructions are defined for both, ARMv6 and
> ARMv7, using the ARM Architecture Reference Manual, ARMv7-A and ARMv7-R
> edition (ARM DDI 0406C.c). For the already existing mcr instruction see
> the newly added comment.
> 
> This patch also unifies handling of ARMv6 and ARMv7, the isb instruction
> can also be done on the latter via mcr which simplifies the code a bit.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  arch/arm/cpu/lowlevel.S | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/lowlevel.S b/arch/arm/cpu/lowlevel.S
> index c615d5b58160..e000cd8eae4c 100644
> --- a/arch/arm/cpu/lowlevel.S
> +++ b/arch/arm/cpu/lowlevel.S
> @@ -11,9 +11,26 @@ ENTRY(arm_cpu_lowlevel_init)
>  	orr	r12, r12, #0xd3
>  	msr	cpsr, r12
>  
> -#if __LINUX_ARM_ARCH__ >= 7
> -	isb
> -#elif __LINUX_ARM_ARCH__ == 6
> +#if __LINUX_ARM_ARCH__ >= 6
> +	/*
> +	 * ICIALLU: Invalidate all instruction caches to PoU,
> +	 * includes flushing of branch predictors.
> +	 * Even if the i-cache is off it might contain stale entries
> +	 * that are better discarded before enabling the cache.
> +	 * Architectually this is even possible after a cold reset.
> +	 */
> +	mcr	p15, 0, r12, c7, c5, 0
> +	/* DSB, ensure completion of the invalidation */
> +	mcr	p15, 0, r12, c7, c10, 4
> +	/*
> +	 * ISB, ensure instruction fetch path is in sync.
> +	 * Note that the ARM Architecture Reference Manual, ARMv7-A and ARMv7-R
> +	 * edition (ARM DDI 0406C.c) doesn't define this instruction in the
> +	 * ARMv6 part (D12.7.10). It only has: "Support of additional
> +	 * operations is IMPLEMENTATION DEFINED".
> +	 * But an earlier version of the ARMARM (ARM DDI 0100I) does define it
> +	 * as "Flush prefetch buffer (PrefetchFlush)".
> +	 */
>  	mcr	p15, 0, r12, c7, c5, 4
>  #endif
I just noticed that this is not optimal here. E.g. for mvebu_defconfig
builds this code is not compiled in because

	CONFIG_ARCH_ARMADA_370=y
	CONFIG_ARCH_ARMADA_XP=y
	CONFIG_ARCH_DOVE=y
	CONFIG_ARCH_KIRKWOOD=y

and so __LINUX_ARM_ARCH__ = 5. That means it's not worse than before my
patch, but still not optimal. I'll check how to fix that.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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