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

[Hollis Blanchard <hollis_blanchard@mentor.com>]

[-- 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 --]