Re: [PATCH] Delegate to target_ops->beneath to read cache lines

Pedro Alves
2013-12-02T10:36:47Z

On 12/02/2013 06:05 AM, Yao Qi wrote:
> On 11/30/2013 03:42 AM, Doug Evans wrote:
>> I think a comment is required here explaining why things are the way they are.
>> i.e., why we use current_target.beneath instead of &current_target.
>
> I find the comments in target_read_memory can do some explanations,
> so I copy that paragraph here. Is it clear to you?
>

I'd much prefer following my suggestion:

https://sourceware.org/ml/gdb-patches/2013-11/msg00941.html

Note we already have a target_write_raw_memory function.

Do you see a problem with this?

---------
Add new target_read_raw_memory function, and consolidate comments.

Tested on x86_64 Fedora 17.

gdb/
2013-12-02 Pedro Alves <palves@redhat.com>

* dcache.c (dcache_read_line): Use target_read_raw_memory.
* target.c (target_read_raw_memory): New function.
(target_read_stack, target_write_memory, target_write_raw_memory):
Update comment.
(target_read_core): Add comment.
* target.h (target_read_raw_memory): Declare.
---

gdb/dcache.c | 11 +++++------
gdb/target.c | 34 ++++++++++++++++++++++++----------
gdb/target.h | 3 +++
3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/gdb/dcache.c b/gdb/dcache.c
index 12d1a4b..804d567 100644
--- a/gdb/dcache.c
+++ b/gdb/dcache.c
@@ -337,14 +337,13 @@ dcache_read_line (DCACHE *dcache, struct dcache_block *db)
continue;
}

- res = target_read (current_target.beneath, TARGET_OBJECT_RAW_MEMORY,
- NULL, myaddr, memaddr, reg_len);
- if (res < reg_len)
+ res = target_read_raw_memory (memaddr, myaddr, reg_len);
+ if (res != 0)
return 0;

- memaddr += res;
- myaddr += res;
- len -= res;
+ memaddr += reg_len;
+ myaddr += reg_len;
+ len -= reg_len;
}

return 1;
diff --git a/gdb/target.c b/gdb/target.c
index 6c72e70..85b5037 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1791,16 +1791,30 @@ target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
return TARGET_XFER_E_IO;
}

+/* Like target_read_memory, but specify explicitly that this is a read
+ from the target's raw memory. That is, this read bypasses the
+ dcache, breakpoint shadowing, etc. */
+
+int
+target_read_raw_memory (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
+{
+ /* See comment in target_read_memory about why the request starts at
+ current_target.beneath. */
+ if (target_read (current_target.beneath, TARGET_OBJECT_RAW_MEMORY, NULL,
+ myaddr, memaddr, len) == len)
+ return 0;
+ else
+ return TARGET_XFER_E_IO;
+}
+
/* Like target_read_memory, but specify explicitly that this is a read from
the target's stack. This may trigger different cache behavior. */

int
target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
{
- /* Dispatch to the topmost target, not the flattened current_target.
- Memory accesses check target->to_has_(all_)memory, and the
- flattened target doesn't inherit those. */
-
+ /* See comment in target_read_memory about why the request starts at
+ current_target.beneath. */
if (target_read (current_target.beneath, TARGET_OBJECT_STACK_MEMORY, NULL,
myaddr, memaddr, len) == len)
return 0;
@@ -1814,6 +1828,8 @@ target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
int
target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
{
+ /* See comment in target_read_memory about why the request starts at
+ current_target.beneath. */
if (target_read (current_target.beneath, TARGET_OBJECT_CODE_MEMORY, NULL,
myaddr, memaddr, len) == len)
return 0;
@@ -1830,9 +1846,8 @@ target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
int
target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
{
- /* Dispatch to the topmost target, not the flattened current_target.
- Memory accesses check target->to_has_(all_)memory, and the
- flattened target doesn't inherit those. */
+ /* See comment in target_read_memory about why the request starts at
+ current_target.beneath. */
if (target_write (current_target.beneath, TARGET_OBJECT_MEMORY, NULL,
myaddr, memaddr, len) == len)
return 0;
@@ -1849,9 +1864,8 @@ target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
int
target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)
{
- /* Dispatch to the topmost target, not the flattened current_target.
- Memory accesses check target->to_has_(all_)memory, and the
- flattened target doesn't inherit those. */
+ /* See comment in target_read_memory about why the request starts at
+ current_target.beneath. */
if (target_write (current_target.beneath, TARGET_OBJECT_RAW_MEMORY, NULL,
myaddr, memaddr, len) == len)
return 0;
diff --git a/gdb/target.h b/gdb/target.h
index 890171d..f22e5c6 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1051,6 +1051,9 @@ extern int target_read_string (CORE_ADDR, char **, int, int *);
extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
ssize_t len);

+extern int target_read_raw_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
+ ssize_t len);
+
extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);

extern int target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len);