missing data cache flush in trap_init?

missing data cache flush in trap_init?

[Jun Sun]

Ralf,

Someone reported this bug to me.  I think it is a valid one.  Basically
trap_init() installs the vectors through kseg0 address and then flushes
icache.  It is possible that the vectors are still in the data cache and not
written back to memory yet.  If an exception happens it may get the corrupted
the vector value.

The following patch should fix it.  I am not sure if I can use
flush_cache_range() to have potentially better performance.

Jun


--- linux/arch/mips/kernel/traps.c.orig Tue Jan  2 16:24:16 2001
+++ linux/arch/mips/kernel/traps.c      Tue Jan  2 16:50:59 2001
@@ -767,7 +767,7 @@
        default:
                panic("Unknown CPU type");
        }
-       flush_icache_range(KSEG0, KSEG0 + 0x200);
+       flush_cache_all();
 
        atomic_inc(&init_mm.mm_count);  /* XXX  UP?  */
        current->active_mm = &init_mm;

Re: missing data cache flush in trap_init?

[Maciej W. Rozycki]

On Tue, 2 Jan 2001, Jun Sun wrote:

> The following patch should fix it.  I am not sure if I can use
> flush_cache_range() to have potentially better performance.

 Yes, flush_cache_range() is the right function and it should be used even
if no performance gain is achieved (it is in this case, though).  I can't
comment if that's needed at all, though.  R3k which I use has its data
cache write-through so it doesn't need such a change.  I might check IDT
R4k and possibly IDT R5k docs yet but I have no idea about other
implementations. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: missing data cache flush in trap_init?

[Ralf Baechle]

On Tue, Jan 02, 2001 at 04:52:22PM -0800, Jun Sun wrote:

> Someone reported this bug to me.  I think it is a valid one.  Basically
> trap_init() installs the vectors through kseg0 address and then flushes
> icache.  It is possible that the vectors are still in the data cache and not
> written back to memory yet.  If an exception happens it may get the corrupted
> the vector value.
> 
> The following patch should fix it.  I am not sure if I can use
> flush_cache_range() to have potentially better performance.

Flush_icache_range is correct;  the function is expected to do any dcache
writebacks etc. to make dcache / icache / memory coherent.

Is it possible that you're using a CPU with additional vectors that aren't
flushed by this flush_icache_call or?

  Ralf

Re: missing data cache flush in trap_init?

[Jun Sun]

Ralf Baechle wrote:
> 
> On Tue, Jan 02, 2001 at 04:52:22PM -0800, Jun Sun wrote:
> 
> > Someone reported this bug to me.  I think it is a valid one.  Basically
> > trap_init() installs the vectors through kseg0 address and then flushes
> > icache.  It is possible that the vectors are still in the data cache and not
> > written back to memory yet.  If an exception happens it may get the corrupted
> > the vector value.
> >
> > The following patch should fix it.  I am not sure if I can use
> > flush_cache_range() to have potentially better performance.
> 
> Flush_icache_range is correct;  the function is expected to do any dcache
> writebacks etc. to make dcache / icache / memory coherent.
> 
> Is it possible that you're using a CPU with additional vectors that aren't
> flushed by this flush_icache_call or?
> 
>   Ralf

You are right - flush_icache_range() practically flushes both caches in the
current implementation.  There might be some other problems.

Aside of that, the name of flush_icache_range() seems to be misleading.  Also
in general how does it know which part of dcache to flush() without a given
process mm struct?  If it does not know, the only choice is to flush the whole
dcache, which seems to make this function very close to flush_all().  

Is this function introduced by other CPU platforms?  How would it make a
difference there?  I am just curious ...

Jun

Re: missing data cache flush in trap_init?

[Ralf Baechle]

On Wed, Jan 03, 2001 at 07:26:23PM -0800, Jun Sun wrote:

> Aside of that, the name of flush_icache_range() seems to be misleading.  Also
> in general how does it know which part of dcache to flush() without a given
> process mm struct?

The function is only intended to flush kernel addresses for which no mm
exists.  Yes, it's being abused in creative ways but that's the purpose
it was designed for ...

>  If it does not know, the only choice is to flush the whole
> dcache, which seems to make this function very close to flush_all().  
> 
> Is this function introduced by other CPU platforms?  How would it make a
> difference there?  I am just curious ...

Others such as for example m68k need it as well.

  Ralf