qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings
@ 2016-02-09  0:03 Hollis Blanchard
  2016-02-09  0:03 ` [Qemu-devel] [PATCH 2/3] trace: split subpage MMIOs into their own trace events Hollis Blanchard
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hollis Blanchard @ 2016-02-09  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, Hollis Blanchard

The simple tracing backend has supported strings for more than three years
(62bab73213ba885426a781eb2741670b9f3cae36).

Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
---
 docs/tracing.txt | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index 3853a6a..f2f553b 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -172,9 +172,6 @@ source tree.  It may not be as powerful as platform-specific or third-party
 trace backends but it is portable.  This is the recommended trace backend
 unless you have specific needs for more advanced backends.
 
-The "simple" backend currently does not capture string arguments, it simply
-records the char* pointer value instead of the string that is pointed to.
-
 === Ftrace ===
 
 The "ftrace" backend writes trace data to ftrace marker. This effectively
-- 
1.9.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 2/3] trace: split subpage MMIOs into their own trace events.
  2016-02-09  0:03 [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings Hollis Blanchard
@ 2016-02-09  0:03 ` Hollis Blanchard
  2016-02-09  0:03 ` [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints Hollis Blanchard
  2016-02-10 10:14 ` [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings Stefan Hajnoczi
  2 siblings, 0 replies; 7+ messages in thread
From: Hollis Blanchard @ 2016-02-09  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, Hollis Blanchard, pbonzini

Previously, a single MMIO could trigger the memory_region_ops tracepoint twice:
once on its way into subpage ops, then later on its way into the model's ops.

Also, the fields previously called "addr" are actually offsets into the memory
region. Rename them to "offset" while we're editing the tracepoint definitions.

Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>

---
As agreed in https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01509.html
and https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01551.html.
---
 memory.c     | 36 ++++++++++++++++++++++++++++++------
 trace-events |  6 ++++--
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/memory.c b/memory.c
index 39c539c..0556700 100644
--- a/memory.c
+++ b/memory.c
@@ -383,7 +383,11 @@ static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr,
     uint64_t tmp;
 
     tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
-    trace_memory_region_ops_read(mr, addr, tmp, size);
+    if (mr->subpage) {
+        trace_memory_region_subpage_read(mr, addr, tmp, size);
+    } else {
+        trace_memory_region_ops_read(mr, addr, tmp, size);
+    }
     *value |= (tmp & mask) << shift;
     return MEMTX_OK;
 }
@@ -399,7 +403,11 @@ static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
     uint64_t tmp;
 
     tmp = mr->ops->read(mr->opaque, addr, size);
-    trace_memory_region_ops_read(mr, addr, tmp, size);
+    if (mr->subpage) {
+        trace_memory_region_subpage_read(mr, addr, tmp, size);
+    } else {
+        trace_memory_region_ops_read(mr, addr, tmp, size);
+    }
     *value |= (tmp & mask) << shift;
     return MEMTX_OK;
 }
@@ -416,7 +424,11 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
     MemTxResult r;
 
     r = mr->ops->read_with_attrs(mr->opaque, addr, &tmp, size, attrs);
-    trace_memory_region_ops_read(mr, addr, tmp, size);
+    if (mr->subpage) {
+        trace_memory_region_subpage_read(mr, addr, tmp, size);
+    } else {
+        trace_memory_region_ops_read(mr, addr, tmp, size);
+    }
     *value |= (tmp & mask) << shift;
     return r;
 }
@@ -432,7 +444,11 @@ static MemTxResult memory_region_oldmmio_write_accessor(MemoryRegion *mr,
     uint64_t tmp;
 
     tmp = (*value >> shift) & mask;
-    trace_memory_region_ops_write(mr, addr, tmp, size);
+    if (mr->subpage) {
+        trace_memory_region_subpage_write(mr, addr, tmp, size);
+    } else {
+        trace_memory_region_ops_write(mr, addr, tmp, size);
+    }
     mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
     return MEMTX_OK;
 }
@@ -448,7 +464,11 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
     uint64_t tmp;
 
     tmp = (*value >> shift) & mask;
