Re: [PATCH] MIPS: BMIPS: fix bmips_wr_vec()

[Ralf Baechle <ralf@linux-mips.org>]

On Thu, May 28, 2015 at 11:25:45AM -0700, Petri Gynther wrote:

> On Thu, May 28, 2015 at 9:47 AM, Kevin Cernekee <cernekee@gmail.com> wrote:
> > On Thu, May 28, 2015 at 9:40 AM, Ralf Baechle <ralf@linux-mips.org> wrote:
> >> On Tue, May 26, 2015 at 11:25:08PM -0700, Petri Gynther wrote:
> >>
> >>> bmips_wr_vec() copies exception vector code from start to dst.
> >>>
> >>> The call to dma_cache_wback() needs to flush (end-start) bytes,
> >>> starting at dst, from write-back cache to memory.
> >>>
> >>> Signed-off-by: Petri Gynther <pgynther@google.com>
> >>> ---
> >>>  arch/mips/kernel/smp-bmips.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/mips/kernel/smp-bmips.c b/arch/mips/kernel/smp-bmips.c
> >>> index fd528d7..336708a 100644
> >>> --- a/arch/mips/kernel/smp-bmips.c
> >>> +++ b/arch/mips/kernel/smp-bmips.c
> >>> @@ -444,7 +444,7 @@ struct plat_smp_ops bmips5000_smp_ops = {
> >>>  static void bmips_wr_vec(unsigned long dst, char *start, char *end)
> >>>  {
> >>>       memcpy((void *)dst, start, end - start);
> >>> -     dma_cache_wback((unsigned long)start, end - start);
> >>> +     dma_cache_wback(dst, end - start);
> >>
> >> dma_cache_wback is a guess what - DMA function.  It doesn't handle
> >> I-caches at all and on some platforms might actually do nothing at all.
> >> or use other optimizations that only work for DMA buffers and it's not
> >> SMP aware - nor will it.  So if it ever worked for your case then just
> >> because you're lucky.  This really should use flush_icache_range which
> >> also conveniently for your code takes an end pointer as argument.
> >
> > This flush isn't intended to handle I$.  It is intended to flush the
> > newly written code all the way out to DRAM (not just to L2) so that it
> > can be executed through an uncached kseg1 alias.  On initial boot, a
> > BMIPS secondary CPU comes up with its I$ disabled (5000) or in an
> > uninitialized state (43xx).
> 
> Just wondering if we should just use:
> r4k_blast_dcache()
> r4k_blast_scache()
> 
> here instead? r4k_blast_dcache() is currently exported, but
> r4k_blast_scache() is not.

There's simply no user of r4k_blast_scache() outside of c-r4k.c so far
but I don't mind exporting the function.  But Kevin has already
convinced me that this is a special use for which none of the existing
functions fits well and it certainly isn't worth to invent a new flush
function for this use, so I've applied your patch.

  Ralf