Re: memory regions, dcache

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

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
>> 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?

Andrew> If a user sets (pick a random attribute) the atomic attribute
Andrew> (see next paragraph) but the target doesn't support it
Andrew> shouldn't the user be told that the attribute was ignored?  I
Andrew> think it is important to not mis-lead the user with this.

I agree.  However, I'm having a hard time thinking of a implementation
that is easy to maintain.  While it wouldn't be difficult to add code
that checks attrib->width and prints a message for unsupported access
types, this would have to be done each time a new low-level attribute
was defined.  This will do if necessary, but I'd prefer something more
robust.  With so many targets, it seems likely that some are going to
be missed.

Andrew> (By atomic I mean read/write a word as a word and not as a
Andrew> sequence of bytes - assuming that is a possible attribute.  On
Andrew> some targets a word read (arm?) returns a different value to a
Andrew> byte read).

See above.  The need for this one feature is what's "funding" this
project.  In our case, we need to read/write memory mapped device
registers with specific memory access sizes.


>> 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.

Andrew> I think the elegant solution is preferable (but as you ask, is
Andrew> it feasible?).

I think it is too.  But to avoid biting off too much at once, I've
decided to get there incrementally.  The low level memory read/write
functions need to be changed to handle attributes anyway, and I need
to some time to think about the ramifications of moving up dcache.
For example, there are now separate caches for each target vector, in
the new scheme there would only be one.

>> 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.

Andrew> You mean delete the command :-)

Andrew> FYI, things have changed a little - its now the static variable
Andrew> dcache.c:dcache_enabled_p and wince.c sets it using
Andrew> dcache.c:set_dcache_state(1).

This is surprising, but perhaps this is due to my utter lack of
knowledge about Win CE's architecture.  In my experience, blanket
memory caching makes it extremely difficult to watch what is going on
whenever there are any asynchronous elements (tasks, threads, drivers,
etc.).

Andrew> I think the command (but not the global variable) would need
Andrew> to be around for at least one more release.  Can it be
Andrew> converted into a recursive call to the CLI with a command that
Andrew> enables/disables all memory regions? 

While there is nothing that would prevent a command which turns off
the cache attribute in all defined memory regions, it may violate the
"principle of least astonishment".  I'd certainly be surprised if the
command did that, especially since at this time there is no way to
modify an already defined attribute.

Andrew> However, is there any evidence that people actually use it?

The cache in general, or the 'set remotecache' command?

        --jtc

-- 
J.T. Conklin
RedBack Networks