-    trace_memory_region_ops_write(mr, addr, tmp, size);
+    if (mr->subpage) {
+        trace_memory_region_subpage_write(mr, addr, tmp, size);
+    } else {
+        trace_memory_region_ops_write(mr, addr, tmp, size);
+    }
     mr->ops->write(mr->opaque, addr, tmp, size);
     return MEMTX_OK;
 }
@@ -464,7 +484,11 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
     uint64_t tmp;
 
     tmp = (*value >> shift) & mask;
-    trace_memory_region_ops_write(mr, addr, tmp, size);
+    if (mr->subpage) {
+        trace_memory_region_subpage_write(mr, addr, tmp, size);
+    } else {
+        trace_memory_region_ops_write(mr, addr, tmp, size);
+    }
     return mr->ops->write_with_attrs(mr->opaque, addr, tmp, size, attrs);
 }
 
diff --git a/trace-events b/trace-events
index c9ac144..f0c8126 100644
--- a/trace-events
+++ b/trace-events
@@ -1604,8 +1604,10 @@ 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 addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %u"
-memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %u"
+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_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"
 
 # qom/object.c
 object_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
-- 
1.9.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints
  2016-02-09  0:03 [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings Hollis Blanchard
  2016-02-09  0:03 ` [Qemu-devel] [PATCH 2/3] trace: split subpage MMIOs into their own trace events Hollis Blanchard
@ 2016-02-09  0:03 ` Hollis Blanchard
  2016-02-09 15:53   ` Stefan Hajnoczi
  2016-02-10 10:14 ` [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings Stefan Hajnoczi
  2 siblings, 1 reply; 7+ messages in thread
From: Hollis Blanchard @ 2016-02-09  0:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha, Hollis Blanchard, pbonzini

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(-)

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"
 
-- 
1.9.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints
  2016-02-09  0:03 ` [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints Hollis Blanchard
@ 2016-02-09 15:53   ` Stefan Hajnoczi
  2016-02-09 17:31     ` Hollis Blanchard
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2016-02-09 15:53 UTC (permalink / raw)
  To: Hollis Blanchard; +Cc: pbonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints
  2016-02-09 15:53   ` Stefan Hajnoczi
@ 2016-02-09 17:31     ` Hollis Blanchard
  2016-02-10 10:11       ` Stefan Hajnoczi
  0 siblings, 1 reply; 7+ messages in thread
From: Hollis Blanchard @ 2016-02-09 17:31 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: pbonzini, qemu-devel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints
  2016-02-09 17:31     ` Hollis Blanchard
@ 2016-02-10 10:11       ` Stefan Hajnoczi
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2016-02-10 10:11 UTC (permalink / raw)
  To: Hollis Blanchard; +Cc: pbonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings
  2016-02-09  0:03 [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings Hollis Blanchard
  2016-02-09  0:03 ` [Qemu-devel] [PATCH 2/3] trace: split subpage MMIOs into their own trace events Hollis Blanchard
  2016-02-09  0:03 ` [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints Hollis Blanchard
@ 2016-02-10 10:14 ` Stefan Hajnoczi
  2 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2016-02-10 10:14 UTC (permalink / raw)
  To: Hollis Blanchard; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

On Mon, Feb 08, 2016 at 04:03:03PM -0800, Hollis Blanchard wrote:
> The simple tracing backend has supported strings for more than three years
> (62bab73213ba885426a781eb2741670b9f3cae36).
> 
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> ---
>  docs/tracing.txt | 3 ---
>  1 file changed, 3 deletions(-)

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-02-10 10:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09  0:03 [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings Hollis Blanchard
2016-02-09  0:03 ` [Qemu-devel] [PATCH 2/3] trace: split subpage MMIOs into their own trace events Hollis Blanchard
2016-02-09  0:03 ` [Qemu-devel] [PATCH 3/3] trace: use addresses instead of offsets in memory tracepoints Hollis Blanchard
2016-02-09 15:53   ` Stefan Hajnoczi
2016-02-09 17:31     ` Hollis Blanchard
2016-02-10 10:11       ` Stefan Hajnoczi
2016-02-10 10:14 ` [Qemu-devel] [PATCH 1/3] trace: docs: "simple" backend does support strings Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).