On 24/02/2026 18:49, Alex Bennée wrote: > Florian Hofhammer <[email protected]> writes: > >> The opaque register handle encodes whether a register is read-only in >> the lowest bit and prevents writing to the register via the plugin API >> in this case. >> >> Signed-off-by: Florian Hofhammer <[email protected]> >> --- >> plugins/api.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/plugins/api.c b/plugins/api.c >> index b2c52d2a09..3a8479ddce 100644 >> --- a/plugins/api.c >> +++ b/plugins/api.c >> @@ -424,6 +424,7 @@ static GArray *create_register_handles(GArray >> *gdbstub_regs) >> for (int i = 0; i < gdbstub_regs->len; i++) { >> GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i); >> qemu_plugin_reg_descriptor desc; >> + gint plugin_ro_bit = 0; >> >> /* skip "un-named" regs */ >> if (!grd->name) { >> @@ -431,7 +432,6 @@ static GArray *create_register_handles(GArray >> *gdbstub_regs) >> } >> >> /* Create a record for the plugin */ >> - desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1); >> desc.name = g_intern_string(grd->name); >> desc.is_readonly = false; >> if (g_strcmp0(desc.name, pc_str) == 0 >> @@ -442,7 +442,9 @@ static GArray *create_register_handles(GArray >> *gdbstub_regs) >> || g_strcmp0(desc.name, rpc_str) == 0 >> ) { >> desc.is_readonly = true; >> + plugin_ro_bit = 1; >> } >> + desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit); >> desc.feature = g_intern_string(grd->feature_name); >> g_array_append_val(find_data, desc); >> } >> @@ -467,7 +469,7 @@ bool qemu_plugin_read_register(struct >> qemu_plugin_register *reg, >> return false; >> } >> >> - return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > >> 0); >> + return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1) >> > 0); >> } >> >> bool qemu_plugin_write_register(struct qemu_plugin_register *reg, >> @@ -476,11 +478,12 @@ bool qemu_plugin_write_register(struct >> qemu_plugin_register *reg, >> g_assert(current_cpu); >> >> if (buf->len == 0 || (qemu_plugin_get_cb_flags() != >> QEMU_PLUGIN_CB_RW_REGS && >> - qemu_plugin_get_cb_flags() != >> QEMU_PLUGIN_CB_RW_REGS_PC)) { >> + qemu_plugin_get_cb_flags() != >> QEMU_PLUGIN_CB_RW_REGS_PC) >> + || (GPOINTER_TO_INT(reg) & 1)) { > > Maybe this is better as: > > g_assert(GPOINTER_TO_INT(reg) & 1 == 0); > > if (buf->len == 0 || (qemu_plugin_get_cb_flags() != > QEMU_PLUGIN_CB_RW_REGS &&... > > again the plugin is trying to do something it shouldn't.
As far as I can tell, there's currently no mechanism in the test setup to check that a certain assertion is triggered. In the previous test, I just checked whether the API returns false when I try to write to a read-only register, but now I'd need to check in my test whether the assert triggers. Should I check that in a wrapper script, test only the read path, or follow a completely different approach for this? > >> return false; >> } >> >> - return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) >> - 1) > 0); >> + return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) >> >> 1) > 0); >> } >> >> void qemu_plugin_set_pc(uint64_t vaddr) > Best regards, Florian
