There seems to be some confusion in the cache op code about what the
functions are supposed to do, and how they're supposed to do them.
(At the very least, the lack of adequate documentation for them is
causing _me_ confusion. 8-)


Things i've noticed:

* the difference between

	locore_mips3.S:1589:

	 * mips3_FlushDCache --   
	 *      
	 *      void mips3_FlushDCache(addr, len)
	 *              vaddr_t addr; vsize_t len;

	locore_mips3.S:1868:

	 * mips3_FlushDCache_2way --
	 *      
	 *      void mips3_FlushDCache_2way(paddr_t addr, len)

  (and the way all of those functions do conversion from the given
  address to the indices used for the cache ops).  I could believe that
  this is cut-and-paste confusion, but there are a lot of other
  issues.

  Also, the lack of consistency about whether or not the aligned line
  address, or whether any address in the line, will work
  (e.g. mips3_FlushICache vs. mips3_FlushDCache), again without any
  discussion in the comments as to why this is necessary.

* the scary bit of code in pmap.c (around line 2136):

        if (CPUISMIPS3 && last != 0) {
                MachFlushDCache(va, PAGE_SIZE); 
                if (mips_L2CachePresent)
                        /*
                         * mips3_MachFlushDCache() converts the address to a
                         * KSEG0 address, and won't properly flush the Level 2
                         * cache.  Do another flush using the physical adddress
                         * to make sure the proper secondary cache lines are 
                         * flushed.  Ugh!
                         */
                        MachFlushDCache(pa, PAGE_SIZE);
        }

  this shows that _some_ assumption is very, very bogus.  In
  particular, I'd guess that the root cause of the problem here is that
 the use of index ops to do range flushes is ... very, very broken.

* code sequences (like pmap.c:983) which look like:

                MachFlushDCache(va, len);
                MachFlushICache(MIPS_PHYS_TO_KSEG0(va &
                    (mips_L1ICacheSize - 1)), len);

  (when there's no substantive difference in the descriptions of those
  two functions' arguments).  This goes back to comments in the first point
  above.

* If i understand things correctly, reading See Mips Run, it looks
  like the way the mips1 cache ops are invoked may be broken (they seem
  to operate on virtual addresses, but See Mips Run says that R2k/R3k
  caches are physically addressed).

* Also, again going by documentation that I have at hand, whether the
  effective address passed to an 'index' cache op is evaluated by the
  TLB on its way to becoming the index used to actually address the
  cache, is actually CPU-dependent.  I.e. the apparent assumption in
  mips3_Flush[ID]Cache that the cache is virtually indexed is ... not
  correct.


My understanding of the current code is that it assumes that the caches are
virtually indexed and tagged, and that index-based cache ops don't have the
TLB applied to the EA before it's converted to an index.

It seems to me that the right thing is to _only_ use the index ops for
whole-cache operations (i.e., only FlushCache), and to ues the hit ops
for operations which are to affect only certain (virtual) addresses.
(And, to go along with this, if my understanding of the mips1 caches
is correct, their use needs to be cleaned up as well.)

Can anybody comment on this?


At minimum, somebody who understands this code needs to completely and
adequately document the existing behaviour required from the
functions, along the lines of functional specifications.  This would
allow people to check the existing functions for correctness, and to
implement new ones as appropriate...



chris