| Message ID | 1447290598-13069-2-git-send-email-hollis_blanchard@mentor.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
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.
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
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
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
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
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
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
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
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
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)"
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(-)