[PATCH] MIPS: BMIPS: fix bmips_wr_vec()

[PATCH] MIPS: BMIPS: fix bmips_wr_vec()

[Petri Gynther]

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);
 	local_flush_icache_range(dst, dst + (end - start));
 	instruction_hazard();
 }
-- 
2.2.0.rc0.207.ga3a616c

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

[Florian Fainelli]

On 26/05/15 23:25, 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>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Fixes: df0ac8a406718 ("MIPS: BMIPS: Add SMP support code for
BMIPS43xx/BMIPS5000")
CC: stable@vger.kernel.org # 3.4+

> ---
>  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);
>  	local_flush_icache_range(dst, dst + (end - start));
>  	instruction_hazard();
>  }
> 


-- 
Florian

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

[Ralf Baechle]

On Wed, May 27, 2015 at 10:36:53AM -0700, Florian Fainelli wrote:

> On 26/05/15 23:25, 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>
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> Fixes: df0ac8a406718 ("MIPS: BMIPS: Add SMP support code for
> BMIPS43xx/BMIPS5000")
> CC: stable@vger.kernel.org # 3.4+

Actually 3.3+ I think.

  Ralf

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

[Kevin Cernekee]

On Tue, May 26, 2015 at 11:25 PM, Petri Gynther <pgynther@google.com> 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);
>         local_flush_icache_range(dst, dst + (end - start));
>         instruction_hazard();
>  }
> --
> 2.2.0.rc0.207.ga3a616c

Thanks Petri for catching this.

Reviewed-by: Kevin Cernekee <cernekee@gmail.com>

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

[Ralf Baechle]

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.

  Ralf

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

[Kevin Cernekee]

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).

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

[Petri Gynther]

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.

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

[Ralf Baechle]

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