On 2024/01/04 21:22, Alex Bennée wrote:
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.
Now I get the idea. Indeed index values are not guaranteed to be stable
across CPUs.
Why don't you pass gdb_reg_num as is then? qemu_plugin_register has the
name member, but it's unused so gdb_reg_num is effectively the only
member we need. You can even cast gdb_reg_num to (struct
qemu_plugin_register *), but I don't think pointers are more opaque or
future-proof than integers.
