Re: [PATCH] MIPS: bugfix of local_r4k_flush_icache_range - added L2 flush

[Ralf Baechle]

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 <Leonid.Yegoshin@xxxxxxxxxx>
> ---
>  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 <Leonid.Yegoshin@xxxxxxxxxx>
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 <Leonid.Yegoshin@xxxxxxxxxx>
    Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/