Re: [Qemu-devel] [PATCH v3 05/50] docs/devel: add plugins.rst design document

[Pranith Kumar]

Hi,

On Fri, Jun 14, 2019 at 10:21 AM Alex Bennée <address@hidden> wrote:
>
> This is mostly extracted from Emilio's more verbose commit comments
> with some additional verbiage from me.
>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  docs/devel/index.rst   |  1 +
>  docs/devel/plugins.rst | 99 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 100 insertions(+)
>  create mode 100644 docs/devel/plugins.rst
>
> diff --git a/docs/devel/index.rst b/docs/devel/index.rst
> index 2a4ddf40ad..7e6d20c970 100644
> --- a/docs/devel/index.rst
> +++ b/docs/devel/index.rst
> @@ -21,3 +21,4 @@ Contents:
>     testing
>     decodetree
>     secure-coding-practices
> +   plugins
> diff --git a/docs/devel/plugins.rst b/docs/devel/plugins.rst
> new file mode 100644
> index 0000000000..b0c30375ef
> --- /dev/null
> +++ b/docs/devel/plugins.rst
> @@ -0,0 +1,99 @@
> +..
> +   Copyright (C) 2017, Emilio G. Cota <address@hidden>
> +   Copyright (c) 2019, Linaro Limited
> +   Written by Emilio Cota and Alex Bennée
> +
> +================
> +QEMU TCG Plugins
> +================
> +
> +QEMU TCG plugins provide a way for users to run experiments taking
> +advantage of the total system control emulation can have over a guest.
> +It provides a mechanism for plugins to subscribe to events during
> +translation and execution and optionally callback into the plugin
> +during these events.
> +
> +API Stability
> +=============
> +
> +This is a new feature for QEMU and it does allow people to develop
> +out-of-tree plugins than can be dynamically linked into a running QEMU

s/than/that/

> +process. However the project reserves the right to change or break the
> +API should it need to do so.
> +
> +Exposure of QEMU internals
> +--------------------------
> +
> +The plugin architecture actively avoids leaking implementation details
> +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.
> +
> +Usage
> +=====
> +
> +The QEMU binary needs to be compiled for plugin support:
> +
> +::
> +    configure --enable-plugins
> +
> +Once built a program can be run with multiple plugins loaded each with
> +their own arguments:
> +
> +::
> +    $QEMU $OTHER_QEMU_ARGS \
> +      -plugin tests/plugin/libhowvec.so,arg=inline,arg=hint \
> +      -plugin tests/plugin/libhotblocks.so

I think this might be a good place to describe what these arguments are.

> +
> +Plugin Life cycle
> +=================
> +
> +First the plugin is loaded and the public qemu_plugin_install function
> +is called. The plugin with then register callbacks for various plugin

s/with/will/

> +events. Generally at least the atexit_cb is registered so the plugin
> +can dump its information at the end of a run.

Is that a hard requirement?

> +
> +When a registered event occurs the plugin callback is called. The

I would prefer 'callback is invoked'.

> +callbacks may provide additional information. In the case of a
> +translation event the plugin has an option to enumerate the
> +instructions in a block of instructions and optionally register
> +callbacks to some or all instructions when they are executed.
> +
> +There is also a facility to add an inline event where code to
> +increment a counter can be directly inlined with the translation.
> +Currently only a simple increment is supported. This is not atomic so
> +the plugin must either keep it's counters separated and indexed by CPU
> +or use a callback which can ensure atomicity.
> +
> +Finally when QEMU exits all the registered atexit callbacks are called

Add period at end of sentence and preferably "s/called/invoked/"

> +
> +Internals
> +=========
> +
> +Locking
> +-------
> +
> +We have to ensure we cannot deadlock, particularly under MTTCG. For
> +this we acquire a lock when called from plugin code. We also keep the
> +list of callbacks under RCU so that we do not have to hold the lock
> +when calling the callbacks. This is also for performance, since some
> +callbacks (e.g. memory access callbacks) might be called very
> +frequently.
> +
> +  * A consequence of this is that we keep our own list of CPUs, so that
> +    we do not have to worry about locking order wrt cpu_list_lock.
> +  * Use a recursive lock, since we can get registration calls from
> +    callbacks.
> +
> +As a result registering/unregistering callbacks is "slow", since it
> +takes a lock. But this is very infrequent; we want performance when
> +calling (or not calling) callbacks, not when registering them. Using
> +RCU is great for this.
> +
> +We support the uninstallation of a plugin at any time (e.g. from plugin
> +callbacks). This means some callbacks might still be called after the 
> uninstall
> +function returns. The plugin isn't completely uninstalled until the
> +safe work has executed while all vCPUs are quiescent.

Isn't this when the atexit callback is invoked? Might add that to make
it clearer.