Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic consistency checks
This proposed text sounds good. Better English is to say "concepts" rather than "conceptions". My plugin currently allocates its own unique user data structure on every call to the instrumentation-time callback. This piece of user data captures the transient data presented from qemu. What's missing from the interface is a callback when qemu can guarantee that the run-time callback will never be called again with this piece of user data. At that point I would free my piece of user data. I'm not too worried about this memory loss, yet. I ran ubuntu for 3 days on qemu+plugins and only observed a tolerable growth in qemu's memory consumption. From: Alex Bennée Sent: Friday, January 24, 2020 11:44 AM To: Robert Henry Cc: Laurent Desnogues ; qemu-devel@nongnu.org Subject: Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic consistency checks Robert Henry writes: > I found at least one problem with my plugin. > > I was assuming that the insn data from > struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i); > could be passed into qemu_plugin_register_vcpu_insn_exec_cb both as the 1st > argument AND as the user data last argument. This assumed that insn would > persist and be unique from when qemu_plugin_register_vcpu_insn_exec_cb was > called to when the execution-time callback (vcpu_insn_exec_before) was called. > > This assumption is not true. > > I now capture pieces of *insn into my own persistent data structure, and pass > that in as void *udata, my problems went away. > > I think this lack of persistence of insn should be documented, or > treated as a bug to be fixed. I thought I had done this but it turns out I only mentioned it for hwaddr: /* * qemu_plugin_get_hwaddr(): * @vaddr: the virtual address of the memory operation * * For system emulation returns a qemu_plugin_hwaddr handle to query * details about the actual physical address backing the virtual * address. For linux-user guests it just returns NULL. * * This handle is *only* valid for the duration of the callback. Any * information about the handle should be recovered before the * callback returns. */ But the concept of handle lifetime is true for all the handles. I propose something like this for the docs: --8<---cut here---start->8--- docs/devel: document query handle lifetimes I forgot to document the lifetime of handles in the developer documentation. Do so now. Signed-off-by: Alex Bennée 1 file changed, 11 insertions(+), 2 deletions(-) docs/devel/tcg-plugins.rst | 13 +++-- modified docs/devel/tcg-plugins.rst @@ -51,8 +51,17 @@ about how QEMU's translation works to the plugins. While there are conceptions such as translation time and translation blocks the details are opaque to plugins. The plugin is able to query select details of instructions and system configuration only through the -exported *qemu_plugin* functions. The types used to describe -instructions and events are opaque to the plugins themselves. +exported *qemu_plugin* functions. + +Query Handle Lifetime +- + +Each callback provides an opaque anonymous information handle which +can usually be further queried to find out information about a +translation, instruction or operation. The handles themselves are only +valid during the lifetime of the callback so it is important that any +information that is needed is extracted during the callback and saved +by the plugin. Usage = --8<---cut here---end--->8--- -- Alex Bennée
Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic consistency checks
Robert Henry writes: > I found at least one problem with my plugin. > > I was assuming that the insn data from > struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i); > could be passed into qemu_plugin_register_vcpu_insn_exec_cb both as the 1st > argument AND as the user data last argument. This assumed that insn would > persist and be unique from when qemu_plugin_register_vcpu_insn_exec_cb was > called to when the execution-time callback (vcpu_insn_exec_before) was called. > > This assumption is not true. > > I now capture pieces of *insn into my own persistent data structure, and pass > that in as void *udata, my problems went away. > > I think this lack of persistence of insn should be documented, or > treated as a bug to be fixed. I thought I had done this but it turns out I only mentioned it for hwaddr: /* * qemu_plugin_get_hwaddr(): * @vaddr: the virtual address of the memory operation * * For system emulation returns a qemu_plugin_hwaddr handle to query * details about the actual physical address backing the virtual * address. For linux-user guests it just returns NULL. * * This handle is *only* valid for the duration of the callback. Any * information about the handle should be recovered before the * callback returns. */ But the concept of handle lifetime is true for all the handles. I propose something like this for the docs: --8<---cut here---start->8--- docs/devel: document query handle lifetimes I forgot to document the lifetime of handles in the developer documentation. Do so now. Signed-off-by: Alex Bennée 1 file changed, 11 insertions(+), 2 deletions(-) docs/devel/tcg-plugins.rst | 13 +++-- modified docs/devel/tcg-plugins.rst @@ -51,8 +51,17 @@ about how QEMU's translation works to the plugins. While there are conceptions such as translation time and translation blocks the details are opaque to plugins. The plugin is able to query select details of instructions and system configuration only through the -exported *qemu_plugin* functions. The types used to describe -instructions and events are opaque to the plugins themselves. +exported *qemu_plugin* functions. + +Query Handle Lifetime +- + +Each callback provides an opaque anonymous information handle which +can usually be further queried to find out information about a +translation, instruction or operation. The handles themselves are only +valid during the lifetime of the callback so it is important that any +information that is needed is extracted during the callback and saved +by the plugin. Usage = --8<---cut here---end--->8--- -- Alex Bennée
Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic consistency checks
I found at least one problem with my plugin.
I was assuming that the insn data from
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
could be passed into qemu_plugin_register_vcpu_insn_exec_cb both as the 1st
argument AND as the user data last argument. This assumed that insn would
persist and be unique from when qemu_plugin_register_vcpu_insn_exec_cb was
called to when the execution-time callback (vcpu_insn_exec_before) was called.
This assumption is not true.
I now capture pieces of *insn into my own persistent data structure, and pass
that in as void *udata, my problems went away.
I think this lack of persistence of insn should be documented, or treated as a
bug to be fixed.
From: Alex Bennée
Sent: Friday, January 24, 2020 8:36 AM
To: Robert Henry
Cc: qemu-devel@nongnu.org
Subject: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic
consistency checks
Robert Henry writes:
> I wrote a QEMU plugin for aarch64 where the insn and mem callbacks
> print out the specifics of the guest instructions as they are
> "executed". I expect this trace stream to be well behaved but it is
> not.
Can you post your plugin? It's hard to diagnose what might be wrong
without the actual code.
> By well-behaved, I expect memory insns print out some memory details,
> non-memory insns don't print anything, and the pc only changes after a
> control flow instruction.
Exactly how are you tracking the PC? You should have the correct PC as
you instrument each instruction. Are you saying qemu_plugin_insn_vaddr()
doesn't report a different PC for each instrumented instruction in a block?
> I don't see that gross correctness about 2%
> of the time.
>
>
> 1. I'm using qemu at tag v4.2.0 (or master head; it doesn't matter),
> running on a x86_64 host.
> 2. I build qemu using ./configure --disable-sdl --enable-gtk
> --enable-plugins --enable-debug --target-list=aarch64-softmmu
> aarch64-linux-user
> 3. I execute qemu from its build area
> build/aarch64-linux-user/qemu-aarch64, with flags --cpu cortex-a72 and the
> appropriate args to --plugin ... -d plugin -D .
> 4. I'm emulating a simple C program in linux emulation mode.
> 5. The resulting qemu execution is valgrind clean (eg, I run qemu under
> valgrind) for my little program save for memory leaks I reported a few days
> ago.
>
> Below is an example of my trace output (the first int printed is the
> cpu_index, checked to be always 0). Note that the ldr instruction at 0x41a608
> sometimes reports a memop, but most of the time it doesn't. Note that
> 0x41a608 is seen, by trace, running back to back. Note that (bottom of trace)
> that the movz instruction reports a memop. (The executed code comes from
> glibc _dl_aux_init, executed before main() is called.)
>
> How should this problem be tackled? I can't figure out how to make each tcg
> block be exactly 1 guest (aarch64) insn, which is where I'd first start out.
>
> 0 0x0041a784 0x0041a784 0xf1000c3f cmp x1, #3
> 0 0x0041a788 0x0041a788 0x54fff401 b.ne #0xfe80
> 0 0x0041a78c 0x0041a78c 0x52800033 movz w19, #0x1
> 0 0x0041a790 0x0041a790 0xf9400416 ldr x22, [x0, #8] 0 mem
> {3 0 0 0} 0x004000800618
> 0 0x0041a794 0x0041a794 0x179d b #0xfe74
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]! 0
> mem {3 0 0 0} 0x004000800620
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]! 0
> mem {3 0 0 0} 0x004000800630
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a608 0x0041a608 0xf8410c01 ldr x1, [x0, #0x10]!
> 0 0x0041a60c 0x0041a60c 0xb4000221 cbz x1, #0x44
> 0
