Akihiko Odaki <akihiko.odaki@xxxxxxxxxx> writes: > On 2024/01/04 2:33, 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. This allows for a bit of future proofing should the >> internals need to be changed while also being hashed against the >> CPUClass so we can handle different register sets per-vCPU in >> hetrogenous situations. >> Having an internal state within the plugins also allows us to expand >> the interface in future (for example providing callbacks on register >> change if the translator can track changes). >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706 >> Cc: Akihiko Odaki <akihiko.odaki@xxxxxxxxxx> >> Based-on: <20231025093128.33116-18-akihiko.odaki@xxxxxxxxxx> >> Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx> >> --- >> v3 >> - also g_intern_string the register name >> - make get_registers documentation a bit less verbose >> v2 >> - use new get whole list api, and expose upwards >> vAJB: >> The main difference to Akikio's version is hiding the gdb register >> detail from the plugin for the reasons described above. >> --- >> include/qemu/qemu-plugin.h | 51 +++++++++++++++++- >> plugins/api.c | 102 +++++++++++++++++++++++++++++++++++ >> plugins/qemu-plugins.symbols | 2 + >> 3 files changed, 153 insertions(+), 2 deletions(-) >> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h >> index 4daab6efd29..95380895f81 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> >> @@ -227,8 +228,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, >> @@ -708,4 +709,50 @@ 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; > > Just in case you missed my comment for the earlier version: > > What about identifying a register with an index in an array returned > by qemu_plugin_get_registers(). That saves troubles having the handle > member in qemu_plugin_reg_descriptor. The handle gets de-referenced internally in the plugin api and additional checking could be added there. If we pass an index then we'd end up having to track the index assigned during get_registers as well as account for a potential skew in the index value if the register layout varies between vCPUs (although I admit this is future proofing for potential heterogeneous models). The concept of opaque handle == pointer is fairly common in the QEMU code base. We are not making it hard for a plugin author to bypass this "protection", just making it clear if you do so your violating the principle of the API. > > Regards, > Akihiko Odaki -- Alex Bennée Virtualisation Tech Lead @ Linaro
