Re: [PATCH 5/8] plugins: implement QPP callbacks

["Alex Bennée" <alex.bennee@linaro.org>]

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