Now gdbstub has gained the ability to extend its command tables we can
allow it to trigger plugin callbacks. This is probably most useful for
QEMU developers debugging plugins themselves but might be useful for
other stuff.

Trigger the callback by sending:

  maintenance packet Qqemu.plugin_cb

I've extended the memory plugin to report on the packet.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/qemu/plugin-event.h  |  1 +
 include/qemu/qemu-plugin.h   | 16 ++++++++++++++++
 plugins/plugin.h             |  9 +++++++++
 plugins/api.c                | 18 ++++++++++++++++++
 plugins/core.c               | 37 ++++++++++++++++++++++++++++++++++++
 tests/tcg/plugins/mem.c      | 11 +++++++++--
 plugins/qemu-plugins.symbols |  1 +
 7 files changed, 91 insertions(+), 2 deletions(-)

diff --git a/include/qemu/plugin-event.h b/include/qemu/plugin-event.h
index 7056d8427b..d9aa56cf4f 100644
--- a/include/qemu/plugin-event.h
+++ b/include/qemu/plugin-event.h
@@ -20,6 +20,7 @@ enum qemu_plugin_event {
     QEMU_PLUGIN_EV_VCPU_SYSCALL_RET,
     QEMU_PLUGIN_EV_FLUSH,
     QEMU_PLUGIN_EV_ATEXIT,
+    QEMU_PLUGIN_EV_GDBSTUB,
     QEMU_PLUGIN_EV_MAX, /* total number of plugin events we support */
 };
 
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 622c9a0232..99c3b365aa 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -802,6 +802,22 @@ QEMU_PLUGIN_API
 void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
                                     qemu_plugin_udata_cb_t cb, void *userdata);
 
+
+/**
+ * qemu_plugin_register_gdb_cb() - register a gdb callback
+ * @id: plugin ID
+ * @cb: callback
+ * @userdata: user data for callback
+ *
+ * When using the gdbstub to debug a guest you can send a command that
+ * will trigger the callback. This is useful if you want the plugin to
+ * print out collected state at particular points when debugging a
+ * program.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_register_gdb_cb(qemu_plugin_id_t id,
+                                 qemu_plugin_udata_cb_t cb, void *userdata);
+
 /* returns how many vcpus were started at this point */
 int qemu_plugin_num_vcpus(void);
 
diff --git a/plugins/plugin.h b/plugins/plugin.h
index 30e2299a54..f37667c9fb 100644
--- a/plugins/plugin.h
+++ b/plugins/plugin.h
@@ -118,4 +118,13 @@ struct qemu_plugin_scoreboard *plugin_scoreboard_new(size_t element_size);
 
 void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score);
 
+/**
+ * plugin_register_gdbstub_commands - register gdbstub commands
+ *
+ * This should only be called once to register gdbstub commands so we
+ * can trigger callbacks if needed.
+ */
+void plugin_register_gdbstub_commands(void);
+
+
 #endif /* PLUGIN_H */
diff --git a/plugins/api.c b/plugins/api.c
index 24ea64e2de..62141616f4 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -681,3 +681,21 @@ void qemu_plugin_update_ns(const void *handle, int64_t new_time)
     }
 #endif
 }
+
+/*
+ * gdbstub hooks
+ */
+
+static bool gdbstub_callbacks;
+
+void qemu_plugin_register_gdb_cb(qemu_plugin_id_t id,
+                                 qemu_plugin_udata_cb_t cb,
+                                 void *udata)
+{
+    plugin_register_cb_udata(id, QEMU_PLUGIN_EV_GDBSTUB, cb, udata);
+
+    if (!gdbstub_callbacks) {
+        plugin_register_gdbstub_commands();
+        gdbstub_callbacks = true;
+    }
+}
diff --git a/plugins/core.c b/plugins/core.c
index bb105e8e68..e7fce08799 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -23,6 +23,7 @@
 #include "qemu/xxhash.h"
 #include "qemu/rcu.h"
 #include "hw/core/cpu.h"
+#include "gdbstub/commands.h"
 
 #include "exec/exec-all.h"
 #include "exec/tb-flush.h"
@@ -147,6 +148,7 @@ static void plugin_cb__udata(enum qemu_plugin_event ev)
 
     switch (ev) {
     case QEMU_PLUGIN_EV_ATEXIT:
+    case QEMU_PLUGIN_EV_GDBSTUB:
         QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
             qemu_plugin_udata_cb_t func = cb->f.udata;
 
@@ -768,3 +770,38 @@ void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score)
     g_array_free(score->data, TRUE);
     g_free(score);
 }
+
+/*
+ * gdbstub integration
+ */
+
+static void handle_plugin_cb(GArray *params, void *user_ctx)
+{
+    plugin_cb__udata(QEMU_PLUGIN_EV_GDBSTUB);
+    gdb_put_packet("OK");
+}
+
+enum Command {
+    PluginCallback,
+    NUM_GDB_CMDS
+};
+
+static const GdbCmdParseEntry cmd_handler_table[NUM_GDB_CMDS] = {
+    [PluginCallback] = {
+        .handler = handle_plugin_cb,
+        .cmd_startswith = true,
+        .cmd = "qemu.plugin_cb",
+        .schema = "s?",
+    },
+};
+
+void plugin_register_gdbstub_commands(void)
+{
+    g_autoptr(GPtrArray) set_table = g_ptr_array_new();
+
+    for (int i = 0; i < NUM_GDB_CMDS; i++) {
+        g_ptr_array_add(set_table, (gpointer) &cmd_handler_table[PluginCallback]);
+    }
+
+    gdb_extend_set_table(set_table);
+}
diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
index b0fa8a9f27..d416d92fc2 100644
--- a/tests/tcg/plugins/mem.c
+++ b/tests/tcg/plugins/mem.c
@@ -75,8 +75,7 @@ static gint addr_order(gconstpointer a, gconstpointer b)
     return na->region_address > nb->region_address ? 1 : -1;
 }
 
-
-static void plugin_exit(qemu_plugin_id_t id, void *p)
+static void plugin_report(qemu_plugin_id_t id, void *p)
 {
     g_autoptr(GString) out = g_string_new("");
 
@@ -90,6 +89,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     }
     qemu_plugin_outs(out->str);
 
+    g_mutex_lock(&lock);
 
     if (do_region_summary) {
         GList *counts = g_hash_table_get_values(regions);
@@ -114,6 +114,12 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
         qemu_plugin_outs(out->str);
     }
 
+    g_mutex_unlock(&lock);
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+    plugin_report(id, p);
     qemu_plugin_scoreboard_free(counts);
 }
 
@@ -400,6 +406,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
         counts, CPUCount, mem_count);
     io_count = qemu_plugin_scoreboard_u64_in_struct(counts, CPUCount, io_count);
     qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+    qemu_plugin_register_gdb_cb(id, plugin_report, NULL);
     qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
     return 0;
 }
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index 032661f9ea..d272e8e0f3 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -25,6 +25,7 @@
   qemu_plugin_read_register;
   qemu_plugin_register_atexit_cb;
   qemu_plugin_register_flush_cb;
+  qemu_plugin_register_gdb_cb;
   qemu_plugin_register_vcpu_exit_cb;
   qemu_plugin_register_vcpu_idle_cb;
   qemu_plugin_register_vcpu_init_cb;
-- 
2.39.5

