| Message ID | 20240227144335.1196131-24-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | maintainer updates for 9.0 pre-PR (tests, plugin register support) | expand |
On 2024/02/27 23:43, Alex Bennée wrote: > We can only request a list of registers once the vCPU has been > initialised so the user needs to use either call the get function on > vCPU initialisation or during the translation phase. > > We don't expose the reg number to the plugin instead hiding it behind > an opaque handle. For now this is just the gdb_regnum encapsulated in > an anonymous GPOINTER but in future as we add more state for plugins > to track we can expand it. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706 > Cc: Akihiko Odaki <akihiko.odaki@daynix.com> > Message-Id: <20240103173349.398526-39-alex.bennee@linaro.org> > Based-on: <20231025093128.33116-18-akihiko.odaki@daynix.com> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > --- > v4 > - the get/read_registers functions are now implicitly for current > vCPU only to accidental cpu != current_cpu uses. > v5 > - make reg_handles as per-CPUPluginState variable. > v6 > - for now just wrap gdb_regnum > v7 > - minor style fixes > --- > include/qemu/qemu-plugin.h | 48 +++++++++++++++++++++++++++++-- > plugins/api.c | 55 ++++++++++++++++++++++++++++++++++++ > plugins/qemu-plugins.symbols | 2 ++ > 3 files changed, 103 insertions(+), 2 deletions(-) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index 93981f8f89f..6c5580f4428 100644 > --- a/include/qemu/qemu-plugin.h > +++ b/include/qemu/qemu-plugin.h > @@ -11,6 +11,7 @@ > #ifndef QEMU_QEMU_PLUGIN_H > #define QEMU_QEMU_PLUGIN_H > > +#include <glib.h> > #include <inttypes.h> > #include <stdbool.h> > #include <stddef.h> > @@ -229,8 +230,8 @@ struct qemu_plugin_insn; > * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs > * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs > * > - * Note: currently unused, plugins cannot read or change system > - * register state. > + * Note: currently QEMU_PLUGIN_CB_RW_REGS is unused, plugins cannot change > + * system register state. > */ > enum qemu_plugin_cb_flags { > QEMU_PLUGIN_CB_NO_REGS, > @@ -707,4 +708,47 @@ uint64_t qemu_plugin_end_code(void); > QEMU_PLUGIN_API > uint64_t qemu_plugin_entry_code(void); > > +/** struct qemu_plugin_register - Opaque handle for register access */ > +struct qemu_plugin_register; > + > +/** > + * typedef qemu_plugin_reg_descriptor - register descriptions > + * > + * @handle: opaque handle for retrieving value with qemu_plugin_read_register > + * @name: register name > + * @feature: optional feature descriptor, can be NULL > + */ > +typedef struct { > + struct qemu_plugin_register *handle; > + const char *name; > + const char *feature; > +} qemu_plugin_reg_descriptor; > + > +/** > + * qemu_plugin_get_registers() - return register list for current vCPU > + * > + * Returns a potentially empty GArray of qemu_plugin_reg_descriptor. > + * Caller frees the array (but not the const strings). > + * > + * Should be used from a qemu_plugin_register_vcpu_init_cb() callback > + * after the vCPU is initialised, i.e. in the vCPU context. > + */ Qualify with QEMU_PLUGIN_API, which was apparently added after this patch was authored. > +GArray *qemu_plugin_get_registers(void); > + > +/** > + * qemu_plugin_read_register() - read register for current vCPU > + * > + * @handle: a @qemu_plugin_reg_handle handle > + * @buf: A GByteArray for the data owned by the plugin > + * > + * This function is only available in a context that register read access is > + * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag. > + * > + * Returns the size of the read register. The content of @buf is in target byte > + * order. On failure returns -1 Add a period after -1. > + */ > +int qemu_plugin_read_register(struct qemu_plugin_register *handle, > + GByteArray *buf); > + > + > #endif /* QEMU_QEMU_PLUGIN_H */ > diff --git a/plugins/api.c b/plugins/api.c > index 54df72c1c00..908fe7e6fa3 100644 > --- a/plugins/api.c > +++ b/plugins/api.c > @@ -8,6 +8,7 @@ > * > * qemu_plugin_tb > * qemu_plugin_insn > + * qemu_plugin_register > * > * Which can then be passed back into the API to do additional things. > * As such all the public functions in here are exported in > @@ -35,10 +36,12 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/main-loop.h" > #include "qemu/plugin.h" > #include "qemu/log.h" > #include "tcg/tcg.h" > #include "exec/exec-all.h" > +#include "exec/gdbstub.h" > #include "exec/ram_addr.h" > #include "disas/disas.h" > #include "plugin.h" > @@ -410,3 +413,55 @@ uint64_t qemu_plugin_entry_code(void) > #endif > return entry; > } > + > +/* > + * Create register handles. > + * > + * We need to create a handle for each register so the plugin > + * infrastructure can call gdbstub to read a register. They are > + * currently just a pointer encapsulation of the gdb_regnum but in s/gdb_regnum/gdb_reg/ With all comments fixed, Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com> > + * future may hold internal plugin state so its important plugin > + * authors are not tempted to treat them as numbers. > + * > + * We also construct a result array with those handles and some > + * ancillary data the plugin might find useful. > + */ > + > +static GArray *create_register_handles(GArray *gdbstub_regs) > +{ > + GArray *find_data = g_array_new(true, true, > + sizeof(qemu_plugin_reg_descriptor)); > + > + for (int i = 0; i < gdbstub_regs->len; i++) { > + GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i); > + qemu_plugin_reg_descriptor desc; > + > + /* skip "un-named" regs */ > + if (!grd->name) { > + continue; > + } > + > + /* Create a record for the plugin */ > + desc.handle = GINT_TO_POINTER(grd->gdb_reg); > + desc.name = g_intern_string(grd->name); > + desc.feature = g_intern_string(grd->feature_name); > + g_array_append_val(find_data, desc); > + } > + > + return find_data; > +} > + > +GArray *qemu_plugin_get_registers(void) > +{ > + g_assert(current_cpu); > + > + g_autoptr(GArray) regs = gdb_get_register_list(current_cpu); > + return create_register_handles(regs); > +} > + > +int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf) > +{ > + g_assert(current_cpu); > + > + return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg)); > +} > diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols > index adb67608598..27fe97239be 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_get_registers; > qemu_plugin_hwaddr_device_name; > qemu_plugin_hwaddr_is_io; > qemu_plugin_hwaddr_phys_addr; > @@ -19,6 +20,7 @@ > qemu_plugin_num_vcpus; > qemu_plugin_outs; > qemu_plugin_path_to_binary; > + qemu_plugin_read_register; > qemu_plugin_register_atexit_cb; > qemu_plugin_register_flush_cb; > qemu_plugin_register_vcpu_exit_cb;
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index 93981f8f89f..6c5580f4428 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -11,6 +11,7 @@ #ifndef QEMU_QEMU_PLUGIN_H #define QEMU_QEMU_PLUGIN_H +#include <glib.h> #include <inttypes.h> #include <stdbool.h> #include <stddef.h> @@ -229,8 +230,8 @@ struct qemu_plugin_insn; * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs * - * Note: currently unused, plugins cannot read or change system - * register state. + * Note: currently QEMU_PLUGIN_CB_RW_REGS is unused, plugins cannot change + * system register state. */ enum qemu_plugin_cb_flags { QEMU_PLUGIN_CB_NO_REGS, @@ -707,4 +708,47 @@ uint64_t qemu_plugin_end_code(void); QEMU_PLUGIN_API uint64_t qemu_plugin_entry_code(void); +/** struct qemu_plugin_register - Opaque handle for register access */ +struct qemu_plugin_register; + +/** + * typedef qemu_plugin_reg_descriptor - register descriptions + * + * @handle: opaque handle for retrieving value with qemu_plugin_read_register + * @name: register name + * @feature: optional feature descriptor, can be NULL + */ +typedef struct { + struct qemu_plugin_register *handle; + const char *name; + const char *feature; +} qemu_plugin_reg_descriptor; + +/** + * qemu_plugin_get_registers() - return register list for current vCPU + * + * Returns a potentially empty GArray of qemu_plugin_reg_descriptor. + * Caller frees the array (but not the const strings). + * + * Should be used from a qemu_plugin_register_vcpu_init_cb() callback + * after the vCPU is initialised, i.e. in the vCPU context. + */ +GArray *qemu_plugin_get_registers(void); + +/** + * qemu_plugin_read_register() - read register for current vCPU + * + * @handle: a @qemu_plugin_reg_handle handle + * @buf: A GByteArray for the data owned by the plugin + * + * This function is only available in a context that register read access is + * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag. + * + * Returns the size of the read register. The content of @buf is in target byte + * order. On failure returns -1 + */ +int qemu_plugin_read_register(struct qemu_plugin_register *handle, + GByteArray *buf); + + #endif /* QEMU_QEMU_PLUGIN_H */ diff --git a/plugins/api.c b/plugins/api.c index 54df72c1c00..908fe7e6fa3 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -8,6 +8,7 @@ * * qemu_plugin_tb * qemu_plugin_insn + * qemu_plugin_register * * Which can then be passed back into the API to do additional things. * As such all the public functions in here are exported in @@ -35,10 +36,12 @@ */ #include "qemu/osdep.h" +#include "qemu/main-loop.h" #include "qemu/plugin.h" #include "qemu/log.h" #include "tcg/tcg.h" #include "exec/exec-all.h" +#include "exec/gdbstub.h" #include "exec/ram_addr.h" #include "disas/disas.h" #include "plugin.h" @@ -410,3 +413,55 @@ uint64_t qemu_plugin_entry_code(void) #endif return entry; } + +/* + * Create register handles. + * + * We need to create a handle for each register so the plugin + * infrastructure can call gdbstub to read a register. They are + * currently just a pointer encapsulation of the gdb_regnum but in + * future may hold internal plugin state so its important plugin + * authors are not tempted to treat them as numbers. + * + * We also construct a result array with those handles and some + * ancillary data the plugin might find useful. + */ + +static GArray *create_register_handles(GArray *gdbstub_regs) +{ + GArray *find_data = g_array_new(true, true, + sizeof(qemu_plugin_reg_descriptor)); + + for (int i = 0; i < gdbstub_regs->len; i++) { + GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i); + qemu_plugin_reg_descriptor desc; + + /* skip "un-named" regs */ + if (!grd->name) { + continue; + } + + /* Create a record for the plugin */ + desc.handle = GINT_TO_POINTER(grd->gdb_reg); + desc.name = g_intern_string(grd->name); + desc.feature = g_intern_string(grd->feature_name); + g_array_append_val(find_data, desc); + } + + return find_data; +} + +GArray *qemu_plugin_get_registers(void) +{ + g_assert(current_cpu); + + g_autoptr(GArray) regs = gdb_get_register_list(current_cpu); + return create_register_handles(regs); +} + +int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf) +{ + g_assert(current_cpu); + + return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg)); +} diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols index adb67608598..27fe97239be 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_get_registers; qemu_plugin_hwaddr_device_name; qemu_plugin_hwaddr_is_io; qemu_plugin_hwaddr_phys_addr; @@ -19,6 +20,7 @@ qemu_plugin_num_vcpus; qemu_plugin_outs; qemu_plugin_path_to_binary; + qemu_plugin_read_register; qemu_plugin_register_atexit_cb; qemu_plugin_register_flush_cb; qemu_plugin_register_vcpu_exit_cb;