On 2/16/26 4:54 AM, Florian Hofhammer wrote:
On 13/02/2026 19:36, Pierrick Bouvier wrote:
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?
The idea of blocking writes to the PC via the register write API came up
in https://lists.gnu.org/archive/html/qemu-devel/2025-12/msg01671.html.
As it stands, writes to the PC are silently swallowed, and preventing
that could reduce confusion for plugin authors. It makes it clear why
there would be a special API for setting the PC instead of using the
generic register write API.
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.
Alternatively, I could add a remark to the documentation for the
register write API that the PC can only be modified via the dedicated
API and not change the implementation at all. But I think I'll have to
defer that decision to you and Alex as the plugin subsystem maintainers.
My original message was for Alex, in case it was not clear, or too implicit.
I don't want to hijack this thread since he explicitely asked for this,
but since it starts to go in a complicated direction, I just felt asking
again "Is it really what we want?".
On my side, it's definitely ok with just a comment in documentation. If
we have a problem later, we can still change things.
So maybe we can just extend API with PC diversion, and not overthink too much
about potential consequences on registers.
Regards,
Pierrick
Best,
Florian
