Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic consistency checks

[Robert Henry]

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 <address@hidden>
Sent: Friday, January 24, 2020 11:44 AM
To: Robert Henry <address@hidden>
Cc: Laurent Desnogues <address@hidden>; address@hidden <address@hidden>
Subject: Re: [EXTERNAL] Re: QEMU for aarch64 with plugins seems to fail basic consistency checks

 

Robert Henry <address@hidden> 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 <address@hidden>

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