On 10/22/24 03:56, Alex Bennée wrote:
> Now gdbstub has gained the ability to extend its command tables we can
> allow it to trigger plugin callbacks. This is probably most useful for
> QEMU developers debugging plugins themselves but might be useful for
> other stuff.
> 
> Trigger the callback by sending:
> 
>    maintenance packet Qqemu.plugin_cb
> 
> I've extended the memory plugin to report on the packet.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/qemu/plugin-event.h  |  1 +
>   include/qemu/qemu-plugin.h   | 16 ++++++++++++++++
>   plugins/plugin.h             |  9 +++++++++
>   plugins/api.c                | 18 ++++++++++++++++++
>   plugins/core.c               | 37 ++++++++++++++++++++++++++++++++++++
>   tests/tcg/plugins/mem.c      | 11 +++++++++--
>   plugins/qemu-plugins.symbols |  1 +
>   7 files changed, 91 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/plugin-event.h b/include/qemu/plugin-event.h
> index 7056d8427b..d9aa56cf4f 100644
> --- a/include/qemu/plugin-event.h
> +++ b/include/qemu/plugin-event.h
> @@ -20,6 +20,7 @@ enum qemu_plugin_event {
>       QEMU_PLUGIN_EV_VCPU_SYSCALL_RET,
>       QEMU_PLUGIN_EV_FLUSH,
>       QEMU_PLUGIN_EV_ATEXIT,
> +    QEMU_PLUGIN_EV_GDBSTUB,
>       QEMU_PLUGIN_EV_MAX, /* total number of plugin events we support */
>   };
>   
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 622c9a0232..99c3b365aa 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -802,6 +802,22 @@ QEMU_PLUGIN_API
>   void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
>                                       qemu_plugin_udata_cb_t cb, void *userdata);
>   
> +
> +/**
> + * qemu_plugin_register_gdb_cb() - register a gdb callback
> + * @id: plugin ID
> + * @cb: callback
> + * @userdata: user data for callback
> + *
> + * When using the gdbstub to debug a guest you can send a command that
> + * will trigger the callback. This is useful if you want the plugin to
> + * print out collected state at particular points when debugging a
> + * program.
> + */
> +QEMU_PLUGIN_API
> +void qemu_plugin_register_gdb_cb(qemu_plugin_id_t id,
> +                                 qemu_plugin_udata_cb_t cb, void *userdata);
> +
>   /* returns how many vcpus were started at this point */
>   int qemu_plugin_num_vcpus(void);
>   
> diff --git a/plugins/plugin.h b/plugins/plugin.h
> index 30e2299a54..f37667c9fb 100644
> --- a/plugins/plugin.h
> +++ b/plugins/plugin.h
> @@ -118,4 +118,13 @@ struct qemu_plugin_scoreboard *plugin_scoreboard_new(size_t element_size);
>   
>   void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score);
>   
> +/**
> + * plugin_register_gdbstub_commands - register gdbstub commands
> + *
> + * This should only be called once to register gdbstub commands so we
> + * can trigger callbacks if needed.
> + */
> +void plugin_register_gdbstub_commands(void);
> +
> +
>   #endif /* PLUGIN_H */
> diff --git a/plugins/api.c b/plugins/api.c
> index 24ea64e2de..62141616f4 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -681,3 +681,21 @@ void qemu_plugin_update_ns(const void *handle, int64_t new_time)
>       }
>   #endif
>   }
> +
> +/*
> + * gdbstub hooks
> + */
> +
> +static bool gdbstub_callbacks;
> +
> +void qemu_plugin_register_gdb_cb(qemu_plugin_id_t id,
> +                                 qemu_plugin_udata_cb_t cb,
> +                                 void *udata)
> +{
> +    plugin_register_cb_udata(id, QEMU_PLUGIN_EV_GDBSTUB, cb, udata);
> +
> +    if (!gdbstub_callbacks) {
> +        plugin_register_gdbstub_commands();
> +        gdbstub_callbacks = true;
> +    }
> +}
> diff --git a/plugins/core.c b/plugins/core.c
> index bb105e8e68..e7fce08799 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -23,6 +23,7 @@
>   #include "qemu/xxhash.h"
>   #include "qemu/rcu.h"
>   #include "hw/core/cpu.h"
> +#include "gdbstub/commands.h"
>   
>   #include "exec/exec-all.h"
>   #include "exec/tb-flush.h"
> @@ -147,6 +148,7 @@ static void plugin_cb__udata(enum qemu_plugin_event ev)
>   
>       switch (ev) {
>       case QEMU_PLUGIN_EV_ATEXIT:
> +    case QEMU_PLUGIN_EV_GDBSTUB:
>           QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
>               qemu_plugin_udata_cb_t func = cb->f.udata;
>   
> @@ -768,3 +770,38 @@ void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score)
>       g_array_free(score->data, TRUE);
>       g_free(score);
>   }
> +
> +/*
> + * gdbstub integration
> + */
> +
> +static void handle_plugin_cb(GArray *params, void *user_ctx)
> +{
> +    plugin_cb__udata(QEMU_PLUGIN_EV_GDBSTUB);
> +    gdb_put_packet("OK");
> +}
> +
> +enum Command {
> +    PluginCallback,
> +    NUM_GDB_CMDS
> +};
> +
> +static const GdbCmdParseEntry cmd_handler_table[NUM_GDB_CMDS] = {
> +    [PluginCallback] = {
> +        .handler = handle_plugin_cb,
> +        .cmd_startswith = true,
> +        .cmd = "qemu.plugin_cb",
> +        .schema = "s?",
> +    },
> +};
> +
> +void plugin_register_gdbstub_commands(void)
> +{
> +    g_autoptr(GPtrArray) set_table = g_ptr_array_new();
> +
> +    for (int i = 0; i < NUM_GDB_CMDS; i++) {
> +        g_ptr_array_add(set_table, (gpointer) &cmd_handler_table[PluginCallback]);
> +    }
> +
> +    gdb_extend_set_table(set_table);
> +}
> diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
> index b0fa8a9f27..d416d92fc2 100644
> --- a/tests/tcg/plugins/mem.c
> +++ b/tests/tcg/plugins/mem.c
> @@ -75,8 +75,7 @@ static gint addr_order(gconstpointer a, gconstpointer b)
>       return na->region_address > nb->region_address ? 1 : -1;
>   }
>   
> -
> -static void plugin_exit(qemu_plugin_id_t id, void *p)
> +static void plugin_report(qemu_plugin_id_t id, void *p)
>   {
>       g_autoptr(GString) out = g_string_new("");
>   
> @@ -90,6 +89,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>       }
>       qemu_plugin_outs(out->str);
>   
> +    g_mutex_lock(&lock);
>   
>       if (do_region_summary) {
>           GList *counts = g_hash_table_get_values(regions);
> @@ -114,6 +114,12 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>           qemu_plugin_outs(out->str);
>       }
>   
> +    g_mutex_unlock(&lock);
> +}
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +    plugin_report(id, p);
>       qemu_plugin_scoreboard_free(counts);
>   }
>   
> @@ -400,6 +406,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>           counts, CPUCount, mem_count);
>       io_count = qemu_plugin_scoreboard_u64_in_struct(counts, CPUCount, io_count);
>       qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_gdb_cb(id, plugin_report, NULL);
>       qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
>       return 0;
>   }
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index 032661f9ea..d272e8e0f3 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -25,6 +25,7 @@
>     qemu_plugin_read_register;
>     qemu_plugin_register_atexit_cb;
>     qemu_plugin_register_flush_cb;
> +  qemu_plugin_register_gdb_cb;
>     qemu_plugin_register_vcpu_exit_cb;
>     qemu_plugin_register_vcpu_idle_cb;
>     qemu_plugin_register_vcpu_init_cb;

This is a very nice addition!
Beyond developers, it can be very useful for plugins because you can 
start to script instrumentation using gdb.

Would that be possible to register several commands, with different 
names? It seems a bit arbitrary to be able to register only one command, 
with a fixed name, when we could have several.

In more, it could be nice to be able to pass a list of parameters (as a 
simple string or string array) to the callback on plugin side.