* [PATCH v2 1/3] arm/cpu/lowlevel: invalidate i-cache before enabling
2014-12-11 20:51 [PATCH v2 0/3] arm/cpu/lowlevel cleanups Uwe Kleine-König
@ 2014-12-11 20:51 ` Uwe Kleine-König
2014-12-18 8:40 ` Uwe Kleine-König
2014-12-11 20:51 ` [PATCH v2 2/3] arm/cpu/lowlevel: add and fix comments for CPSR and SCTLR accesses Uwe Kleine-König
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2014-12-11 20:51 UTC (permalink / raw)
To: barebox
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
--
2.1.3
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/3] arm/cpu/lowlevel: invalidate i-cache before enabling
2014-12-11 20:51 ` [PATCH v2 1/3] arm/cpu/lowlevel: invalidate i-cache before enabling Uwe Kleine-König
@ 2014-12-18 8:40 ` Uwe Kleine-König
0 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2014-12-18 8:40 UTC (permalink / raw)
To: barebox
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] arm/cpu/lowlevel: add and fix comments for CPSR and SCTLR accesses
2014-12-11 20:51 [PATCH v2 0/3] arm/cpu/lowlevel cleanups Uwe Kleine-König
2014-12-11 20:51 ` [PATCH v2 1/3] arm/cpu/lowlevel: invalidate i-cache before enabling Uwe Kleine-König
@ 2014-12-11 20:51 ` Uwe Kleine-König
2014-12-11 20:51 ` [PATCH v2 3/3] arm/cpu/lowlevel: Don't save the return address in another register Uwe Kleine-König
2014-12-15 9:58 ` [PATCH v2 0/3] arm/cpu/lowlevel cleanups Sascha Hauer
3 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2014-12-11 20:51 UTC (permalink / raw)
To: barebox
A part of the existing comments was incomplete or missleading.
Adding the register name to mcr/mrc instructions helps finding the
corresponding documentation in the manuals.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/arm/cpu/lowlevel.S | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/lowlevel.S b/arch/arm/cpu/lowlevel.S
index e000cd8eae4c..de7afba2c317 100644
--- a/arch/arm/cpu/lowlevel.S
+++ b/arch/arm/cpu/lowlevel.S
@@ -5,7 +5,7 @@
.section ".text_bare_init_","ax"
ENTRY(arm_cpu_lowlevel_init)
mov r2, lr
- /* set the cpu to SVC32 mode */
+ /* set the cpu to SVC32 mode, mask irq and fiq */
mrs r12, cpsr
bic r12, r12, #0x1f
orr r12, r12, #0xd3
@@ -34,10 +34,12 @@ ENTRY(arm_cpu_lowlevel_init)
mcr p15, 0, r12, c7, c5, 4
#endif
- /* disable MMU stuff and caches */
- mrc p15, 0, r12, c1, c0, 0
- bic r12, r12 , #(CR_M | CR_C | CR_B)
+ /* disable MMU stuff and data/unified caches */
+ mrc p15, 0, r12, c1, c0, 0 /* SCTLR */
+ bic r12, r12, #(CR_M | CR_C | CR_B)
bic r12, r12, #(CR_S | CR_R | CR_V)
+
+ /* enable instruction cache */
orr r12, r12, #CR_I
#if __LINUX_ARM_ARCH__ >= 6
@@ -51,7 +53,7 @@ ENTRY(arm_cpu_lowlevel_init)
orr r12, r12, #CR_B
#endif
- mcr p15, 0, r12, c1, c0, 0
+ mcr p15, 0, r12, c1, c0, 0 /* SCTLR */
mov pc, r2
ENDPROC(arm_cpu_lowlevel_init)
--
2.1.3
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] arm/cpu/lowlevel: Don't save the return address in another register
2014-12-11 20:51 [PATCH v2 0/3] arm/cpu/lowlevel cleanups Uwe Kleine-König
2014-12-11 20:51 ` [PATCH v2 1/3] arm/cpu/lowlevel: invalidate i-cache before enabling Uwe Kleine-König
2014-12-11 20:51 ` [PATCH v2 2/3] arm/cpu/lowlevel: add and fix comments for CPSR and SCTLR accesses Uwe Kleine-König
@ 2014-12-11 20:51 ` Uwe Kleine-König
2014-12-15 9:58 ` [PATCH v2 0/3] arm/cpu/lowlevel cleanups Sascha Hauer
3 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2014-12-11 20:51 UTC (permalink / raw)
To: barebox
The corresponding code doesn't use the lr register (neither explicitly
nor implicitly by the bl instruction), so there is no gain in using r2
here.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/arm/cpu/lowlevel.S | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/cpu/lowlevel.S b/arch/arm/cpu/lowlevel.S
index de7afba2c317..b76222d8f3c0 100644
--- a/arch/arm/cpu/lowlevel.S
+++ b/arch/arm/cpu/lowlevel.S
@@ -4,7 +4,6 @@
.section ".text_bare_init_","ax"
ENTRY(arm_cpu_lowlevel_init)
- mov r2, lr
/* set the cpu to SVC32 mode, mask irq and fiq */
mrs r12, cpsr
bic r12, r12, #0x1f
@@ -55,5 +54,5 @@ ENTRY(arm_cpu_lowlevel_init)
mcr p15, 0, r12, c1, c0, 0 /* SCTLR */
- mov pc, r2
+ mov pc, lr
ENDPROC(arm_cpu_lowlevel_init)
--
2.1.3
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/3] arm/cpu/lowlevel cleanups
2014-12-11 20:51 [PATCH v2 0/3] arm/cpu/lowlevel cleanups Uwe Kleine-König
` (2 preceding siblings ...)
2014-12-11 20:51 ` [PATCH v2 3/3] arm/cpu/lowlevel: Don't save the return address in another register Uwe Kleine-König
@ 2014-12-15 9:58 ` Sascha Hauer
3 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2014-12-15 9:58 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: barebox
On Thu, Dec 11, 2014 at 09:51:30PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> compared to (implicit) v1 this changes the order to make the i-cache fix the
> first patch to address Sascha's review remarks. I also removed the explicit
> dropping of branch predictor entries which Lucas pointed out to be unnecessary.
>
> Uwe Kleine-König (3):
> arm/cpu/lowlevel: invalidate i-cache before enabling
> arm/cpu/lowlevel: add and fix comments for CPSR and SCTLR accesses
> arm/cpu/lowlevel: Don't save the return address in another register
>
> arch/arm/cpu/lowlevel.S | 38 ++++++++++++++++++++++++++++----------
> 1 file changed, 28 insertions(+), 10 deletions(-)
Applied, thanks
Sascha
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread