[Qemu-devel] [PATCH 1/2] docs: "simple" trace backend does support strings

[Qemu-devel] [PATCH 1/2] docs: "simple" trace backend does support strings

[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

[Qemu-devel] [PATCH 2/2] trace: show MemoryRegion name, not address

[Hollis Blanchard]

Recording the MemoryRegion pointers isn't helpful, especially since no trace
data allows us to correlate those pointers to devices. Instead, record the
MemoryRegion name.

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

diff --git a/memory.c b/memory.c
index c435c88..9bd4c31 100644
--- a/memory.c
+++ b/memory.c
@@ -381,7 +381,7 @@ 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);
+    trace_memory_region_ops_read(mr->name, addr, tmp, size);
     *value |= (tmp & mask) << shift;
     return MEMTX_OK;
 }
@@ -397,7 +397,7 @@ 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);
+    trace_memory_region_ops_read(mr->name, addr, tmp, size);
     *value |= (tmp & mask) << shift;
     return MEMTX_OK;
 }
@@ -414,7 +414,7 @@ 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);
+    trace_memory_region_ops_read(mr->name, addr, tmp, size);
     *value |= (tmp & mask) << shift;
     return r;
 }
@@ -430,7 +430,7 @@ 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);
+    trace_memory_region_ops_write(mr->name, addr, tmp, size);
     mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
     return MEMTX_OK;
 }
@@ -446,7 +446,7 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
     uint64_t tmp;
 
     tmp = (*value >> shift) & mask;
-    trace_memory_region_ops_write(mr, addr, tmp, size);
+    trace_memory_region_ops_write(mr->name, addr, tmp, size);
     mr->ops->write(mr->opaque, addr, tmp, size);
     return MEMTX_OK;
 }
@@ -462,7 +462,7 @@ 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);
+    trace_memory_region_ops_write(mr->name, addr, tmp, size);
     return mr->ops->write_with_attrs(mr->opaque, addr, tmp, size, attrs);
 }
 
diff --git a/trace-events b/trace-events
index ef6bc41..23c11c3 100644
--- a/trace-events
+++ b/trace-events
@@ -1594,8 +1594,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 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(const char *mr_name, uint64_t addr, uint64_t value, unsigned size) "dev %s addr %#"PRIx64" value %#"PRIx64" size %u"
+memory_region_ops_write(const char *mr_name, uint64_t addr, uint64_t value, unsigned size) "dev %s addr %#"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

Re: [Qemu-devel] [PATCH 2/2] trace: show MemoryRegion name, not address

[Stefan Hajnoczi]

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

On Wed, Nov 11, 2015 at 05:09:58PM -0800, Hollis Blanchard wrote:
> Recording the MemoryRegion pointers isn't helpful, especially since no trace
> data allows us to correlate those pointers to devices. Instead, record the
> MemoryRegion name.
> 
> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> ---
>  memory.c     | 12 ++++++------
>  trace-events |  4 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index c435c88..9bd4c31 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -381,7 +381,7 @@ 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);
> +    trace_memory_region_ops_read(mr->name, addr, tmp, size);

mr->name may be NULL.  There is a memory_region_name() function that
always produces a real string.  Perhaps it's best to use it.

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

Re: [Qemu-devel] [PATCH 2/2] trace: show MemoryRegion name, not address

[Paolo Bonzini]

On 13/11/2015 11:23, Stefan Hajnoczi wrote:
>> >      tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
>> > -    trace_memory_region_ops_read(mr, addr, tmp, size);
>> > +    trace_memory_region_ops_read(mr->name, addr, tmp, size);
> mr->name may be NULL.  There is a memory_region_name() function that
> always produces a real string.  Perhaps it's best to use it.

It's also useful to have mr in the trace, for use within a debugger.

Paolo

Re: [Qemu-devel] [PATCH 2/2] trace: show MemoryRegion name, not address

[Hollis Blanchard]

On 11/13/2015 02:23 AM, Stefan Hajnoczi wrote:
> On Wed, Nov 11, 2015 at 05:09:58PM -0800, Hollis Blanchard wrote:
>> Recording the MemoryRegion pointers isn't helpful, especially since no trace
>> data allows us to correlate those pointers to devices. Instead, record the
>> MemoryRegion name.
>>
>> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
>> ---
>>   memory.c     | 12 ++++++------
>>   trace-events |  4 ++--
>>   2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/memory.c b/memory.c
>> index c435c88..9bd4c31 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -381,7 +381,7 @@ 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);
>> +    trace_memory_region_ops_read(mr->name, addr, tmp, size);
> mr->name may be NULL.  There is a memory_region_name() function that
> always produces a real string.  Perhaps it's best to use it.

Using memory_region_name() yields this:
** ERROR **: file qom/object.c: line 1427 
(object_get_canonical_path_component): assertion failed: (obj->parent != 
NULL)
aborting...

The offending MemoryRegion seems to be a subpage one, which has no name. 
I can tell because ops contains links to subpage_read() and subpage_write().

"info mtree" uses memory_region_name() and works fine, but perhaps 
that's because it only goes 2 levels deep?

Hollis Blanchard
Mentor Graphics Emulation Division

Re: [Qemu-devel] [PATCH 2/2] trace: show MemoryRegion name, not address

[Stefan Hajnoczi]

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

On Tue, Nov 17, 2015 at 04:37:48PM -0800, Hollis Blanchard wrote:
> On 11/13/2015 02:23 AM, Stefan Hajnoczi wrote:
> >On Wed, Nov 11, 2015 at 05:09:58PM -0800, Hollis Blanchard wrote:
> >>Recording the MemoryRegion pointers isn't helpful, especially since no trace
> >>data allows us to correlate those pointers to devices. Instead, record the
> >>MemoryRegion name.
> >>
> >>Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> >>---
> >>  memory.c     | 12 ++++++------
> >>  trace-events |  4 ++--
> >>  2 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/memory.c b/memory.c
> >>index c435c88..9bd4c31 100644
> >>--- a/memory.c
> >>+++ b/memory.c
> >>@@ -381,7 +381,7 @@ 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);
> >>+    trace_memory_region_ops_read(mr->name, addr, tmp, size);
> >mr->name may be NULL.  There is a memory_region_name() function that
> >always produces a real string.  Perhaps it's best to use it.
> 
> Using memory_region_name() yields this:
> ** ERROR **: file qom/object.c: line 1427
> (object_get_canonical_path_component): assertion failed: (obj->parent !=
> NULL)
> aborting...
> 
> The offending MemoryRegion seems to be a subpage one, which has no name. I
> can tell because ops contains links to subpage_read() and subpage_write().
> 
> "info mtree" uses memory_region_name() and works fine, but perhaps that's
> because it only goes 2 levels deep?

I'm not very familiar with the memory API so I'm afraid I don't know the
best solution.  My concern about a NULL string pointer is that some
operating systems ship a libc that segfaults instead of snprintf(...,
"%s", NULL) to "(null)".  So the stderr trace backend could crash on
those operating systems.

Stefan

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

[Qemu-devel] subpage_write() and duplicated memory_region_ops_write tracepoints

[Hollis Blanchard]

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

On 11/24/2015 11:20 PM, Stefan Hajnoczi wrote:
> On Tue, Nov 17, 2015 at 04:37:48PM -0800, Hollis Blanchard wrote:
>> On 11/13/2015 02:23 AM, Stefan Hajnoczi wrote:
>>> On Wed, Nov 11, 2015 at 05:09:58PM -0800, Hollis Blanchard wrote:
>>>> Recording the MemoryRegion pointers isn't helpful, especially since no trace
>>>> data allows us to correlate those pointers to devices. Instead, record the
>>>> MemoryRegion name.
>>>>
>>>> Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
>>>> ---
>>>>   memory.c     | 12 ++++++------
>>>>   trace-events |  4 ++--
>>>>   2 files changed, 8 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/memory.c b/memory.c
>>>> index c435c88..9bd4c31 100644
>>>> --- a/memory.c
>>>> +++ b/memory.c
>>>> @@ -381,7 +381,7 @@ 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);
>>>> +    trace_memory_region_ops_read(mr->name, addr, tmp, size);
>>> mr->name may be NULL.  There is a memory_region_name() function that
>>> always produces a real string.  Perhaps it's best to use it.
>> Using memory_region_name() yields this:
>> ** ERROR **: file qom/object.c: line 1427
>> (object_get_canonical_path_component): assertion failed: (obj->parent !=
>> NULL)
>> aborting...
>>
>> The offending MemoryRegion seems to be a subpage one, which has no name. I
>> can tell because ops contains links to subpage_read() and subpage_write().
>>
>> "info mtree" uses memory_region_name() and works fine, but perhaps that's
>> because it only goes 2 levels deep?
> I'm not very familiar with the memory API so I'm afraid I don't know the
> best solution.  My concern about a NULL string pointer is that some
> operating systems ship a libc that segfaults instead of snprintf(...,
> "%s", NULL) to "(null)".  So the stderr trace backend could crash on
> those operating systems.
I now have a patch that calculates the absolute address, so printing the 
MemoryRegion->name would be convenient but not a critical issue. I say 
"convenient" because it would still be nice to be able to tell what 
device handled the write without needing to look up the address in the 
machine's memory map.

However, I noticed something odd... there are a lot of duplicated write 
tracepoints:

memory_region_ops_write 281.000 pid=29100 mr=0x19138f0 addr=0x0 value=0x3 size=0x4
memory_region_ops_write 2.000 pid=29100 mr=0x185a620 addr=0x0 value=0x3 size=0x4
memory_region_ops_write 227.000 pid=29100 mr=0x19138f0 addr=0x4 value=0x80 size=0x4
memory_region_ops_write 2.000 pid=29100 mr=0x185a620 addr=0x4 value=0x80 size=0x4
memory_region_ops_write 764.000 pid=29100 mr=0x19124c0 addr=0x24 value=0x10 size=0x4
memory_region_ops_write 2.000 pid=29100 mr=0x18d4488 addr=0x24 value=0x10 size=0x4
memory_region_ops_write 275.000 pid=29100 mr=0x19124c0 addr=0x30 value=0xc301 size=0x4
memory_region_ops_write 2.000 pid=29100 mr=0x18d4488 addr=0x30 value=0xc301 size=0x4


It's coming from subpage_write():

#0  trace_memory_region_ops_write (mr=0x185b620, addr=16, absaddr=738205712, value=136, size=4)
     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/trace/generated-tracers.h:7374
#1  0x000000000045eb8a in*memory_region_write_with_attrs_accessor*  (mr=0x185b620, addr=16,
     value=0x45203338, size=4, shift=0, mask=4294967295, attrs=...)
     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:513
#2  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x45203338, size=4,
     access_size_min=1, access_size_max=4, access=0x45eb15 <memory_region_write_with_attrs_accessor>,
     mr=0x185b620, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
#3  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x185b620, addr=16, data=136, size=4,
     attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
#4  0x0000000000411bbf in address_space_rw (as=0x11f3440, addr=738205712, attrs=...,
     buf=0x45203490 "\210", len=4, is_write=true)
     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2497
#5  0x0000000000411ea9 in address_space_write (as=0x11f3440, addr=738205712, attrs=...,
     buf=0x45203490 "\210", len=4) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2579
#6  0x0000000000410d89 in subpage_write (opaque=0x19148f0, addr=16, value=136, len=4, attrs=...)
     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2139
#7  0x000000000045ebb2 in*memory_region_write_with_attrs_accessor*  (mr=0x19148f0, addr=16,
     value=0x452035a8, size=4, shift=0, mask=4294967295, attrs=...)
     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:516
#8  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x452035a8, size=4,
     access_size_min=1, access_size_max=8, access=0x45eb15 <memory_region_write_with_attrs_accessor>,
     mr=0x19148f0, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
#9  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x19148f0, addr=16, data=136, size=4,
     attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
#10 0x000000000046c61c in io_writel (env=0x2aabace89268, iotlbentry=0x2aabace99808, val=136,
     addr=18446743523953745936, retaddr=1107508028)
     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:470
#11 0x000000000046c3cb in helper_le_stl_mmu (env=0x2aabace89268, addr=18446743523953745936, val=136,
     oi=33, retaddr=1107508028)
     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:510
#12 0x0000000042033b3e in code_gen_buffer ()


The first tracepoint in each pair is an artifact, and should be omitted. 
Any suggestions? We could special case "if (mr->ops->write != 
subpage_write) { emit tracepoint }", but that's a bit of a hack... :-)

Hollis Blanchard
Mentor Graphics Emulation Division

[-- Attachment #2: Type: text/html, Size: 6733 bytes --]

Re: [Qemu-devel] subpage_write() and duplicated memory_region_ops_write tracepoints

[Paolo Bonzini]

On 09/12/2015 21:54, Hollis Blanchard wrote:
> 
> #0  trace_memory_region_ops_write (mr=0x185b620, addr=16, absaddr=738205712, value=136, size=4)
>     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/trace/generated-tracers.h:7374
> #1  0x000000000045eb8a in *memory_region_write_with_attrs_accessor* (mr=0x185b620, addr=16, 
>     value=0x45203338, size=4, shift=0, mask=4294967295, attrs=...)
>     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:513
> #2  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x45203338, size=4, 
>     access_size_min=1, access_size_max=4, access=0x45eb15 <memory_region_write_with_attrs_accessor>, 
>     mr=0x185b620, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
> #3  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x185b620, addr=16, data=136, size=4, 
>     attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
> #4  0x0000000000411bbf in address_space_rw (as=0x11f3440, addr=738205712, attrs=..., 
>     buf=0x45203490 "\210", len=4, is_write=true)
>     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2497
> #5  0x0000000000411ea9 in address_space_write (as=0x11f3440, addr=738205712, attrs=..., 
>     buf=0x45203490 "\210", len=4) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2579
> #6  0x0000000000410d89 in subpage_write (opaque=0x19148f0, addr=16, value=136, len=4, attrs=...)
>     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2139
> #7  0x000000000045ebb2 in *memory_region_write_with_attrs_accessor* (mr=0x19148f0, addr=16, 
>     value=0x452035a8, size=4, shift=0, mask=4294967295, attrs=...)
>     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:516
> #8  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x452035a8, size=4, 
>     access_size_min=1, access_size_max=8, access=0x45eb15 <memory_region_write_with_attrs_accessor>, 
>     mr=0x19148f0, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
> #9  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x19148f0, addr=16, data=136, size=4, 
>     attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
> #10 0x000000000046c61c in io_writel (env=0x2aabace89268, iotlbentry=0x2aabace99808, val=136, 
>     addr=18446743523953745936, retaddr=1107508028)
>     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:470
> #11 0x000000000046c3cb in helper_le_stl_mmu (env=0x2aabace89268, addr=18446743523953745936, val=136, 
>     oi=33, retaddr=1107508028)
>     at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:510
> #12 0x0000000042033b3e in code_gen_buffer ()
> 
> 
> The first tracepoint in each pair is an artifact, and should be omitted.
> Any suggestions? We could special case "if (mr->ops->write !=
> subpage_write) { emit tracepoint }", but that's a bit of a hack... :-)

You can add an argument to the tracepoint for mr->subpage.  I would keep
the duplicate however.

Paolo

Re: [Qemu-devel] subpage_write() and duplicated memory_region_ops_write tracepoints

[Hollis Blanchard]

On 12/09/2015 01:12 PM, Paolo Bonzini wrote:
> On 09/12/2015 21:54, Hollis Blanchard wrote:
>> #0  trace_memory_region_ops_write (mr=0x185b620, addr=16, absaddr=738205712, value=136, size=4)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/trace/generated-tracers.h:7374
>> #1  0x000000000045eb8a in memory_region_write_with_attrs_accessor (mr=0x185b620, addr=16,
>>      value=0x45203338, size=4, shift=0, mask=4294967295, attrs=...)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:513
>> #2  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x45203338, size=4,
>>      access_size_min=1, access_size_max=4, access=0x45eb15 <memory_region_write_with_attrs_accessor>,
>>      mr=0x185b620, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
>> #3  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x185b620, addr=16, data=136, size=4,
>>      attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
>> #4  0x0000000000411bbf in address_space_rw (as=0x11f3440, addr=738205712, attrs=...,
>>      buf=0x45203490 "\210", len=4, is_write=true)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2497
>> #5  0x0000000000411ea9 in address_space_write (as=0x11f3440, addr=738205712, attrs=...,
>>      buf=0x45203490 "\210", len=4) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2579
>> #6  0x0000000000410d89 in subpage_write (opaque=0x19148f0, addr=16, value=136, len=4, attrs=...)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2139
>> #7  0x000000000045ebb2 in memory_region_write_with_attrs_accessor (mr=0x19148f0, addr=16,
>>      value=0x452035a8, size=4, shift=0, mask=4294967295, attrs=...)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:516
>> #8  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x452035a8, size=4,
>>      access_size_min=1, access_size_max=8, access=0x45eb15 <memory_region_write_with_attrs_accessor>,
>>      mr=0x19148f0, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
>> #9  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x19148f0, addr=16, data=136, size=4,
>>      attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
>> #10 0x000000000046c61c in io_writel (env=0x2aabace89268, iotlbentry=0x2aabace99808, val=136,
>>      addr=18446743523953745936, retaddr=1107508028)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:470
>> #11 0x000000000046c3cb in helper_le_stl_mmu (env=0x2aabace89268, addr=18446743523953745936, val=136,
>>      oi=33, retaddr=1107508028)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:510
>> #12 0x0000000042033b3e in code_gen_buffer ()
>>
>>
>> The first tracepoint in each pair is an artifact, and should be omitted.
>> Any suggestions? We could special case "if (mr->ops->write !=
>> subpage_write) { emit tracepoint }", but that's a bit of a hack... :-)
> You can add an argument to the tracepoint for mr->subpage.  I would keep
> the duplicate however.

Not sure I understand. Do you mean something like this?
-trace_memory_region_ops_write(mr, addr, tmp, size);
+trace_memory_region_ops_write(mr, addr, mr->ops->write == 
subpage_write, tmp, size);

Maybe this isn't the right tracepoint to begin with. I'm trying to trace 
guest MMIO activity; is there a better place to intercept that?

Hollis Blanchard
Mentor Graphics Emulation Division

Re: [Qemu-devel] subpage_write() and duplicated memory_region_ops_write tracepoints

[Paolo Bonzini]

On 10/12/2015 01:39, Hollis Blanchard wrote:
> 
> Not sure I understand. Do you mean something like this?
> -trace_memory_region_ops_write(mr, addr, tmp, size);
> +trace_memory_region_ops_write(mr, addr, mr->ops->write ==
> subpage_write, tmp, size);

There is also a mr->subpage that you can use.

> Maybe this isn't the right tracepoint to begin with. I'm trying to trace
> guest MMIO activity; is there a better place to intercept that?

