Re: Memory attributes triumphs over dcache

[jtc at redback dot com (J.T. Conklin)]

>>>>> "Frank" == Frank Ch Eigler <fche@redhat.com> writes:
Frank> : [...]
Frank> : I think you may be misunderstanding the code.  [...]

Frank> Yeah, possibly.  I don't actually see any target looking at the
Frank> MEM_WIDTH_* fields!  

Correct for the time being.  I want to fix this as soon as I can with
the remote protocol.  I want to get it right, so not to introduce any
more cruft.  It should be easy to support the width attribute for ROM
monitors that use the infrastructure in monitor.c.  It should also be
easy to add it to the SDS back end, since SDS' SingleStep debugger has
a similar feature.  I don't have a description of the protocol, or I'd
do it myself.  Perhaps no one does, remote-sds.c looks like it was
reverse engineered.

Frank> (BTW, mem_command fails to initialize the new mem_attrib struct
Frank> rigorously.  It should assign defaults to all the optional
Frank> flags explicitly.)

Eh?  Line 142 of memattr.c is "attrib = default_mem_attrib".

Frank> What got me worried is the effect of defining a memory region (with
Frank> caching but no other flags), then doing a "target remote" download.
Frank> It proceeded one byte at a time - yuck!  If some combination of the
Frank> dcache and memattr system is at fault, this still needs to be
Frank> improved.

Since enabling caching should not effect accesses that don't cross
(dcache) line boundaries, this sounds like a bug.  

Perhaps dcache_write_line is breaking things up into byte sized chunks
when it shouldn't be.  I don't use download, so this might be a little
difficult to reproduce.  I appreciate any assistance you might be able
to offer.

Frank> : [...]
Frank> : Note that dcache itself chunks things into cache line sized accesses.

Frank> (... which is too small when dealing with large cacheable regions,
Frank> talking across a latency-bound protocol ...)

Agreed.  

I've been given patches so that the cache line size and the number of
cache lines can be specified by the user.  It's a trade off.  A large
cache line might be good for downloads, not so good for interactive
use.

Frank> : Frank> Also, there is no automation in defining memory attributes.  [...]
Frank> : 
Frank> : I can see .text and .rodata section being marked read-only and
Frank> : cacheable safely by default, 

Frank> If .text is read-only, the breakpoint instruction insertion
Frank> can't work.  (You're talking "read-write"/"read-only" from the
Frank> point of view of the debugger, not of the inferior program!)

In general, you are correct.  I forgot about that because on the
targets I use breakpoints are inserted and removed by higher level
commands rather than memory reads and writes.

Frank> : but not .data, since I/O devices might
Frank> : DMA into the data region while the target is stopped.
Frank>
Frank> I suppose so, if this is really that frequent or likely an occurrence.
Frank> If it is infrequent, then making the default the opposite makes sense.

Perhaps it is infrequent.  But i've done it from time to time.  More
often the memory is in the heap, not in .data.

What's more likely are cases when the user is attached to one thread,
while other threads are still executing.  My users do this all the
time.

In general, my philosophy is that GDB should do what's always safe by
default, and provide knobs so users can tweak the default behavior when
they know they can get away with it.

-- 
J.T. Conklin
RedBack Networks