* [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush
@ 2019-10-01 9:09 Ahmad Fatoum
2019-10-01 9:09 ` [PATCH 2/2] ARM: cache_64: invalidate dcache in arm_early_mmu_cache_invalidate Ahmad Fatoum
2019-10-02 7:38 ` [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush Rouven Czerwinski
0 siblings, 2 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2019-10-01 9:09 UTC (permalink / raw)
To: barebox; +Cc: andrew.smirnov, lst, Ahmad Fatoum
So far arm_early_mmu_cache_flush has only been used in preparation for
executing newly-written code. For this reason, on ARMv7 and below,
it had always invalidate the icache after the dcache flush.
We don't do this on ARM64, but sync_caches_for_execution depends on this,
which had this comment that didn't hold true for ARM64:
> Despite the name arm_early_mmu_cache_flush not only flushes the
> data cache, but also invalidates the instruction cache.
It might be worthwhile to decouple dcache flushing from icache
invalidation, but for now, align what we do on ARM64 with what we do for
32-bit ARMs.
This fixes a potential read of stale instructions when loading
second-stage barebox from the PBL with MMU disabled.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/cpu/cache_64.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/cache_64.c b/arch/arm/cpu/cache_64.c
index 45f01e8dc9cf..847323525424 100644
--- a/arch/arm/cpu/cache_64.c
+++ b/arch/arm/cpu/cache_64.c
@@ -27,6 +27,7 @@ int arm_set_cache_functions(void)
void arm_early_mmu_cache_flush(void)
{
v8_flush_dcache_all();
+ v8_invalidate_icache_all();
}
void arm_early_mmu_cache_invalidate(void)
--
2.23.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 2/2] ARM: cache_64: invalidate dcache in arm_early_mmu_cache_invalidate
2019-10-01 9:09 [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush Ahmad Fatoum
@ 2019-10-01 9:09 ` Ahmad Fatoum
2019-10-01 9:15 ` Ahmad Fatoum
2019-10-02 7:38 ` [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush Rouven Czerwinski
1 sibling, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2019-10-01 9:09 UTC (permalink / raw)
To: barebox; +Cc: andrew.smirnov, lst, Ahmad Fatoum
On some ARM cores, cache contents are indeterminate after a Power-On
Reset. Turning on the MMU on such cores risks interpreting random cache
lines as valid, causing hard-to-debug errors.
For this reason, we always invalidate the dcache on <= ARMv7. Let's do
likewise for ARM64. Newer ARM cores tend to come up with their dcaches
invalidated already, but for some, like the Cortex-A72, L2 caches are
invalidated dependent on a signal sampled at reset, so better play
it safe.
The icache invalidate here seems to serve no useful purpose. It's kept
for now for symmetry with ARM32.
Note that this is wrong should barebox be entered with the MMU enabled,
but this is so far not the case with any ARM64 platform we support.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
arch/arm/cpu/cache_64.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/cpu/cache_64.c b/arch/arm/cpu/cache_64.c
index 847323525424..6e18d981a434 100644
--- a/arch/arm/cpu/cache_64.c
+++ b/arch/arm/cpu/cache_64.c
@@ -32,5 +32,6 @@ void arm_early_mmu_cache_flush(void)
void arm_early_mmu_cache_invalidate(void)
{
+ v8_invalidate_dcache_all();
v8_invalidate_icache_all();
}
--
2.23.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush
2019-10-01 9:09 [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush Ahmad Fatoum
2019-10-01 9:09 ` [PATCH 2/2] ARM: cache_64: invalidate dcache in arm_early_mmu_cache_invalidate Ahmad Fatoum
@ 2019-10-02 7:38 ` Rouven Czerwinski
2019-10-02 7:43 ` Ahmad Fatoum
1 sibling, 1 reply; 9+ messages in thread
From: Rouven Czerwinski @ 2019-10-02 7:38 UTC (permalink / raw)
To: Ahmad Fatoum, barebox; +Cc: andrew.smirnov, lst
Hello Ahmad,
with these two patches applied, the NXP i.MX8MQ no longer starts into
barebox.
- rcz
On Tue, 2019-10-01 at 11:09 +0200, Ahmad Fatoum wrote:
> So far arm_early_mmu_cache_flush has only been used in preparation
> for
> executing newly-written code. For this reason, on ARMv7 and below,
> it had always invalidate the icache after the dcache flush.
> We don't do this on ARM64, but sync_caches_for_execution depends on
> this,
> which had this comment that didn't hold true for ARM64:
>
> > Despite the name arm_early_mmu_cache_flush not only flushes the
> > data cache, but also invalidates the instruction cache.
>
> It might be worthwhile to decouple dcache flushing from icache
> invalidation, but for now, align what we do on ARM64 with what we do
> for
> 32-bit ARMs.
>
> This fixes a potential read of stale instructions when loading
> second-stage barebox from the PBL with MMU disabled.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> arch/arm/cpu/cache_64.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/cpu/cache_64.c b/arch/arm/cpu/cache_64.c
> index 45f01e8dc9cf..847323525424 100644
> --- a/arch/arm/cpu/cache_64.c
> +++ b/arch/arm/cpu/cache_64.c
> @@ -27,6 +27,7 @@ int arm_set_cache_functions(void)
> void arm_early_mmu_cache_flush(void)
> {
> v8_flush_dcache_all();
> + v8_invalidate_icache_all();
> }
>
> void arm_early_mmu_cache_invalidate(void)
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush
2019-10-02 7:38 ` [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush Rouven Czerwinski
@ 2019-10-02 7:43 ` Ahmad Fatoum
2019-10-02 7:46 ` Rouven Czerwinski
0 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2019-10-02 7:43 UTC (permalink / raw)
To: Rouven Czerwinski, barebox; +Cc: andrew.smirnov, lst
On 10/2/19 9:38 AM, Rouven Czerwinski wrote:
> Hello Ahmad,
>
> with these two patches applied, the NXP i.MX8MQ no longer starts into
> barebox.
Can you try each patch separately to see which one is the culprit?
Thanks for testing,
Ahmad
>
> - rcz
>
> On Tue, 2019-10-01 at 11:09 +0200, Ahmad Fatoum wrote:
>> So far arm_early_mmu_cache_flush has only been used in preparation
>> for
>> executing newly-written code. For this reason, on ARMv7 and below,
>> it had always invalidate the icache after the dcache flush.
>> We don't do this on ARM64, but sync_caches_for_execution depends on
>> this,
>> which had this comment that didn't hold true for ARM64:
>>
>>> Despite the name arm_early_mmu_cache_flush not only flushes the
>>> data cache, but also invalidates the instruction cache.
>>
>> It might be worthwhile to decouple dcache flushing from icache
>> invalidation, but for now, align what we do on ARM64 with what we do
>> for
>> 32-bit ARMs.
>>
>> This fixes a potential read of stale instructions when loading
>> second-stage barebox from the PBL with MMU disabled.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> arch/arm/cpu/cache_64.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/cpu/cache_64.c b/arch/arm/cpu/cache_64.c
>> index 45f01e8dc9cf..847323525424 100644
>> --- a/arch/arm/cpu/cache_64.c
>> +++ b/arch/arm/cpu/cache_64.c
>> @@ -27,6 +27,7 @@ int arm_set_cache_functions(void)
>> void arm_early_mmu_cache_flush(void)
>> {
>> v8_flush_dcache_all();
>> + v8_invalidate_icache_all();
>> }
>>
>> void arm_early_mmu_cache_invalidate(void)
>
>
--
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] 9+ messages in thread* Re: [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush
2019-10-02 7:43 ` Ahmad Fatoum
@ 2019-10-02 7:46 ` Rouven Czerwinski
2019-10-02 7:57 ` [PATCH] ARM: aarch64: save clobbered registers in __barebox_arm_entry Ahmad Fatoum
0 siblings, 1 reply; 9+ messages in thread
From: Rouven Czerwinski @ 2019-10-02 7:46 UTC (permalink / raw)
To: Ahmad Fatoum, barebox; +Cc: andrew.smirnov, lst
On Wed, 2019-10-02 at 09:43 +0200, Ahmad Fatoum wrote:
> On 10/2/19 9:38 AM, Rouven Czerwinski wrote:
> > Hello Ahmad,
> >
> > with these two patches applied, the NXP i.MX8MQ no longer starts
> > into
> > barebox.
>
> Can you try each patch separately to see which one is the culprit?
Patch 2/2 is the culprit.
-rcz
> Thanks for testing,
> Ahmad
>
> > - rcz
> >
> > On Tue, 2019-10-01 at 11:09 +0200, Ahmad Fatoum wrote:
> > > So far arm_early_mmu_cache_flush has only been used in
> > > preparation
> > > for
> > > executing newly-written code. For this reason, on ARMv7 and
> > > below,
> > > it had always invalidate the icache after the dcache flush.
> > > We don't do this on ARM64, but sync_caches_for_execution depends
> > > on
> > > this,
> > > which had this comment that didn't hold true for ARM64:
> > >
> > > > Despite the name arm_early_mmu_cache_flush not only flushes the
> > > > data cache, but also invalidates the instruction cache.
> > >
> > > It might be worthwhile to decouple dcache flushing from icache
> > > invalidation, but for now, align what we do on ARM64 with what we
> > > do
> > > for
> > > 32-bit ARMs.
> > >
> > > This fixes a potential read of stale instructions when loading
> > > second-stage barebox from the PBL with MMU disabled.
> > >
> > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > ---
> > > arch/arm/cpu/cache_64.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/arm/cpu/cache_64.c b/arch/arm/cpu/cache_64.c
> > > index 45f01e8dc9cf..847323525424 100644
> > > --- a/arch/arm/cpu/cache_64.c
> > > +++ b/arch/arm/cpu/cache_64.c
> > > @@ -27,6 +27,7 @@ int arm_set_cache_functions(void)
> > > void arm_early_mmu_cache_flush(void)
> > > {
> > > v8_flush_dcache_all();
> > > + v8_invalidate_icache_all();
> > > }
> > >
> > > void arm_early_mmu_cache_invalidate(void)
>
>
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] ARM: aarch64: save clobbered registers in __barebox_arm_entry
2019-10-02 7:46 ` Rouven Czerwinski
@ 2019-10-02 7:57 ` Ahmad Fatoum
2019-10-02 9:30 ` Rouven Czerwinski
0 siblings, 1 reply; 9+ messages in thread
From: Ahmad Fatoum @ 2019-10-02 7:57 UTC (permalink / raw)
To: rcz; +Cc: barebox, Ahmad Fatoum
arm_early_mmu_cache_invalidate now clobbers x0, x1, x2, which might be
passed by a previous stage bootloader. Have the caller save them.
---
Rouven, does this work for you?
---
arch/arm/cpu/entry_ll_64.S | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/arm/cpu/entry_ll_64.S b/arch/arm/cpu/entry_ll_64.S
index 37e0cb66b549..41d6cfb6a851 100644
--- a/arch/arm/cpu/entry_ll_64.S
+++ b/arch/arm/cpu/entry_ll_64.S
@@ -10,14 +10,16 @@
.section .text.__barebox_arm_entry
ENTRY(__barebox_arm_entry)
mov sp, x3
- /*
- * arm_early_mmu_cache_invalidate is jsut a call to
- * v8_invalidate_icache_all() which doesn't clobber x0, x1 or x2
- */
+ mov x4, x0
+ mov x5, x1
+ mov x6, x2
bl arm_early_mmu_cache_invalidate
+ mov x0, x4
+ mov x1, x5
+ mov x2, x6
#if IS_ENABLED(CONFIG_PBL_IMAGE)
b barebox_pbl_start
#else
b barebox_non_pbl_start
#endif
-ENDPROC(__barebox_arm_entry)
\ No newline at end of file
+ENDPROC(__barebox_arm_entry)
--
2.23.0
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: aarch64: save clobbered registers in __barebox_arm_entry
2019-10-02 7:57 ` [PATCH] ARM: aarch64: save clobbered registers in __barebox_arm_entry Ahmad Fatoum
@ 2019-10-02 9:30 ` Rouven Czerwinski
2019-10-02 10:45 ` Ahmad Fatoum
0 siblings, 1 reply; 9+ messages in thread
From: Rouven Czerwinski @ 2019-10-02 9:30 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox
On Wed, 2019-10-02 at 09:57 +0200, Ahmad Fatoum wrote:
> arm_early_mmu_cache_invalidate now clobbers x0, x1, x2, which might
> be
> passed by a previous stage bootloader. Have the caller save them.
> ---
> Rouven, does this work for you?
No, this does not fix the issue. I'll take some time to look into this
at a later point.
- rcz
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ARM: aarch64: save clobbered registers in __barebox_arm_entry
2019-10-02 9:30 ` Rouven Czerwinski
@ 2019-10-02 10:45 ` Ahmad Fatoum
0 siblings, 0 replies; 9+ messages in thread
From: Ahmad Fatoum @ 2019-10-02 10:45 UTC (permalink / raw)
To: Rouven Czerwinski; +Cc: barebox
On 10/2/19 11:30 AM, Rouven Czerwinski wrote:
> On Wed, 2019-10-02 at 09:57 +0200, Ahmad Fatoum wrote:
>> arm_early_mmu_cache_invalidate now clobbers x0, x1, x2, which might
>> be
>> passed by a previous stage bootloader. Have the caller save them.
>> ---
>> Rouven, does this work for you?
>
> No, this does not fix the issue. I'll take some time to look into this
> at a later point.
Hmm, could you tell me a bit about how the i.MX8 boots?
At what stage does it break? barebox PBL? barebox proper?
Is there something running prior to the barebox PBL?
Is the MMU off when barebox is run?
>
> - rcz
>
>
--
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] 9+ messages in thread
end of thread, other threads:[~2019-10-02 10:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-01 9:09 [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush Ahmad Fatoum
2019-10-01 9:09 ` [PATCH 2/2] ARM: cache_64: invalidate dcache in arm_early_mmu_cache_invalidate Ahmad Fatoum
2019-10-01 9:15 ` Ahmad Fatoum
2019-10-02 7:38 ` [PATCH 1/2] ARM: cache_64: invalidate icache in arm_early_mmu_cache_flush Rouven Czerwinski
2019-10-02 7:43 ` Ahmad Fatoum
2019-10-02 7:46 ` Rouven Czerwinski
2019-10-02 7:57 ` [PATCH] ARM: aarch64: save clobbered registers in __barebox_arm_entry Ahmad Fatoum
2019-10-02 9:30 ` Rouven Czerwinski
2019-10-02 10:45 ` Ahmad Fatoum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox