qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Inter-plugin interactions with QPP
@ 2022-12-13 21:37 Andrew Fasano
  2022-12-13 21:37 ` [PATCH 1/8] docs/devel: describe QPP API Andrew Fasano
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

Hello,

This is a series of patches expanding the TCG plugin system to add what we're
calling the "QEMU Plugin-to-Plugin (QPP)" interface that allows for
interactions between TCG plugins. This is a follow up to the Sept 1 2022 RFC
"Support interactions between TCG plugins." The goal of this interface is to
enable plugins to expand on other plugins and reduce code duplication. This
patch series includes documentation, tests and commented code, but a
high-level summary is below along with a discussion of the current
implementation, how it would affect plugin developers, and a summary of the
changes since the original RFC.

**Summary**
The QPP interface allows two types of interactions between plugins:
1) Exported functions: A plugin may wish to allow other plugins to call
one of the functions it has defined. To do this, the plugin must mark
the function definition as publicly visible with the QEMU_PLUGIN_EXPORT
macro and place a definition in an included header file using the
QPP_FUN_PROTOTYPE macro. Other plugins can then include this header and
call the exported function by combining the name of the target plugin
with the name of the exported function and "_qpp" at the end.
For example, consider a hypothetical plugin that collects a list of
cache misses. This plugin could export two functions using the QPP
interface: one to allow another plugin to query this list and another
to empty the list. This would enable the development of another plugin
that examines guest CPU state to identify process changes and reports
the cache misses per process. With the QPP interface, this second plugin
would not need to duplicate any logic from the first.

2) Callbacks: Multiple plugins may wish to take some action when some
event of interest occurs inside a running guest. To support modularity
and reduce code duplication, the QPP callback system allows this logic
to be contained in single plugin that detects whenever a given event
occurs and exposes a callback with a given name. Another plugin can then
request to have one of its own functions run whenever this event occurs.
Additional plugins could also use this same callback to run additional
logic whenever this event occurs.

For example, consider (again) a hypothetical plugin that detects when
the current guest process changes by analyzing the guest CPU state. This
plugin could define a callback named "on_process_change" and trigger
this callback event whenever it detects a process change. Other plugins
could then be developed that take various actions on process changes by
registering internal functions to run on this event.
These patches and examples are inspired by the PANDA project
(https://panda.re and https://github.com/panda-re/panda), a fork of QEMU
modified to support dynamic program analysis and reverse engineering.
PANDA also includes a large plugin system with a similar interface for
interactions between plugins. We are a part of the PANDA team and have
seen how the ability for plugins to interact with other plugins reduces
code duplication and enables the creation of many useful plugins.

**Implementation Overview**
These patches increment the TCG plugin version from 1 to 2 and require
plugins of v2 or greater to specify a plugin name. During plugin load, an
error is raised if two plugins are loaded that use the same name.
Using these unique names, plugins can import functions from other
plugins and register local functions to be run on callback events
managed by other plugins. To accomplish this, these patches add
the following functions to the core plugin code:
 - qemu_plugin_import_function: this is used by the QPP_FUN_PROTOTYPE
   macro to find and resolve the handle of the function to be
   imported given the target plugin name and function name.
 - qemu_plugin_create_callback: this can be called by plugins to
   create a new callback using the unique plugin id and the callback
   name.
 - qemu_plugin_run_callback: this is called by plugins to define when
   registered callback functions get run for the named callback and
   specify the args that are passed to those functions.
 - qemu_plugin_reg_callback: this can be used by plugins to register
   functions on another plugin's defined callback.
 - qemu_plugin_unreg_callback: this removes a previously registered
   function from the callback's list of registered functions if it
   exists there.

The QPP implementation is mostly contained within the api, but we
did add a helpful macro for qemu_plugin_import_function inside a
header file plugin-qpp.h called QPP_FUN_PROTOTYPE. This macro
must be used after defining the qemu_plugin_name variable (would
love to change this part of the design if anyone has better ideas).

The QPP_FUN_PROTOYPE macro enables a plugin to expose a function
it defines to other plugins. This macro enables a plugin to provide a single
header file that both defines the exported functions for the plugin
that provides them as well as providing a definition and auto-resolving the
function pointers for other plugins that wish to call them. This macro is used
in a header file that is included by both the plugin that defines a function as
well as plugins that wish to use the function. This prototype creates a
constructor function that runs on plugin load. If the target plugin
name differs from the value of the current plugin name, this function
will use qemu_plugin_import_function to resolve the target function in
that plugin. If this fails, it will print a warning and abort.

Finally, these patches include a pair of test plugins, qpp_srv and
qpp_client. The former provides a trivial QPP callback which simply runs
when the plugin is unloaded as well as two functions to add and subtract
integers. The latter of these plugins registers a function to run
on this callback and uses the two exported functions. These plugins are
found in tests/plugin_qpp and have been integrated into the make check-tcg
tests.

**Changes Since the 9/1/22 RFC**
Based on the feedback we received from Alex, we changed our design to use
the QEMU plugin APIs instead of allowing plugins to directly dlopen and
interact with one another.
We bumped the QPP version number and mandated that developers specify a unique
name for each plugin instead of using the source filenames as the plugin names.
We moved our pair of example plugins to the tests directory and configured
them to be run and tested as part of the check-tcg tests.

**Next steps**
We are working on building some more useful example plugins that builds on this
API. Pending feedback on this patch series, we would be happy to either merge
these API changes on their own or hold off until such these example plugins
are ready.

We have developed some plugins already, but our initial examples depend on
some poorly-written APIs we've added to access guest registers and memory.
Those APIs aren't ready for upstreaming yet, which is why we aren't including
those patches and example plugins in this patch series. However the code is
available in our tree if anyone is interested:
  - Register accessing: https://github.com/panda-re/qemu/commit/b97c5a56edd0ba3b5f6ab16bf531ac1f7abaac04
  - Memory accessing: https://github.com/panda-re/qemu/commit/dba9d1ceffeeeddf2b5232d64ca97147a4f5a3a5

Using the QPP system from this patch series and the ability to access guest
registers and memory, we developed have a set of three of plugins that provide
OS introspection, syscall detection, and logging such that on every system
call in a Linux guest we can record the system call number and
information on the current running process. These generate output like:
  Syscall at pc 7f0b815478cd: number 3: Process 'init', pid 211, ppid 1
  Syscall at pc 7fff775feb58: number 228: Process 'systemd-udevd', pid 137, ppid 1

As our register and memory access APIs aren't yet ready to merge, we
haven't included these plugins as a part of this patch series, but their
code is also available in our tree:
  - Linux OS Introspection: https://github.com/panda-re/qemu/blob/ppp_beta_with_plugins/contrib/plugins/osi_linux/
  - Syscall detection: https://github.com/panda-re/qemu/blob/ppp_beta_with_plugins/contrib/plugins/syscalls.c
  - Syscall logging with process details: https://github.com/panda-re/qemu/blob/ppp_beta_with_plugins/contrib/plugins/syscalls_logger.c

We plan to explore integrating this with qemu's existing user-mode syscall
tracing system and would love to eventually merge these plugins if there's
interest and the necessary APIs (QPP + register/memory access) become
available.

We welcome any feedback for this patch series and the next steps we're
interested in pursuing.
Thank you,
Andrew Fasano & Elysia Witham

Elysia Witham (8):
  docs/devel: describe QPP API
  plugins: version 2, require unique plugin names
  plugins: add id_to_plugin_name
  plugins: add core API functions for QPP callbacks
  plugins: implement QPP callbacks
  plugins: implement QPP import function
  include/qemu: added macro for QPP import function
  tests: build and run QPP tests

 docs/devel/tcg-plugins.rst                |  91 ++++++++++++++-
 include/qemu/plugin-qpp.h                 |  54 +++++++++
 include/qemu/qemu-plugin.h                |  68 ++++++++++-
 plugins/api.c                             | 132 ++++++++++++++++++++++
 plugins/core.c                            |  47 ++++++++
 plugins/loader.c                          |  61 +++++++++-
 plugins/plugin.h                          |  24 ++++
 plugins/qemu-plugins.symbols              |   5 +
 tests/Makefile.include                    |   2 +-
 tests/meson.build                         |   1 +
 tests/plugin/bb.c                         |   1 +
 tests/plugin/empty.c                      |   1 +
 tests/plugin/insn.c                       |   1 +
 tests/plugin/mem.c                        |   1 +
 tests/plugin/syscall.c                    |   1 +
 tests/plugin_qpp/meson.build              |   7 ++
 tests/plugin_qpp/qpp_client.c             |  32 ++++++
 tests/plugin_qpp/qpp_srv.c                |  37 ++++++
 tests/plugin_qpp/qpp_srv.h                |  12 ++
 tests/tcg/Makefile.target                 |  29 +++++
 tests/tcg/aarch64/Makefile.softmmu-target |   2 +
 tests/tcg/arm/Makefile.softmmu-target     |   1 +
 tests/tcg/arm/Makefile.target             |   2 +
 tests/tcg/multiarch/Makefile.target       |   2 +
 24 files changed, 605 insertions(+), 9 deletions(-)
 create mode 100644 include/qemu/plugin-qpp.h
 create mode 100644 tests/plugin_qpp/meson.build
 create mode 100644 tests/plugin_qpp/qpp_client.c
 create mode 100644 tests/plugin_qpp/qpp_srv.c
 create mode 100644 tests/plugin_qpp/qpp_srv.h

-- 
2.34.1



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/8] docs/devel: describe QPP API
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2023-03-09 14:15   ` Alex Bennée
  2022-12-13 21:37 ` [PATCH 2/8] plugins: version 2, require unique plugin names Andrew Fasano
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

The new QPP API allows plugin-to-plugin interaction for creating
and using callbacks as well as importing and exporting functions.
The new test plugins qpp_srv and qpp_client demonstrate how
plugins use the new API.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 docs/devel/tcg-plugins.rst | 91 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 1 deletion(-)

diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 9740a70406..70ac09b96b 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -281,6 +281,14 @@ run::
   160          1      0
   135          1      0
 
+- tests/plugins/qpp_srv.c & tests/plugins/qpp_client.c
+
+These plugins demonstrate QPP interactions. The qpp_srv plugin defines
+a few exported functions and its own callback which are then imported and
+used by the qpp_client plugin. The qpp_client plugin registers its own
+function to run on qpp_srv's defined callback. The tests for these plugins
+are modified as both plugins must be loaded in order to work.
+
 - contrib/plugins/hotblocks.c
 
 The hotblocks plugin allows you to examine the where hot paths of
@@ -573,6 +581,88 @@ The plugin has a number of arguments, all of them are optional:
   configuration arguments implies ``l2=on``.
   (default: N = 2097152 (2MB), B = 64, A = 16)
 
+Plugin-to-Plugin Interactions
+-----------------------------
+
+Plugins may interact with other plugins through the QEMU Plugin-to-Plugin
+("QPP") API by including ``<plugin-qpp.h>`` in addition to ``<qemu_plugin.h>``.
+This API supports direct function calls between plugins. An inter-plugin
+callback system is supported within the core code as long as
+``qemu_plugin_version >= 2``.
+
+Plugin names
+~~~~~~~~~~~~
+Plugin names must be exported as ``qemu_plugin_name`` to use the QPP API in the same way
+``qemu_plugin_version`` is exported. This name can then be used by other plugins
+to import functions and use callbacks belonging to that plugin.
+
+Plugin dependencies
+~~~~~~~~~~~~~~~~~~~
+For a plugin to use another plugin's functions or callbacks it must declare that
+dependency through exporting ``qemu_plugin_uses`` which is a string array containing
+names of plugins used by that plugin. Those plugins must be loaded first. Note that this
+array must be null terminated, e.g. ``{plugin_a, NULL}``.
+
+QPP function calls
+~~~~~~~~~~~~~~~~~~
+When a plugin (e.g., ``plugin_a``) wishes to make some of its functions (e.g.,
+``func_1``) available to other plugins, it must:
+
+1. Mark the function definition with the ``QEMU_PLUGIN_EXPORT`` macro. For
+example : ``QEMU_PLUGIN_EXPORT int func_1(int x) {...}``
+2. Provide prototypes for exported functions in a header file using the macro
+``QPP_FUN_PROTOTYPE`` with arguments of the plugin's name, the function's
+return type, the function's name, and any arguments the function takes. For
+example: ``QPP_FUN_PROTOTYPE(my_plugin, int, do_add, int);``.
+3. Import this header from the plugin.
+
+When other plugins wish to use the functions exported by ``plugin_a``, they
+must:
+
+1. Import the header file with the function prototype(s).
+2. Call the function when desired by combining the target plugin name, an
+   underscore, and the target function name with ``_qpp`` on the end,
+   e.g., ``plugin_a_func_1_qpp()``.
+
+QPP callbacks
+~~~~~~~~~~~~~
+
+The QPP API also allows a plugin to define callback events and for other plugins
+to request to be notified whenever these events happens. The plugin that defines
+the callback is responsible for triggering the callback when it so wishes. Other
+plugins that wish to be notified on these events must define a function of an
+appropriate type and register it to run on this event.
+In particular, these plugins must:
+
+
+When a plugin (e.g., ``plugin_a``) wishes to define a callback (an event that
+other plugins can request to be notified about), it must:
+
+1. Define the callback using the ``qemu_plugin_create_callback`` function which
+   takes two arguments: the unique ``qemu_plugin_id_t id`` and the callback name.
+2. Call ``qemu_plugin_run_callback`` at appropriate places in the code to call registered
+   callback functions. It takes four arguments: the unique ``qemu_plugin_id_t id``,
+   the callback name, and the callback arguments which are standardized to be
+   ``gpointer evdata, gpointer udata``. The callback arguments point to two structs
+   which are defined by the plugin and can vary based on the use case of the callback.
+
+When other plugins wish to register a function to run on such an event, they
+must:
+
+1. Define a function that matches the ``cb_func_t`` type:
+   ``typedef void (*cb_func_t) (gpointer evdata, gpointer udata)``.
+2. Register this function to be run on the plugin defined callback using
+   ``qemu_plugin_reg_callback``. This function takes three arguments: the name of the
+   plugin which defines the callback, the callback name, and a ``cb_func_t`` function
+   pointer.
+
+When other plugins wish to unregister a function which is registered to run on a plugin
+defined event callback, they must:
+
+1. Call ``qemu_plugin_unreg_callback``. This function takes the same arguments as
+   ``qemu_plugin_reg_callback``. It will return true if it successfully finds and
+   unregisters the function.
+
 API
 ---
 
@@ -581,4 +671,3 @@ The following API is generated from the inline documentation in
 include the full kernel-doc annotations.
 
 .. kernel-doc:: include/qemu/qemu-plugin.h
-
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/8] plugins: version 2, require unique plugin names
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
  2022-12-13 21:37 ` [PATCH 1/8] docs/devel: describe QPP API Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2023-03-09 14:17   ` Alex Bennée
  2022-12-13 21:37 ` [PATCH 3/8] plugins: add id_to_plugin_name Andrew Fasano
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

In order for the QPP API to resolve interactions between plugins,
plugins must export their own names which cannot match any other
loaded plugins.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 include/qemu/qemu-plugin.h |  2 +-
 plugins/core.c             | 12 +++++++++
 plugins/loader.c           | 50 +++++++++++++++++++++++++++++++++-----
 plugins/plugin.h           |  7 ++++++
 tests/plugin/bb.c          |  1 +
 tests/plugin/empty.c       |  1 +
 tests/plugin/insn.c        |  1 +
 tests/plugin/mem.c         |  1 +
 tests/plugin/syscall.c     |  1 +
 9 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index d0e9d03adf..5326f33ce8 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -51,7 +51,7 @@ typedef uint64_t qemu_plugin_id_t;
 
 extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
 
-#define QEMU_PLUGIN_VERSION 1
+#define QEMU_PLUGIN_VERSION 2
 
 /**
  * struct qemu_info_t - system information for plugins
diff --git a/plugins/core.c b/plugins/core.c
index ccb770a485..5fbdcb5768 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -236,6 +236,18 @@ void qemu_plugin_vcpu_exit_hook(CPUState *cpu)
     qemu_rec_mutex_unlock(&plugin.lock);
 }
 
+int name_to_plugin_version(const char *name)
+{
+    struct qemu_plugin_ctx *ctx;
+    QTAILQ_FOREACH(ctx, &plugin.ctxs, entry) {
+        if (strcmp(ctx->name, name) == 0) {
+            return ctx->version;
+        }
+    }
+    warn_report("Could not find any plugin named %s.", name);
+    return -1;
+}
+
 struct plugin_for_each_args {
     struct qemu_plugin_ctx *ctx;
     qemu_plugin_vcpu_simple_cb_t cb;
diff --git a/plugins/loader.c b/plugins/loader.c
index 88c30bde2d..12c0680e03 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -177,7 +177,7 @@ QEMU_DISABLE_CFI
 static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, Error **errp)
 {
     qemu_plugin_install_func_t install;
-    struct qemu_plugin_ctx *ctx;
+    struct qemu_plugin_ctx *ctx, *ctx2;
     gpointer sym;
     int rc;
 
@@ -208,17 +208,55 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, E
                    desc->path, g_module_error());
         goto err_symbol;
     } else {
-        int version = *(int *)sym;
-        if (version < QEMU_PLUGIN_MIN_VERSION) {
+        ctx->version = *(int *)sym;
+        if (ctx->version < QEMU_PLUGIN_MIN_VERSION) {
             error_setg(errp, "Could not load plugin %s: plugin requires API version %d, but "
                        "this QEMU supports only a minimum version of %d",
-                       desc->path, version, QEMU_PLUGIN_MIN_VERSION);
+                       desc->path, ctx->version, QEMU_PLUGIN_MIN_VERSION);
             goto err_symbol;
-        } else if (version > QEMU_PLUGIN_VERSION) {
+        } else if (ctx->version > QEMU_PLUGIN_VERSION) {
             error_setg(errp, "Could not load plugin %s: plugin requires API version %d, but "
                        "this QEMU supports only up to version %d",
-                       desc->path, version, QEMU_PLUGIN_VERSION);
+                       desc->path, ctx->version, QEMU_PLUGIN_VERSION);
             goto err_symbol;
+        } else if (ctx->version < QPP_MINIMUM_VERSION) {
+            ctx->name = NULL;
+        } else {
+            if (!g_module_symbol(ctx->handle, "qemu_plugin_name", &sym)) {
+                error_setg(errp, "Could not load plugin %s: plugin does not "
+                           "declare plugin name %s",
+                           desc->path, g_module_error());
+                goto err_symbol;
+            }
+            ctx->name = (const char *)strdup(*(const char **)sym);
+            QTAILQ_FOREACH(ctx2, &plugin.ctxs, entry) {
+                if (strcmp(ctx2->name, ctx->name) == 0) {
+                    error_setg(errp, "Could not load plugin %s as the name %s "
+                               "is already in use by plugin at %s",
+                               desc->path, ctx->name, ctx2->desc->path);
+                    goto err_symbol;
+                }
+            }
+            if (g_module_symbol(ctx->handle, "qemu_plugin_uses", &sym)) {
+                const char **dependencies = &(*(const char **)sym);
+                bool found = false;
+                while (*dependencies) {
+                    found = false;
+                    QTAILQ_FOREACH(ctx2, &plugin.ctxs, entry) {
+                        if (strcmp(ctx2->name, *dependencies) == 0) {
+                            dependencies++;
+                            found = true;
+                            break;
+                        }
+                    }
+                    if (!found) {
+                        error_setg(errp, "Could not load plugin %s as it is "
+                                   "dependent on %s which is not loaded",
+                                   ctx->name, *dependencies);
+                        goto err_symbol;
+                    }
+                }
+            }
         }
     }
 
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 5eb2fdbc85..ce885bfa98 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -16,6 +16,7 @@
 #include "qemu/qht.h"
 
 #define QEMU_PLUGIN_MIN_VERSION 0
+#define QPP_MINIMUM_VERSION 2
 
 /* global state */
 struct qemu_plugin_state {
@@ -50,6 +51,8 @@ struct qemu_plugin_state {
 struct qemu_plugin_ctx {
     GModule *handle;
     qemu_plugin_id_t id;
+    const char *name;
+    int version;
     struct qemu_plugin_cb *callbacks[QEMU_PLUGIN_EV_MAX];
     QTAILQ_ENTRY(qemu_plugin_ctx) entry;
     /*
@@ -64,6 +67,8 @@ struct qemu_plugin_ctx {
 
 struct qemu_plugin_ctx *plugin_id_to_ctx_locked(qemu_plugin_id_t id);
 
+struct qemu_plugin_ctx *plugin_name_to_ctx_locked(const char* name);
+
 void plugin_register_inline_op(GArray **arr,
                                enum qemu_plugin_mem_rw rw,
                                enum qemu_plugin_op op, void *ptr,
@@ -97,4 +102,6 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
 
 void exec_inline_op(struct qemu_plugin_dyn_cb *cb);
 
+int name_to_plugin_version(const char *name);
+
 #endif /* PLUGIN_H */
diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index 7d470a1011..c04e5aaf90 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -15,6 +15,7 @@
 #include <qemu-plugin.h>
 
 QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "bb";
 
 typedef struct {
     GMutex lock;
diff --git a/tests/plugin/empty.c b/tests/plugin/empty.c
index 8fa6bacd93..0f3d2b92b9 100644
--- a/tests/plugin/empty.c
+++ b/tests/plugin/empty.c
@@ -14,6 +14,7 @@
 #include <qemu-plugin.h>
 
 QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "empty";
 
 /*
  * Empty TB translation callback.
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index cd5ea5d4ae..3f71138139 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -15,6 +15,7 @@
 #include <qemu-plugin.h>
 
 QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "insn";
 
 #define MAX_CPUS 8 /* lets not go nuts */
 
diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
index 4570f7d815..35e5d7fe2a 100644
--- a/tests/plugin/mem.c
+++ b/tests/plugin/mem.c
@@ -15,6 +15,7 @@
 #include <qemu-plugin.h>
 
 QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "mem";
 
 static uint64_t inline_mem_count;
 static uint64_t cb_mem_count;
diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c
index 96040c578f..922bdbd2e6 100644
--- a/tests/plugin/syscall.c
+++ b/tests/plugin/syscall.c
@@ -15,6 +15,7 @@
 #include <qemu-plugin.h>
 
 QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "syscall";
 
 typedef struct {
     int64_t num;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/8] plugins: add id_to_plugin_name
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
  2022-12-13 21:37 ` [PATCH 1/8] docs/devel: describe QPP API Andrew Fasano
  2022-12-13 21:37 ` [PATCH 2/8] plugins: version 2, require unique plugin names Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2023-03-09 14:45   ` Alex Bennée
  2022-12-13 21:37 ` [PATCH 4/8] plugins: add core API functions for QPP callbacks Andrew Fasano
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

Plugins will pass their unique id when creating callbacks to
ensure they are associated with the correct plugin. This
internal function resolves those ids to the declared names.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 plugins/core.c   | 12 ++++++++++++
 plugins/plugin.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/plugins/core.c b/plugins/core.c
index 5fbdcb5768..6a50b4a6e6 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -248,6 +248,18 @@ int name_to_plugin_version(const char *name)
     return -1;
 }
 
+const char *id_to_plugin_name(qemu_plugin_id_t id)
+{
+    const char *plugin = plugin_id_to_ctx_locked(id)->name;
+    if (plugin) {
+        return plugin;
+    } else {
+        warn_report("Unnamed plugin cannot use QPP, not supported in plugin "
+                    "version. Please update plugin.");
+        return NULL;
+    }
+}
+
 struct plugin_for_each_args {
     struct qemu_plugin_ctx *ctx;
     qemu_plugin_vcpu_simple_cb_t cb;
diff --git a/plugins/plugin.h b/plugins/plugin.h
index ce885bfa98..9e710c23a7 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -104,4 +104,6 @@ void exec_inline_op(struct qemu_plugin_dyn_cb *cb);
 
 int name_to_plugin_version(const char *name);
 
+const char *id_to_plugin_name(qemu_plugin_id_t id);
+
 #endif /* PLUGIN_H */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/8] plugins: add core API functions for QPP callbacks
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
                   ` (2 preceding siblings ...)
  2022-12-13 21:37 ` [PATCH 3/8] plugins: add id_to_plugin_name Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2023-03-09 16:09   ` Alex Bennée
  2022-12-13 21:37 ` [PATCH 5/8] plugins: implement " Andrew Fasano
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

Plugin callbacks and their registered functions are stored in a
separate struct which is accessible from the plugin's ctx.
In order for plugins to use another plugin's callbacks, we have
internal functions that resolve a plugin's name to its ctx and
find a target plugin.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 include/qemu/qemu-plugin.h | 10 ++++++++++
 plugins/core.c             | 23 +++++++++++++++++++++++
 plugins/loader.c           | 10 ++++++++++
 plugins/plugin.h           | 15 +++++++++++++++
 4 files changed, 58 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 5326f33ce8..3ec82ce97f 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -14,6 +14,7 @@
 #include <inttypes.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <gmodule.h>
 
 /*
  * For best performance, build the plugin with -fvisibility=hidden so that
@@ -38,6 +39,15 @@
  */
 typedef uint64_t qemu_plugin_id_t;
 
+/**
+ * typedef cb_func_t - callback function pointer type
+ * @evdata: plugin callback defined event data
+ * @udata: plugin defined user data
+ *
+ * No return value.
+ */
+typedef void (*cb_func_t) (gpointer evdata, gpointer udata);
+
 /*
  * Versioning plugins:
  *
diff --git a/plugins/core.c b/plugins/core.c
index 6a50b4a6e6..0415a55ec5 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -236,6 +236,17 @@ void qemu_plugin_vcpu_exit_hook(CPUState *cpu)
     qemu_rec_mutex_unlock(&plugin.lock);
 }
 
+struct qemu_plugin_ctx *plugin_name_to_ctx_locked(const char* name)
+{
+    struct qemu_plugin_ctx *ctx;
+    QTAILQ_FOREACH(ctx, &plugin.ctxs, entry) {
+        if (strcmp(ctx->name, name) == 0) {
+            return plugin_id_to_ctx_locked(ctx->id);
+        }
+    }
+    return NULL;
+}
+
 int name_to_plugin_version(const char *name)
 {
     struct qemu_plugin_ctx *ctx;
@@ -260,6 +271,18 @@ const char *id_to_plugin_name(qemu_plugin_id_t id)
     }
 }
 
+struct qemu_plugin_qpp_cb *plugin_find_qpp_cb(struct qemu_plugin_ctx *ctx,
+                                              const char *name)
+{
+    struct qemu_plugin_qpp_cb *cb;
+    QTAILQ_FOREACH(cb, &ctx->qpp_cbs, entry) {
+        if (strcmp(cb->name, name) == 0) {
+            return cb;
+        }
+    }
+    return NULL;
+}
+
 struct plugin_for_each_args {
     struct qemu_plugin_ctx *ctx;
     qemu_plugin_vcpu_simple_cb_t cb;
diff --git a/plugins/loader.c b/plugins/loader.c
index 12c0680e03..ab01d0753c 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -277,6 +277,7 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, E
             break;
         }
     }
+    QTAILQ_INIT(&ctx->qpp_cbs);
     QTAILQ_INSERT_TAIL(&plugin.ctxs, ctx, entry);
     ctx->installing = true;
     rc = install(ctx->id, info, desc->argc, desc->argv);
@@ -303,6 +304,15 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, E
     return 1;
 }
 
+void plugin_add_qpp_cb(struct qemu_plugin_ctx *ctx, const char *name)
+{
+    struct qemu_plugin_qpp_cb *new_cb;
+    new_cb = qemu_memalign(qemu_dcache_linesize, sizeof(*new_cb));
+    memset(new_cb, 0, sizeof(*new_cb));
+    new_cb->name = name;
+    QTAILQ_INSERT_TAIL(&ctx->qpp_cbs, new_cb, entry);
+}
+
 /* call after having removed @desc from the list */
 static void plugin_desc_free(struct qemu_plugin_desc *desc)
 {
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 9e710c23a7..fee4741bc6 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -47,6 +47,14 @@ struct qemu_plugin_state {
     struct qht dyn_cb_arr_ht;
 };
 
+typedef void (*cb_func_t) (gpointer evdata, gpointer udata);
+
+struct qemu_plugin_qpp_cb {
+    const char *name;
+    cb_func_t registered_cb_funcs[QEMU_PLUGIN_EV_MAX];
+    int counter;
+    QTAILQ_ENTRY(qemu_plugin_qpp_cb) entry;
+};
 
 struct qemu_plugin_ctx {
     GModule *handle;
@@ -54,6 +62,7 @@ struct qemu_plugin_ctx {
     const char *name;
     int version;
     struct qemu_plugin_cb *callbacks[QEMU_PLUGIN_EV_MAX];
+    QTAILQ_HEAD(, qemu_plugin_qpp_cb) qpp_cbs;
     QTAILQ_ENTRY(qemu_plugin_ctx) entry;
     /*
      * keep a reference to @desc until uninstall, so that plugins do not have
@@ -106,4 +115,10 @@ int name_to_plugin_version(const char *name);
 
 const char *id_to_plugin_name(qemu_plugin_id_t id);
 
+struct qemu_plugin_qpp_cb *plugin_find_qpp_cb(struct qemu_plugin_ctx *plugin_ctx,
+                                              const char *cb_name);
+
+/* loader.c */
+void plugin_add_qpp_cb(struct qemu_plugin_ctx *ctx, const char *name);
+
 #endif /* PLUGIN_H */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/8] plugins: implement QPP callbacks
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
                   ` (3 preceding siblings ...)
  2022-12-13 21:37 ` [PATCH 4/8] plugins: add core API functions for QPP callbacks Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2023-03-09 16:17   ` Alex Bennée
  2022-12-13 21:37 ` [PATCH 6/8] plugins: implement QPP import function Andrew Fasano
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

Plugins are able to use API functions which are explained in
<qemu-plugin.h> to create and run their own callbacks and register
functions on another plugin's callbacks.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 include/qemu/qemu-plugin.h   |  46 +++++++++++++++
 plugins/api.c                | 111 +++++++++++++++++++++++++++++++++++
 plugins/loader.c             |   1 +
 plugins/qemu-plugins.symbols |   4 ++
 4 files changed, 162 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 3ec82ce97f..4221545015 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -354,6 +354,52 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
  */
 uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
 
+/**
+ * qemu_plugin_create_callback() - create a new cb with given name
+ * @id: unique plugin id
+ * @name: name of cb
+ *
+ * Returns: true on success, false otherwise
+ */
+bool qemu_plugin_create_callback(qemu_plugin_id_t id, const char *name);
+
+/**
+ * qemu_plugin_run_callback() - run all functions registered to cb with given
+ * name using given args
+ * @id: unique plugin id
+ * @name: name of cb
+ * @evdata: pointer to evdata struct
+ * @udata: pointer to udata struct
+ *
+ * Returns: true if any registerd functions were run, false otherwise
+ */
+bool qemu_plugin_run_callback(qemu_plugin_id_t id, const char *name,
+                              gpointer evdata, gpointer udata);
+
+/**
+ * qemu_plugin_reg_callback() - register a function to be called on cb with
+ * given name
+ * @target_plugin: name of plugin that provides the callback
+ * @cb_name: name of the callback
+ * @function_pointer: pointer to function being registered
+ *
+ * Returns: true if function was registered successfully, false otherwise
+ */
+bool qemu_plugin_reg_callback(const char *target_plugin, const char *cb_name,
+                              cb_func_t function_pointer);
+
+/**
+ * qemu_plugin_unreg_callback() - unregister a function to be called on cb
+ * with given name
+ * @target_plugin: name of plugin that provides the callback
+ * @cb_name: name of the callback
+ * @function_pointer: pointer to function being unregistered
+ *
+ * Returns: true if function was unregistered successfully, false otherwise
+ */
+bool qemu_plugin_unreg_callback(const char *target_plugin, const char *cb_name,
+                                cb_func_t function_pointer);
+
 /**
  * qemu_plugin_tb_get_insn() - retrieve handle for instruction
  * @tb: opaque handle to TB passed to callback
diff --git a/plugins/api.c b/plugins/api.c
index 2078b16edb..1f7ea718dc 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -400,6 +400,117 @@ bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
     return name && value && qapi_bool_parse(name, value, ret, NULL);
 }
 
+bool qemu_plugin_create_callback(qemu_plugin_id_t id, const char *cb_name)
+{
+    struct qemu_plugin_ctx *ctx = plugin_id_to_ctx_locked(id);
+    if (ctx == NULL) {
+        error_report("Cannot create callback with invalid plugin ID");
+        return false;
+    }
+
+    if (ctx->version < QPP_MINIMUM_VERSION) {
+        error_report("Plugin %s cannot create callbacks as its PLUGIN_VERSION"
+                     " %d is below QPP_MINIMUM_VERSION (%d).",
+                     ctx->name, ctx->version, QPP_MINIMUM_VERSION);
+        return false;
+    }
+
+    if (plugin_find_qpp_cb(ctx, cb_name)) {
+        error_report("Plugin %s already created callback %s", ctx->name,
+                     cb_name);
+        return false;
+    }
+
+    plugin_add_qpp_cb(ctx, cb_name);
+    return true;
+}
+
+bool qemu_plugin_run_callback(qemu_plugin_id_t id, const char *cb_name,
+                              gpointer evdata, gpointer udata) {
+    struct qemu_plugin_ctx *ctx = plugin_id_to_ctx_locked(id);
+    if (ctx == NULL) {
+        error_report("Cannot run callback with invalid plugin ID");
+        return false;
+    }
+
+    struct qemu_plugin_qpp_cb *cb = plugin_find_qpp_cb(ctx, cb_name);
+    if (!cb) {
+        error_report("Can not run previously-unregistered callback %s in "
+                     "plugin %s", cb_name, ctx->name);
+        return false;
+    }
+
+    for (int i = 0; i < cb->counter; i++) {
+        cb_func_t qpp_cb_func = cb->registered_cb_funcs[i];
+        qpp_cb_func(evdata, udata);
+    }
+
+    return (cb->registered_cb_funcs[0] != NULL);
+}
+
+bool qemu_plugin_reg_callback(const char *target_plugin, const char *cb_name,
+                              cb_func_t function_pointer) {
+    struct qemu_plugin_ctx *ctx = plugin_name_to_ctx_locked(target_plugin);
+    if (ctx == NULL) {
+        error_report("Cannot register callback with unknown plugin %s",
+                     target_plugin);
+      return false;
+    }
+
+    struct qemu_plugin_qpp_cb *cb = plugin_find_qpp_cb(ctx, cb_name);
+    if (!cb) {
+        error_report("Cannot register a function to run on callback %s in "
+                     "plugin %s as that callback does not exist",
+                     cb_name, target_plugin);
+        return false;
+    }
+
+    if (cb->counter == QEMU_PLUGIN_EV_MAX) {
+        error_report("The maximum number of allowed callbacks are already "
+                     "registered for callback %s in plugin %s",
+                     cb_name, target_plugin);
+        return false;
+    }
+
+    cb->registered_cb_funcs[cb->counter] = function_pointer;
+    cb->counter++;
+    return true;
+}
+
+bool qemu_plugin_unreg_callback(const char *target_plugin, const char *cb_name,
+                              cb_func_t function_pointer) {
+    struct qemu_plugin_ctx *ctx = plugin_name_to_ctx_locked(target_plugin);
+    if (ctx == NULL) {
+        error_report("Cannot remove callback function from unknown plugin %s",
+                     target_plugin);
+        return false;
+    }
+
+    struct qemu_plugin_qpp_cb *cb = plugin_find_qpp_cb(ctx, cb_name);
+    if (!cb) {
+        error_report("Cannot remove a function to run on callback %s in "
+                     "plugin %s as that callback does not exist",
+                     cb_name, target_plugin);
+        return false;
+    }
+
+    for (int i = 0; i < cb->counter; i++) {
+        if (cb->registered_cb_funcs[i] == function_pointer) {
+            for (int j = i + 1; j < cb->counter; j++) {
+                cb->registered_cb_funcs[i] = cb->registered_cb_funcs[j];
+                i++;
+            }
+            cb->registered_cb_funcs[i] = NULL;
+            cb->counter--;
+            return true;
+        }
+    }
+    error_report("Function to remove not found in registered functions "
+                 "for callback %s in plugin %s",
+                 cb_name, target_plugin);
+    return false;
+}
+
 /*
  * Binary path, start and end locations
  */
diff --git a/plugins/loader.c b/plugins/loader.c
index ab01d0753c..7f047ebc99 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -310,6 +310,7 @@ void plugin_add_qpp_cb(struct qemu_plugin_ctx *ctx, const char *name)
     new_cb = qemu_memalign(qemu_dcache_linesize, sizeof(*new_cb));
     memset(new_cb, 0, sizeof(*new_cb));
     new_cb->name = name;
+    new_cb->counter = 0;
     QTAILQ_INSERT_TAIL(&ctx->qpp_cbs, new_cb, entry);
 }
 
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index 71f6c90549..b7013980cf 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -3,6 +3,10 @@
   qemu_plugin_end_code;
   qemu_plugin_entry_code;
   qemu_plugin_get_hwaddr;
+  qemu_plugin_create_callback;
+  qemu_plugin_run_callback;
+  qemu_plugin_reg_callback;
+  qemu_plugin_unreg_callback;
   qemu_plugin_hwaddr_device_name;
   qemu_plugin_hwaddr_is_io;
   qemu_plugin_hwaddr_phys_addr;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/8] plugins: implement QPP import function
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
                   ` (4 preceding siblings ...)
  2022-12-13 21:37 ` [PATCH 5/8] plugins: implement " Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2023-03-09 16:26   ` Alex Bennée
  2022-12-13 21:37 ` [PATCH 7/8] include/qemu: added macro for " Andrew Fasano
  2022-12-13 21:37 ` [PATCH 8/8] tests: build and run QPP tests Andrew Fasano
  7 siblings, 1 reply; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

Plugins can export functions or import functions from other
plugins using their name and the function name. This is also
described in <qemu-plugin.h>.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 include/qemu/qemu-plugin.h   | 10 ++++++++++
 plugins/api.c                | 21 +++++++++++++++++++++
 plugins/qemu-plugins.symbols |  1 +
 3 files changed, 32 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 4221545015..a0516e9a0e 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -354,6 +354,16 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
  */
 uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
 
+/**
+ * qemu_plugin_import_function() - return pointer to a function in another
+ * plugin
+ * @plugin: plugin name
+ * @function: function name
+ *
+ * Returns: NULL on failure, function pointer on success
+ */
+gpointer qemu_plugin_import_function(const char *plugin, const char *function);
+
 /**
  * qemu_plugin_create_callback() - create a new cb with given name
  * @id: unique plugin id
diff --git a/plugins/api.c b/plugins/api.c
index 1f7ea718dc..a998df6942 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -400,6 +400,27 @@ bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
     return name && value && qapi_bool_parse(name, value, ret, NULL);
 }
 
+/*
+ * QPP: inter-plugin function resolution and callbacks
+ */
+
+gpointer qemu_plugin_import_function(const char *target_plugin,
+                                     const char *function) {
+    gpointer function_pointer = NULL;
+    struct qemu_plugin_ctx *ctx = plugin_name_to_ctx_locked(target_plugin);
+    if (ctx == NULL) {
+        error_report("Unable to load plugin %s by name", target_plugin);
+    } else if (g_module_symbol(ctx->handle, function,
+               (gpointer *)&function_pointer)) {
+        return function_pointer;
+    } else {
+      error_report("function: %s not found in plugin: %s", function,
+                   target_plugin);
+    }
+    abort();
+    return NULL;
+}
+
 bool qemu_plugin_create_callback(qemu_plugin_id_t id, const char *cb_name)
 {
     struct qemu_plugin_ctx *ctx = plugin_id_to_ctx_locked(id);
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index b7013980cf..70a518839d 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -3,6 +3,7 @@
   qemu_plugin_end_code;
   qemu_plugin_entry_code;
   qemu_plugin_get_hwaddr;
+  qemu_plugin_import_function;
   qemu_plugin_create_callback;
   qemu_plugin_run_callback;
   qemu_plugin_reg_callback;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/8] include/qemu: added macro for QPP import function
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
                   ` (5 preceding siblings ...)
  2022-12-13 21:37 ` [PATCH 6/8] plugins: implement QPP import function Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2022-12-13 21:37 ` [PATCH 8/8] tests: build and run QPP tests Andrew Fasano
  7 siblings, 0 replies; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

Plugins can use this macro in a header file which can be included
by both the exporting and importing plugins. The macro will
either use qemu_plugin_import_function to import the function
or just define it if the plugin is the same one that exports it.
If importing a function, "_qpp" will be appended to the end of the
function name.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 include/qemu/plugin-qpp.h | 54 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 include/qemu/plugin-qpp.h

diff --git a/include/qemu/plugin-qpp.h b/include/qemu/plugin-qpp.h
new file mode 100644
index 0000000000..7aea98a14d
--- /dev/null
+++ b/include/qemu/plugin-qpp.h
@@ -0,0 +1,54 @@
+#ifndef PLUGIN_QPP_H
+#define PLUGIN_QPP_H
+
+/*
+ * Facilities for "Plugin to plugin" (QPP) interactions between tcg plugins.
+ * These allows for direct function calls between loaded plugins. For more
+ * details see docs/devel/plugin.rst.
+ */
+
+
+/*
+ * Internal macros
+ */
+#define _PLUGIN_STR(s) #s
+#define PLUGIN_STR(s) _PLUGIN_STR(s)
+#define _PLUGIN_CONCAT(x, y) x##y
+#define PLUGIN_CONCAT(x, y) _PLUGIN_CONCAT(x, y)
+#define _QPP_SETUP_NAME(fn) PLUGIN_CONCAT(_qpp_setup_, fn)
+
+/*
+ * A header file that defines an exported function should use
+ * the QPP_FUN_PROTOTYPE macro to create the necessary types.
+ *
+ * The generated function named after the output of QPP_SETUP_NAME should
+ * dynamically resolve a target function in another plugin or raise a fatal
+ * error on failure. This function has the constructor attribute so it will
+ * run immediately when the plugin shared object object is loaded.
+ *
+ * Note that the variable qemu_plugin_name must be set before this macro is
+ * used. In other words the plugin that includes a header file with these
+ * macros should set qemu_plugin_name before including such headers. When the
+ * generated function is run it compares the current plugin name to the name
+ * of the plugin that provides the target function.
+ *
+ * If the target plugin is not the current plugin it will resolve the function
+ * pointer from qemu_plugin_import_function, correctly cast it, and assign the
+ * function pointer "[function_name]_qpp" which can then be used by the plugin
+ * that imported it.
+ */
+
+#define QPP_FUN_PROTOTYPE(plugin_name, fn_ret, fn, args)                      \
+  fn_ret fn(args);                                                            \
+  typedef fn_ret(*PLUGIN_CONCAT(fn, _t))(args);                               \
+  fn##_t fn##_qpp;                                                            \
+  void _QPP_SETUP_NAME(fn) (void);                                            \
+                                                                              \
+  void __attribute__ ((constructor)) _QPP_SETUP_NAME(fn) (void) {             \
+    if (strcmp(qemu_plugin_name, #plugin_name) != 0) {                        \
+        fn##_qpp = (fn##_t)qemu_plugin_import_function(                       \
+                                                      PLUGIN_STR(plugin_name),\
+                                                      PLUGIN_STR(fn));        \
+    }                                                                         \
+  }
+#endif /* PLUGIN_QPP_H */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 8/8] tests: build and run QPP tests
  2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
                   ` (6 preceding siblings ...)
  2022-12-13 21:37 ` [PATCH 7/8] include/qemu: added macro for " Andrew Fasano
@ 2022-12-13 21:37 ` Andrew Fasano
  2023-03-09 16:37   ` Alex Bennée
  7 siblings, 1 reply; 16+ messages in thread
From: Andrew Fasano @ 2022-12-13 21:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: elysia.witham, alex.bennee, erdnaxe, ma.mandourr, Andrew Fasano

From: Elysia Witham <elysia.witham@ll.mit.edu>

These test plugins demonstrate the QPP API changes by exporting
and importing functions and creating and registering callbacks.
These tests are integrated into the `make check-tcg` tests.

Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
Signed-off-by: Andrew Fasano <fasano@mit.edu>
---
 tests/Makefile.include                    |  2 +-
 tests/meson.build                         |  1 +
 tests/plugin_qpp/meson.build              |  7 +++++
 tests/plugin_qpp/qpp_client.c             | 32 ++++++++++++++++++++
 tests/plugin_qpp/qpp_srv.c                | 37 +++++++++++++++++++++++
 tests/plugin_qpp/qpp_srv.h                | 12 ++++++++
 tests/tcg/Makefile.target                 | 29 ++++++++++++++++++
 tests/tcg/aarch64/Makefile.softmmu-target |  2 ++
 tests/tcg/arm/Makefile.softmmu-target     |  1 +
 tests/tcg/arm/Makefile.target             |  2 ++
 tests/tcg/multiarch/Makefile.target       |  2 ++
 11 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 tests/plugin_qpp/meson.build
 create mode 100644 tests/plugin_qpp/qpp_client.c
 create mode 100644 tests/plugin_qpp/qpp_srv.c
 create mode 100644 tests/plugin_qpp/qpp_srv.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9422ddaece..08dd667aad 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -73,7 +73,7 @@ $(TCG_TESTS_TARGETS:%=distclean-tcg-tests-%): distclean-tcg-tests-%:
 build-tcg: $(BUILD_TCG_TARGET_RULES)
 
 .PHONY: check-tcg
-.ninja-goals.check-tcg = all $(if $(CONFIG_PLUGIN),test-plugins)
+.ninja-goals.check-tcg = all $(if $(CONFIG_PLUGIN),test-plugins test-plugins-qpp)
 check-tcg: $(RUN_TCG_TARGET_RULES)
 
 .PHONY: clean-tcg
diff --git a/tests/meson.build b/tests/meson.build
index 8e318ec513..12d3ec9c4b 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -86,6 +86,7 @@ endif
 if get_option('tcg').allowed()
   if 'CONFIG_PLUGIN' in config_host
     subdir('plugin')
+    subdir('plugin_qpp')
   endif
 endif
 
diff --git a/tests/plugin_qpp/meson.build b/tests/plugin_qpp/meson.build
new file mode 100644
index 0000000000..25555d0ec2
--- /dev/null
+++ b/tests/plugin_qpp/meson.build
@@ -0,0 +1,7 @@
+t = []
+foreach i : ['qpp_srv', 'qpp_client']
+  t += shared_module(i, files(i + '.c'),
+                     include_directories: '../../include/qemu',
+                     dependencies: glib)
+endforeach
+alias_target('test-plugins-qpp', t)
diff --git a/tests/plugin_qpp/qpp_client.c b/tests/plugin_qpp/qpp_client.c
new file mode 100644
index 0000000000..69b7cc4ac5
--- /dev/null
+++ b/tests/plugin_qpp/qpp_client.c
@@ -0,0 +1,32 @@
+#include <stdio.h>
+#include <qemu-plugin.h>
+#include <plugin-qpp.h>
+#include <glib.h>
+
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "qpp_client";
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_uses[] = {"qpp_srv", NULL};
+
+#include "qpp_srv.h"
+
+void my_cb_exit_callback(gpointer evdata, gpointer udata);
+
+QEMU_PLUGIN_EXPORT void my_cb_exit_callback(gpointer evdata, gpointer udata)
+{
+    *(bool *)evdata = true;
+    qemu_plugin_outs("called my on exit callback\n");
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+                   const qemu_info_t *info, int argc, char **argv) {
+
+    g_autoptr(GString) report = g_string_new("QPP client: Call "
+                                             "qpp_srv's do_add(0) do_sub(3) => ");
+    g_string_append_printf(report, "%d %d\n", qpp_srv_do_add_qpp(0),
+                           qpp_srv_do_sub_qpp(3));
+    qemu_plugin_outs(report->str);
+    qemu_plugin_reg_callback("qpp_srv", "my_on_exit", &my_cb_exit_callback);
+
+    return 0;
+}
+
diff --git a/tests/plugin_qpp/qpp_srv.c b/tests/plugin_qpp/qpp_srv.c
new file mode 100644
index 0000000000..88b1907a7e
--- /dev/null
+++ b/tests/plugin_qpp/qpp_srv.c
@@ -0,0 +1,37 @@
+#include <stdio.h>
+#include <qemu-plugin.h>
+#include <plugin-qpp.h>
+#include <gmodule.h>
+#include <assert.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "qpp_srv";
+#include "qpp_srv.h"
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+  qemu_plugin_outs("QPP srv: exit triggered, running all registered"
+                  " QPP callbacks\n");
+  bool called = false;
+  qemu_plugin_run_callback(id, "my_on_exit", &called, NULL);
+  assert(called);
+}
+
+QEMU_PLUGIN_EXPORT int qpp_srv_do_add(int x)
+{
+  return x + 1;
+}
+
+QEMU_PLUGIN_EXPORT int qpp_srv_do_sub(int x)
+{
+  return x - 1;
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+                   const qemu_info_t *info, int argc, char **argv) {
+    qemu_plugin_outs("qpp_srv loaded\n");
+    qemu_plugin_create_callback(id, "my_on_exit");
+    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+
+    return 0;
+}
diff --git a/tests/plugin_qpp/qpp_srv.h b/tests/plugin_qpp/qpp_srv.h
new file mode 100644
index 0000000000..16d9bc2df4
--- /dev/null
+++ b/tests/plugin_qpp/qpp_srv.h
@@ -0,0 +1,12 @@
+#ifndef QPP_SRV_H
+#define QPP_SRV_H
+
+
+/*
+ * Prototypes for the do_add and do_sub functions. Both return an int and
+ * take an int as an argument.
+ */
+QPP_FUN_PROTOTYPE(qpp_srv, int, qpp_srv_do_add, int);
+QPP_FUN_PROTOTYPE(qpp_srv, int, qpp_srv_do_sub, int);
+
+#endif /* QPP_SRV_H */
diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 75257f2b29..c478f1b2de 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -145,6 +145,9 @@ RUN_TESTS=$(patsubst %,run-%, $(TESTS))
 ifeq ($(CONFIG_PLUGIN),y)
 PLUGIN_SRC=$(SRC_PATH)/tests/plugin
 PLUGIN_LIB=../../plugin
+
+PLUGIN_LIB_QPP=../../plugin_qpp
+
 VPATH+=$(PLUGIN_LIB)
 PLUGINS=$(patsubst %.c, lib%.so, $(notdir $(wildcard $(PLUGIN_SRC)/*.c)))
 
@@ -156,6 +159,11 @@ $(foreach p,$(PLUGINS), \
 	$(foreach t,$(TESTS),\
 		$(eval run-plugin-$(t)-with-$(p): $t $p) \
 		$(eval RUN_TESTS+=run-plugin-$(t)-with-$(p))))
+
+$(foreach t,$(TESTS),\
+	$(eval run-plugin-qpp-$(t): $t) \
+	$(eval RUN_TESTS+=run-plugin-qpp-$(t)))
+
 endif
 
 strip-plugin = $(wordlist 1, 1, $(subst -with-, ,$1))
@@ -167,18 +175,39 @@ ifeq ($(filter %-softmmu, $(TARGET)),)
 run-%: %
 	$(call run-test, $<, $(QEMU) $(QEMU_OPTS) $<)
 
+run-plugin-qpp-%:
+	$(call run-test, $@, \
+	  $(QEMU) \
+			-plugin $(PLUGIN_LIB_QPP)/libqpp_srv.so \
+			-plugin $(PLUGIN_LIB_QPP)/libqpp_client.so \
+			$(call extract-plugin,$@) \
+			-d plugin -D $*.pout \
+		$(QEMU_OPTS) $<)
+
 run-plugin-%:
 	$(call run-test, $@, $(QEMU) $(QEMU_OPTS) \
 		-plugin $(PLUGIN_LIB)/$(call extract-plugin,$@) \
 		-d plugin -D $*.pout \
 		 $(call strip-plugin,$<))
+
 else
+
 run-%: %
 	$(call run-test, $<, \
 	  $(QEMU) -monitor none -display none \
 		  -chardev file$(COMMA)path=$<.out$(COMMA)id=output \
 		  $(QEMU_OPTS) $<)
 
+run-plugin-qpp-%:
+	$(call run-test, $@, \
+	  $(QEMU) -monitor none -display none \
+		-chardev file$(COMMA)path=$@.out$(COMMA)id=output \
+			-plugin $(PLUGIN_LIB_QPP)/libqpp_srv.so \
+			-plugin $(PLUGIN_LIB_QPP)/libqpp_client.so \
+			$(call extract-plugin,$@) \
+				-d plugin -D $*.pout \
+		$(QEMU_OPTS) $<)
+
 run-plugin-%:
 	$(call run-test, $@, \
 	  $(QEMU) -monitor none -display none \
diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target
index a1368905f5..f87a1a799b 100644
--- a/tests/tcg/aarch64/Makefile.softmmu-target
+++ b/tests/tcg/aarch64/Makefile.softmmu-target
@@ -45,6 +45,8 @@ QEMU_SEMIHOST=-chardev stdio,mux=on,id=stdio0 -semihosting-config enable=on,char
 run-semiconsole: QEMU_OPTS=$(QEMU_BASE_MACHINE) $(QEMU_SEMIHOST)  -kernel
 run-semiconsole: semiconsole
 	$(call skip-test, $<, "MANUAL ONLY")
+run-plugin-qpp-semiconsole: semiconsole
+	$(call skip-test, $<, "MANUAL ONLY")
 run-plugin-semiconsole-with-%: semiconsole
 	$(call skip-test, $<, "MANUAL ONLY")
 
diff --git a/tests/tcg/arm/Makefile.softmmu-target b/tests/tcg/arm/Makefile.softmmu-target
index 7df88ddea8..f4bfab41bf 100644
--- a/tests/tcg/arm/Makefile.softmmu-target
+++ b/tests/tcg/arm/Makefile.softmmu-target
@@ -24,3 +24,4 @@ test-armv6m-undef: EXTRA_CFLAGS+=-mcpu=cortex-m0 -mfloat-abi=soft
 
 run-test-armv6m-undef: QEMU_OPTS+=-semihosting -M microbit -kernel
 run-plugin-test-armv6m-undef-%: QEMU_OPTS+=-semihosting -M microbit -kernel
+run-plugin-qpp-test-armv6m-undef: QEMU_OPTS+=-semihosting -M microbit -kernel
diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
index b3b1504a1c..2aa3479952 100644
--- a/tests/tcg/arm/Makefile.target
+++ b/tests/tcg/arm/Makefile.target
@@ -62,6 +62,8 @@ semiconsole-arm: semihosting.c
 run-semiconsole-arm: semiconsole-arm
 	$(call skip-test, $<, "MANUAL ONLY")
 
+run-plugin-qpp-semiconsole-arm:
+	$(call skip-test, $<, "MANUAL ONLY")
 run-plugin-semiconsole-arm-with-%:
 	$(call skip-test, $<, "MANUAL ONLY")
 
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 5f0fee1aad..a77b30aa94 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -108,6 +108,8 @@ semiconsole: CFLAGS+=-I$(SRC_PATH)/tests/tcg/$(TARGET_NAME)
 run-semiconsole: semiconsole
 	$(call skip-test, $<, "MANUAL ONLY")
 
+run-plugin-qpp-semiconsole:
+	$(call skip-test, $<, "MANUAL ONLY")
 run-plugin-semiconsole-with-%:
 	$(call skip-test, $<, "MANUAL ONLY")
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/8] docs/devel: describe QPP API
  2022-12-13 21:37 ` [PATCH 1/8] docs/devel: describe QPP API Andrew Fasano
@ 2023-03-09 14:15   ` Alex Bennée
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2023-03-09 14:15 UTC (permalink / raw)
  To: Andrew Fasano; +Cc: qemu-devel, elysia.witham, erdnaxe, ma.mandourr


Andrew Fasano <fasano@mit.edu> writes:

> From: Elysia Witham <elysia.witham@ll.mit.edu>
>
> The new QPP API allows plugin-to-plugin interaction for creating
> and using callbacks as well as importing and exporting functions.
> The new test plugins qpp_srv and qpp_client demonstrate how
> plugins use the new API.
>
> Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>

Sorry about the delay getting to look at this.

> ---
>  docs/devel/tcg-plugins.rst | 91 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 90 insertions(+), 1 deletion(-)
>
> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> index 9740a70406..70ac09b96b 100644
> --- a/docs/devel/tcg-plugins.rst
> +++ b/docs/devel/tcg-plugins.rst
> @@ -281,6 +281,14 @@ run::
>    160          1      0
>    135          1      0
>  
> +- tests/plugins/qpp_srv.c & tests/plugins/qpp_client.c
> +
> +These plugins demonstrate QPP interactions. The qpp_srv plugin defines
> +a few exported functions and its own callback which are then imported and
> +used by the qpp_client plugin. The qpp_client plugin registers its own
> +function to run on qpp_srv's defined callback. The tests for these plugins
> +are modified as both plugins must be loaded in order to work.
> +

This section should be split to where the plugins are introduced.

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/8] plugins: version 2, require unique plugin names
  2022-12-13 21:37 ` [PATCH 2/8] plugins: version 2, require unique plugin names Andrew Fasano
@ 2023-03-09 14:17   ` Alex Bennée
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2023-03-09 14:17 UTC (permalink / raw)
  To: Andrew Fasano; +Cc: qemu-devel, elysia.witham, erdnaxe, ma.mandourr


Andrew Fasano <fasano@mit.edu> writes:

> From: Elysia Witham <elysia.witham@ll.mit.edu>
>
> In order for the QPP API to resolve interactions between plugins,
> plugins must export their own names which cannot match any other
> loaded plugins.
>
> Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>
> ---
>  include/qemu/qemu-plugin.h |  2 +-
>  plugins/core.c             | 12 +++++++++
>  plugins/loader.c           | 50 +++++++++++++++++++++++++++++++++-----
>  plugins/plugin.h           |  7 ++++++
>  tests/plugin/bb.c          |  1 +
>  tests/plugin/empty.c       |  1 +
>  tests/plugin/insn.c        |  1 +
>  tests/plugin/mem.c         |  1 +
>  tests/plugin/syscall.c     |  1 +
>  9 files changed, 69 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index d0e9d03adf..5326f33ce8 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -51,7 +51,7 @@ typedef uint64_t qemu_plugin_id_t;
>  
>  extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
>  
> -#define QEMU_PLUGIN_VERSION 1
> +#define QEMU_PLUGIN_VERSION 2
>  
>  /**
>   * struct qemu_info_t - system information for plugins
> diff --git a/plugins/core.c b/plugins/core.c
> index ccb770a485..5fbdcb5768 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -236,6 +236,18 @@ void qemu_plugin_vcpu_exit_hook(CPUState *cpu)
>      qemu_rec_mutex_unlock(&plugin.lock);
>  }
>  
> +int name_to_plugin_version(const char *name)
> +{
> +    struct qemu_plugin_ctx *ctx;
> +    QTAILQ_FOREACH(ctx, &plugin.ctxs, entry) {
> +        if (strcmp(ctx->name, name) == 0) {
> +            return ctx->version;
> +        }
> +    }
> +    warn_report("Could not find any plugin named %s.", name);
> +    return -1;
> +}
> +

This function seems to be unused.

>  struct plugin_for_each_args {
>      struct qemu_plugin_ctx *ctx;
>      qemu_plugin_vcpu_simple_cb_t cb;
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 88c30bde2d..12c0680e03 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -177,7 +177,7 @@ QEMU_DISABLE_CFI
>  static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, Error **errp)
>  {
>      qemu_plugin_install_func_t install;
> -    struct qemu_plugin_ctx *ctx;
> +    struct qemu_plugin_ctx *ctx, *ctx2;
>      gpointer sym;
>      int rc;
>  
> @@ -208,17 +208,55 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, E
>                     desc->path, g_module_error());
>          goto err_symbol;
>      } else {
> -        int version = *(int *)sym;
> -        if (version < QEMU_PLUGIN_MIN_VERSION) {
> +        ctx->version = *(int *)sym;
> +        if (ctx->version < QEMU_PLUGIN_MIN_VERSION) {
>              error_setg(errp, "Could not load plugin %s: plugin requires API version %d, but "
>                         "this QEMU supports only a minimum version of %d",
> -                       desc->path, version, QEMU_PLUGIN_MIN_VERSION);
> +                       desc->path, ctx->version, QEMU_PLUGIN_MIN_VERSION);
>              goto err_symbol;
> -        } else if (version > QEMU_PLUGIN_VERSION) {
> +        } else if (ctx->version > QEMU_PLUGIN_VERSION) {
>              error_setg(errp, "Could not load plugin %s: plugin requires API version %d, but "
>                         "this QEMU supports only up to version %d",
> -                       desc->path, version, QEMU_PLUGIN_VERSION);
> +                       desc->path, ctx->version, QEMU_PLUGIN_VERSION);
>              goto err_symbol;
> +        } else if (ctx->version < QPP_MINIMUM_VERSION) {
> +            ctx->name = NULL;

A comment wouldn't go amiss here. Something like:

  "Older plugins will not be available for QPP calls".

> +        } else {
> +            if (!g_module_symbol(ctx->handle, "qemu_plugin_name", &sym)) {
> +                error_setg(errp, "Could not load plugin %s: plugin does not "
> +                           "declare plugin name %s",
> +                           desc->path, g_module_error());
> +                goto err_symbol;
> +            }
> +            ctx->name = (const char *)strdup(*(const char **)sym);
> +            QTAILQ_FOREACH(ctx2, &plugin.ctxs, entry) {
> +                if (strcmp(ctx2->name, ctx->name) == 0) {
> +                    error_setg(errp, "Could not load plugin %s as the name %s "
> +                               "is already in use by plugin at %s",
> +                               desc->path, ctx->name, ctx2->desc->path);
> +                    goto err_symbol;
> +                }
> +            }
> +            if (g_module_symbol(ctx->handle, "qemu_plugin_uses", &sym)) {
> +                const char **dependencies = &(*(const char **)sym);
> +                bool found = false;
> +                while (*dependencies) {
> +                    found = false;
> +                    QTAILQ_FOREACH(ctx2, &plugin.ctxs, entry) {
> +                        if (strcmp(ctx2->name, *dependencies) == 0) {

Lets use glib where we can, in this case g_strcmp0().

> +                            dependencies++;
> +                            found = true;
> +                            break;
> +                        }
> +                    }
> +                    if (!found) {
> +                        error_setg(errp, "Could not load plugin %s as it is "
> +                                   "dependent on %s which is not loaded",
> +                                   ctx->name, *dependencies);
> +                        goto err_symbol;
> +                    }

We are implying a load order here which we should document. Ideally we
could avoid it but I suspect that requires too much hoop jumping.

> +                }
> +            }
>          }
>      }
>  
> diff --git a/plugins/plugin.h b/plugins/plugin.h
> index 5eb2fdbc85..ce885bfa98 100644
> --- a/plugins/plugin.h
> +++ b/plugins/plugin.h
> @@ -16,6 +16,7 @@
>  #include "qemu/qht.h"
>  
>  #define QEMU_PLUGIN_MIN_VERSION 0
> +#define QPP_MINIMUM_VERSION 2
>  
>  /* global state */
>  struct qemu_plugin_state {
> @@ -50,6 +51,8 @@ struct qemu_plugin_state {
>  struct qemu_plugin_ctx {
>      GModule *handle;
>      qemu_plugin_id_t id;
> +    const char *name;
> +    int version;
>      struct qemu_plugin_cb *callbacks[QEMU_PLUGIN_EV_MAX];
>      QTAILQ_ENTRY(qemu_plugin_ctx) entry;
>      /*
> @@ -64,6 +67,8 @@ struct qemu_plugin_ctx {
>  
>  struct qemu_plugin_ctx *plugin_id_to_ctx_locked(qemu_plugin_id_t id);
>  
> +struct qemu_plugin_ctx *plugin_name_to_ctx_locked(const char* name);
> +
>  void plugin_register_inline_op(GArray **arr,
>                                 enum qemu_plugin_mem_rw rw,
>                                 enum qemu_plugin_op op, void *ptr,
> @@ -97,4 +102,6 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
>  
>  void exec_inline_op(struct qemu_plugin_dyn_cb *cb);
>  
> +int name_to_plugin_version(const char *name);
> +
>  #endif /* PLUGIN_H */
> diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
> index 7d470a1011..c04e5aaf90 100644
> --- a/tests/plugin/bb.c
> +++ b/tests/plugin/bb.c
> @@ -15,6 +15,7 @@
>  #include <qemu-plugin.h>
>  
>  QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "bb";
>  
>  typedef struct {
>      GMutex lock;
> diff --git a/tests/plugin/empty.c b/tests/plugin/empty.c
> index 8fa6bacd93..0f3d2b92b9 100644
> --- a/tests/plugin/empty.c
> +++ b/tests/plugin/empty.c
> @@ -14,6 +14,7 @@
>  #include <qemu-plugin.h>
>  
>  QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "empty";
>  
>  /*
>   * Empty TB translation callback.
> diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
> index cd5ea5d4ae..3f71138139 100644
> --- a/tests/plugin/insn.c
> +++ b/tests/plugin/insn.c
> @@ -15,6 +15,7 @@
>  #include <qemu-plugin.h>
>  
>  QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "insn";
>  
>  #define MAX_CPUS 8 /* lets not go nuts */
>  
> diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
> index 4570f7d815..35e5d7fe2a 100644
> --- a/tests/plugin/mem.c
> +++ b/tests/plugin/mem.c
> @@ -15,6 +15,7 @@
>  #include <qemu-plugin.h>
>  
>  QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "mem";
>  
>  static uint64_t inline_mem_count;
>  static uint64_t cb_mem_count;
> diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c
> index 96040c578f..922bdbd2e6 100644
> --- a/tests/plugin/syscall.c
> +++ b/tests/plugin/syscall.c
> @@ -15,6 +15,7 @@
>  #include <qemu-plugin.h>
>  
>  QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +QEMU_PLUGIN_EXPORT const char *qemu_plugin_name = "syscall";
>  
>  typedef struct {
>      int64_t num;

You should update the plugins in contrib/plugins as well.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/8] plugins: add id_to_plugin_name
  2022-12-13 21:37 ` [PATCH 3/8] plugins: add id_to_plugin_name Andrew Fasano
@ 2023-03-09 14:45   ` Alex Bennée
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2023-03-09 14:45 UTC (permalink / raw)
  To: Andrew Fasano; +Cc: qemu-devel, elysia.witham, erdnaxe, ma.mandourr


Andrew Fasano <fasano@mit.edu> writes:

> From: Elysia Witham <elysia.witham@ll.mit.edu>
>
> Plugins will pass their unique id when creating callbacks to
> ensure they are associated with the correct plugin. This
> internal function resolves those ids to the declared names.
>
> Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>
> ---
>  plugins/core.c   | 12 ++++++++++++
>  plugins/plugin.h |  2 ++
>  2 files changed, 14 insertions(+)
>
> diff --git a/plugins/core.c b/plugins/core.c
> index 5fbdcb5768..6a50b4a6e6 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -248,6 +248,18 @@ int name_to_plugin_version(const char *name)
>      return -1;
>  }
>  
> +const char *id_to_plugin_name(qemu_plugin_id_t id)
> +{
> +    const char *plugin = plugin_id_to_ctx_locked(id)->name;
> +    if (plugin) {
> +        return plugin;
> +    } else {
> +        warn_report("Unnamed plugin cannot use QPP, not supported in plugin "
> +                    "version. Please update plugin.");
> +        return NULL;
> +    }
> +}
> +

I don't see this function being used in this series.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/8] plugins: add core API functions for QPP callbacks
  2022-12-13 21:37 ` [PATCH 4/8] plugins: add core API functions for QPP callbacks Andrew Fasano
@ 2023-03-09 16:09   ` Alex Bennée
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2023-03-09 16:09 UTC (permalink / raw)
  To: Andrew Fasano; +Cc: qemu-devel, elysia.witham, erdnaxe, ma.mandourr


Andrew Fasano <fasano@mit.edu> writes:

> From: Elysia Witham <elysia.witham@ll.mit.edu>
>
> Plugin callbacks and their registered functions are stored in a
> separate struct which is accessible from the plugin's ctx.
> In order for plugins to use another plugin's callbacks, we have
> internal functions that resolve a plugin's name to its ctx and
> find a target plugin.
>
> Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>
> ---
>  include/qemu/qemu-plugin.h | 10 ++++++++++
>  plugins/core.c             | 23 +++++++++++++++++++++++
>  plugins/loader.c           | 10 ++++++++++
>  plugins/plugin.h           | 15 +++++++++++++++
>  4 files changed, 58 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 5326f33ce8..3ec82ce97f 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -14,6 +14,7 @@
>  #include <inttypes.h>
>  #include <stdbool.h>
>  #include <stddef.h>
> +#include <gmodule.h>
>  
>  /*
>   * For best performance, build the plugin with -fvisibility=hidden so that
> @@ -38,6 +39,15 @@
>   */
>  typedef uint64_t qemu_plugin_id_t;
>  
> +/**
> + * typedef cb_func_t - callback function pointer type
> + * @evdata: plugin callback defined event data
> + * @udata: plugin defined user data
> + *
> + * No return value.
> + */
> +typedef void (*cb_func_t) (gpointer evdata, gpointer udata);
> +
>  /*
>   * Versioning plugins:
>   *
> diff --git a/plugins/core.c b/plugins/core.c
> index 6a50b4a6e6..0415a55ec5 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -236,6 +236,17 @@ void qemu_plugin_vcpu_exit_hook(CPUState *cpu)
>      qemu_rec_mutex_unlock(&plugin.lock);
>  }
>

This looks like another unused function. I was looking to see what
locking was done before calling it as the suffix implies there should be
some.

> +struct qemu_plugin_ctx *plugin_name_to_ctx_locked(const char* name)
> +{
> +    struct qemu_plugin_ctx *ctx;
> +    QTAILQ_FOREACH(ctx, &plugin.ctxs, entry) {
> +        if (strcmp(ctx->name, name) == 0) {
> +            return plugin_id_to_ctx_locked(ctx->id);
> +        }
> +    }
> +    return NULL;
> +}
> +
>  int name_to_plugin_version(const char *name)
>  {
>      struct qemu_plugin_ctx *ctx;
> @@ -260,6 +271,18 @@ const char *id_to_plugin_name(qemu_plugin_id_t id)
>      }
>  }
>

This one too.

> +struct qemu_plugin_qpp_cb *plugin_find_qpp_cb(struct qemu_plugin_ctx *ctx,
> +                                              const char *name)
> +{
> +    struct qemu_plugin_qpp_cb *cb;
> +    QTAILQ_FOREACH(cb, &ctx->qpp_cbs, entry) {
> +        if (strcmp(cb->name, name) == 0) {
> +            return cb;
> +        }
> +    }
> +    return NULL;
> +}
> +
>  struct plugin_for_each_args {
>      struct qemu_plugin_ctx *ctx;
>      qemu_plugin_vcpu_simple_cb_t cb;
> diff --git a/plugins/loader.c b/plugins/loader.c
> index 12c0680e03..ab01d0753c 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -277,6 +277,7 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, E
>              break;
>          }
>      }
> +    QTAILQ_INIT(&ctx->qpp_cbs);
>      QTAILQ_INSERT_TAIL(&plugin.ctxs, ctx, entry);
>      ctx->installing = true;
>      rc = install(ctx->id, info, desc->argc, desc->argv);
> @@ -303,6 +304,15 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, E
>      return 1;
>  }
>  
> +void plugin_add_qpp_cb(struct qemu_plugin_ctx *ctx, const char *name)
> +{
> +    struct qemu_plugin_qpp_cb *new_cb;
> +    new_cb = qemu_memalign(qemu_dcache_linesize, sizeof(*new_cb));
> +    memset(new_cb, 0, sizeof(*new_cb));

Is there a reason to do aligned allocation here. Have you measured a
difference between this and a normal g_new0() in performance?

> +    new_cb->name = name;
> +    QTAILQ_INSERT_TAIL(&ctx->qpp_cbs, new_cb, entry);
> +}
> +

I think we need some rules for this that can be enforced. Can a plugin
create a cb at any time? Or only during initialisation?

If it's at any time we need some sort of serialisation to prevent races.

>  /* call after having removed @desc from the list */
>  static void plugin_desc_free(struct qemu_plugin_desc *desc)
>  {
> diff --git a/plugins/plugin.h b/plugins/plugin.h
> index 9e710c23a7..fee4741bc6 100644
> --- a/plugins/plugin.h
> +++ b/plugins/plugin.h
> @@ -47,6 +47,14 @@ struct qemu_plugin_state {
>      struct qht dyn_cb_arr_ht;
>  };
>  
> +typedef void (*cb_func_t) (gpointer evdata, gpointer udata);
> +
> +struct qemu_plugin_qpp_cb {
> +    const char *name;
> +    cb_func_t registered_cb_funcs[QEMU_PLUGIN_EV_MAX];
> +    int counter;
> +    QTAILQ_ENTRY(qemu_plugin_qpp_cb) entry;
> +};
>  
>  struct qemu_plugin_ctx {
>      GModule *handle;
> @@ -54,6 +62,7 @@ struct qemu_plugin_ctx {
>      const char *name;
>      int version;
>      struct qemu_plugin_cb *callbacks[QEMU_PLUGIN_EV_MAX];
> +    QTAILQ_HEAD(, qemu_plugin_qpp_cb) qpp_cbs;
>      QTAILQ_ENTRY(qemu_plugin_ctx) entry;
>      /*
>       * keep a reference to @desc until uninstall, so that plugins do not have
> @@ -106,4 +115,10 @@ int name_to_plugin_version(const char *name);
>  
>  const char *id_to_plugin_name(qemu_plugin_id_t id);
>  
> +struct qemu_plugin_qpp_cb *plugin_find_qpp_cb(struct qemu_plugin_ctx *plugin_ctx,
> +                                              const char *cb_name);
> +
> +/* loader.c */
> +void plugin_add_qpp_cb(struct qemu_plugin_ctx *ctx, const char *name);
> +
>  #endif /* PLUGIN_H */


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 5/8] plugins: implement QPP callbacks
  2022-12-13 21:37 ` [PATCH 5/8] plugins: implement " Andrew Fasano
@ 2023-03-09 16:17   ` Alex Bennée
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2023-03-09 16:17 UTC (permalink / raw)
  To: Andrew Fasano; +Cc: qemu-devel, elysia.witham, erdnaxe, ma.mandourr


Andrew Fasano <fasano@mit.edu> writes:

> From: Elysia Witham <elysia.witham@ll.mit.edu>
>
> Plugins are able to use API functions which are explained in
> <qemu-plugin.h> to create and run their own callbacks and register
> functions on another plugin's callbacks.
>
> Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>
> ---
>  include/qemu/qemu-plugin.h   |  46 +++++++++++++++
>  plugins/api.c                | 111 +++++++++++++++++++++++++++++++++++
>  plugins/loader.c             |   1 +
>  plugins/qemu-plugins.symbols |   4 ++
>  4 files changed, 162 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 3ec82ce97f..4221545015 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -354,6 +354,52 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
>   */
>  uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
>  
> +/**
> + * qemu_plugin_create_callback() - create a new cb with given name
> + * @id: unique plugin id
> + * @name: name of cb
> + *
> + * Returns: true on success, false otherwise
> + */
> +bool qemu_plugin_create_callback(qemu_plugin_id_t id, const char *name);
> +
> +/**
> + * qemu_plugin_run_callback() - run all functions registered to cb with given
> + * name using given args
> + * @id: unique plugin id
> + * @name: name of cb
> + * @evdata: pointer to evdata struct
> + * @udata: pointer to udata struct
> + *
> + * Returns: true if any registerd functions were run, false otherwise
> + */
> +bool qemu_plugin_run_callback(qemu_plugin_id_t id, const char *name,
> +                              gpointer evdata, gpointer udata);
> +
> +/**
> + * qemu_plugin_reg_callback() - register a function to be called on cb with
> + * given name
> + * @target_plugin: name of plugin that provides the callback
> + * @cb_name: name of the callback
> + * @function_pointer: pointer to function being registered
> + *
> + * Returns: true if function was registered successfully, false otherwise
> + */
> +bool qemu_plugin_reg_callback(const char *target_plugin, const char *cb_name,
> +                              cb_func_t function_pointer);
> +
> +/**
> + * qemu_plugin_unreg_callback() - unregister a function to be called on cb
> + * with given name
> + * @target_plugin: name of plugin that provides the callback
> + * @cb_name: name of the callback
> + * @function_pointer: pointer to function being unregistered
> + *
> + * Returns: true if function was unregistered successfully, false otherwise
> + */
> +bool qemu_plugin_unreg_callback(const char *target_plugin, const char *cb_name,
> +                                cb_func_t function_pointer);
> +
>  /**
>   * qemu_plugin_tb_get_insn() - retrieve handle for instruction
>   * @tb: opaque handle to TB passed to callback
> diff --git a/plugins/api.c b/plugins/api.c
> index 2078b16edb..1f7ea718dc 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -400,6 +400,117 @@ bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
>      return name && value && qapi_bool_parse(name, value, ret, NULL);
>  }
>  
> +bool qemu_plugin_create_callback(qemu_plugin_id_t id, const char *cb_name)
> +{
> +    struct qemu_plugin_ctx *ctx = plugin_id_to_ctx_locked(id);

c.f. last patch. I see no locking going on here.

> +    if (ctx == NULL) {
> +        error_report("Cannot create callback with invalid plugin ID");
> +        return false;
> +    }
> +
> +    if (ctx->version < QPP_MINIMUM_VERSION) {
> +        error_report("Plugin %s cannot create callbacks as its PLUGIN_VERSION"
> +                     " %d is below QPP_MINIMUM_VERSION (%d).",
> +                     ctx->name, ctx->version, QPP_MINIMUM_VERSION);
> +        return false;
> +    }
> +
> +    if (plugin_find_qpp_cb(ctx, cb_name)) {
> +        error_report("Plugin %s already created callback %s", ctx->name,
> +                     cb_name);
> +        return false;
> +    }
> +
> +    plugin_add_qpp_cb(ctx, cb_name);
> +    return true;
> +}
> +
> +bool qemu_plugin_run_callback(qemu_plugin_id_t id, const char *cb_name,
> +                              gpointer evdata, gpointer udata) {
> +    struct qemu_plugin_ctx *ctx = plugin_id_to_ctx_locked(id);

Or here.

> +    if (ctx == NULL) {
> +        error_report("Cannot run callback with invalid plugin ID");
> +        return false;
> +    }
> +
> +    struct qemu_plugin_qpp_cb *cb = plugin_find_qpp_cb(ctx, cb_name);
> +    if (!cb) {
> +        error_report("Can not run previously-unregistered callback %s in "
> +                     "plugin %s", cb_name, ctx->name);
> +        return false;
> +    }
> +
> +    for (int i = 0; i < cb->counter; i++) {
> +        cb_func_t qpp_cb_func = cb->registered_cb_funcs[i];
> +        qpp_cb_func(evdata, udata);
> +    }
> +
> +    return (cb->registered_cb_funcs[0] != NULL);
> +}
> +
> +bool qemu_plugin_reg_callback(const char *target_plugin, const char *cb_name,
> +                              cb_func_t function_pointer) {
> +    struct qemu_plugin_ctx *ctx = plugin_name_to_ctx_locked(target_plugin);
> +    if (ctx == NULL) {
> +        error_report("Cannot register callback with unknown plugin %s",
> +                     target_plugin);
> +      return false;
> +    }
> +
> +    struct qemu_plugin_qpp_cb *cb = plugin_find_qpp_cb(ctx, cb_name);
> +    if (!cb) {
> +        error_report("Cannot register a function to run on callback %s in "
> +                     "plugin %s as that callback does not exist",
> +                     cb_name, target_plugin);
> +        return false;
> +    }
> +
> +    if (cb->counter == QEMU_PLUGIN_EV_MAX) {

I'm a little confused why there is a relation to the number of callbacks
QPP can support and the list of qemu callback events.

> +        error_report("The maximum number of allowed callbacks are already "
> +                     "registered for callback %s in plugin %s",
> +                     cb_name, target_plugin);
> +        return false;
> +    }
> +
> +    cb->registered_cb_funcs[cb->counter] = function_pointer;
> +    cb->counter++;
> +    return true;
> +}
> +
> +bool qemu_plugin_unreg_callback(const char *target_plugin, const char *cb_name,
> +                              cb_func_t function_pointer) {
> +    struct qemu_plugin_ctx *ctx =
> plugin_name_to_ctx_locked(target_plugin);

Locking.

> +    if (ctx == NULL) {
> +        error_report("Cannot remove callback function from unknown plugin %s",
> +                     target_plugin);
> +        return false;
> +    }
> +
> +    struct qemu_plugin_qpp_cb *cb = plugin_find_qpp_cb(ctx, cb_name);
> +    if (!cb) {
> +        error_report("Cannot remove a function to run on callback %s in "
> +                     "plugin %s as that callback does not exist",
> +                     cb_name, target_plugin);
> +        return false;
> +    }
> +
> +    for (int i = 0; i < cb->counter; i++) {
> +        if (cb->registered_cb_funcs[i] == function_pointer) {
> +            for (int j = i + 1; j < cb->counter; j++) {
> +                cb->registered_cb_funcs[i] = cb->registered_cb_funcs[j];
> +                i++;
> +            }
> +            cb->registered_cb_funcs[i] = NULL;
> +            cb->counter--;
> +            return true;
> +        }
> +    }
> +    error_report("Function to remove not found in registered functions "
> +                 "for callback %s in plugin %s",
> +                 cb_name, target_plugin);
> +    return false;
> +}
> +
>  /*
>   * Binary path, start and end locations
>   */
> diff --git a/plugins/loader.c b/plugins/loader.c
> index ab01d0753c..7f047ebc99 100644
> --- a/plugins/loader.c
> +++ b/plugins/loader.c
> @@ -310,6 +310,7 @@ void plugin_add_qpp_cb(struct qemu_plugin_ctx *ctx, const char *name)
>      new_cb = qemu_memalign(qemu_dcache_linesize, sizeof(*new_cb));
>      memset(new_cb, 0, sizeof(*new_cb));
>      new_cb->name = name;
> +    new_cb->counter = 0;
>      QTAILQ_INSERT_TAIL(&ctx->qpp_cbs, new_cb, entry);
>  }
>  
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index 71f6c90549..b7013980cf 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -3,6 +3,10 @@
>    qemu_plugin_end_code;
>    qemu_plugin_entry_code;
>    qemu_plugin_get_hwaddr;
> +  qemu_plugin_create_callback;
> +  qemu_plugin_run_callback;
> +  qemu_plugin_reg_callback;
> +  qemu_plugin_unreg_callback;
>    qemu_plugin_hwaddr_device_name;
>    qemu_plugin_hwaddr_is_io;
>    qemu_plugin_hwaddr_phys_addr;


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 6/8] plugins: implement QPP import function
  2022-12-13 21:37 ` [PATCH 6/8] plugins: implement QPP import function Andrew Fasano
@ 2023-03-09 16:26   ` Alex Bennée
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2023-03-09 16:26 UTC (permalink / raw)
  To: Andrew Fasano; +Cc: qemu-devel, elysia.witham, erdnaxe, ma.mandourr


Andrew Fasano <fasano@mit.edu> writes:

> From: Elysia Witham <elysia.witham@ll.mit.edu>
>
> Plugins can export functions or import functions from other
> plugins using their name and the function name. This is also
> described in <qemu-plugin.h>.
>
> Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>
> ---
>  include/qemu/qemu-plugin.h   | 10 ++++++++++
>  plugins/api.c                | 21 +++++++++++++++++++++
>  plugins/qemu-plugins.symbols |  1 +
>  3 files changed, 32 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 4221545015..a0516e9a0e 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -354,6 +354,16 @@ size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
>   */
>  uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
>  
> +/**
> + * qemu_plugin_import_function() - return pointer to a function in another
> + * plugin
> + * @plugin: plugin name
> + * @function: function name
> + *
> + * Returns: NULL on failure, function pointer on success
> + */
> +gpointer qemu_plugin_import_function(const char *plugin, const char *function);
> +
>  /**
>   * qemu_plugin_create_callback() - create a new cb with given name
>   * @id: unique plugin id
> diff --git a/plugins/api.c b/plugins/api.c
> index 1f7ea718dc..a998df6942 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -400,6 +400,27 @@ bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
>      return name && value && qapi_bool_parse(name, value, ret, NULL);
>  }
>  
> +/*
> + * QPP: inter-plugin function resolution and callbacks
> + */
> +
> +gpointer qemu_plugin_import_function(const char *target_plugin,
> +                                     const char *function) {
> +    gpointer function_pointer = NULL;
> +    struct qemu_plugin_ctx *ctx = plugin_name_to_ctx_locked(target_plugin);
> +    if (ctx == NULL) {
> +        error_report("Unable to load plugin %s by name", target_plugin);
> +    } else if (g_module_symbol(ctx->handle, function,
> +               (gpointer *)&function_pointer)) {
> +        return function_pointer;
> +    } else {
> +      error_report("function: %s not found in plugin: %s", function,
> +                   target_plugin);
> +    }
> +    abort();
> +    return NULL;

Hmm when does __attribute__ ((constructor)) get called during the
g_module_open() of the plugin? I think aborting is a is a poor failure
mode in this case as you can bring down the whole translator with a poor
plugin load. I'd rather fail gracefully and uninstall the plugin.

> +}
> +
>  bool qemu_plugin_create_callback(qemu_plugin_id_t id, const char *cb_name)
>  {
>      struct qemu_plugin_ctx *ctx = plugin_id_to_ctx_locked(id);
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index b7013980cf..70a518839d 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -3,6 +3,7 @@
>    qemu_plugin_end_code;
>    qemu_plugin_entry_code;
>    qemu_plugin_get_hwaddr;
> +  qemu_plugin_import_function;
>    qemu_plugin_create_callback;
>    qemu_plugin_run_callback;
>    qemu_plugin_reg_callback;


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 8/8] tests: build and run QPP tests
  2022-12-13 21:37 ` [PATCH 8/8] tests: build and run QPP tests Andrew Fasano
@ 2023-03-09 16:37   ` Alex Bennée
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Bennée @ 2023-03-09 16:37 UTC (permalink / raw)
  To: Andrew Fasano; +Cc: qemu-devel, elysia.witham, erdnaxe, ma.mandourr


Andrew Fasano <fasano@mit.edu> writes:

> From: Elysia Witham <elysia.witham@ll.mit.edu>
>
> These test plugins demonstrate the QPP API changes by exporting
> and importing functions and creating and registering callbacks.
> These tests are integrated into the `make check-tcg` tests.
>
> Signed-off-by: Elysia Witham <elysia.witham@ll.mit.edu>
> Signed-off-by: Andrew Fasano <fasano@mit.edu>

I think this is fine for testing. I'd still like to see something useful
that uses QPP up-streamed into contrib/plugins.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-03-09 16:41 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-13 21:37 [PATCH 0/8] Inter-plugin interactions with QPP Andrew Fasano
2022-12-13 21:37 ` [PATCH 1/8] docs/devel: describe QPP API Andrew Fasano
2023-03-09 14:15   ` Alex Bennée
2022-12-13 21:37 ` [PATCH 2/8] plugins: version 2, require unique plugin names Andrew Fasano
2023-03-09 14:17   ` Alex Bennée
2022-12-13 21:37 ` [PATCH 3/8] plugins: add id_to_plugin_name Andrew Fasano
2023-03-09 14:45   ` Alex Bennée
2022-12-13 21:37 ` [PATCH 4/8] plugins: add core API functions for QPP callbacks Andrew Fasano
2023-03-09 16:09   ` Alex Bennée
2022-12-13 21:37 ` [PATCH 5/8] plugins: implement " Andrew Fasano
2023-03-09 16:17   ` Alex Bennée
2022-12-13 21:37 ` [PATCH 6/8] plugins: implement QPP import function Andrew Fasano
2023-03-09 16:26   ` Alex Bennée
2022-12-13 21:37 ` [PATCH 7/8] include/qemu: added macro for " Andrew Fasano
2022-12-13 21:37 ` [PATCH 8/8] tests: build and run QPP tests Andrew Fasano
2023-03-09 16:37   ` Alex Bennée

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).