Re: [PATCH 1/5] ARM: cache-armv7: add work-around for errata 814220

From: Lucas Stach <l.stach@pengutronix.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>, barebox@lists.infradead.org
Cc: ore@pengutronix.de, rcz@pengutronix.de
Subject: Re: [PATCH 1/5] ARM: cache-armv7: add work-around for errata 814220
Date: Thu, 25 Apr 2019 12:02:22 +0200	[thread overview]
Message-ID: <1556186542.2584.9.camel@pengutronix.de> (raw)
In-Reply-To: <20190423171852.26126-2-a.fatoum@pengutronix.de>

Am Dienstag, den 23.04.2019, 19:18 +0200 schrieb Ahmad Fatoum:
> The v7 ARM states that all cache and branch predictor maintenance operations
> that do not specify an address execute, relative to each other, in program
> order. However, because of this erratum, an L2 set/way cache maintenance
> operation can overtake an L1 set/way cache maintenance operation, this would
> cause the data corruption.
> 
> This ERRATA affected the Cortex-A7 and present in r0p2, r0p3, r0p4, r0p5.
> 
> This patch is the SW workaround by adding a DSB before changing cache levels as
> the ARM ERRATA: ARM/MP: 814220 told in the ARM ERRATA documentation.
> 
> Signed-off-by: Jason Liu <r64343@freescale.com>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> [afa: picked from LKML: <20190214083145.15148-1-benjamin.gaignard@linaro.org>]
> [afa: edited commit message headline]
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/Kconfig           | 12 ++++++++++++
>  arch/arm/cpu/cache-armv7.S |  3 +++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index a683c9c86661..fc622640aa2b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -448,4 +448,16 @@ config ARM_PSCI_DEBUG
>  	  putc function.
>  	  Only use for debugging.
>  
> +config ARM_ERRATA_814220
> +       bool "ARM errata: Cache maintenance by set/way operations can execute out of order"
> +       depends on CPU_V7
> +       help
> +         The v7 ARM states that all cache and branch predictor maintenance
> +         operations that do not specify an address execute, relative to
> +         each other, in program order.
> +         However, because of this erratum, an L2 set/way cache maintenance
> +         operation can overtake an L1 set/way cache maintenance operation.
> +         This ERRATA only affected the Cortex-A7 and present in r0p2, r0p3,
> +         r0p4, r0p5.
> +
>  endmenu
> diff --git a/arch/arm/cpu/cache-armv7.S b/arch/arm/cpu/cache-armv7.S
> index 7a1c5c01894c..0eb0ecfee756 100644
> --- a/arch/arm/cpu/cache-armv7.S
> +++ b/arch/arm/cpu/cache-armv7.S
> @@ -120,6 +120,9 @@ THUMB(		ite	eq			)
>  skip:
>  		add	r12, r12, #2		@ increment cache number
>  		cmp	r3, r12
> +#ifdef CONFIG_ARM_ERRATA_814220
> +		dsb
> +#endif

I don't think we need a Kconfig option here. This function is not
really performance critical. The short pipeline stall introduced by the
dsb when switching the cache level is minor compared to the time it
takes to actually move the cache blocks on a clean.

Just always execute the sdb and add a comment on why it is needed.

Regards,
Lucas

>  		bgt	loop1
>  finished:
>  		ldmfd	sp!, {r4-r11}

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