memory regions, dcache

[jtc@redback.com (J.T. Conklin)]

Setting aside symbol table performance for the moment, I'm back
finishing up memory region attributes.

In previous messages, I've shared my changes to target_xfer_memory().
Those have not changed significantly.  Since that time, I've added a
'mem_attrib *attrib' argument to the target vectors to_xfer_memory()
function, and have fixed up all the code for all the targets.  While I
was there, I noticed that some target's *_xfer_memory() function did
not declare the 'struct target_ops *target' argument, so I fixed that
up at the same time.  (Btw, Are there any *_xfer_memory() functions
that use the 'target' argument?  I didn't see any while I was adding
the new argument, but I wasn't looking for it either.  If nothing uses
it, I wouldn't mind removing it as part of this change.)

Now that the mem_attrib argument is available, target *_xfer_memory()
functions may be able to tailor their behavior accordingly.  I say
'may be able' because currently most of the functions simply ignore
the new arguement (either because the function has not been changed to
use the attribute, or perhaps because the host/target interface/
protocol cannot support the attribute(s) selected.  Is there anything
I should do about this now, or is the fact that some attributes are
not supported on all targets a given that just needs to be documented?

In most of the embedded remote targets, the target vector code calls
dcache_init() in the *_open() function to register functions used to
read and write memory, and the *_xfer_memory() functions calls
dcache_xfer_memory() which uses the functions registered earlier to
perform the I/O.  The problem is that there is no way to pass the
attribute argument through dcache to the functions.  

This should be fairly easy to address --- just a lot of grunt work
fixing up dcache_xfer_memory() and the read and write functions to
take the new argument.  However, I wonder whether it would be better
(cleaner, more "elegant", etc.) to move dcache up a layer and put it
between target_xfer_memory() and the target *_xfer_memory() functions.

For example, instead of calling a target vector's *to_xfer_memory(),
target_xfer_memory() would call dcache_xfer_memory() with a target
vector pointer.  If dcache_xfer_memory() had to do i/o, it would call
the target vector's *to_xfer_memory() function.

Having the dcache at a higher level might be useful for implementing
the verify (read and compare after writes) attribute.  As I imagine
things, verify support would be implemented in target_xfer_memory()
and would re-read and compare after a write.  But if the target code
uses dcache, the read would not come from the target but the cache.
On the other hand, target_xfer_memory() could do a cache invalidate
before reading (and a more refined cache invalidate could easily be
written).

All that being said, I think that going the simple route and adding
the parameter to dcache_xfer_memory() and to the target read/write
functions may be the right thing for now.  It won't be difficult to
re-evaluate this decision and go for the more integrated approch if
that turns out to be the right thing over time.

One more thing before this gets too long.  There is a remotecache
variable that globally enables the cache which is currently disabled.
With attributes, each memory region has a cache/nocache atttribute.
Should the remotecache variable override the attribute.  Or can we
depricate remotecache completely?  Since when no memory attributes
are defined the default attribute is used which disables the cache,
there isn't much need for a global flag any more.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From kevinb@cygnus.com Fri Jun 09 12:22:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: gdb@sourceware.cygnus.com
Subject: Comments on U_REGS_OFFSET and fetch_register...
Date: Fri, 09 Jun 2000 12:22:00 -0000
Message-id: <1000609192235.ZM24244@ocotillo.lan>
X-SW-Source: 2000-06/msg00067.html
Content-length: 1586

In infptrace.c, U_REGS_OFFSET is (conditionally) defined as follows:

/* U_REGS_OFFSET is the offset of the registers within the u area.  */
#if !defined (U_REGS_OFFSET)
#define U_REGS_OFFSET \
  ptrace (PT_READ_U, inferior_pid, \
	  (PTRACE_ARG3_TYPE) (offsetof (struct user, u_ar0)), 0) \
    - KERNEL_U_ADDR
#endif

And later on, in fetch_register(), we have the following code:

  /* Overload thread id onto process id */
  if ((tid = TIDGET (inferior_pid)) == 0)
    tid = inferior_pid;		/* no thread id, just use process id */

  offset = U_REGS_OFFSET;

  ...

      *(PTRACE_XFER_TYPE *) & buf[i] = ptrace (PT_READ_U, tid,
					       (PTRACE_ARG3_TYPE) regaddr, 0);


I have two problems with this code.

 1) U_REGS_OFFSET is still using the composite thread id / process id.
    It should only be using the actual pid extracted from
    inferior_pid.  

    In my opinion, U_REGS_OFFSET should be changed (everywhere) to
    take the pid as an argument.

 2) The tid being extracted is later passed to ptrace().  While this
    is fine for linux, where the tid is actually a pid which makes
    sense to pass to ptrace(), I really doubt it makes sense for other
    threads implementations where the thread id is something else
    entirely.

    I'm not sure what to do about this problem.  Perhaps its a moot
    point since native ports where this could be a problem likely
    define FETCH_INFERIOR_REGISTERS in order to do the right thing
    when it comes to the thread id.  Still it looks more than a little
    strange to be passing the thread id to ptrace().

Kevin