On Thu, May 28, 2015 at 01:37:24PM -0700, Leonid Yegoshin wrote:
> This function is used to flush code used in NMI and EJTAG debug exceptions.
> However, during that exceptions the Status.ERL bit is set, which means
> that code runs as UNCACHABLE. So, flush code down to memory is needed.
>
> Signed-off-by: Leonid Yegoshin <[email protected]>
> ---
> arch/mips/mm/c-r4k.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
> index 0dbb65a51ce5..9f0299bb9a2a 100644
> --- a/arch/mips/mm/c-r4k.c
> +++ b/arch/mips/mm/c-r4k.c
> @@ -666,17 +666,9 @@ static inline void local_r4k_flush_icache_range(unsigned long start, unsigned lo
> break;
> }
> }
> -#ifdef CONFIG_EVA
> - /*
> - * Due to all possible segment mappings, there might cache aliases
> - * caused by the bootloader being in non-EVA mode, and the CPU switching
> - * to EVA during early kernel init. It's best to flush the scache
> - * to avoid having secondary cores fetching stale data and lead to
> - * kernel crashes.
> - */
> +
> bc_wback_inv(start, (end - start));
> __sync();
> -#endif
> }
I was wondering why there was a cache flush at all so I dove into git
history and found:
commit 4676f9359fa5190ee6f42bbf2c27d28beb14d26a
Author: Leonid Yegoshin <[email protected]>
Date: Tue Jan 21 09:48:48 2014 +0000
MIPS: mm: c-r4k: Flush scache to avoid cache aliases
There is a chance for the secondary cache to have memory
aliases. This can happen if the bootloader is in a non-EVA mode
(or even in EVA mode but with different mapping from the kernel)
and the kernel switching to EVA afterwards. It's best to flush
the icache to avoid having the secondary CPUs fetching stale
data from it.
Signed-off-by: Leonid Yegoshin <[email protected]>
Signed-off-by: Markos Chandras <[email protected]>
flush_icache_range() really only is meant to deal with I-cache coherency
issues as they appear during normal kernel operation, that is code is
modified and will be executed from RAM. I doesn't know about aliases
and it's not meant to know.
As I understand you only need this on startup. Making this function work
for your special use results in a performance penalty for every other user
of this function.
How about reverting this commit and calling __flush_cache_all() to
make sure your kernel code gets flushed out to the other end of the
universe - or memory, what ever comes first?
Ralf
On 06/10/2015 03:28 AM, Ralf Baechle wrote:
> On Thu, May 28, 2015 at 01:37:24PM -0700, Leonid Yegoshin wrote:
>
>> ...
> I was wondering why there was a cache flush at all so I dove into git
> history and found:
>
> commit 4676f9359fa5190ee6f42bbf2c27d28beb14d26a
> Author: Leonid Yegoshin <[email protected]>
> Date: Tue Jan 21 09:48:48 2014 +0000
>
> MIPS: mm: c-r4k: Flush scache to avoid cache aliases
>
> There is a chance for the secondary cache to have memory
> aliases. This can happen if the bootloader is in a non-EVA mode
> (or even in EVA mode but with different mapping from the kernel)
> and the kernel switching to EVA afterwards. It's best to flush
> the icache to avoid having the secondary CPUs fetching stale
> data from it.
>
> Signed-off-by: Leonid Yegoshin <[email protected]>
> Signed-off-by: Markos Chandras <[email protected]>
>
> flush_icache_range() really only is meant to deal with I-cache coherency
> issues as they appear during normal kernel operation, that is code is
> modified and will be executed from RAM. I doesn't know about aliases
> and it's not meant to know.
1) OK, here is an explanation of memory aliasing (don't mix with cache
aliasing).
Memory blocks bigger than 512MB should be put somewhere above 512MB
physical address due to IO hole but we still need some memory at
physical address 0x0000000. Board designers often prefer using masking
of some address bits to MIRROR 256-512MB into 0x00000000 from a primary
memory space location (for Malta - physical address 0x80000000). It
decreases latency by some bus clocks (which are not CPU clocks but slowly).
So, in Malta the word at physical address 0x00000000-256MB is the same
as word at physical address 0x80000000-0x90000000 (other boards may have
another location). But cache subsystem knows NOTHING about it and caches
that memory twice. This is a memory aliasing and it may cause a lot of
trouble if system uses both aliasing address ranges. But it is pretty
often that system initially uses low block 0x00000000->512MB during core
startup and later switches to primary address space. So, some full L1D
and L2 cache flush is needed after cache is initialized. However, after
creation of exception handler code it is still needed additionally a
flush of that code too, in L1D/I and including L2 in EVA, to avoid
memory aliasing problem.
2) This patch actually notices that EJTAG and NMI exception handlers
work with Status.ERL bit and pull exception handler code from uncachable
memory, so a flushing exception handlers down to memory (including L2)
is needed for that because of different reason. Actually, it is a result
of debugging NMI with U-Boot. It is just by chance that it uses the same
code and solution as EVA flushing.
>
> As I understand you only need this on startup. Making this function work
> for your special use results in a performance penalty for every other user
> of this function.
Function local_r4k_flush_icache_range() can be used only at startup,
before SMP is inialized. Original code has comment above this function:
/* This function is used only for local CPU only while boot etc */
I don't know why it disappeared.
>
> How about reverting this commit and calling __flush_cache_all() to
> make sure your kernel code gets flushed out to the other end of the
> universe - or memory, what ever comes first?
That function may call SMP IPI in some vendor CPUs and is not suitable.
But I agree that name local_r4k_flush_icache_range() is a not good now
and it may be changed to something more appropriate. I am just not good
in name changing game.
- Leonid.
>
> Ralf