* [PATCH v3 0/5] Enable PC diversion via the plugin API
@ 2026-01-20 15:19 Florian Hofhammer
2026-01-20 15:22 ` [PATCH v3 1/5] plugins: add PC diversion API function Florian Hofhammer
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Florian Hofhammer @ 2026-01-20 15:19 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Hi,
This patch series builds on top of the discussion from the thread at
https://lore.kernel.org/qemu-devel/e9bcd7c7-2d67-469e-b2f3-d1a68e456b2b@epfl.ch/
and adds a plugin API function to set the program counter of the guest,
as just writing to it via qemu_plugin_write_register() has no direct
effect.
Based on the discussion in the above thread, the series also introduces
a means to declare registers as read-only from the plugin side, which
prevents plugins from writing to them via qemu_plugin_write_register().
This for now is only applied to the PC, and finding the PC register is
done via some rather hacky strcmp()s. In the above thread, we also
discussed encoding the read-only property in a custom attribute in the
GDB XMLs, but that would (1) make syncing with GDB harder, (2) not
cover all architectures, as there's not an XML description of all
architectures available in the gdb-xml/ directory, and (3) require quite
some changes to the whole GDB infrastructure in gdbstub/ to even encode
the attribute in the correct structs and pass them on over the different
layers up into the plugin API.
This patch series does not (yet) bump the plugin API version, as I've
sent another patch yesterday (see
https://lore.kernel.org/qemu-devel/f877dd79-1285-4752-811e-f0d430ff27fe@fhofhammer.de/)
that makes some changes to qemu_plugin_{read,write}_register() as well
and I'll adjust the plugin API version bump and API usage in a v4 once I
have an idea whether the patches will make it into a release or not to
avoid conflicts later on.
Best regards,
Florian
Changes:
v3:
- make PC registers read-only across architectures
- add tests for read-only registers
- adjust test structure for qemu_plugin_set_pc() by moving
architecture-specific tests into corresponding directories
v2:
- add setjmp() in syscall handling path to allow PC redirection from
syscall callbacks (via longjmp(), the cpu_loop()'s setjmp() for
exiting a TB would not be live anymore in syscall handlers)
- add flags to ensure the qemu_plugin_set_pc() API is only called from
contexts where the CPU is live
- add test for qemu_plugin_set_pc() API
v1:
- initial version
Florian Hofhammer (5):
plugins: add PC diversion API function
plugins: add read-only property for registers
plugins: prohibit writing to read-only registers
tests/tcg: add test for qemu_plugin_set_pc API
tests/tcg/plugins: test register readonly feature
include/qemu/qemu-plugin.h | 17 +++++
linux-user/aarch64/cpu_loop.c | 2 +-
linux-user/alpha/cpu_loop.c | 2 +-
linux-user/arm/cpu_loop.c | 2 +-
linux-user/hexagon/cpu_loop.c | 2 +-
linux-user/hppa/cpu_loop.c | 4 ++
linux-user/i386/cpu_loop.c | 8 ++-
linux-user/include/special-errno.h | 8 +++
linux-user/loongarch64/cpu_loop.c | 5 +-
linux-user/m68k/cpu_loop.c | 2 +-
linux-user/microblaze/cpu_loop.c | 2 +-
linux-user/mips/cpu_loop.c | 5 +-
linux-user/openrisc/cpu_loop.c | 2 +-
linux-user/ppc/cpu_loop.c | 6 +-
linux-user/riscv/cpu_loop.c | 2 +-
linux-user/s390x/cpu_loop.c | 2 +-
linux-user/sh4/cpu_loop.c | 2 +-
linux-user/sparc/cpu_loop.c | 4 +-
linux-user/syscall.c | 8 +++
linux-user/xtensa/cpu_loop.c | 3 +
plugins/api.c | 41 +++++++++--
plugins/core.c | 25 ++++---
tests/tcg/arm/Makefile.target | 6 ++
tests/tcg/hexagon/Makefile.target | 7 ++
tests/tcg/mips/Makefile.target | 6 +-
tests/tcg/mips64/Makefile.target | 15 ++++
tests/tcg/mips64el/Makefile.target | 15 ++++
tests/tcg/mipsel/Makefile.target | 15 ++++
tests/tcg/multiarch/Makefile.target | 20 +++++-
.../{ => plugin}/check-plugin-output.sh | 0
.../{ => plugin}/test-plugin-mem-access.c | 0
.../plugin/test-plugin-skip-syscalls.c | 26 +++++++
tests/tcg/plugins/meson.build | 2 +-
tests/tcg/plugins/registers.c | 71 +++++++++++++++++++
tests/tcg/plugins/syscall.c | 6 ++
tests/tcg/sparc64/Makefile.target | 16 +++++
36 files changed, 321 insertions(+), 38 deletions(-)
create mode 100644 tests/tcg/mips64/Makefile.target
create mode 100644 tests/tcg/mips64el/Makefile.target
create mode 100644 tests/tcg/mipsel/Makefile.target
rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
create mode 100644 tests/tcg/plugins/registers.c
create mode 100644 tests/tcg/sparc64/Makefile.target
base-commit: 38879a667fbb4ef54c70de71494882615f600a64
--
2.52.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/5] plugins: add PC diversion API function
2026-01-20 15:19 [PATCH v3 0/5] Enable PC diversion via the plugin API Florian Hofhammer
@ 2026-01-20 15:22 ` Florian Hofhammer
2026-01-26 20:31 ` Alex Bennée
2026-01-20 15:23 ` [PATCH v3 2/5] plugins: add read-only property for registers Florian Hofhammer
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Florian Hofhammer @ 2026-01-20 15:22 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
This patch adds a plugin API function that allows diverting the program
counter during execution. A potential use case for this functionality is
to skip over parts of the code, e.g., by hooking into a specific
instruction and setting the PC to the next instruction in the callback.
Redirecting control flow via cpu_loop_exit() works fine in callbacks
that are triggered during code execution due to cpu_exec_setjmp() still
being part of the call stack. If we want to use the new API in syscall
callbacks, cpu_exec_setjmp() already returned and the longjmp()
triggered by cpu_loop_exit() is undefined behavior. For this reason, we
introduce a new return constant QEMU_ESETPC and do another setjmp()
before executing syscall plugin callbacks and potentially the syscall
itself.
Link: https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00656.html
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
include/qemu/qemu-plugin.h | 15 +++++++++++++++
linux-user/aarch64/cpu_loop.c | 2 +-
linux-user/alpha/cpu_loop.c | 2 +-
linux-user/arm/cpu_loop.c | 2 +-
linux-user/hexagon/cpu_loop.c | 2 +-
linux-user/hppa/cpu_loop.c | 4 ++++
linux-user/i386/cpu_loop.c | 8 +++++---
linux-user/include/special-errno.h | 8 ++++++++
linux-user/loongarch64/cpu_loop.c | 5 +++--
linux-user/m68k/cpu_loop.c | 2 +-
linux-user/microblaze/cpu_loop.c | 2 +-
linux-user/mips/cpu_loop.c | 5 +++--
linux-user/openrisc/cpu_loop.c | 2 +-
linux-user/ppc/cpu_loop.c | 6 ++++--
linux-user/riscv/cpu_loop.c | 2 +-
linux-user/s390x/cpu_loop.c | 2 +-
linux-user/sh4/cpu_loop.c | 2 +-
linux-user/sparc/cpu_loop.c | 4 +++-
linux-user/syscall.c | 8 ++++++++
linux-user/xtensa/cpu_loop.c | 3 +++
plugins/api.c | 17 ++++++++++++++++-
plugins/core.c | 25 ++++++++++++++-----------
22 files changed, 96 insertions(+), 32 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 60de4fdd3f..f976b62030 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -321,11 +321,14 @@ typedef struct {
* @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
* @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
* @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
+ * @QEMU_PLUGIN_CB_RW_REGS_PC: callback reads and writes the CPU's
+ * regs and updates the PC
*/
enum qemu_plugin_cb_flags {
QEMU_PLUGIN_CB_NO_REGS,
QEMU_PLUGIN_CB_R_REGS,
QEMU_PLUGIN_CB_RW_REGS,
+ QEMU_PLUGIN_CB_RW_REGS_PC,
};
enum qemu_plugin_mem_rw {
@@ -1003,6 +1006,18 @@ QEMU_PLUGIN_API
int qemu_plugin_write_register(struct qemu_plugin_register *handle,
GByteArray *buf);
+/**
+ * qemu_plugin_set_pc() - set the program counter for the current vCPU
+ *
+ * @vaddr: the new virtual (guest) address for the program counter
+ *
+ * This function sets the program counter for the current vCPU to @vaddr and
+ * resumes execution at that address. This function only returns in case of
+ * errors.
+ */
+QEMU_PLUGIN_API
+void qemu_plugin_set_pc(uint64_t vaddr);
+
/**
* qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
*
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 7f66a879ea..e7f643d69d 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -181,7 +181,7 @@ void cpu_loop(CPUARMState *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->xregs[0] = ret;
}
break;
diff --git a/linux-user/alpha/cpu_loop.c b/linux-user/alpha/cpu_loop.c
index f93597c400..bef196b1f5 100644
--- a/linux-user/alpha/cpu_loop.c
+++ b/linux-user/alpha/cpu_loop.c
@@ -82,7 +82,7 @@ void cpu_loop(CPUAlphaState *env)
env->pc -= 4;
break;
}
- if (sysret == -QEMU_ESIGRETURN) {
+ if (sysret == -QEMU_ESIGRETURN || sysret == -QEMU_ESETPC) {
break;
}
/* Syscall writes 0 to V0 to bypass error check, similar
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 40aefc4c1d..19874f4c72 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -399,7 +399,7 @@ void cpu_loop(CPUARMState *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->regs[15] -= env->thumb ? 2 : 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[0] = ret;
}
}
diff --git a/linux-user/hexagon/cpu_loop.c b/linux-user/hexagon/cpu_loop.c
index 1941f4c9c1..9adb3ec4c6 100644
--- a/linux-user/hexagon/cpu_loop.c
+++ b/linux-user/hexagon/cpu_loop.c
@@ -56,7 +56,7 @@ void cpu_loop(CPUHexagonState *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->gpr[HEX_REG_PC] -= 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->gpr[0] = ret;
}
break;
diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index 972e85c487..bd3b67059b 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -17,6 +17,7 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "qemu/compiler.h"
#include "qemu/osdep.h"
#include "qemu.h"
#include "user-internals.h"
@@ -123,7 +124,10 @@ void cpu_loop(CPUHPPAState *env)
env->iaoq_b = env->iaoq_f + 4;
break;
case -QEMU_ERESTARTSYS:
+ QEMU_FALLTHROUGH;
case -QEMU_ESIGRETURN:
+ QEMU_FALLTHROUGH;
+ case -QEMU_ESETPC:
break;
}
break;
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index f3f58576af..fe922fceb5 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -181,7 +181,9 @@ static void emulate_vsyscall(CPUX86State *env)
if (ret == -TARGET_EFAULT) {
goto sigsegv;
}
- env->regs[R_EAX] = ret;
+ if (ret != -QEMU_ESETPC) {
+ env->regs[R_EAX] = ret;
+ }
/* Emulate a ret instruction to leave the vsyscall page. */
env->eip = caller;
@@ -234,7 +236,7 @@ void cpu_loop(CPUX86State *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->eip -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[R_EAX] = ret;
}
break;
@@ -253,7 +255,7 @@ void cpu_loop(CPUX86State *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->eip -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[R_EAX] = ret;
}
break;
diff --git a/linux-user/include/special-errno.h b/linux-user/include/special-errno.h
index 4120455baa..1db757241a 100644
--- a/linux-user/include/special-errno.h
+++ b/linux-user/include/special-errno.h
@@ -29,4 +29,12 @@
*/
#define QEMU_ESIGRETURN 513
+/*
+ * This is returned after a plugin has used the qemu_plugin_set_pc API, to
+ * indicate that the plugin deliberately changed the PC and potentially
+ * modified the register values. The main loop should not touch the guest
+ * registers for this reason.
+ */
+#define QEMU_ESETPC 514
+
#endif /* SPECIAL_ERRNO_H */
diff --git a/linux-user/loongarch64/cpu_loop.c b/linux-user/loongarch64/cpu_loop.c
index 26a5ce3a93..603fcc39c7 100644
--- a/linux-user/loongarch64/cpu_loop.c
+++ b/linux-user/loongarch64/cpu_loop.c
@@ -44,9 +44,10 @@ void cpu_loop(CPULoongArchState *env)
env->pc -= 4;
break;
}
- if (ret == -QEMU_ESIGRETURN) {
+ if (ret == -QEMU_ESIGRETURN || ret == -QEMU_ESETPC) {
/*
- * Returning from a successful sigreturn syscall.
+ * Returning from a successful sigreturn syscall or from
+ * control flow diversion in a plugin callback.
* Avoid clobbering register state.
*/
break;
diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 2c9f628241..b98ca8ff7b 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -66,7 +66,7 @@ void cpu_loop(CPUM68KState *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->dregs[0] = ret;
}
}
diff --git a/linux-user/microblaze/cpu_loop.c b/linux-user/microblaze/cpu_loop.c
index 78506ab23d..06d92c0b90 100644
--- a/linux-user/microblaze/cpu_loop.c
+++ b/linux-user/microblaze/cpu_loop.c
@@ -54,7 +54,7 @@ void cpu_loop(CPUMBState *env)
if (ret == -QEMU_ERESTARTSYS) {
/* Wind back to before the syscall. */
env->pc -= 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[3] = ret;
}
/* All syscall exits result in guest r14 being equal to the
diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
index 2365de1de1..af98138eb2 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -140,8 +140,9 @@ done_syscall:
env->active_tc.PC -= 4;
break;
}
- if (ret == -QEMU_ESIGRETURN) {
- /* Returning from a successful sigreturn syscall.
+ if (ret == -QEMU_ESIGRETURN || ret == -QEMU_ESETPC) {
+ /* Returning from a successful sigreturn syscall or from
+ control flow diversion in a plugin callback.
Avoid clobbering register state. */
break;
}
diff --git a/linux-user/openrisc/cpu_loop.c b/linux-user/openrisc/cpu_loop.c
index 2167d880d5..e7e9929e6f 100644
--- a/linux-user/openrisc/cpu_loop.c
+++ b/linux-user/openrisc/cpu_loop.c
@@ -48,7 +48,7 @@ void cpu_loop(CPUOpenRISCState *env)
cpu_get_gpr(env, 8), 0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
cpu_set_gpr(env, 11, ret);
}
break;
diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
index b0b0cb14b4..1f8aae14bb 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -340,8 +340,10 @@ void cpu_loop(CPUPPCState *env)
env->nip -= 4;
break;
}
- if (ret == (target_ulong)(-QEMU_ESIGRETURN)) {
- /* Returning from a successful sigreturn syscall.
+ if (ret == (target_ulong)(-QEMU_ESIGRETURN)
+ || ret == (target_ulong)(-QEMU_ESETPC)) {
+ /* Returning from a successful sigreturn syscall or from
+ control flow diversion in a plugin callback.
Avoid corrupting register state. */
break;
}
diff --git a/linux-user/riscv/cpu_loop.c b/linux-user/riscv/cpu_loop.c
index ce542540c2..eecc8d1517 100644
--- a/linux-user/riscv/cpu_loop.c
+++ b/linux-user/riscv/cpu_loop.c
@@ -65,7 +65,7 @@ void cpu_loop(CPURISCVState *env)
}
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 4;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->gpr[xA0] = ret;
}
if (cs->singlestep_enabled) {
diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index 4929b32e1f..67d2a803fb 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -83,7 +83,7 @@ void cpu_loop(CPUS390XState *env)
env->regs[6], env->regs[7], 0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->psw.addr -= env->int_svc_ilen;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->regs[2] = ret;
}
diff --git a/linux-user/sh4/cpu_loop.c b/linux-user/sh4/cpu_loop.c
index 0c9d7e9c46..ee2958d0d9 100644
--- a/linux-user/sh4/cpu_loop.c
+++ b/linux-user/sh4/cpu_loop.c
@@ -50,7 +50,7 @@ void cpu_loop(CPUSH4State *env)
0, 0);
if (ret == -QEMU_ERESTARTSYS) {
env->pc -= 2;
- } else if (ret != -QEMU_ESIGRETURN) {
+ } else if (ret != -QEMU_ESIGRETURN && ret != -QEMU_ESETPC) {
env->gregs[0] = ret;
}
break;
diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
index 7391e2add8..f054316dce 100644
--- a/linux-user/sparc/cpu_loop.c
+++ b/linux-user/sparc/cpu_loop.c
@@ -229,7 +229,9 @@ void cpu_loop (CPUSPARCState *env)
env->regwptr[2], env->regwptr[3],
env->regwptr[4], env->regwptr[5],
0, 0);
- if (ret == -QEMU_ERESTARTSYS || ret == -QEMU_ESIGRETURN) {
+ if (ret == -QEMU_ERESTARTSYS
+ || ret == -QEMU_ESIGRETURN
+ || ret == -QEMU_ESETPC) {
break;
}
if ((abi_ulong)ret >= (abi_ulong)(-515)) {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3601715769..b48ea6debd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -43,6 +43,7 @@
#include <linux/capability.h>
#include <sched.h>
#include <sys/timex.h>
+#include <setjmp.h>
#include <sys/socket.h>
#include <linux/sockios.h>
#include <sys/un.h>
@@ -584,6 +585,9 @@ const char *target_strerror(int err)
if (err == QEMU_ESIGRETURN) {
return "Successful exit from sigreturn";
}
+ if (err == QEMU_ESETPC) {
+ return "Successfully redirected control flow via qemu_plugin_set_pc";
+ }
return strerror(target_to_host_errno(err));
}
@@ -14200,6 +14204,10 @@ abi_long do_syscall(CPUArchState *cpu_env, int num, abi_long arg1,
return -QEMU_ESIGRETURN;
}
+ if (unlikely(sigsetjmp(cpu->jmp_env, 0) != 0)) {
+ return -QEMU_ESETPC;
+ }
+
record_syscall_start(cpu, num, arg1,
arg2, arg3, arg4, arg5, arg6, arg7, arg8);
diff --git a/linux-user/xtensa/cpu_loop.c b/linux-user/xtensa/cpu_loop.c
index a0ff10eff8..7680e243bb 100644
--- a/linux-user/xtensa/cpu_loop.c
+++ b/linux-user/xtensa/cpu_loop.c
@@ -17,6 +17,7 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "qemu/compiler.h"
#include "qemu/osdep.h"
#include "qemu.h"
#include "user-internals.h"
@@ -185,6 +186,8 @@ void cpu_loop(CPUXtensaState *env)
env->pc -= 3;
break;
+ case -QEMU_ESETPC:
+ QEMU_FALLTHROUGH;
case -QEMU_ESIGRETURN:
break;
}
diff --git a/plugins/api.c b/plugins/api.c
index eac04cc1f6..fc19bdb40b 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -41,6 +41,7 @@
#include "qemu/log.h"
#include "system/memory.h"
#include "tcg/tcg.h"
+#include "exec/cpu-common.h"
#include "exec/gdbstub.h"
#include "exec/target_page.h"
#include "exec/translation-block.h"
@@ -450,13 +451,27 @@ int 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) {
+ 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)) {
return -1;
}
return gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1);
}
+void qemu_plugin_set_pc(uint64_t vaddr)
+{
+ g_assert(current_cpu);
+
+ if (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC) {
+ return;
+ }
+
+ cpu_set_pc(current_cpu, vaddr);
+ cpu_loop_exit(current_cpu);
+}
+
bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len)
{
g_assert(current_cpu);
diff --git a/plugins/core.c b/plugins/core.c
index b4b783008f..9e9a066669 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -395,15 +395,16 @@ void plugin_register_dyn_cb__udata(GArray **arr,
enum qemu_plugin_cb_flags flags,
void *udata)
{
- static TCGHelperInfo info[3] = {
+ static TCGHelperInfo info[4] = {
[QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
[QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
[QEMU_PLUGIN_CB_RW_REGS].flags = 0,
+ [QEMU_PLUGIN_CB_RW_REGS_PC].flags = 0,
/*
* Match qemu_plugin_vcpu_udata_cb_t:
* void (*)(uint32_t, void *)
*/
- [0 ... 2].typemask = (dh_typemask(void, 0) |
+ [0 ... 3].typemask = (dh_typemask(void, 0) |
dh_typemask(i32, 1) |
dh_typemask(ptr, 2))
};
@@ -425,15 +426,16 @@ void plugin_register_dyn_cond_cb__udata(GArray **arr,
uint64_t imm,
void *udata)
{
- static TCGHelperInfo info[3] = {
+ static TCGHelperInfo info[4] = {
[QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
[QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
[QEMU_PLUGIN_CB_RW_REGS].flags = 0,
+ [QEMU_PLUGIN_CB_RW_REGS_PC].flags = 0,
/*
* Match qemu_plugin_vcpu_udata_cb_t:
* void (*)(uint32_t, void *)
*/
- [0 ... 2].typemask = (dh_typemask(void, 0) |
+ [0 ... 3].typemask = (dh_typemask(void, 0) |
dh_typemask(i32, 1) |
dh_typemask(ptr, 2))
};
@@ -464,15 +466,16 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
!__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t) &&
!__builtin_types_compatible_p(qemu_plugin_meminfo_t, int32_t));
- static TCGHelperInfo info[3] = {
+ static TCGHelperInfo info[4] = {
[QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
[QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
[QEMU_PLUGIN_CB_RW_REGS].flags = 0,
+ [QEMU_PLUGIN_CB_RW_REGS_PC].flags = 0,
/*
* Match qemu_plugin_vcpu_mem_cb_t:
* void (*)(uint32_t, qemu_plugin_meminfo_t, uint64_t, void *)
*/
- [0 ... 2].typemask =
+ [0 ... 3].typemask =
(dh_typemask(void, 0) |
dh_typemask(i32, 1) |
(__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t)
@@ -534,7 +537,7 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
qemu_plugin_vcpu_syscall_cb_t func = cb->f.vcpu_syscall;
- qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
func(cb->ctx->id, cpu->cpu_index, num, a1, a2, a3, a4, a5, a6, a7, a8);
qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
@@ -558,7 +561,7 @@ void qemu_plugin_vcpu_syscall_ret(CPUState *cpu, int64_t num, int64_t ret)
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
qemu_plugin_vcpu_syscall_ret_cb_t func = cb->f.vcpu_syscall_ret;
- qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
func(cb->ctx->id, cpu->cpu_index, num, ret);
qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
@@ -568,7 +571,7 @@ void qemu_plugin_vcpu_idle_cb(CPUState *cpu)
{
/* idle and resume cb may be called before init, ignore in this case */
if (cpu->cpu_index < plugin.num_vcpus) {
- qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_IDLE);
qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
@@ -577,7 +580,7 @@ void qemu_plugin_vcpu_idle_cb(CPUState *cpu)
void qemu_plugin_vcpu_resume_cb(CPUState *cpu)
{
if (cpu->cpu_index < plugin.num_vcpus) {
- qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_RESUME);
qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
}
@@ -848,6 +851,6 @@ enum qemu_plugin_cb_flags tcg_call_to_qemu_plugin_cb_flags(int flags)
} else if (flags & TCG_CALL_NO_WG) {
return QEMU_PLUGIN_CB_R_REGS;
} else {
- return QEMU_PLUGIN_CB_RW_REGS;
+ return QEMU_PLUGIN_CB_RW_REGS_PC;
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/5] plugins: add read-only property for registers
2026-01-20 15:19 [PATCH v3 0/5] Enable PC diversion via the plugin API Florian Hofhammer
2026-01-20 15:22 ` [PATCH v3 1/5] plugins: add PC diversion API function Florian Hofhammer
@ 2026-01-20 15:23 ` Florian Hofhammer
2026-01-26 20:18 ` Alex Bennée
2026-01-20 15:23 ` [PATCH v3 3/5] plugins: prohibit writing to read-only registers Florian Hofhammer
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Florian Hofhammer @ 2026-01-20 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
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 <florian.hofhammer@epfl.ch>
---
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
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);
+ 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;
desc.feature = g_intern_string(grd->feature_name);
g_array_append_val(find_data, desc);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 3/5] plugins: prohibit writing to read-only registers
2026-01-20 15:19 [PATCH v3 0/5] Enable PC diversion via the plugin API Florian Hofhammer
2026-01-20 15:22 ` [PATCH v3 1/5] plugins: add PC diversion API function Florian Hofhammer
2026-01-20 15:23 ` [PATCH v3 2/5] plugins: add read-only property for registers Florian Hofhammer
@ 2026-01-20 15:23 ` Florian Hofhammer
2026-01-20 15:24 ` [PATCH v3 4/5] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Florian Hofhammer @ 2026-01-20 15:23 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
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 <florian.hofhammer@epfl.ch>
---
plugins/api.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/plugins/api.c b/plugins/api.c
index de8c32db50..4555f048a1 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -425,7 +425,6 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
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);
if (!strcmp(desc.name, pc_str)
|| !strcmp(desc.name, eip_str)
@@ -436,6 +435,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
) {
plugin_ro_bit = 1;
}
+ desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit);
desc.is_readonly = plugin_ro_bit == 1 ? true : false;
desc.feature = g_intern_string(grd->feature_name);
g_array_append_val(find_data, desc);
@@ -460,7 +460,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
return -1;
}
- return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
+ return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1);
}
int qemu_plugin_write_register(struct qemu_plugin_register *reg,
@@ -470,11 +470,12 @@ int qemu_plugin_write_register(struct qemu_plugin_register *reg,
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)) {
return -1;
}
- return gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1);
+ return gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) >> 1);
}
void qemu_plugin_set_pc(uint64_t vaddr)
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 4/5] tests/tcg: add test for qemu_plugin_set_pc API
2026-01-20 15:19 [PATCH v3 0/5] Enable PC diversion via the plugin API Florian Hofhammer
` (2 preceding siblings ...)
2026-01-20 15:23 ` [PATCH v3 3/5] plugins: prohibit writing to read-only registers Florian Hofhammer
@ 2026-01-20 15:24 ` Florian Hofhammer
2026-01-20 15:26 ` [PATCH v3 5/5] tests/tcg/plugins: test register readonly feature Florian Hofhammer
2026-01-26 19:17 ` [PATCH v3 0/5] Enable PC diversion via the plugin API Pierrick Bouvier
5 siblings, 0 replies; 15+ messages in thread
From: Florian Hofhammer @ 2026-01-20 15:24 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
The test executes a non-existent syscall, which the syscall plugin
intercepts and redirects to a clean exit.
Due to architecture-specific quirks, the architecture-specific Makefiles
require setting specific compiler and linker flags in some cases.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
tests/tcg/arm/Makefile.target | 6 +++++
tests/tcg/hexagon/Makefile.target | 7 +++++
tests/tcg/mips/Makefile.target | 6 ++++-
tests/tcg/mips64/Makefile.target | 15 +++++++++++
tests/tcg/mips64el/Makefile.target | 15 +++++++++++
tests/tcg/mipsel/Makefile.target | 15 +++++++++++
tests/tcg/multiarch/Makefile.target | 20 +++++++++++++-
.../{ => plugin}/check-plugin-output.sh | 0
.../{ => plugin}/test-plugin-mem-access.c | 0
.../plugin/test-plugin-skip-syscalls.c | 26 +++++++++++++++++++
tests/tcg/plugins/syscall.c | 6 +++++
tests/tcg/sparc64/Makefile.target | 16 ++++++++++++
12 files changed, 130 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/mips64/Makefile.target
create mode 100644 tests/tcg/mips64el/Makefile.target
create mode 100644 tests/tcg/mipsel/Makefile.target
rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
create mode 100644 tests/tcg/sparc64/Makefile.target
diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
index 6189d7a0e2..0d8be9cd80 100644
--- a/tests/tcg/arm/Makefile.target
+++ b/tests/tcg/arm/Makefile.target
@@ -78,4 +78,10 @@ sha512-vector: sha512.c
ARM_TESTS += sha512-vector
+ifeq ($(CONFIG_PLUGIN),y)
+# Require emitting arm32 instructions, otherwise the vCPU might accidentally
+# try to execute Thumb instructions in arm32 mode after qemu_plugin_set_pc()
+test-plugin-skip-syscalls: CFLAGS+=-marm
+endif
+
TESTS += $(ARM_TESTS)
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index e5182c01d8..428b0112e0 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -130,3 +130,10 @@ v73_scalar: CFLAGS += -Wno-unused-function
hvx_histogram: hvx_histogram.c hvx_histogram_row.S
$(CC) $(CFLAGS) $(CROSS_CC_GUEST_CFLAGS) $^ -o $@ $(LDFLAGS)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# hexagon uses clang/lld which does not support -Ttext-segment but GNU ld does
+# not generally support --image-base. Therefore, the multiarch Makefile uses
+# the GNU ld flag and we special-case here for hexagon.
+override LDFLAG_TEXT_BASE = -Wl,--image-base=0x40000
+endif
diff --git a/tests/tcg/mips/Makefile.target b/tests/tcg/mips/Makefile.target
index 5d17c1706e..d08138f17b 100644
--- a/tests/tcg/mips/Makefile.target
+++ b/tests/tcg/mips/Makefile.target
@@ -9,11 +9,15 @@ MIPS_SRC=$(SRC_PATH)/tests/tcg/mips
VPATH += $(MIPS_SRC)
# hello-mips is 32 bit only
-ifeq ($(findstring 64,$(TARGET_NAME)),)
MIPS_TESTS=hello-mips
TESTS += $(MIPS_TESTS)
hello-mips: CFLAGS+=-mno-abicalls -fno-PIC -fno-stack-protector -mabi=32
hello-mips: LDFLAGS+=-nostdlib
+
+ifeq ($(CONFIG_PLUGIN),y)
+# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+ $(call skip-test, $<, "qemu-mips does not execute invalid syscalls")
endif
diff --git a/tests/tcg/mips64/Makefile.target b/tests/tcg/mips64/Makefile.target
new file mode 100644
index 0000000000..5386855efc
--- /dev/null
+++ b/tests/tcg/mips64/Makefile.target
@@ -0,0 +1,15 @@
+# -*- Mode: makefile -*-
+#
+# MIPS64 - included from tests/tcg/Makefile.target
+#
+
+MIPS64_SRC=$(SRC_PATH)/tests/tcg/mips64
+
+# Set search path for all sources
+VPATH += $(MIPS64_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
+test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
+test-plugin-skip-syscalls: LDFLAGS+=-no-pie
+endif
diff --git a/tests/tcg/mips64el/Makefile.target b/tests/tcg/mips64el/Makefile.target
new file mode 100644
index 0000000000..77ac8815fe
--- /dev/null
+++ b/tests/tcg/mips64el/Makefile.target
@@ -0,0 +1,15 @@
+# -*- Mode: makefile -*-
+#
+# MIPS64EL - included from tests/tcg/Makefile.target
+#
+
+MIPS64EL_SRC=$(SRC_PATH)/tests/tcg/mips64el
+
+# Set search path for all sources
+VPATH += $(MIPS64EL_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# Require no ABI calls to avoid $t9-relative .got address calculation on MIPS64
+test-plugin-skip-syscalls: CFLAGS+=-mno-abicalls -fno-pie
+test-plugin-skip-syscalls: LDFLAGS+=-no-pie
+endif
diff --git a/tests/tcg/mipsel/Makefile.target b/tests/tcg/mipsel/Makefile.target
new file mode 100644
index 0000000000..bf1bdb56b3
--- /dev/null
+++ b/tests/tcg/mipsel/Makefile.target
@@ -0,0 +1,15 @@
+# -*- Mode: makefile -*-
+#
+# MIPSEL - included from tests/tcg/Makefile.target
+#
+
+MIPSEL_SRC=$(SRC_PATH)/tests/tcg/mipsel
+
+# Set search path for all sources
+VPATH += $(MIPSEL_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# qemu-mips(el) returns ENOSYS without triggering syscall plugin callbacks
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+ $(call skip-test, $<, "qemu-mipsel does not execute invalid syscalls")
+endif
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index f5b4d2b813..25df6e7211 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -14,6 +14,10 @@ ifeq ($(filter %-linux-user, $(TARGET)),$(TARGET))
VPATH += $(MULTIARCH_SRC)/linux
MULTIARCH_SRCS += $(notdir $(wildcard $(MULTIARCH_SRC)/linux/*.c))
endif
+ifeq ($(CONFIG_PLUGIN),y)
+VPATH += $(MULTIARCH_SRC)/plugin
+MULTIARCH_SRCS += $(notdir $(wildcard $(MULTIARCH_SRC)/plugin/*.c))
+endif
MULTIARCH_TESTS = $(MULTIARCH_SRCS:.c=)
#
@@ -200,10 +204,24 @@ run-plugin-test-plugin-mem-access-with-libmem.so: \
PLUGIN_ARGS=$(COMMA)print-accesses=true
run-plugin-test-plugin-mem-access-with-libmem.so: \
CHECK_PLUGIN_OUTPUT_COMMAND= \
- $(SRC_PATH)/tests/tcg/multiarch/check-plugin-output.sh \
+ $(SRC_PATH)/tests/tcg/multiarch/plugin/check-plugin-output.sh \
$(QEMU) $<
EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so
+
+# Test plugin control flow redirection by skipping system calls
+LDFLAG_TEXT_BASE = -Wl,-Ttext-segment=0x40000
+test-plugin-skip-syscalls: LDFLAGS += $(LDFLAG_TEXT_BASE)
+test-plugin-skip-syscalls: LDFLAGS += -Wl,--section-start,.redirect=0x20000
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+
+EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-skip-syscalls-with-libsyscall.so
+
+else # CONFIG_PLUGIN=n
+# Do not build the syscall skipping test if it's not tested with a plugin
+# because it will simply return an error and fail the test.
+MULTIARCH_TESTS := $(filter-out test-plugin-skip-syscalls, $(MULTIARCH_TESTS))
+
endif
# Update TESTS
diff --git a/tests/tcg/multiarch/check-plugin-output.sh b/tests/tcg/multiarch/plugin/check-plugin-output.sh
similarity index 100%
rename from tests/tcg/multiarch/check-plugin-output.sh
rename to tests/tcg/multiarch/plugin/check-plugin-output.sh
diff --git a/tests/tcg/multiarch/test-plugin-mem-access.c b/tests/tcg/multiarch/plugin/test-plugin-mem-access.c
similarity index 100%
rename from tests/tcg/multiarch/test-plugin-mem-access.c
rename to tests/tcg/multiarch/plugin/test-plugin-mem-access.c
diff --git a/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
new file mode 100644
index 0000000000..1f5cbc3851
--- /dev/null
+++ b/tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
@@ -0,0 +1,26 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This test attempts to execute an invalid syscall. The syscall test plugin
+ * should intercept this.
+ */
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+void exit_success(void) __attribute__((section(".redirect"), noinline,
+ noreturn, used));
+
+void exit_success(void) {
+ _exit(EXIT_SUCCESS);
+}
+
+int main(int argc, char *argv[]) {
+ long ret = syscall(0xc0deUL);
+ if (ret != 0L) {
+ perror("");
+ }
+ /* We should never get here */
+ return EXIT_FAILURE;
+}
diff --git a/tests/tcg/plugins/syscall.c b/tests/tcg/plugins/syscall.c
index 42801f5c86..c5bac2d928 100644
--- a/tests/tcg/plugins/syscall.c
+++ b/tests/tcg/plugins/syscall.c
@@ -148,6 +148,12 @@ static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
fprintf(stderr, "Error reading memory from vaddr %"PRIu64"\n", a2);
}
}
+
+ if (num == 0xc0deUL) {
+ /* Special syscall to test the control flow redirection functionality. */
+ qemu_plugin_outs("Marker syscall detected, jump to clean exit\n");
+ qemu_plugin_set_pc(0x20000);
+ }
}
static void vcpu_syscall_ret(qemu_plugin_id_t id, unsigned int vcpu_idx,
diff --git a/tests/tcg/sparc64/Makefile.target b/tests/tcg/sparc64/Makefile.target
new file mode 100644
index 0000000000..516927a3fc
--- /dev/null
+++ b/tests/tcg/sparc64/Makefile.target
@@ -0,0 +1,16 @@
+# -*- Mode: makefile -*-
+#
+# Sparc64 - included from tests/tcg/Makefile.target
+#
+
+SPARC64_SRC=$(SRC_PATH)/tests/tcg/sparc64
+
+# Set search path for all sources
+VPATH += $(SPARC64_SRC)
+
+ifeq ($(CONFIG_PLUGIN),y)
+# The defined addresses for the binary are not aligned correctly for sparc64
+# but adjusting them breaks other architectures, so just skip it on sparc64.
+run-plugin-test-plugin-skip-syscalls-with-libsyscall.so:
+ $(call skip-test, $<, "qemu-sparc64 does not allow mapping at our given fixed address")
+endif
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 5/5] tests/tcg/plugins: test register readonly feature
2026-01-20 15:19 [PATCH v3 0/5] Enable PC diversion via the plugin API Florian Hofhammer
` (3 preceding siblings ...)
2026-01-20 15:24 ` [PATCH v3 4/5] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
@ 2026-01-20 15:26 ` Florian Hofhammer
2026-01-26 19:17 ` [PATCH v3 0/5] Enable PC diversion via the plugin API Pierrick Bouvier
5 siblings, 0 replies; 15+ messages in thread
From: Florian Hofhammer @ 2026-01-20 15:26 UTC (permalink / raw)
To: qemu-devel
Cc: alex.bennee, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
This commit tests the readonly feature for registers exposed via the
plugin API introduced earlier in the patch series.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
tests/tcg/plugins/meson.build | 2 +-
tests/tcg/plugins/registers.c | 71 +++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/plugins/registers.c
diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
index 561584159e..52831a75d4 100644
--- a/tests/tcg/plugins/meson.build
+++ b/tests/tcg/plugins/meson.build
@@ -1,6 +1,6 @@
t = []
if get_option('plugins')
- foreach i : ['bb', 'discons', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall', 'patch']
+ foreach i : ['bb', 'discons', 'empty', 'inline', 'insn', 'mem', 'reset', 'syscall', 'patch', 'registers']
if host_os == 'windows'
t += shared_module(i, files(i + '.c') + '../../../contrib/plugins/win32_linker.c',
include_directories: '../../../include/qemu',
diff --git a/tests/tcg/plugins/registers.c b/tests/tcg/plugins/registers.c
new file mode 100644
index 0000000000..b738971551
--- /dev/null
+++ b/tests/tcg/plugins/registers.c
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
+ *
+ * License: GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "glib.h"
+#include <inttypes.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#include <qemu-plugin.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+/*
+ * This callback is called when a vCPU is initialized. It tests whether we
+ * successfully read from a register and write value back to it. It also tests
+ * that read-only registers cannot be written to, i.e., we can read a read-only
+ * register but writing to it fails.
+ */
+static void vcpu_init_cb(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+ g_autoptr(GArray) regs = qemu_plugin_get_registers();
+ g_autoptr(GByteArray) buf = g_byte_array_sized_new(0);
+ int sz = 0;
+
+ /* Make sure we can read and write an arbitrary register */
+ qemu_plugin_reg_descriptor *reg_desc = &g_array_index(regs,
+ qemu_plugin_reg_descriptor, 0);
+ g_assert(reg_desc->is_readonly == false);
+ sz = qemu_plugin_read_register(reg_desc->handle, buf);
+ g_assert(sz > 0);
+ g_assert(sz == buf->len);
+ sz = qemu_plugin_write_register(reg_desc->handle, buf);
+ g_assert(sz > 0);
+ g_assert(sz == buf->len);
+
+ /*
+ * Reset the buffer and find a read-only register. On each architecture, at
+ * least the PC should be read-only because it's only supposed to be
+ * modified via the qemu_plugin_set_pc() function.
+ */
+ g_byte_array_set_size(buf, 0);
+ for (size_t i = 0; i < regs->len; i++) {
+ reg_desc = &g_array_index(regs, qemu_plugin_reg_descriptor, i);
+ if (reg_desc->is_readonly) {
+ sz = qemu_plugin_read_register(reg_desc->handle, buf);
+ g_assert(sz > 0);
+ g_assert(sz == buf->len);
+ break;
+ } else {
+ reg_desc = NULL;
+ }
+ }
+ /* Make sure there is a read-only register and we cannot write to it */
+ g_assert(reg_desc != NULL);
+ sz = qemu_plugin_write_register(reg_desc->handle, buf);
+ g_assert(sz == -1);
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+ const qemu_info_t *info,
+ int argc, char **argv)
+{
+ qemu_plugin_register_vcpu_init_cb(id, vcpu_init_cb);
+ return 0;
+}
--
2.52.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/5] Enable PC diversion via the plugin API
2026-01-20 15:19 [PATCH v3 0/5] Enable PC diversion via the plugin API Florian Hofhammer
` (4 preceding siblings ...)
2026-01-20 15:26 ` [PATCH v3 5/5] tests/tcg/plugins: test register readonly feature Florian Hofhammer
@ 2026-01-26 19:17 ` Pierrick Bouvier
5 siblings, 0 replies; 15+ messages in thread
From: Pierrick Bouvier @ 2026-01-26 19:17 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: alex.bennee, richard.henderson, laurent, imp, berrange
On 1/20/26 7:19 AM, Florian Hofhammer wrote:
> Hi,
>
> This patch series builds on top of the discussion from the thread at
> https://lore.kernel.org/qemu-devel/e9bcd7c7-2d67-469e-b2f3-d1a68e456b2b@epfl.ch/
> and adds a plugin API function to set the program counter of the guest,
> as just writing to it via qemu_plugin_write_register() has no direct
> effect.
>
> Based on the discussion in the above thread, the series also introduces
> a means to declare registers as read-only from the plugin side, which
> prevents plugins from writing to them via qemu_plugin_write_register().
> This for now is only applied to the PC, and finding the PC register is
> done via some rather hacky strcmp()s. In the above thread, we also
> discussed encoding the read-only property in a custom attribute in the
> GDB XMLs, but that would (1) make syncing with GDB harder, (2) not
> cover all architectures, as there's not an XML description of all
> architectures available in the gdb-xml/ directory, and (3) require quite
> some changes to the whole GDB infrastructure in gdbstub/ to even encode
> the attribute in the correct structs and pass them on over the different
> layers up into the plugin API.
>
> This patch series does not (yet) bump the plugin API version, as I've
> sent another patch yesterday (see
> https://lore.kernel.org/qemu-devel/f877dd79-1285-4752-811e-f0d430ff27fe@fhofhammer.de/)
> that makes some changes to qemu_plugin_{read,write}_register() as well
> and I'll adjust the plugin API version bump and API usage in a v4 once I
> have an idea whether the patches will make it into a release or not to
> avoid conflicts later on.
>
> Best regards,
> Florian
>
> Changes:
> v3:
> - make PC registers read-only across architectures
> - add tests for read-only registers
> - adjust test structure for qemu_plugin_set_pc() by moving
> architecture-specific tests into corresponding directories
> v2:
> - add setjmp() in syscall handling path to allow PC redirection from
> syscall callbacks (via longjmp(), the cpu_loop()'s setjmp() for
> exiting a TB would not be live anymore in syscall handlers)
> - add flags to ensure the qemu_plugin_set_pc() API is only called from
> contexts where the CPU is live
> - add test for qemu_plugin_set_pc() API
> v1:
> - initial version
>
> Florian Hofhammer (5):
> plugins: add PC diversion API function
> plugins: add read-only property for registers
> plugins: prohibit writing to read-only registers
> tests/tcg: add test for qemu_plugin_set_pc API
> tests/tcg/plugins: test register readonly feature
>
> include/qemu/qemu-plugin.h | 17 +++++
> linux-user/aarch64/cpu_loop.c | 2 +-
> linux-user/alpha/cpu_loop.c | 2 +-
> linux-user/arm/cpu_loop.c | 2 +-
> linux-user/hexagon/cpu_loop.c | 2 +-
> linux-user/hppa/cpu_loop.c | 4 ++
> linux-user/i386/cpu_loop.c | 8 ++-
> linux-user/include/special-errno.h | 8 +++
> linux-user/loongarch64/cpu_loop.c | 5 +-
> linux-user/m68k/cpu_loop.c | 2 +-
> linux-user/microblaze/cpu_loop.c | 2 +-
> linux-user/mips/cpu_loop.c | 5 +-
> linux-user/openrisc/cpu_loop.c | 2 +-
> linux-user/ppc/cpu_loop.c | 6 +-
> linux-user/riscv/cpu_loop.c | 2 +-
> linux-user/s390x/cpu_loop.c | 2 +-
> linux-user/sh4/cpu_loop.c | 2 +-
> linux-user/sparc/cpu_loop.c | 4 +-
> linux-user/syscall.c | 8 +++
> linux-user/xtensa/cpu_loop.c | 3 +
> plugins/api.c | 41 +++++++++--
> plugins/core.c | 25 ++++---
> tests/tcg/arm/Makefile.target | 6 ++
> tests/tcg/hexagon/Makefile.target | 7 ++
> tests/tcg/mips/Makefile.target | 6 +-
> tests/tcg/mips64/Makefile.target | 15 ++++
> tests/tcg/mips64el/Makefile.target | 15 ++++
> tests/tcg/mipsel/Makefile.target | 15 ++++
> tests/tcg/multiarch/Makefile.target | 20 +++++-
> .../{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> .../plugin/test-plugin-skip-syscalls.c | 26 +++++++
> tests/tcg/plugins/meson.build | 2 +-
> tests/tcg/plugins/registers.c | 71 +++++++++++++++++++
> tests/tcg/plugins/syscall.c | 6 ++
> tests/tcg/sparc64/Makefile.target | 16 +++++
> 36 files changed, 321 insertions(+), 38 deletions(-)
> create mode 100644 tests/tcg/mips64/Makefile.target
> create mode 100644 tests/tcg/mips64el/Makefile.target
> create mode 100644 tests/tcg/mipsel/Makefile.target
> rename tests/tcg/multiarch/{ => plugin}/check-plugin-output.sh (100%)
> rename tests/tcg/multiarch/{ => plugin}/test-plugin-mem-access.c (100%)
> create mode 100644 tests/tcg/multiarch/plugin/test-plugin-skip-syscalls.c
> create mode 100644 tests/tcg/plugins/registers.c
> create mode 100644 tests/tcg/sparc64/Makefile.target
>
>
> base-commit: 38879a667fbb4ef54c70de71494882615f600a64
Thanks for posting Florian.
I'll let Alex review it to see if it matches his original vision, and
the readonly approach for pc register.
Regards,
Pierrick
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] plugins: add read-only property for registers
2026-01-20 15:23 ` [PATCH v3 2/5] plugins: add read-only property for registers Florian Hofhammer
@ 2026-01-26 20:18 ` Alex Bennée
2026-01-26 20:46 ` Alex Bennée
0 siblings, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2026-01-26 20:18 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> 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 <florian.hofhammer@epfl.ch>
> ---
> 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.
> desc.feature = g_intern_string(grd->feature_name);
> g_array_append_val(find_data, desc);
> }
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/5] plugins: add PC diversion API function
2026-01-20 15:22 ` [PATCH v3 1/5] plugins: add PC diversion API function Florian Hofhammer
@ 2026-01-26 20:31 ` Alex Bennée
0 siblings, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2026-01-26 20:31 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> This patch adds a plugin API function that allows diverting the program
> counter during execution. A potential use case for this functionality is
> to skip over parts of the code, e.g., by hooking into a specific
> instruction and setting the PC to the next instruction in the callback.
>
> Redirecting control flow via cpu_loop_exit() works fine in callbacks
> that are triggered during code execution due to cpu_exec_setjmp() still
> being part of the call stack. If we want to use the new API in syscall
> callbacks, cpu_exec_setjmp() already returned and the longjmp()
> triggered by cpu_loop_exit() is undefined behavior. For this reason, we
> introduce a new return constant QEMU_ESETPC and do another setjmp()
> before executing syscall plugin callbacks and potentially the syscall
> itself.
I think we want to split this up into:
- introduce the flags
- re-factor the linux-user to use an inline helper (qemu_syscall_finalize_pc(ret)?)
- implement the api call, tweak qemu_finalize_pc
>
> Link: https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00656.html
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> include/qemu/qemu-plugin.h | 15 +++++++++++++++
> linux-user/aarch64/cpu_loop.c | 2 +-
> linux-user/alpha/cpu_loop.c | 2 +-
> linux-user/arm/cpu_loop.c | 2 +-
> linux-user/hexagon/cpu_loop.c | 2 +-
> linux-user/hppa/cpu_loop.c | 4 ++++
> linux-user/i386/cpu_loop.c | 8 +++++---
> linux-user/include/special-errno.h | 8 ++++++++
> linux-user/loongarch64/cpu_loop.c | 5 +++--
> linux-user/m68k/cpu_loop.c | 2 +-
> linux-user/microblaze/cpu_loop.c | 2 +-
> linux-user/mips/cpu_loop.c | 5 +++--
> linux-user/openrisc/cpu_loop.c | 2 +-
> linux-user/ppc/cpu_loop.c | 6 ++++--
> linux-user/riscv/cpu_loop.c | 2 +-
> linux-user/s390x/cpu_loop.c | 2 +-
> linux-user/sh4/cpu_loop.c | 2 +-
> linux-user/sparc/cpu_loop.c | 4 +++-
> linux-user/syscall.c | 8 ++++++++
> linux-user/xtensa/cpu_loop.c | 3 +++
> plugins/api.c | 17 ++++++++++++++++-
> plugins/core.c | 25 ++++++++++++++-----------
> 22 files changed, 96 insertions(+), 32 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 60de4fdd3f..f976b62030 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -321,11 +321,14 @@ typedef struct {
> * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
> * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
> * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
> + * @QEMU_PLUGIN_CB_RW_REGS_PC: callback reads and writes the CPU's
> + * regs and updates the PC
> */
> enum qemu_plugin_cb_flags {
> QEMU_PLUGIN_CB_NO_REGS,
> QEMU_PLUGIN_CB_R_REGS,
> QEMU_PLUGIN_CB_RW_REGS,
> + QEMU_PLUGIN_CB_RW_REGS_PC,
> };
Hmm should we consider making this an enum? or...
>
<snip>
> diff --git a/plugins/core.c b/plugins/core.c
> index b4b783008f..9e9a066669 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -395,15 +395,16 @@ void plugin_register_dyn_cb__udata(GArray **arr,
> enum qemu_plugin_cb_flags flags,
> void *udata)
> {
> - static TCGHelperInfo info[3] = {
> + static TCGHelperInfo info[4] = {
> [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
> [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
> [QEMU_PLUGIN_CB_RW_REGS].flags = 0,
> + [QEMU_PLUGIN_CB_RW_REGS_PC].flags = 0,
> /*
> * Match qemu_plugin_vcpu_udata_cb_t:
> * void (*)(uint32_t, void *)
> */
> - [0 ... 2].typemask = (dh_typemask(void, 0) |
> + [0 ... 3].typemask = (dh_typemask(void, 0) |
> dh_typemask(i32, 1) |
> dh_typemask(ptr, 2))
...are we effectively ordering callback types here?
> };
> @@ -425,15 +426,16 @@ void plugin_register_dyn_cond_cb__udata(GArray **arr,
> uint64_t imm,
> void *udata)
> {
> - static TCGHelperInfo info[3] = {
> + static TCGHelperInfo info[4] = {
> [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
> [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
> [QEMU_PLUGIN_CB_RW_REGS].flags = 0,
> + [QEMU_PLUGIN_CB_RW_REGS_PC].flags = 0,
> /*
> * Match qemu_plugin_vcpu_udata_cb_t:
> * void (*)(uint32_t, void *)
> */
> - [0 ... 2].typemask = (dh_typemask(void, 0) |
> + [0 ... 3].typemask = (dh_typemask(void, 0) |
> dh_typemask(i32, 1) |
> dh_typemask(ptr, 2))
> };
> @@ -464,15 +466,16 @@ void plugin_register_vcpu_mem_cb(GArray **arr,
> !__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t) &&
> !__builtin_types_compatible_p(qemu_plugin_meminfo_t, int32_t));
>
> - static TCGHelperInfo info[3] = {
> + static TCGHelperInfo info[4] = {
> [QEMU_PLUGIN_CB_NO_REGS].flags = TCG_CALL_NO_RWG,
> [QEMU_PLUGIN_CB_R_REGS].flags = TCG_CALL_NO_WG,
> [QEMU_PLUGIN_CB_RW_REGS].flags = 0,
> + [QEMU_PLUGIN_CB_RW_REGS_PC].flags = 0,
> /*
> * Match qemu_plugin_vcpu_mem_cb_t:
> * void (*)(uint32_t, qemu_plugin_meminfo_t, uint64_t, void *)
> */
> - [0 ... 2].typemask =
> + [0 ... 3].typemask =
> (dh_typemask(void, 0) |
> dh_typemask(i32, 1) |
> (__builtin_types_compatible_p(qemu_plugin_meminfo_t, uint32_t)
> @@ -534,7 +537,7 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
> QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
> qemu_plugin_vcpu_syscall_cb_t func = cb->f.vcpu_syscall;
>
> - qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
> + qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
> func(cb->ctx->id, cpu->cpu_index, num, a1, a2, a3, a4, a5, a6, a7, a8);
> qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
> }
> @@ -558,7 +561,7 @@ void qemu_plugin_vcpu_syscall_ret(CPUState *cpu, int64_t num, int64_t ret)
> QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
> qemu_plugin_vcpu_syscall_ret_cb_t func = cb->f.vcpu_syscall_ret;
>
> - qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
> + qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
> func(cb->ctx->id, cpu->cpu_index, num, ret);
> qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
> }
> @@ -568,7 +571,7 @@ void qemu_plugin_vcpu_idle_cb(CPUState *cpu)
> {
> /* idle and resume cb may be called before init, ignore in this case */
> if (cpu->cpu_index < plugin.num_vcpus) {
> - qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
> + qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
> plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_IDLE);
> qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
> }
> @@ -577,7 +580,7 @@ void qemu_plugin_vcpu_idle_cb(CPUState *cpu)
> void qemu_plugin_vcpu_resume_cb(CPUState *cpu)
> {
> if (cpu->cpu_index < plugin.num_vcpus) {
> - qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
> + qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
> plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_RESUME);
> qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_NO_REGS);
> }
> @@ -848,6 +851,6 @@ enum qemu_plugin_cb_flags tcg_call_to_qemu_plugin_cb_flags(int flags)
> } else if (flags & TCG_CALL_NO_WG) {
> return QEMU_PLUGIN_CB_R_REGS;
> } else {
> - return QEMU_PLUGIN_CB_RW_REGS;
> + return QEMU_PLUGIN_CB_RW_REGS_PC;
> }
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] plugins: add read-only property for registers
2026-01-26 20:18 ` Alex Bennée
@ 2026-01-26 20:46 ` Alex Bennée
2026-02-13 13:38 ` Florian Hofhammer
0 siblings, 1 reply; 15+ messages in thread
From: Alex Bennée @ 2026-01-26 20:46 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
Alex Bennée <alex.bennee@linaro.org> writes:
> Florian Hofhammer <florian.hofhammer@epfl.ch> 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 <florian.hofhammer@epfl.ch>
>> ---
>> 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?
>
>> desc.feature = g_intern_string(grd->feature_name);
>> g_array_append_val(find_data, desc);
>> }
>
> Otherwise:
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] plugins: add read-only property for registers
2026-01-26 20:46 ` Alex Bennée
@ 2026-02-13 13:38 ` Florian Hofhammer
2026-02-13 18:36 ` Pierrick Bouvier
0 siblings, 1 reply; 15+ messages in thread
From: Florian Hofhammer @ 2026-02-13 13:38 UTC (permalink / raw)
To: Alex Bennée
Cc: qemu-devel, pierrick.bouvier, richard.henderson, laurent, imp,
berrange
[-- Attachment #1: Type: text/plain, Size: 4309 bytes --]
On 26/01/2026 21:46, Alex Bennée wrote:> Alex Bennée <alex.bennee@linaro.org> writes:
>
>> Florian Hofhammer <florian.hofhammer@epfl.ch> 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 <florian.hofhammer@epfl.ch>
>>> ---
>>> 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 <alex.bennee@linaro.org>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] plugins: add read-only property for registers
2026-02-13 13:38 ` Florian Hofhammer
@ 2026-02-13 18:36 ` Pierrick Bouvier
2026-02-16 12:54 ` Florian Hofhammer
0 siblings, 1 reply; 15+ messages in thread
From: Pierrick Bouvier @ 2026-02-13 18:36 UTC (permalink / raw)
To: Florian Hofhammer, Alex Bennée
Cc: qemu-devel, richard.henderson, laurent, imp, berrange
On 2/13/26 5:38 AM, Florian Hofhammer wrote:
> On 26/01/2026 21:46, Alex Bennée wrote:> Alex Bennée <alex.bennee@linaro.org> writes:
>>
>>> Florian Hofhammer <florian.hofhammer@epfl.ch> 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 <florian.hofhammer@epfl.ch>
>>>> ---
>>>> 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 <alex.bennee@linaro.org>
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] plugins: add read-only property for registers
2026-02-13 18:36 ` Pierrick Bouvier
@ 2026-02-16 12:54 ` Florian Hofhammer
2026-02-17 2:30 ` Pierrick Bouvier
2026-02-20 16:34 ` Alex Bennée
0 siblings, 2 replies; 15+ messages in thread
From: Florian Hofhammer @ 2026-02-16 12:54 UTC (permalink / raw)
To: Pierrick Bouvier, Alex Bennée
Cc: qemu-devel, richard.henderson, laurent, imp, berrange
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 <alex.bennee@linaro.org> writes:
>>>
>>>> Florian Hofhammer <florian.hofhammer@epfl.ch> 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 <florian.hofhammer@epfl.ch>
>>>>> ---
>>>>> 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 <alex.bennee@linaro.org>
>
> 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.
> So maybe we can just extend API with PC diversion, and not overthink too much about potential consequences on registers.
>
> Regards,
> Pierrick
Best,
Florian
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] plugins: add read-only property for registers
2026-02-16 12:54 ` Florian Hofhammer
@ 2026-02-17 2:30 ` Pierrick Bouvier
2026-02-20 16:34 ` Alex Bennée
1 sibling, 0 replies; 15+ messages in thread
From: Pierrick Bouvier @ 2026-02-17 2:30 UTC (permalink / raw)
To: Florian Hofhammer, Alex Bennée
Cc: qemu-devel, richard.henderson, laurent, imp, berrange
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 <alex.bennee@linaro.org> writes:
>>>>
>>>>> Florian Hofhammer <florian.hofhammer@epfl.ch> 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 <florian.hofhammer@epfl.ch>
>>>>>> ---
>>>>>> 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 <alex.bennee@linaro.org>
>>
>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/5] plugins: add read-only property for registers
2026-02-16 12:54 ` Florian Hofhammer
2026-02-17 2:30 ` Pierrick Bouvier
@ 2026-02-20 16:34 ` Alex Bennée
1 sibling, 0 replies; 15+ messages in thread
From: Alex Bennée @ 2026-02-20 16:34 UTC (permalink / raw)
To: Florian Hofhammer
Cc: Pierrick Bouvier, qemu-devel, richard.henderson, laurent, imp,
berrange
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> 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 <alex.bennee@linaro.org> writes:
>>>>
>>>>> Florian Hofhammer <florian.hofhammer@epfl.ch> 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 <florian.hofhammer@epfl.ch>
>>>>>> ---
>>>>>> 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 <alex.bennee@linaro.org>
>>
>> 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.
Agreed.
>
>> 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.
We definitely want to make it clear to use the specific function. We
just need a solution for making sure they can always find out the
current pc when we have multiple virtual mappings to the same place.
>
>> So maybe we can just extend API with PC diversion, and not overthink too much about potential consequences on registers.
>>
>> Regards,
>> Pierrick
>
> Best,
> Florian
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-02-20 16:35 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-20 15:19 [PATCH v3 0/5] Enable PC diversion via the plugin API Florian Hofhammer
2026-01-20 15:22 ` [PATCH v3 1/5] plugins: add PC diversion API function Florian Hofhammer
2026-01-26 20:31 ` Alex Bennée
2026-01-20 15:23 ` [PATCH v3 2/5] plugins: add read-only property for registers Florian Hofhammer
2026-01-26 20:18 ` Alex Bennée
2026-01-26 20:46 ` Alex Bennée
2026-02-13 13:38 ` Florian Hofhammer
2026-02-13 18:36 ` Pierrick Bouvier
2026-02-16 12:54 ` Florian Hofhammer
2026-02-17 2:30 ` Pierrick Bouvier
2026-02-20 16:34 ` Alex Bennée
2026-01-20 15:23 ` [PATCH v3 3/5] plugins: prohibit writing to read-only registers Florian Hofhammer
2026-01-20 15:24 ` [PATCH v3 4/5] tests/tcg: add test for qemu_plugin_set_pc API Florian Hofhammer
2026-01-20 15:26 ` [PATCH v3 5/5] tests/tcg/plugins: test register readonly feature Florian Hofhammer
2026-01-26 19:17 ` [PATCH v3 0/5] Enable PC diversion via the plugin API Pierrick Bouvier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.