| Message ID | 1375014954-31916-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 28.07.2013 14:35, schrieb Paolo Bonzini: > This is quite handy to debug softmmu targets. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > memory.c | 5 +++++ > trace-events | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/memory.c b/memory.c > index 1494e95..ac6f3c6 100644 > --- a/memory.c > +++ b/memory.c > @@ -19,6 +19,7 @@ > #include "qemu/bitops.h" > #include "qom/object.h" > #include "sysemu/kvm.h" > +#include "trace.h" > #include <assert.h> > > #include "exec/memory-internal.h" > @@ -388,6 +389,7 @@ static void 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); > *value |= (tmp & mask) << shift; > } > > @@ -404,6 +406,7 @@ static void memory_region_read_accessor(MemoryRegion *mr, > qemu_flush_coalesced_mmio_buffer(); > } > tmp = mr->ops->read(mr->opaque, addr, size); > + trace_memory_region_ops_read(mr, addr, tmp, size); > *value |= (tmp & mask) << shift; > } > > @@ -417,6 +420,7 @@ static void memory_region_oldmmio_write_accessor(MemoryRegion *mr, > uint64_t tmp; > > tmp = (*value >> shift) & mask; > + trace_memory_region_ops_write(mr, addr, tmp, size); > mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp); > } > > @@ -433,6 +437,7 @@ static void memory_region_write_accessor(MemoryRegion *mr, > qemu_flush_coalesced_mmio_buffer(); > } > tmp = (*value >> shift) & mask; > + trace_memory_region_ops_write(mr, addr, tmp, size); > mr->ops->write(mr->opaque, addr, tmp, size); > } > > diff --git a/trace-events b/trace-events > index 002df83..3e0dd74 100644 > --- a/trace-events > +++ b/trace-events > @@ -1165,6 +1165,10 @@ kvm_vm_ioctl(int type, void *arg) "type %d, arg %p" > kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p" > kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d" > > +# memory.c > +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" > +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" %u would seem more correct for unsigned - if you fix that, Reviewed-by: Andreas Färber <afaerber@suse.de> and ACK that's it's good for 1.6. Andreas > + > # 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)" > object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)" >
Il 28/07/2013 14:45, Andreas Färber ha scritto: > Am 28.07.2013 14:35, schrieb Paolo Bonzini: >> This is quite handy to debug softmmu targets. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> memory.c | 5 +++++ >> trace-events | 4 ++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/memory.c b/memory.c >> index 1494e95..ac6f3c6 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -19,6 +19,7 @@ >> #include "qemu/bitops.h" >> #include "qom/object.h" >> #include "sysemu/kvm.h" >> +#include "trace.h" >> #include <assert.h> >> >> #include "exec/memory-internal.h" >> @@ -388,6 +389,7 @@ static void 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); >> *value |= (tmp & mask) << shift; >> } >> >> @@ -404,6 +406,7 @@ static void memory_region_read_accessor(MemoryRegion *mr, >> qemu_flush_coalesced_mmio_buffer(); >> } >> tmp = mr->ops->read(mr->opaque, addr, size); >> + trace_memory_region_ops_read(mr, addr, tmp, size); >> *value |= (tmp & mask) << shift; >> } >> >> @@ -417,6 +420,7 @@ static void memory_region_oldmmio_write_accessor(MemoryRegion *mr, >> uint64_t tmp; >> >> tmp = (*value >> shift) & mask; >> + trace_memory_region_ops_write(mr, addr, tmp, size); >> mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp); >> } >> >> @@ -433,6 +437,7 @@ static void memory_region_write_accessor(MemoryRegion *mr, >> qemu_flush_coalesced_mmio_buffer(); >> } >> tmp = (*value >> shift) & mask; >> + trace_memory_region_ops_write(mr, addr, tmp, size); >> mr->ops->write(mr->opaque, addr, tmp, size); >> } >> >> diff --git a/trace-events b/trace-events >> index 002df83..3e0dd74 100644 >> --- a/trace-events >> +++ b/trace-events >> @@ -1165,6 +1165,10 @@ kvm_vm_ioctl(int type, void *arg) "type %d, arg %p" >> kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p" >> kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d" >> >> +# memory.c >> +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" >> +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" > > %u would seem more correct for unsigned - if you fix that, Reviewed-by: > Andreas Färber <afaerber@suse.de> and ACK that's it's good for 1.6. Ok, though it can only be 1/2/4/8. Paolo
Am 28.07.2013 14:55, schrieb Paolo Bonzini: > Il 28/07/2013 14:45, Andreas Färber ha scritto: >> Am 28.07.2013 14:35, schrieb Paolo Bonzini: >>> diff --git a/trace-events b/trace-events >>> index 002df83..3e0dd74 100644 >>> --- a/trace-events >>> +++ b/trace-events >>> @@ -1165,6 +1165,10 @@ kvm_vm_ioctl(int type, void *arg) "type %d, arg %p" >>> kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p" >>> kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d" >>> >>> +# memory.c >>> +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" >>> +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" >> >> %u would seem more correct for unsigned - if you fix that, Reviewed-by: >> Andreas Färber <afaerber@suse.de> and ACK that's it's good for 1.6. > > Ok, though it can only be 1/2/4/8. I had static analysis in mind. ;) Andreas
On 28 July 2013 13:35, Paolo Bonzini <pbonzini@redhat.com> wrote: > This is quite handy to debug softmmu targets. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> (not a nak, just a tangential thought) It would be cute if you could just insert a "trace this access and pass it through" container MemoryRegion at an arbitrary point in the memory region hierarchy. That would let us do "trace accesses to device X" without device X having to have boilerplate tracing code in its read/write functions. -- PMM
Am 28.07.2013 16:34, schrieb Peter Maydell: > On 28 July 2013 13:35, Paolo Bonzini <pbonzini@redhat.com> wrote: >> This is quite handy to debug softmmu targets. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > (not a nak, just a tangential thought) > It would be cute if you could just insert a "trace this access > and pass it through" container MemoryRegion at an arbitrary > point in the memory region hierarchy. That would let us do > "trace accesses to device X" without device X having to > have boilerplate tracing code in its read/write functions. The SystemTap backend for instance allows you to filter what you print based on arguments, i.e. address and size. Are you suggesting to make the trace points in this patch conditional on some new mr->tracing_enabled flag in the MemoryRegion? Andreas
On 07/28/13 08:35, Paolo Bonzini wrote: > This is quite handy to debug softmmu targets. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > memory.c | 5 +++++ > trace-events | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/memory.c b/memory.c > index 1494e95..ac6f3c6 100644 > --- a/memory.c > +++ b/memory.c > @@ -19,6 +19,7 @@ > #include "qemu/bitops.h" > #include "qom/object.h" > #include "sysemu/kvm.h" > +#include "trace.h" > #include <assert.h> > > #include "exec/memory-internal.h" > @@ -388,6 +389,7 @@ static void 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); > *value |= (tmp & mask) << shift; > } > > @@ -404,6 +406,7 @@ static void memory_region_read_accessor(MemoryRegion *mr, > qemu_flush_coalesced_mmio_buffer(); > } > tmp = mr->ops->read(mr->opaque, addr, size); > + trace_memory_region_ops_read(mr, addr, tmp, size); > *value |= (tmp & mask) << shift; > } > > @@ -417,6 +420,7 @@ static void memory_region_oldmmio_write_accessor(MemoryRegion *mr, > uint64_t tmp; > > tmp = (*value >> shift) & mask; > + trace_memory_region_ops_write(mr, addr, tmp, size); > mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp); > } > > @@ -433,6 +437,7 @@ static void memory_region_write_accessor(MemoryRegion *mr, > qemu_flush_coalesced_mmio_buffer(); > } > tmp = (*value >> shift) & mask; > + trace_memory_region_ops_write(mr, addr, tmp, size); > mr->ops->write(mr->opaque, addr, tmp, size); > } > > diff --git a/trace-events b/trace-events > index 002df83..3e0dd74 100644 > --- a/trace-events > +++ b/trace-events > @@ -1165,6 +1165,10 @@ kvm_vm_ioctl(int type, void *arg) "type %d, arg %p" > kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p" > kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d" > > +# memory.c > +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" > +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" > + Sorry about the late mail. Did you mean to have the stderr trace text to be the same? -Don Slutz > # 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)" > object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
On 08/05/2013 05:18 PM, Don Slutz wrote: >> diff --git a/trace-events b/trace-events >> index 002df83..3e0dd74 100644 >> --- a/trace-events >> +++ b/trace-events >> @@ -1165,6 +1165,10 @@ kvm_vm_ioctl(int type, void *arg) "type %d, arg >> %p" >> kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, >> type %d, arg %p" >> kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d" >> +# memory.c >> +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, >> unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" >> +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, >> unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" >> + > Sorry about the late mail. Did you mean to have the stderr trace text > to be the same? Yes, the name of the tracepoint should come before the text. See scripts/tracetool/backend/stderr.py for the generator source code. Paolo
diff --git a/memory.c b/memory.c index 1494e95..ac6f3c6 100644 --- a/memory.c +++ b/memory.c @@ -19,6 +19,7 @@ #include "qemu/bitops.h" #include "qom/object.h" #include "sysemu/kvm.h" +#include "trace.h" #include <assert.h> #include "exec/memory-internal.h" @@ -388,6 +389,7 @@ static void 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); *value |= (tmp & mask) << shift; } @@ -404,6 +406,7 @@ static void memory_region_read_accessor(MemoryRegion *mr, qemu_flush_coalesced_mmio_buffer(); } tmp = mr->ops->read(mr->opaque, addr, size); + trace_memory_region_ops_read(mr, addr, tmp, size); *value |= (tmp & mask) << shift; } @@ -417,6 +420,7 @@ static void memory_region_oldmmio_write_accessor(MemoryRegion *mr, uint64_t tmp; tmp = (*value >> shift) & mask; + trace_memory_region_ops_write(mr, addr, tmp, size); mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp); } @@ -433,6 +437,7 @@ static void memory_region_write_accessor(MemoryRegion *mr, qemu_flush_coalesced_mmio_buffer(); } tmp = (*value >> shift) & mask; + trace_memory_region_ops_write(mr, addr, tmp, size); mr->ops->write(mr->opaque, addr, tmp, size); } diff --git a/trace-events b/trace-events index 002df83..3e0dd74 100644 --- a/trace-events +++ b/trace-events @@ -1165,6 +1165,10 @@ kvm_vm_ioctl(int type, void *arg) "type %d, arg %p" kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type %d, arg %p" kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d" +# memory.c +memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" +memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %d" + # 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)" object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "%s->%s (%s:%d:%s)"
This is quite handy to debug softmmu targets. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- memory.c | 5 +++++ trace-events | 4 ++++ 2 files changed, 9 insertions(+)