Re: More questions on memory read/write methods

[Daniel Jacobowitz <drow at false dot org>]

Hi Vlad,

I don't know if any of these questions are still current - we've talked
about a lot of this stuff in the past two weeks.  But I'll try to
answer them for the list archives anyway.

On Thu, Jun 29, 2006 at 03:52:58PM +0400, Vladimir Prus wrote:
> Suppose we call target_write_memory, then the code will work like this:
> 
>     - if target_stack->to_xfer_partial == default_xfer_partial
> 
>         - target_xfer_memory is called
> 
>             (inside target_xfer_memory)
> 
>             - lookup_mem_region is called
> 
>             - memory request is clipped to the memory region size,
>               region flags like "read-only" are checked.
> 
>             - Successive calls to do_xfer_memory are done until all memory
>               is transferred.
> 
>                 (inside do_xfer_memory)
> 
>                 - an attempt is made to fulfill read request right from
>                   object file (for write operation -- noop)
>  
>                 - calls to target_ops->deprecated_xfer_memory are done, for
>                   each target in target_stack
> 
>     - if target_stack->to_xfer_partial != default_xfer_partial
> 
>         - xfer_using_stratum is called
> 
>             (inside xfer_using_stratum)
> 
>             -  Walks over target_stack, calling target_xfer_partial
> 
>                 (inside target_xfer_partial)
>                 
>                 - Direct call to target_ops->to_xfer_partial is done
> 
> 
> Now the questions:
> 
> 1. If target_stack->to_xfer_partial == default_xfer_partial, then we use 
> deprecated_xfer_memory to do the memory transfer, we never call 
> default_xfer_partial inside target_write_memory. Is that right?

I think so.

> 2. If, yes, then default_xfer_partial can only be called from either 
> target_read_partial, or target_write_partial, which are explicitly called by 
> user, or by target_write/target_read with TARGET_OBJECT_MEMORY as parameter.
> 
> Is that's true, then why default_xfer_partial for the case where object == 
> TARGET_OBJECT_MEMORY does not repeat the logic done by  target_xfer_memory?
> It does no clipping of memory to memory regions, no checking of RO flags of 
> memory, and does not iterate over target_stack. It just calls 
> target_ops->deprecated_xfer_memory.

I have no idea.  Really this should all be cleaned up.  If every target
implemented memory xfer using to_xfer_partial, we could delete half
this code.  Once that happened we could look at what we were deleting,
and make sure the non-deleted paths did the right things.

> 3. If target_stack->to_xfer_partial != default_xfer_partial, then a direct
> call to it is made. Why the above-mentioned clipping to memory region is not 
> performed in this case? Is the assumption that to_xfer_partial will do it? 
> Then why the same assumption is not applied to deprecated_xfer_memory, and 
> why this memory region clipping is done at all?

You're making a very common mistake when reading source code -
assuming that it was all designed at the same time.  The
to_xfer_partial case has absolutely no support for memory regions,
presumably because no one bothered to implement it.

> 4. The do_xfer_memory will try to read data from binary file, if 
> 'trust_readonly' variable is set. This does not happens when 
> target_stack->to_xfer_partial != default_xfer_partial. Why?

Ditto.

I think Michael Snyder posted a patch for this and there was some
discussion.  I don't know what became of it.

> 5. Would it be reasonable for xfer_using_stratum, when object == 
> TARGET_OBJECT_MEMORY, to check to 'to_has_memory' and 'to_has_all_memory' 
> fields?

Maybe?  This all relates to the "This is stupid" comment above
xfer_using_stratum.  Personally I'm not convinced that the comment is
right.

Probably it would be reasonable.

> 6. Would it be reasonable for do_xfer_memory, instead of iterating over target 
> stack itself, just call xfer_using_stratum?

Probably, but it would also be reasonable to delete do_xfer_memory. 
It's just some work.

-- 
Daniel Jacobowitz
CodeSourcery