dcache.c write behaviour

dcache.c write behaviour

[Doug Evans]

Hi.
dcache.c's support for writes seems busted and methinks the right fix is to
throw out all handling of dirty lines until someone actually implements this:

  /* FIXME: There may be some benefit from moving the cache writeback
     to a higher layer, as it could occur after a sequence of smaller
     writes have been completed (as when a stack frame is constructed
     for an inferior function call).  Note that only moving it up one
     level to target_xfer_memory() (also target_xfer_memory_partial())
     is not sufficent, since we want to coalesce memory transfers that
     are "logically" connected but not actually a single call to one
     of the memory transfer functions. */

Consider:

(gdb) set *(char*) 0 = 0
Cannot access memory at address 0x0
(gdb) p *(char*) 0
Cannot access memory at address 0x0
(gdb) mem 0 0xffffffff rw 8 cache
(gdb) set *(char*) 0 = 0
(gdb)

Where's the error from the second write?
It's gone because the return code of dcache_writeback is ignore here:

  if (should_write)
    dcache_writeback (dcache);

Even if the return code wasn't ignored one couldn't return a proper
value which is the number of bytes actually written because the information
isn't available at this point.

Furthermore, the dcache now contains a valid value for *(char*) 0:

(gdb) p *(char*) 0
$1 = 0 '\0'
(gdb) 

There is this comment at the top of the file:

   The ENTRY_DIRTY state is necessary because GDB likes to write large
   lumps of memory in small bits.  If the caching mechanism didn't
   maintain the DIRTY information, then something like a two byte
   write would mean that the entire cache line would have to be read,
   the two bytes modified and then written out again.  The alternative
   would be to not read in the cache line in the first place, and just
   write the two bytes directly into target memory.  The trouble with
   that is that it really nails performance, because of the remote
   protocol overhead.  This way, all those little writes are bundled
   up into an entire cache line write in one go, without having to
   read the cache line in the first place.

I understand the attempt at speeding up writes, but given that the
cache is currently flushed after every write, there currently is no
win for writes.

I think caching of reads is far more valuable than caching of writes
(at least until caching of writes works),
so I propose fixing the above missing error by having dcache_xfer_memory
first write to the target and then record in the cache only the bytes that
were successfully written.  Doing this means there's no such thing
as a dirty line.  We _could_ keep the code that tracks dirty lines,
but it'd be easy to bring back said code if/when the above FIXME is implemented
and until then the code would be easier to understand if we removed it.

Comments?