* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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 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