It's the right one, but the problem with tracepoints is that other
people may use them for something other than what you envisioned.   For
them, including subpages may be the right thing.  Adding a flag to the
tracepoint is the middle ground.

I'm okay also with the solution you proposed in the other message.

Paolo

Re: [Qemu-devel] subpage_write() and duplicated memory_region_ops_write tracepoints

[Hollis Blanchard]

On 12/09/2015 01:12 PM, Paolo Bonzini wrote:
>
> On 09/12/2015 21:54, Hollis Blanchard wrote:
>> #0  trace_memory_region_ops_write (mr=0x185b620, addr=16, absaddr=738205712, value=136, size=4)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/trace/generated-tracers.h:7374
>> #1  0x000000000045eb8a in *memory_region_write_with_attrs_accessor* (mr=0x185b620, addr=16,
>>      value=0x45203338, size=4, shift=0, mask=4294967295, attrs=...)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:513
>> #2  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x45203338, size=4,
>>      access_size_min=1, access_size_max=4, access=0x45eb15 <memory_region_write_with_attrs_accessor>,
>>      mr=0x185b620, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
>> #3  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x185b620, addr=16, data=136, size=4,
>>      attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
>> #4  0x0000000000411bbf in address_space_rw (as=0x11f3440, addr=738205712, attrs=...,
>>      buf=0x45203490 "\210", len=4, is_write=true)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2497
>> #5  0x0000000000411ea9 in address_space_write (as=0x11f3440, addr=738205712, attrs=...,
>>      buf=0x45203490 "\210", len=4) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2579
>> #6  0x0000000000410d89 in subpage_write (opaque=0x19148f0, addr=16, value=136, len=4, attrs=...)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/exec.c:2139
>> #7  0x000000000045ebb2 in *memory_region_write_with_attrs_accessor* (mr=0x19148f0, addr=16,
>>      value=0x452035a8, size=4, shift=0, mask=4294967295, attrs=...)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:516
>> #8  0x000000000045ed08 in access_with_adjusted_size (addr=16, value=0x452035a8, size=4,
>>      access_size_min=1, access_size_max=8, access=0x45eb15 <memory_region_write_with_attrs_accessor>,
>>      mr=0x19148f0, attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:556
>> #9  0x0000000000461ed7 in memory_region_dispatch_write (mr=0x19148f0, addr=16, data=136, size=4,
>>      attrs=...) at /scratch1/hblancha/install/customq/qemu-2.4.0/src/memory.c:1214
>> #10 0x000000000046c61c in io_writel (env=0x2aabace89268, iotlbentry=0x2aabace99808, val=136,
>>      addr=18446743523953745936, retaddr=1107508028)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:470
>> #11 0x000000000046c3cb in helper_le_stl_mmu (env=0x2aabace89268, addr=18446743523953745936, val=136,
>>      oi=33, retaddr=1107508028)
>>      at /scratch1/hblancha/install/customq/qemu-2.4.0/src/softmmu_template.h:510
>> #12 0x0000000042033b3e in code_gen_buffer ()
>>
>>
>> The first tracepoint in each pair is an artifact, and should be omitted.
>> Any suggestions? We could special case "if (mr->ops->write !=
>> subpage_write) { emit tracepoint }", but that's a bit of a hack... :-)
> You can add an argument to the tracepoint for mr->subpage.  I would keep
> the duplicate however.
OK, I understand your suggestion now, but not your rationale. Are you 
thinking about QEMU developers needing to debug the subpage 
implementation? If so, I think tracing guest MMIO activity is a far more 
common need... perhaps we could use a different tracepoint for the latter?

if (mr->subpage)
     trace_memory_region_ops_write_subpage(mr, addr, tmp, size);
else
     trace_memory_region_ops_write(mr, addr, tmp, size);

That way, people debugging guest behavior can enable just the 
memory_region_ops_write tracepoint, and QEMU developers can also enable 
the other.

Hollis Blanchard
Mentor Graphics Emulation Division