Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough

[Blue Swirl]

On Mon, Aug 29, 2011 at 12:17 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Fri, Aug 26, 2011 at 8:06 PM, Blue Swirl <address@hidden> wrote:
>> Let guests inject tracepoint data via fw_cfg device.
>>
>> Signed-off-by: Blue Swirl <address@hidden>
>> ---
>> The patch is used like this:
>> ../configure --with-guest-trace-file=/src/openbios-devel/trace-events
>> make
>> sparc64-softmmu/qemu-system-sparc64 -trace file=foo
>> # ugly hack to combine the file, but my laziness^Wpython-fu is too
>> weak to add handling of "--guest-trace-file
>> /src/openbios-devel/trace-events" to simpletrace.py
>> cat ../trace-events /src/openbios-devel/trace-events >/tmp/trace-events
>> # examine trace file with OpenBIOS trace data with simpletrace.py
>> ../scripts/simpletrace.py /tmp/trace-events foo
>> ob_ide_read_blocks 0.000 dest=0xfff0bed0 blk=0x0 n=0x1
>> ob_ide_read_blocks 6491.806 dest=0xfff0bed0 blk=0x0 n=0x1
>>
>> An example of a generated guest-trace.c file:
>> /* This file is autogenerated by tracetool, do not edit. */
>> #include "trace.h"
>> #include "guest-trace.h"
>>
>> void guest_trace(uint64_t event_id, uint64_t arg1, uint64_t arg2,
>>                 uint64_t arg3, uint64_t arg4, uint64_t arg5, uint64_t arg6)
>>
>> {
>>    switch (event_id) {
>>
>>    case 0:
>>        trace_esp_do_command(arg1, arg2, arg3);
>>        break;
>>
>>    case 1:
>>        trace_ob_ide_pio_insw(arg1);
>>        break;
>>
>>    case 2:
>>        trace_ob_ide_read_blocks(arg1, arg2, arg3);
>>        break;
>>
>>    default:
>>        break;
>>   }
>> }
>>
>> ---
>>  Makefile.objs     |   15 +++++++++++-
>>  configure         |    9 +++++++-
>>  guest-trace.h     |    3 ++
>>  hw/fw_cfg.c       |   31 +++++++++++++++++++++++++++
>>  hw/fw_cfg.h       |   10 ++++++--
>>  scripts/tracetool |   60 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  6 files changed, 120 insertions(+), 8 deletions(-)
>>  create mode 100644 guest-trace.h
>
> The ability to trace from the guest can be handy, so I think we should
> have this feature.  Please add documentation on how to hook it up
> (e.g. how people would use this for other firmware/guest code and/or
> other architectures).

OK. The format should be the same as raw simpletrace data, but words
always in little endian like used elsewhere with fw_cfg. BTW,
currently simpletrace file format depends on host endianness, is that
intentional?

> Guest and QEMU need to agree on event IDs.  The guest code needs to be
> built with QEMU and they may not function with other QEMU builds or
> guest builds.  This is fine for development but not feasible when QEMU
> and the guest code are built or provided separately.

Yes, for this part I'm not so happy about the code. It should be
possible to replace the (very simple) switch with (very complicated)
code to dynamically read and handle the guest tracepoint list, but
that can be added later if ever.

> I suggest we merge this as a development feature that can be used when
> bringing up new architectures, debugging guest code, or for some types
> of performance work.  This feature falls under the Do-It-Yourself
> area, where things could break relatively easy but developers who wish
> to use it should be able to get it working in their area.
>
>> +linetog_all()
>> +{
>> +    local name args argc arg
>> +    name=$(get_name "$1")
>> +    argc=$(get_argc "$1")
>> +    fieldno=1
>
> local fieldno

Thanks for the review.