On 2/13/26 5:38 AM, Florian Hofhammer wrote:
On 26/01/2026 21:46, Alex Bennée wrote:> Alex Bennée <[email protected]>
writes:
Florian Hofhammer <[email protected]> writes:
Some registers should be marked as read-only from a plugin API
perspective, as writing to them via qemu_plugin_write_register has no
effect. This includes the program counter, and we expose this fact to
the plugins with this patch.
Signed-off-by: Florian Hofhammer <[email protected]>
---
include/qemu/qemu-plugin.h | 2 ++
plugins/api.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index f976b62030..1f25fb2b40 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -942,11 +942,13 @@ struct qemu_plugin_register;
* writing value with qemu_plugin_write_register
* @name: register name
* @feature: optional feature descriptor, can be NULL
+ * @is_readonly: true if the register cannot be written via
qemu_plugin_write_register
*/
typedef struct {
struct qemu_plugin_register *handle;
const char *name;
const char *feature;
+ bool is_readonly;
} qemu_plugin_reg_descriptor;
/**
diff --git a/plugins/api.c b/plugins/api.c
index fc19bdb40b..de8c32db50 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -403,6 +403,12 @@ bool qemu_plugin_bool_parse(const char *name, const char
*value, bool *ret)
* ancillary data the plugin might find useful.
*/
+static const char pc_str[] = "pc"; // generic name for program counter
+static const char eip_str[] = "eip"; // x86 specific name for program counter
+static const char rip_str[] = "rip"; // x86_64 specific name for program
counter
+static const char pswa_str[] = "pswa"; // s390x specific name for program
counter
+static const char iaoq_str[] = "iaoq"; // HP/PA specific name for program
counter
+static const char rpc_str[] = "rpc"; // microblaze specific name for
program counter
It's ugly but I can't think of anything better as you say in the commit message.
static GArray *create_register_handles(GArray *gdbstub_regs)
{
GArray *find_data = g_array_new(true, true,
@@ -417,9 +423,20 @@ static GArray *create_register_handles(GArray
*gdbstub_regs)
continue;
}
+ gint plugin_ro_bit = 0;
/* Create a record for the plugin */
desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
desc.name = g_intern_string(grd->name);
Lets just set desc.is_readonly to false here.
+ if (!strcmp(desc.name, pc_str)
+ || !strcmp(desc.name, eip_str)
+ || !strcmp(desc.name, rip_str)
+ || !strcmp(desc.name, pswa_str)
+ || !strcmp(desc.name, iaoq_str)
+ || !strcmp(desc.name, rpc_str)
+ ) {
+ plugin_ro_bit = 1;
+ }
+ desc.is_readonly = plugin_ro_bit == 1 ? true : false;
And fold setting it to true into the if statement. I have a marginal
preference for g_strcmp0(desc.name, eip_str) == 0 style tests.
The option of course would be to skip the register all together. Do we
have code which will have multiple vaddr's for the same TB where using
the TB data wouldn't be sufficient?
Skipping the register altogether would basically make it invisible to
plugins and therefore remove functionality. A plugin might want to get a
list of all registers (which would not be complete anymore), or get the
current PC value. If I didn't miss anything, the latter might not
necessarily be possible anymore, as a callback (especially the simple
callback type) doesn't have access to the current TB data and can't
necessarily pass userdata to the callback, so it wouldn't be able to get
the PC from there anymore.
desc.feature = g_intern_string(grd->feature_name);
g_array_append_val(find_data, desc);
}
Otherwise:
Reviewed-by: Alex Bennée <[email protected]>
Seeing there are some questions around this, I have another one.
Does it really matter if a user can read/write pc register?
It seems to bring a lot of complexity to prevent this (especially the
string per arch approach, even though that's the best we can do), and
I'm not sure what's the actual benefit. If someone tries something QEMU
plugins don't support in general, they can always send a bug report, or
even a patch.
I'm not really sure to why someone would have a bad idea mixing register
writes and pc diversion. And if they do, it would not be surprising it
breaks things.
So maybe we can just extend API with PC diversion, and not overthink too
much about potential consequences on registers.
Regards,
Pierrick
