diff mbox

[3/3] trace: use addresses instead of offsets in memory tracepoints

Message ID 1454976185-30095-3-git-send-email-hollis_blanchard@mentor.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hollis Blanchard Feb. 9, 2016, 12:03 a.m. UTC 
When memory_region_ops tracepoints are enabled, calculate and record the
absolute address being accessed. Otherwise, we only get offsets into the
memory region instead of addresses.

Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
---
 memory.c     | 44 ++++++++++++++++++++++++++++++++------------
 trace-events |  4 ++--
 2 files changed, 34 insertions(+), 14 deletions(-)

Comments

Stefan Hajnoczi Feb. 9, 2016, 3:53 p.m. UTC | #1 
On Mon, Feb 08, 2016 at 04:03:05PM -0800, Hollis Blanchard wrote:
> -memory_region_ops_read(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> -memory_region_ops_write(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"

Please update the format string too ("offset" -> "addr").

Stefan
Hollis Blanchard Feb. 9, 2016, 5:31 p.m. UTC | #2 
On 02/09/2016 07:53 AM, Stefan Hajnoczi wrote:
> On Mon, Feb 08, 2016 at 04:03:05PM -0800, Hollis Blanchard wrote:
>> -memory_region_ops_read(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
>> -memory_region_ops_write(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
>> +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
>> +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> Please update the format string too ("offset" -> "addr").

Oops, thanks. Do you want me to re-send the series, or just this patch?

Hollis Blanchard
Mentor Graphics Emulation Division
Stefan Hajnoczi Feb. 10, 2016, 10:11 a.m. UTC | #3 
On Tue, Feb 09, 2016 at 09:31:23AM -0800, Hollis Blanchard wrote:
> On 02/09/2016 07:53 AM, Stefan Hajnoczi wrote:
> >On Mon, Feb 08, 2016 at 04:03:05PM -0800, Hollis Blanchard wrote:
> >>-memory_region_ops_read(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> >>-memory_region_ops_write(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> >>+memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> >>+memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
> >Please update the format string too ("offset" -> "addr").
> 
> Oops, thanks. Do you want me to re-send the series, or just this patch?

I had no other comments and this is a small change so I can do it myself
when merging the patches.  No need to resend.

Thanks,
Stefan
diff mbox

Patch

diff --git a/memory.c b/memory.c
index 0556700..8a61ad2 100644
--- a/memory.c
+++ b/memory.c
@@ -372,6 +372,20 @@  static void adjust_endianness(MemoryRegion *mr, uint64_t *data, unsigned size)
     }
 }
 
+static hwaddr memory_region_to_absolute_addr(MemoryRegion *mr, hwaddr offset)
+{
+    MemoryRegion *root;
+    hwaddr abs_addr = offset;
+
+    abs_addr += mr->addr;
+    for (root = mr; root->container; ) {
+        root = root->container;
+        abs_addr += root->addr;
+    }
+
+    return abs_addr;
+}
+
 static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr,
                                                        hwaddr addr,
                                                        uint64_t *value,
@@ -385,8 +399,9 @@  static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr,
     tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
     if (mr->subpage) {
         trace_memory_region_subpage_read(mr, addr, tmp, size);
-    } else {
-        trace_memory_region_ops_read(mr, addr, tmp, size);
+    } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
+        hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
+        trace_memory_region_ops_read(mr, abs_addr, tmp, size);
     }
     *value |= (tmp & mask) << shift;
     return MEMTX_OK;
@@ -405,8 +420,9 @@  static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
     tmp = mr->ops->read(mr->opaque, addr, size);
     if (mr->subpage) {
         trace_memory_region_subpage_read(mr, addr, tmp, size);
-    } else {
-        trace_memory_region_ops_read(mr, addr, tmp, size);
+    } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
+        hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
+        trace_memory_region_ops_read(mr, abs_addr, tmp, size);
     }
     *value |= (tmp & mask) << shift;
     return MEMTX_OK;
@@ -426,8 +442,9 @@  static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
     r = mr->ops->read_with_attrs(mr->opaque, addr, &tmp, size, attrs);
     if (mr->subpage) {
         trace_memory_region_subpage_read(mr, addr, tmp, size);
-    } else {
-        trace_memory_region_ops_read(mr, addr, tmp, size);
+    } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
+        hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
+        trace_memory_region_ops_read(mr, abs_addr, tmp, size);
     }
     *value |= (tmp & mask) << shift;
     return r;
@@ -446,8 +463,9 @@  static MemTxResult memory_region_oldmmio_write_accessor(MemoryRegion *mr,
     tmp = (*value >> shift) & mask;
     if (mr->subpage) {
         trace_memory_region_subpage_write(mr, addr, tmp, size);
-    } else {
-        trace_memory_region_ops_write(mr, addr, tmp, size);
+    } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
+        hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
+        trace_memory_region_ops_write(mr, abs_addr, tmp, size);
     }
     mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
     return MEMTX_OK;
@@ -466,8 +484,9 @@  static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
     tmp = (*value >> shift) & mask;
     if (mr->subpage) {
         trace_memory_region_subpage_write(mr, addr, tmp, size);
-    } else {
-        trace_memory_region_ops_write(mr, addr, tmp, size);
+    } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
+        hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
+        trace_memory_region_ops_write(mr, abs_addr, tmp, size);
     }
     mr->ops->write(mr->opaque, addr, tmp, size);
     return MEMTX_OK;
@@ -486,8 +505,9 @@  static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
     tmp = (*value >> shift) & mask;
     if (mr->subpage) {
         trace_memory_region_subpage_write(mr, addr, tmp, size);
-    } else {
-        trace_memory_region_ops_write(mr, addr, tmp, size);
+    } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
+        hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
+        trace_memory_region_ops_write(mr, abs_addr, tmp, size);
     }
     return mr->ops->write_with_attrs(mr->opaque, addr, tmp, size, attrs);
 }
diff --git a/trace-events b/trace-events
index f0c8126..292be8e 100644
--- a/trace-events
+++ b/trace-events
@@ -1604,8 +1604,8 @@  disable exec_tb_exit(void *next_tb, unsigned int flags) "tb:%p flags=%x"
 translate_block(void *tb, uintptr_t pc, uint8_t *tb_code) "tb:%p, pc:0x%"PRIxPTR", tb_code:%p"
 
 # memory.c
-memory_region_ops_read(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
-memory_region_ops_write(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
+memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
+memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
 memory_region_subpage_read(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"
 memory_region_subpage_write(void *mr, uint64_t offset, uint64_t value, unsigned size) "mr %p offset %#"PRIx64" value %#"PRIx64" size %u"