RE: MPC8260 cache patch

[Gary Thomas <gary at mlbassoc dot com>]

On Mon, 2003-03-31 at 08:14, Patrick Doyle wrote:
> > I think we should leave the cache functions as it - in the variant -
> > since the behaviour (and even the instructions used) are very processor
> > specific.
> So, I think I am misunderstanding something here -- if the "dcbf"
> instruction is defined by the architecture to flush the data cache, why is
> it _not_ a good idea to put it in the architecture directory?  If particular
> variants are broken, or have more efficient implementations, they can always
> override the architecture defined implementation.
> 

My comment was that the dcbf instruction can't be used for generic
flushing - it simply isn't designed for that purpose.  All that dbcf 
does is say "if the cache holds data for address XYZ, flush it to memory
and invalidate the line".  Since we don't know what addresses are 
currently in the cache, we can't use this instruction to totally
flush out the cache.  This instruction works fine for insuring that
a given region of memory is consistent with the cache.

> > I don't think that the code using 'dcbf' works :-(  The way I read the
> > documentation, it only flushes lines which match the given address.
> > Since the code enumerates address [0..DCACHE_SIZE-1], this only insures
> > that memory is coherent with those addresses - not any others that might
> > be in the cache (whose address falls outside that range).
> >
> > I'm pretty sure that the only way to do this properly is like it is done
> > on the PPC60x and even then, I think we need the 2x fix in there.
> 
> My understanding of the use of "dcbf" instruction is that we would simple
> replace the "lwz" instruction in the loop with the "dcbf" instruction -- we
> would still have to traverse all of the lines of the cache, but we would not
> have to traverse twice the number of lines in the cache.  The downside is
> that, after executing "dcbf" on a line in the cache, that line is also
> marked as invalid.  But, if we just loaded up the cache with a region of
> memory that we are not accessing that much anyway (which is what the current
> implementation does), I don't see that there is a problem with that.
> 
> Of course, I am away from my PowerPC books and board today, so I can't
> verify this, but it seems like the right thing to do.
> 
> >
> > In other words, the versions that use 'dcbf' for the SYNC() function are
> > simply wrong.  We probably never noticed because the only 40x platform
> I don't understand why.
> 
> > that was ever exercised much was a 403 and the IIRC either it had no
> > caches or they didn't work (maybe this was why).
> >
> Boy am I really confused -- because the ppc40x variant was the only one
> where I saw "dcbf" used.  Oh well...

I think this is because of the history of the various ports :-)

Nonetheless, we need to understand the issues and fix it right,
now and forever more (I hope).

-- 
------------------------------------------------------------
Gary Thomas                 |
MLB Associates              |  Consulting for the
+1 (970) 229-1963           |    Embedded world
http://www.mlbassoc.com/    |
email: <gary at mlbassoc dot com>  |
gpg: http://www.chez-thomas.org/gary/gpg_key.asc
------------------------------------------------------------