* [PATCH v6 1/7] plugins: add flag to specify whether PC is rw
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
@ 2026-03-03 13:07 ` Florian Hofhammer
2026-03-03 18:08 ` Pierrick Bouvier
2026-03-03 13:07 ` [PATCH v6 2/7] linux-user: make syscall emulation interruptible Florian Hofhammer
` (6 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-03 13:07 UTC (permalink / raw)
To: qemu-devel
Cc: Florian Hofhammer, Alex Bennée, Pierrick Bouvier,
Laurent Vivier, berrange, richard.henderson, imp
In addition to the flags specifying whether general-purpose registers
are read-write (rw) during a plugin callback, we add an additional flag
explicitly stating whether the PC is writable. This is in preparation of
a patch that allows to explicitly set the PC to divert control flow from
within a plugin callback, which is currently not possible.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
include/plugins/qemu-plugin.h | 3 +++
plugins/api.c | 4 +++-
plugins/core.c | 29 ++++++++++++++++-------------
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
index 17a834dca9..a6ec8e275d 100644
--- a/include/plugins/qemu-plugin.h
+++ b/include/plugins/qemu-plugin.h
@@ -325,11 +325,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 {
diff --git a/plugins/api.c b/plugins/api.c
index 04ca7da7f1..32eb086300 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -458,7 +458,9 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
{
g_assert(current_cpu);
- if (buf->len == 0 || qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS) {
+ 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 false;
}
diff --git a/plugins/core.c b/plugins/core.c
index 42fd986593..7220b9dbb4 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -119,7 +119,7 @@ static void plugin_vcpu_cb__discon(CPUState *cpu,
struct qemu_plugin_cb *cb, *next;
uint64_t to = cpu->cc->get_pc(cpu);
- qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
if (cpu->cpu_index < plugin.num_vcpus) {
/* iterate safely; plugins might uninstall themselves at any time */
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
@@ -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);
}
@@ -584,7 +587,7 @@ qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
return false;
}
- qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
+ qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
qemu_plugin_vcpu_syscall_filter_cb_t func = cb->f.vcpu_syscall_filter;
@@ -605,7 +608,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);
}
@@ -614,7 +617,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);
}
@@ -885,6 +888,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.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v6 1/7] plugins: add flag to specify whether PC is rw
2026-03-03 13:07 ` [PATCH v6 1/7] plugins: add flag to specify whether PC is rw Florian Hofhammer
@ 2026-03-03 18:08 ` Pierrick Bouvier
0 siblings, 0 replies; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-03 18:08 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/3/26 5:07 AM, Florian Hofhammer wrote:
> In addition to the flags specifying whether general-purpose registers
> are read-write (rw) during a plugin callback, we add an additional flag
> explicitly stating whether the PC is writable. This is in preparation of
> a patch that allows to explicitly set the PC to divert control flow from
> within a plugin callback, which is currently not possible.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> include/plugins/qemu-plugin.h | 3 +++
> plugins/api.c | 4 +++-
> plugins/core.c | 29 ++++++++++++++++-------------
> 3 files changed, 22 insertions(+), 14 deletions(-)
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v6 2/7] linux-user: make syscall emulation interruptible
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
2026-03-03 13:07 ` [PATCH v6 1/7] plugins: add flag to specify whether PC is rw Florian Hofhammer
@ 2026-03-03 13:07 ` Florian Hofhammer
2026-03-03 13:07 ` [PATCH v6 3/7] plugins: add PC diversion API function Florian Hofhammer
` (5 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-03 13:07 UTC (permalink / raw)
To: qemu-devel
Cc: Florian Hofhammer, Alex Bennée, Pierrick Bouvier,
Laurent Vivier, berrange, richard.henderson, imp
The syscall emulation code previously wasn't interruptible via
cpu_loop_exit(), as this construct relies on a longjmp target that is not
live anymore in the syscall handling code. Consequently, longjmp() would
operate on a (potentially overwritten) stale jump buffer. This patch adds an additional
setjmp and the necessary handling around it to make longjmp() (and by
proxy cpu_loop_exit() safe to call even within a syscall context.
Reviewed-by: Warner Losh <imp@bsdimp.com>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
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 | 1 +
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 | 9 ++++++---
linux-user/or1k/cpu_loop.c | 2 +-
linux-user/ppc/cpu_loop.c | 10 +++++++---
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 | 16 ++++++++++++++++
linux-user/xtensa/cpu_loop.c | 1 +
19 files changed, 60 insertions(+), 22 deletions(-)
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 5711055aff..9464246e9e 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..4b4b663052 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -124,6 +124,7 @@ void cpu_loop(CPUHPPAState *env)
break;
case -QEMU_ERESTARTSYS:
case -QEMU_ESIGRETURN:
+ 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..fa264b27ec 100644
--- a/linux-user/mips/cpu_loop.c
+++ b/linux-user/mips/cpu_loop.c
@@ -140,9 +140,12 @@ done_syscall:
env->active_tc.PC -= 4;
break;
}
- if (ret == -QEMU_ESIGRETURN) {
- /* Returning from a successful sigreturn syscall.
- Avoid clobbering register state. */
+ 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;
}
if ((abi_ulong)ret >= (abi_ulong)-1133) {
diff --git a/linux-user/or1k/cpu_loop.c b/linux-user/or1k/cpu_loop.c
index 2167d880d5..e7e9929e6f 100644
--- a/linux-user/or1k/cpu_loop.c
+++ b/linux-user/or1k/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..1f9ee20bd0 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -340,9 +340,13 @@ void cpu_loop(CPUPPCState *env)
env->nip -= 4;
break;
}
- if (ret == (target_ulong)(-QEMU_ESIGRETURN)) {
- /* Returning from a successful sigreturn syscall.
- Avoid corrupting register state. */
+ 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;
}
if (ret > (target_ulong)(-515)) {
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..ab633eeae3 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 d466d0e32f..99e1ed97d9 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>
@@ -600,6 +601,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";
+ }
return strerror(target_to_host_errno(err));
}
@@ -14410,6 +14414,18 @@ abi_long do_syscall(CPUArchState *cpu_env, int num, abi_long arg1,
return -QEMU_ESIGRETURN;
}
+ /*
+ * Set up a longjmp target here so that we can call cpu_loop_exit to
+ * redirect control flow back to the main loop even from within
+ * syscall-related plugin callbacks.
+ * For other types of callbacks or longjmp call sites, the longjmp target
+ * is set up in the cpu loop itself but in syscalls the target is not live
+ * anymore.
+ */
+ 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..d2b4ccdfad 100644
--- a/linux-user/xtensa/cpu_loop.c
+++ b/linux-user/xtensa/cpu_loop.c
@@ -186,6 +186,7 @@ void cpu_loop(CPUXtensaState *env)
break;
case -QEMU_ESIGRETURN:
+ case -QEMU_ESETPC:
break;
}
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v6 3/7] plugins: add PC diversion API function
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
2026-03-03 13:07 ` [PATCH v6 1/7] plugins: add flag to specify whether PC is rw Florian Hofhammer
2026-03-03 13:07 ` [PATCH v6 2/7] linux-user: make syscall emulation interruptible Florian Hofhammer
@ 2026-03-03 13:07 ` Florian Hofhammer
2026-03-03 13:07 ` [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API Florian Hofhammer
` (4 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-03 13:07 UTC (permalink / raw)
To: qemu-devel
Cc: Florian Hofhammer, Alex Bennée, Pierrick Bouvier,
Laurent Vivier, berrange, richard.henderson, imp
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.
Link: https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00656.html
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
include/plugins/qemu-plugin.h | 13 +++++++++++++
plugins/api.c | 11 +++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
index a6ec8e275d..f083c30fd3 100644
--- a/include/plugins/qemu-plugin.h
+++ b/include/plugins/qemu-plugin.h
@@ -76,6 +76,7 @@ typedef uint64_t qemu_plugin_id_t;
*
* version 6:
* - changed return value of qemu_plugin_{read,write}_register from int to bool
+ * - added qemu_plugin_set_pc
*/
extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
@@ -1042,6 +1043,18 @@ QEMU_PLUGIN_API
bool 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 does not return.
+ */
+QEMU_PLUGIN_API
+G_NORETURN
+void qemu_plugin_set_pc(uint64_t vaddr);
+
/**
* qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
*
diff --git a/plugins/api.c b/plugins/api.c
index 32eb086300..23c291f644 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"
@@ -467,6 +468,16 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1) > 0);
}
+void qemu_plugin_set_pc(uint64_t vaddr)
+{
+ g_assert(current_cpu);
+
+ g_assert(qemu_plugin_get_cb_flags() == QEMU_PLUGIN_CB_RW_REGS_PC);
+
+ 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);
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (2 preceding siblings ...)
2026-03-03 13:07 ` [PATCH v6 3/7] plugins: add PC diversion API function Florian Hofhammer
@ 2026-03-03 13:07 ` Florian Hofhammer
2026-03-03 19:46 ` Pierrick Bouvier
2026-03-03 13:07 ` [PATCH v6 5/7] plugins: add read-only property for registers Florian Hofhammer
` (3 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-03 13:07 UTC (permalink / raw)
To: qemu-devel
Cc: Florian Hofhammer, Alex Bennée, Pierrick Bouvier,
Laurent Vivier, berrange, richard.henderson, imp
The test plugin intercepts execution in different contexts. Without the
plugin, any of the implemented test functions would trigger an assert
and fail. With the plugin, control flow is redirected to skip the assert
and return cleanly via the qemu_plugin_set_pc() API.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
MAINTAINERS | 1 +
tests/tcg/arm/Makefile.target | 6 +
tests/tcg/multiarch/Makefile.target | 17 ++-
.../multiarch/{ => plugin}/check-plugin-output.sh | 0
.../{ => plugin}/test-plugin-mem-access.c | 0
tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
tests/tcg/plugins/meson.build | 1 +
tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
8 files changed, 282 insertions(+), 3 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 6698e5ff69..63c0af4d86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4104,6 +4104,7 @@ S: Maintained
F: docs/devel/tcg-plugins.rst
F: plugins/
F: tests/tcg/plugins/
+F: tests/tcg/multiarch/plugin/
F: tests/functional/aarch64/test_tcg_plugins.py
F: contrib/plugins/
F: scripts/qemu-plugin-symbols.py
diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
index 6189d7a0e2..613bbf0939 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-set-pc: CFLAGS+=-marm
+endif
+
TESTS += $(ARM_TESTS)
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 07d0b27bdd..a347efbadf 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,13 +204,20 @@ 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) $<
run-plugin-test-plugin-syscall-filter-with-libsyscall.so:
+run-plugin-test-plugin-set-pc-with-libsetpc.so:
EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
- run-plugin-test-plugin-syscall-filter-with-libsyscall.so
-else
+ run-plugin-test-plugin-syscall-filter-with-libsyscall.so \
+ run-plugin-test-plugin-set-pc-with-libsetpc.so
+
+else # CONFIG_PLUGIN=n
+# Do not build the syscall skipping test if it's not tested with the setpc
+# plugin because it will simply fail the test.
+MULTIARCH_TESTS := $(filter-out test-plugin-set-pc, $(MULTIARCH_TESTS))
+
# test-plugin-syscall-filter needs syscall plugin to succeed
test-plugin-syscall-filter: CFLAGS+=-DSKIP
endif
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-set-pc.c b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
new file mode 100644
index 0000000000..40d9a9e8f0
--- /dev/null
+++ b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
@@ -0,0 +1,140 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
+ *
+ * This test set exercises the qemu_plugin_set_pc() function in four different
+ * contexts:
+ * 1. in a syscall callback,
+ * 2. in an instruction callback during normal execution,
+ * 3. in an instruction callback during signal handling,
+ * 4. in a memory access callback.
+ * Note: using the volatile guards is necessary to prevent the compiler from
+ * doing dead code elimination even on -O0, which would cause everything after
+ * the asserts and thus also the target labels to be optimized away.
+ */
+#include <assert.h>
+#include <signal.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <setjmp.h>
+
+#define NOINLINE __attribute__((noinline))
+#define NORETURN __attribute__((noreturn))
+
+static int signal_handled;
+/*
+ * The volatile variable is used as a guard to prevent the compiler from
+ * optimizing away "unreachable" labels.
+ */
+static volatile uint32_t guard = 1;
+
+/*
+ * This test executes a magic syscall which communicates two addresses to the
+ * plugin via the syscall arguments. Whenever we reach the "bad" instruction
+ * during normal execution, the plugin should redirect control flow to the
+ * "good" instruction instead.
+ */
+NOINLINE void test_insn(void)
+{
+ long ret = syscall(4095, &&bad_insn, &&good_insn, NULL);
+ assert(ret == 0 && "Syscall filter did not return expected value");
+ if (guard) {
+bad_insn:
+ assert(0 && "PC redirection in instruction callback failed");
+ } else {
+good_insn:
+ return;
+ }
+}
+
+/*
+ * This signal handler communicates a "bad" and a "good" address to the plugin
+ * similar to the previous test, and skips to the "good" address when the "bad"
+ * one is reached. This serves to test whether PC redirection via
+ * qemu_plugin_set_pc() also works properly in a signal handler context.
+ */
+NOINLINE void usr1_handler(int signum)
+{
+ long ret = syscall(4095, &&bad_signal, &&good_signal, NULL);
+ assert(ret == 0 && "Syscall filter did not return expected value");
+ if (guard) {
+bad_signal:
+ assert(0 && "PC redirection in instruction callback failed");
+ } else {
+good_signal:
+ signal_handled = 1;
+ return;
+ }
+}
+
+/*
+ * This test sends a signal to the process, which should trigger the above
+ * signal handler. The signal handler should then exercise the PC redirection
+ * functionality in the context of a signal handler, which behaves a bit
+ * differently from normal execution.
+ */
+NOINLINE void test_sighandler(void)
+{
+ struct sigaction sa = {0};
+ sa.sa_handler = usr1_handler;
+ sigaction(SIGUSR1, &sa, NULL);
+ pid_t pid = getpid();
+ kill(pid, SIGUSR1);
+ assert(signal_handled == 1 && "Signal handler was not executed properly");
+}
+
+/*
+ * This test communicates a "good" address and the address of a local variable
+ * to the plugin. Upon accessing the local variable, the plugin should then
+ * redirect control flow to the "good" address via qemu_plugin_set_pc().
+ */
+NOINLINE void test_mem(void)
+{
+ long ret = syscall(4095, NULL, &&good_mem, &guard);
+ assert(ret == 0 && "Syscall filter did not return expected value");
+ if (guard) {
+ assert(0 && "PC redirection in memory access callback failed");
+ } else {
+good_mem:
+ return;
+ }
+}
+
+/*
+ * This test executes a magic syscall which is intercepted and its actual
+ * execution skipped via the qemu_plugin_set_pc() API. In a proper plugin,
+ * syscall skipping would rather be implemented via the syscall filtering
+ * callback, but we want to make sure qemu_plugin_set_pc() works in different
+ * contexts.
+ */
+NOINLINE NORETURN
+void test_syscall(void)
+{
+ syscall(4096, &&good_syscall);
+ if (guard) {
+ assert(0 && "PC redirection in syscall callback failed");
+ } else {
+good_syscall:
+ /*
+ * Note: we execute this test last and exit straight from here because
+ * when the plugin redirects control flow upon syscall, the stack frame
+ * for the syscall function (and potential other functions in the call
+ * chain in libc) is still live and the stack is not unwound properly.
+ * Thus, returning from here is risky and breaks on some architectures,
+ * so we just exit directly from this test.
+ */
+ _exit(EXIT_SUCCESS);
+ }
+}
+
+
+int main(int argc, char *argv[])
+{
+ test_insn();
+ test_sighandler();
+ test_mem();
+ test_syscall();
+}
diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
index c5e49753fd..b3e3a9a6d0 100644
--- a/tests/tcg/plugins/meson.build
+++ b/tests/tcg/plugins/meson.build
@@ -7,6 +7,7 @@ test_plugins = [
'mem.c',
'patch.c',
'reset.c',
+'setpc.c',
'syscall.c',
]
diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c
new file mode 100644
index 0000000000..72ae31a0ef
--- /dev/null
+++ b/tests/tcg/plugins/setpc.c
@@ -0,0 +1,120 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
+ */
+#include <assert.h>
+#include <glib.h>
+#include <inttypes.h>
+#include <unistd.h>
+
+#include <qemu-plugin.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+static uint64_t source_pc;
+static uint64_t target_pc;
+static uint64_t target_vaddr;
+
+static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
+ int64_t num, uint64_t a1, uint64_t a2,
+ uint64_t a3, uint64_t a4, uint64_t a5,
+ uint64_t a6, uint64_t a7, uint64_t a8)
+{
+ if (num == 4096) {
+ qemu_plugin_outs("Marker syscall detected, jump to clean return\n");
+ qemu_plugin_set_pc(a1);
+ }
+}
+
+static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
+ int64_t num, uint64_t a1, uint64_t a2,
+ uint64_t a3, uint64_t a4, uint64_t a5,
+ uint64_t a6, uint64_t a7, uint64_t a8,
+ uint64_t *sysret)
+{
+ if (num == 4095) {
+ qemu_plugin_outs("Communication syscall detected, set target_pc / "
+ "target_vaddr\n");
+ source_pc = a1;
+ target_pc = a2;
+ target_vaddr = a3;
+ if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
+ /*
+ * Some architectures (e.g., m68k) use 32-bit addresses with the
+ * top bit set, which causes them to get sign-extended somewhere in
+ * the chain to this callback. We mask the top bits off here to get
+ * the actual addresses.
+ */
+ qemu_plugin_outs("High bit in addresses detected: possible sign "
+ "extension in syscall, masking off top bits\n");
+ source_pc &= UINT32_MAX;
+ target_pc &= UINT32_MAX;
+ target_vaddr &= UINT32_MAX;
+ }
+ *sysret = 0;
+ return true;
+ }
+ return false;
+}
+
+static void vcpu_insn_exec(unsigned int vcpu_index, void *userdata)
+{
+ uint64_t vaddr = (uint64_t)userdata;
+ if (vaddr == source_pc) {
+ g_assert(target_pc != 0);
+ g_assert(target_vaddr == 0);
+
+ qemu_plugin_outs("Marker instruction detected, jump to clean return\n");
+ qemu_plugin_set_pc(target_pc);
+ }
+}
+
+static void vcpu_mem_access(unsigned int vcpu_index,
+ qemu_plugin_meminfo_t info,
+ uint64_t vaddr, void *userdata)
+{
+ if (vaddr != 0 && vaddr == target_vaddr) {
+ g_assert(source_pc == 0);
+ g_assert(target_pc != 0);
+ qemu_plugin_mem_value val = qemu_plugin_mem_get_value(info);
+ /* target_vaddr points to our volatile guard ==> should always be 1 */
+ g_assert(val.type == QEMU_PLUGIN_MEM_VALUE_U32);
+ g_assert(val.data.u32 == 1);
+
+ qemu_plugin_outs("Marker mem access detected, jump to clean return\n");
+ qemu_plugin_set_pc(target_pc);
+ }
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+ size_t insns = qemu_plugin_tb_n_insns(tb);
+ for (size_t i = 0; i < insns; i++) {
+ struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
+ uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn);
+ /*
+ * Note: we cannot only register the callbacks if the instruction is
+ * in one of the functions of interest, because symbol lookup for
+ * filtering does not work for all architectures (e.g., ppc64).
+ */
+ qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
+ QEMU_PLUGIN_CB_RW_REGS_PC,
+ (void *)insn_vaddr);
+ qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem_access,
+ QEMU_PLUGIN_CB_RW_REGS_PC,
+ QEMU_PLUGIN_MEM_R, NULL);
+ }
+}
+
+
+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_syscall_cb(id, vcpu_syscall);
+ qemu_plugin_register_vcpu_syscall_filter_cb(id, vcpu_syscall_filter);
+ qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+ return 0;
+}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-03 13:07 ` [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API Florian Hofhammer
@ 2026-03-03 19:46 ` Pierrick Bouvier
2026-03-04 15:35 ` Florian Hofhammer
0 siblings, 1 reply; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-03 19:46 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/3/26 5:07 AM, Florian Hofhammer wrote:
> The test plugin intercepts execution in different contexts. Without the
> plugin, any of the implemented test functions would trigger an assert
> and fail. With the plugin, control flow is redirected to skip the assert
> and return cleanly via the qemu_plugin_set_pc() API.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> MAINTAINERS | 1 +
> tests/tcg/arm/Makefile.target | 6 +
> tests/tcg/multiarch/Makefile.target | 17 ++-
> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
> tests/tcg/plugins/meson.build | 1 +
> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
> 8 files changed, 282 insertions(+), 3 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6698e5ff69..63c0af4d86 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4104,6 +4104,7 @@ S: Maintained
> F: docs/devel/tcg-plugins.rst
> F: plugins/
> F: tests/tcg/plugins/
> +F: tests/tcg/multiarch/plugin/
> F: tests/functional/aarch64/test_tcg_plugins.py
> F: contrib/plugins/
> F: scripts/qemu-plugin-symbols.py
> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
> index 6189d7a0e2..613bbf0939 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-set-pc: CFLAGS+=-marm
> +endif
> +
Is that still needed?
> TESTS += $(ARM_TESTS)
> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
> index 07d0b27bdd..a347efbadf 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,13 +204,20 @@ 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) $<
> run-plugin-test-plugin-syscall-filter-with-libsyscall.so:
> +run-plugin-test-plugin-set-pc-with-libsetpc.so:
>
> EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
> - run-plugin-test-plugin-syscall-filter-with-libsyscall.so
> -else
> + run-plugin-test-plugin-syscall-filter-with-libsyscall.so \
> + run-plugin-test-plugin-set-pc-with-libsetpc.so
> +
> +else # CONFIG_PLUGIN=n
> +# Do not build the syscall skipping test if it's not tested with the setpc
> +# plugin because it will simply fail the test.
> +MULTIARCH_TESTS := $(filter-out test-plugin-set-pc, $(MULTIARCH_TESTS))
> +
> # test-plugin-syscall-filter needs syscall plugin to succeed
> test-plugin-syscall-filter: CFLAGS+=-DSKIP
> endif
> 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-set-pc.c b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
> new file mode 100644
> index 0000000000..40d9a9e8f0
> --- /dev/null
> +++ b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
> @@ -0,0 +1,140 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
> + *
> + * This test set exercises the qemu_plugin_set_pc() function in four different
> + * contexts:
> + * 1. in a syscall callback,
> + * 2. in an instruction callback during normal execution,
> + * 3. in an instruction callback during signal handling,
> + * 4. in a memory access callback.
> + * Note: using the volatile guards is necessary to prevent the compiler from
> + * doing dead code elimination even on -O0, which would cause everything after
> + * the asserts and thus also the target labels to be optimized away.
> + */
> +#include <assert.h>
> +#include <signal.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <setjmp.h>
> +
> +#define NOINLINE __attribute__((noinline))
Test being compiled in O0, it should not need any noinline attribute.
> +#define NORETURN __attribute__((noreturn))
It's used in a single place, simply move the the attribute there, no
need for another define.
> +
> +static int signal_handled;
> +/*
> + * The volatile variable is used as a guard to prevent the compiler from
> + * optimizing away "unreachable" labels.
> + */
> +static volatile uint32_t guard = 1;
> +
As mentioned on v4, you can simply use an external function for failing
the test, which won't be tainted with noreturn attribute.
This way, you don't need any guard at all or assert(0), and resulting
code is linear and easier to read.
It test completes, it all worked. If it crashes, something was wrong.
void panic(void)
{
g_assert_not_reached();
}
void test_...() {
...
on_panic:
panic();
after_panic:
printf("Hello World\n");
}
> +/*
> + * This test executes a magic syscall which communicates two addresses to the
> + * plugin via the syscall arguments. Whenever we reach the "bad" instruction
> + * during normal execution, the plugin should redirect control flow to the
> + * "good" instruction instead.
> + */
> +NOINLINE void test_insn(void)
> +{
> + long ret = syscall(4095, &&bad_insn, &&good_insn, NULL);
> + assert(ret == 0 && "Syscall filter did not return expected value");
> + if (guard) {
> +bad_insn:
> + assert(0 && "PC redirection in instruction callback failed");
> + } else {
> +good_insn:
> + return;
> + }
> +}
> +
> +/*
> + * This signal handler communicates a "bad" and a "good" address to the plugin
> + * similar to the previous test, and skips to the "good" address when the "bad"
> + * one is reached. This serves to test whether PC redirection via
> + * qemu_plugin_set_pc() also works properly in a signal handler context.
> + */
> +NOINLINE void usr1_handler(int signum)
> +{
> + long ret = syscall(4095, &&bad_signal, &&good_signal, NULL);
> + assert(ret == 0 && "Syscall filter did not return expected value");
> + if (guard) {
> +bad_signal:
> + assert(0 && "PC redirection in instruction callback failed");
> + } else {
> +good_signal:
> + signal_handled = 1;
> + return;
> + }
> +}
> +
> +/*
> + * This test sends a signal to the process, which should trigger the above
> + * signal handler. The signal handler should then exercise the PC redirection
> + * functionality in the context of a signal handler, which behaves a bit
> + * differently from normal execution.
> + */
> +NOINLINE void test_sighandler(void)
> +{
> + struct sigaction sa = {0};
> + sa.sa_handler = usr1_handler;
> + sigaction(SIGUSR1, &sa, NULL);
> + pid_t pid = getpid();
> + kill(pid, SIGUSR1);
> + assert(signal_handled == 1 && "Signal handler was not executed properly");
> +}
> +
> +/*
> + * This test communicates a "good" address and the address of a local variable
> + * to the plugin. Upon accessing the local variable, the plugin should then
> + * redirect control flow to the "good" address via qemu_plugin_set_pc().
> + */
> +NOINLINE void test_mem(void)
> +{
> + long ret = syscall(4095, NULL, &&good_mem, &guard);
Since you use two different syscall numbers, it's worth adding two
defines, that you will duplicate in test and plugin.
Alternatively, you can use the first parameter of syscall to identify
which kind of "service" you want from plugin, with defines duplicated
between test and plugin again.
Feel free to pick the solution you prefer.
Reading 4095 the first time here felt like it was a typo mistake.
> + assert(ret == 0 && "Syscall filter did not return expected value");
> + if (guard) {
> + assert(0 && "PC redirection in memory access callback failed");
> + } else {
> +good_mem:
> + return;
> + }
> +}
> +
> +/*
> + * This test executes a magic syscall which is intercepted and its actual
> + * execution skipped via the qemu_plugin_set_pc() API. In a proper plugin,
> + * syscall skipping would rather be implemented via the syscall filtering
> + * callback, but we want to make sure qemu_plugin_set_pc() works in different
> + * contexts.
> + */
> +NOINLINE NORETURN
> +void test_syscall(void)
> +{
> + syscall(4096, &&good_syscall);
> + if (guard) {
> + assert(0 && "PC redirection in syscall callback failed");
> + } else {
> +good_syscall:
> + /*
> + * Note: we execute this test last and exit straight from here because
> + * when the plugin redirects control flow upon syscall, the stack frame
> + * for the syscall function (and potential other functions in the call
> + * chain in libc) is still live and the stack is not unwound properly.
> + * Thus, returning from here is risky and breaks on some architectures,
> + * so we just exit directly from this test.
> + */
> + _exit(EXIT_SUCCESS);
> + }
> +}
> +
> +
> +int main(int argc, char *argv[])
> +{
> + test_insn();
> + test_sighandler();
> + test_mem();
> + test_syscall();
> +}
> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
> index c5e49753fd..b3e3a9a6d0 100644
> --- a/tests/tcg/plugins/meson.build
> +++ b/tests/tcg/plugins/meson.build
> @@ -7,6 +7,7 @@ test_plugins = [
> 'mem.c',
> 'patch.c',
> 'reset.c',
> +'setpc.c',
> 'syscall.c',
> ]
>
> diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c
> new file mode 100644
> index 0000000000..72ae31a0ef
> --- /dev/null
> +++ b/tests/tcg/plugins/setpc.c
> @@ -0,0 +1,120 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
> + */
> +#include <assert.h>
> +#include <glib.h>
> +#include <inttypes.h>
> +#include <unistd.h>
> +
> +#include <qemu-plugin.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +static uint64_t source_pc;
> +static uint64_t target_pc;
> +static uint64_t target_vaddr;
> +
> +static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
> + int64_t num, uint64_t a1, uint64_t a2,
> + uint64_t a3, uint64_t a4, uint64_t a5,
> + uint64_t a6, uint64_t a7, uint64_t a8)
> +{
> + if (num == 4096) {
> + qemu_plugin_outs("Marker syscall detected, jump to clean return\n");
> + qemu_plugin_set_pc(a1);
> + }
> +}
> +
> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
> + int64_t num, uint64_t a1, uint64_t a2,
> + uint64_t a3, uint64_t a4, uint64_t a5,
> + uint64_t a6, uint64_t a7, uint64_t a8,
> + uint64_t *sysret)
> +{
> + if (num == 4095) {
> + qemu_plugin_outs("Communication syscall detected, set target_pc / "
> + "target_vaddr\n");
> + source_pc = a1;
> + target_pc = a2;
> + target_vaddr = a3;
> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
> + /*
> + * Some architectures (e.g., m68k) use 32-bit addresses with the
> + * top bit set, which causes them to get sign-extended somewhere in
> + * the chain to this callback. We mask the top bits off here to get
> + * the actual addresses.
> + */
This looks like an actual bug. Using a backtrace here, can you find
which function extended it?
Parameter should be treated as unsigned everywhere, else something is
really going wrong.
> + qemu_plugin_outs("High bit in addresses detected: possible sign "
> + "extension in syscall, masking off top bits\n");
> + source_pc &= UINT32_MAX;
> + target_pc &= UINT32_MAX;
> + target_vaddr &= UINT32_MAX;
> + }
> + *sysret = 0;
> + return true;
> + }
> + return false;
> +}
> +
> +static void vcpu_insn_exec(unsigned int vcpu_index, void *userdata)
> +{
> + uint64_t vaddr = (uint64_t)userdata;
> + if (vaddr == source_pc) {
> + g_assert(target_pc != 0);
> + g_assert(target_vaddr == 0);
> +
> + qemu_plugin_outs("Marker instruction detected, jump to clean return\n");
> + qemu_plugin_set_pc(target_pc);
> + }
> +}
> +
> +static void vcpu_mem_access(unsigned int vcpu_index,
> + qemu_plugin_meminfo_t info,
> + uint64_t vaddr, void *userdata)
> +{
> + if (vaddr != 0 && vaddr == target_vaddr) {
> + g_assert(source_pc == 0);
> + g_assert(target_pc != 0);
> + qemu_plugin_mem_value val = qemu_plugin_mem_get_value(info);
> + /* target_vaddr points to our volatile guard ==> should always be 1 */
> + g_assert(val.type == QEMU_PLUGIN_MEM_VALUE_U32);
> + g_assert(val.data.u32 == 1);
> +
> + qemu_plugin_outs("Marker mem access detected, jump to clean return\n");
> + qemu_plugin_set_pc(target_pc);
> + }
Thanks for adding this case also!
So you'll definitely need a read for this precise test, to be able to
keep a load. If it would work with a local variable, that's good, else,
try with a local static (eventually marked volatile if it helps), and in
last case, use a global variable.
> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> + size_t insns = qemu_plugin_tb_n_insns(tb);
> + for (size_t i = 0; i < insns; i++) {
> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> + uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn);
> + /*
> + * Note: we cannot only register the callbacks if the instruction is
> + * in one of the functions of interest, because symbol lookup for
> + * filtering does not work for all architectures (e.g., ppc64).
> + */
Too sad :)
It's not a problem to instrument all instructions though.
> + qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
> + QEMU_PLUGIN_CB_RW_REGS_PC,
> + (void *)insn_vaddr);
> + qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem_access,
> + QEMU_PLUGIN_CB_RW_REGS_PC,
> + QEMU_PLUGIN_MEM_R, NULL);
> + }
> +}
> +
> +
> +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_syscall_cb(id, vcpu_syscall);
> + qemu_plugin_register_vcpu_syscall_filter_cb(id, vcpu_syscall_filter);
> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> + return 0;
> +}
>
Thanks for the update.
Test looks good, and minus the style nits reported, it looks quite ready.
It would be nice if we could sort the signed parameter extension also to
avoid a hack on this.
Regards,
Pierrick
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-03 19:46 ` Pierrick Bouvier
@ 2026-03-04 15:35 ` Florian Hofhammer
2026-03-04 15:45 ` Florian Hofhammer
2026-03-04 17:44 ` Pierrick Bouvier
0 siblings, 2 replies; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-04 15:35 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
[-- Attachment #1: Type: text/plain, Size: 19842 bytes --]
On 03/03/2026 20:46, Pierrick Bouvier wrote:
> On 3/3/26 5:07 AM, Florian Hofhammer wrote:
>> The test plugin intercepts execution in different contexts. Without the
>> plugin, any of the implemented test functions would trigger an assert
>> and fail. With the plugin, control flow is redirected to skip the assert
>> and return cleanly via the qemu_plugin_set_pc() API.
>>
>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>> ---
>> MAINTAINERS | 1 +
>> tests/tcg/arm/Makefile.target | 6 +
>> tests/tcg/multiarch/Makefile.target | 17 ++-
>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>> .../{ => plugin}/test-plugin-mem-access.c | 0
>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>> tests/tcg/plugins/meson.build | 1 +
>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>> 8 files changed, 282 insertions(+), 3 deletions(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6698e5ff69..63c0af4d86 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4104,6 +4104,7 @@ S: Maintained
>> F: docs/devel/tcg-plugins.rst
>> F: plugins/
>> F: tests/tcg/plugins/
>> +F: tests/tcg/multiarch/plugin/
>> F: tests/functional/aarch64/test_tcg_plugins.py
>> F: contrib/plugins/
>> F: scripts/qemu-plugin-symbols.py
>> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
>> index 6189d7a0e2..613bbf0939 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-set-pc: CFLAGS+=-marm
>> +endif
>> +
>
> Is that still needed?
Yes, unfortunately. The compiler emits Thumb2 instructions where it
deems it appropriate but the addresses of functions/labels are still at
least 2-byte aligned and therefore don't have the bottom bit set.
Setting the PC to an even address therefore makes the vCPU think the
target is in arm32 mode, and it tries to execute the Thumb2 instructions
in arm32 mode. I'd either need to ensure that the targets are always in
Thumb2 mode and set the bottom bit myself, or ensure that everything is
in arm32 mode from the start.
>> TESTS += $(ARM_TESTS)
>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
>> index 07d0b27bdd..a347efbadf 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,13 +204,20 @@ 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) $<
>> run-plugin-test-plugin-syscall-filter-with-libsyscall.so:
>> +run-plugin-test-plugin-set-pc-with-libsetpc.so:
>> EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
>> - run-plugin-test-plugin-syscall-filter-with-libsyscall.so
>> -else
>> + run-plugin-test-plugin-syscall-filter-with-libsyscall.so \
>> + run-plugin-test-plugin-set-pc-with-libsetpc.so
>> +
>> +else # CONFIG_PLUGIN=n
>> +# Do not build the syscall skipping test if it's not tested with the setpc
>> +# plugin because it will simply fail the test.
>> +MULTIARCH_TESTS := $(filter-out test-plugin-set-pc, $(MULTIARCH_TESTS))
>> +
>> # test-plugin-syscall-filter needs syscall plugin to succeed
>> test-plugin-syscall-filter: CFLAGS+=-DSKIP
>> endif
>> 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-set-pc.c b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
>> new file mode 100644
>> index 0000000000..40d9a9e8f0
>> --- /dev/null
>> +++ b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
>> @@ -0,0 +1,140 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
>> + *
>> + * This test set exercises the qemu_plugin_set_pc() function in four different
>> + * contexts:
>> + * 1. in a syscall callback,
>> + * 2. in an instruction callback during normal execution,
>> + * 3. in an instruction callback during signal handling,
>> + * 4. in a memory access callback.
>> + * Note: using the volatile guards is necessary to prevent the compiler from
>> + * doing dead code elimination even on -O0, which would cause everything after
>> + * the asserts and thus also the target labels to be optimized away.
>> + */
>> +#include <assert.h>
>> +#include <signal.h>
>> +#include <stdint.h>
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +#include <setjmp.h>
>> +
>> +#define NOINLINE __attribute__((noinline))
>
> Test being compiled in O0, it should not need any noinline attribute.
Ack, removed.
>> +#define NORETURN __attribute__((noreturn))
>
> It's used in a single place, simply move the the attribute there, no need for another define.
Ditto.
>> +
>> +static int signal_handled;
>> +/*
>> + * The volatile variable is used as a guard to prevent the compiler from
>> + * optimizing away "unreachable" labels.
>> + */
>> +static volatile uint32_t guard = 1;
>> +
>
> As mentioned on v4, you can simply use an external function for failing the test, which won't be tainted with noreturn attribute.
> This way, you don't need any guard at all or assert(0), and resulting code is linear and easier to read.
> It test completes, it all worked. If it crashes, something was wrong.
>
> void panic(void)
> {
> g_assert_not_reached();
> }
>
> void test_...() {
> ...
> on_panic:
> panic();
> after_panic:
> printf("Hello World\n");
> }
I've reworked it to look similar to this (without g_assert_not_reached()
though, because glib is not available in the test itself). Still
required some shuffling of stuff to make sure the compiler doesn't
optimize things away then :) Especially on hexagon, which is the only
target (by default) built with clang instead of gcc. Clang/LLVM seems to
be way more aggressive even at -O0 to do deadcode elimination and
inlining, so I had to add extra CFLAGS for the hexagon target.
>> + * This test executes a magic syscall which communicates two addresses to the
>> + * plugin via the syscall arguments. Whenever we reach the "bad" instruction
>> + * during normal execution, the plugin should redirect control flow to the
>> + * "good" instruction instead.
>> + */
>> +NOINLINE void test_insn(void)
>> +{
>> + long ret = syscall(4095, &&bad_insn, &&good_insn, NULL);
>> + assert(ret == 0 && "Syscall filter did not return expected value");
>> + if (guard) {
>> +bad_insn:
>> + assert(0 && "PC redirection in instruction callback failed");
>> + } else {
>> +good_insn:
>> + return;
>> + }
>> +}
>> +
>> +/*
>> + * This signal handler communicates a "bad" and a "good" address to the plugin
>> + * similar to the previous test, and skips to the "good" address when the "bad"
>> + * one is reached. This serves to test whether PC redirection via
>> + * qemu_plugin_set_pc() also works properly in a signal handler context.
>> + */
>> +NOINLINE void usr1_handler(int signum)
>> +{
>> + long ret = syscall(4095, &&bad_signal, &&good_signal, NULL);
>> + assert(ret == 0 && "Syscall filter did not return expected value");
>> + if (guard) {
>> +bad_signal:
>> + assert(0 && "PC redirection in instruction callback failed");
>> + } else {
>> +good_signal:
>> + signal_handled = 1;
>> + return;
>> + }
>> +}
>> +
>> +/*
>> + * This test sends a signal to the process, which should trigger the above
>> + * signal handler. The signal handler should then exercise the PC redirection
>> + * functionality in the context of a signal handler, which behaves a bit
>> + * differently from normal execution.
>> + */
>> +NOINLINE void test_sighandler(void)
>> +{
>> + struct sigaction sa = {0};
>> + sa.sa_handler = usr1_handler;
>> + sigaction(SIGUSR1, &sa, NULL);
>> + pid_t pid = getpid();
>> + kill(pid, SIGUSR1);
>> + assert(signal_handled == 1 && "Signal handler was not executed properly");
>> +}
>> +
>> +/*
>> + * This test communicates a "good" address and the address of a local variable
>> + * to the plugin. Upon accessing the local variable, the plugin should then
>> + * redirect control flow to the "good" address via qemu_plugin_set_pc().
>> + */
>> +NOINLINE void test_mem(void)
>> +{
>> + long ret = syscall(4095, NULL, &&good_mem, &guard);
>
> Since you use two different syscall numbers, it's worth adding two defines, that you will duplicate in test and plugin.
> Alternatively, you can use the first parameter of syscall to identify which kind of "service" you want from plugin, with defines duplicated between test and plugin again.
> Feel free to pick the solution you prefer.
>
> Reading 4095 the first time here felt like it was a typo mistake.
Ack, reworked this to be clearer.
>
>> + assert(ret == 0 && "Syscall filter did not return expected value");
>> + if (guard) {
>> + assert(0 && "PC redirection in memory access callback failed");
>> + } else {
>> +good_mem:
>> + return;
>> + }
>> +}
>> +
>> +/*
>> + * This test executes a magic syscall which is intercepted and its actual
>> + * execution skipped via the qemu_plugin_set_pc() API. In a proper plugin,
>> + * syscall skipping would rather be implemented via the syscall filtering
>> + * callback, but we want to make sure qemu_plugin_set_pc() works in different
>> + * contexts.
>> + */
>> +NOINLINE NORETURN
>> +void test_syscall(void)
>> +{
>> + syscall(4096, &&good_syscall);
>> + if (guard) {
>> + assert(0 && "PC redirection in syscall callback failed");
>> + } else {
>> +good_syscall:
>> + /*
>> + * Note: we execute this test last and exit straight from here because
>> + * when the plugin redirects control flow upon syscall, the stack frame
>> + * for the syscall function (and potential other functions in the call
>> + * chain in libc) is still live and the stack is not unwound properly.
>> + * Thus, returning from here is risky and breaks on some architectures,
>> + * so we just exit directly from this test.
>> + */
>> + _exit(EXIT_SUCCESS);
>> + }
>> +}
>> +
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + test_insn();
>> + test_sighandler();
>> + test_mem();
>> + test_syscall();
>> +}
>> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
>> index c5e49753fd..b3e3a9a6d0 100644
>> --- a/tests/tcg/plugins/meson.build
>> +++ b/tests/tcg/plugins/meson.build
>> @@ -7,6 +7,7 @@ test_plugins = [
>> 'mem.c',
>> 'patch.c',
>> 'reset.c',
>> +'setpc.c',
>> 'syscall.c',
>> ]
>> diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c
>> new file mode 100644
>> index 0000000000..72ae31a0ef
>> --- /dev/null
>> +++ b/tests/tcg/plugins/setpc.c
>> @@ -0,0 +1,120 @@
>> +/*
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + *
>> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
>> + */
>> +#include <assert.h>
>> +#include <glib.h>
>> +#include <inttypes.h>
>> +#include <unistd.h>
>> +
>> +#include <qemu-plugin.h>
>> +
>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>> +
>> +static uint64_t source_pc;
>> +static uint64_t target_pc;
>> +static uint64_t target_vaddr;
>> +
>> +static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>> + int64_t num, uint64_t a1, uint64_t a2,
>> + uint64_t a3, uint64_t a4, uint64_t a5,
>> + uint64_t a6, uint64_t a7, uint64_t a8)
>> +{
>> + if (num == 4096) {
>> + qemu_plugin_outs("Marker syscall detected, jump to clean return\n");
>> + qemu_plugin_set_pc(a1);
>> + }
>> +}
>> +
>> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>> + int64_t num, uint64_t a1, uint64_t a2,
>> + uint64_t a3, uint64_t a4, uint64_t a5,
>> + uint64_t a6, uint64_t a7, uint64_t a8,
>> + uint64_t *sysret)
>> +{
>> + if (num == 4095) {
>> + qemu_plugin_outs("Communication syscall detected, set target_pc / "
>> + "target_vaddr\n");
>> + source_pc = a1;
>> + target_pc = a2;
>> + target_vaddr = a3;
>> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
>> + /*
>> + * Some architectures (e.g., m68k) use 32-bit addresses with the
>> + * top bit set, which causes them to get sign-extended somewhere in
>> + * the chain to this callback. We mask the top bits off here to get
>> + * the actual addresses.
>> + */
>
> This looks like an actual bug. Using a backtrace here, can you find which function extended it?
> Parameter should be treated as unsigned everywhere, else something is really going wrong.
tl;dr: register values are considered signed in the syscall handling
code, which seems incorrect to me. Details below.
This seems to be a bigger issue in the syscall handling code and I'm
surprised this never surfaced before. Generally, the CPUArchState struct
in target/*/cpu.h defines the registers as unsigned types. When a
syscall is encountered, the main cpu loop calls do_syscall() from
linux-user/syscall.c, which takes and returns abi_long values. abi_long
is defined as either int32_t or target_long in include/user/abitypes.h
and therefore a signed type, so the register values get converted from
unsigned to signed types here. do_syscall() passes those abi_long values
on, e.g., to record_syscall_start() and send_through_syscall_filters(),
which still take the register values in as abi_long. Those in turn
however pass the values on to qemu_plugin_vcpu_syscall() and
qemu_plugin_vcpu_syscall_filter(), respectively. Those two functions now
take uint64_t values for the arguments, so for 32-bit architectures the
conversion chain is uint32_t (CPUArchState) -> int32_t (do_syscall) ->
uint64_t (callbacks).
This seems to be some bigger refactoring changing all those abi_longs to
abi_ulongs. Would you like me to prepare a separate patch series for
this?
>
>> + qemu_plugin_outs("High bit in addresses detected: possible sign "
>> + "extension in syscall, masking off top bits\n");
>> + source_pc &= UINT32_MAX;
>> + target_pc &= UINT32_MAX;
>> + target_vaddr &= UINT32_MAX;
>> + }
>> + *sysret = 0;
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +static void vcpu_insn_exec(unsigned int vcpu_index, void *userdata)
>> +{
>> + uint64_t vaddr = (uint64_t)userdata;
>> + if (vaddr == source_pc) {
>> + g_assert(target_pc != 0);
>> + g_assert(target_vaddr == 0);
>> +
>> + qemu_plugin_outs("Marker instruction detected, jump to clean return\n");
>> + qemu_plugin_set_pc(target_pc);
>> + }
>> +}
>> +
>> +static void vcpu_mem_access(unsigned int vcpu_index,
>> + qemu_plugin_meminfo_t info,
>> + uint64_t vaddr, void *userdata)
>> +{
>> + if (vaddr != 0 && vaddr == target_vaddr) {
>> + g_assert(source_pc == 0);
>> + g_assert(target_pc != 0);
>> + qemu_plugin_mem_value val = qemu_plugin_mem_get_value(info);
>> + /* target_vaddr points to our volatile guard ==> should always be 1 */
>> + g_assert(val.type == QEMU_PLUGIN_MEM_VALUE_U32);
>> + g_assert(val.data.u32 == 1);
>> +
>> + qemu_plugin_outs("Marker mem access detected, jump to clean return\n");
>> + qemu_plugin_set_pc(target_pc);
>> + }
>
> Thanks for adding this case also!
> So you'll definitely need a read for this precise test, to be able to keep a load. If it would work with a local variable, that's good, else, try with a local static (eventually marked volatile if it helps), and in last case, use a global variable.
Works fine with a local variable!
>
>> +}
>> +
>> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>> +{
>> + size_t insns = qemu_plugin_tb_n_insns(tb);
>> + for (size_t i = 0; i < insns; i++) {
>> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
>> + uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn);
>> + /*
>> + * Note: we cannot only register the callbacks if the instruction is
>> + * in one of the functions of interest, because symbol lookup for
>> + * filtering does not work for all architectures (e.g., ppc64).
>> + */
>
> Too sad :)
> It's not a problem to instrument all instructions though.
>
>> + qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>> + (void *)insn_vaddr);
>> + qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem_access,
>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>> + QEMU_PLUGIN_MEM_R, NULL);
>> + }
>> +}
>> +
>> +
>> +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_syscall_cb(id, vcpu_syscall);
>> + qemu_plugin_register_vcpu_syscall_filter_cb(id, vcpu_syscall_filter);
>> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>> + return 0;
>> +}
>>
>
> Thanks for the update.
> Test looks good, and minus the style nits reported, it looks quite ready.
> It would be nice if we could sort the signed parameter extension also to avoid a hack on this.
>
> Regards,
> Pierrick
Best regards,
Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-04 15:35 ` Florian Hofhammer
@ 2026-03-04 15:45 ` Florian Hofhammer
2026-03-04 16:18 ` Florian Hofhammer
2026-03-04 17:44 ` Pierrick Bouvier
1 sibling, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-04 15:45 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
[-- Attachment #1: Type: text/plain, Size: 20885 bytes --]
On 04/03/2026 16:35, Florian Hofhammer wrote:
> On 03/03/2026 20:46, Pierrick Bouvier wrote:
>> On 3/3/26 5:07 AM, Florian Hofhammer wrote:
>>> The test plugin intercepts execution in different contexts. Without the
>>> plugin, any of the implemented test functions would trigger an assert
>>> and fail. With the plugin, control flow is redirected to skip the assert
>>> and return cleanly via the qemu_plugin_set_pc() API.
>>>
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> MAINTAINERS | 1 +
>>> tests/tcg/arm/Makefile.target | 6 +
>>> tests/tcg/multiarch/Makefile.target | 17 ++-
>>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>>> tests/tcg/plugins/meson.build | 1 +
>>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>>> 8 files changed, 282 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 6698e5ff69..63c0af4d86 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -4104,6 +4104,7 @@ S: Maintained
>>> F: docs/devel/tcg-plugins.rst
>>> F: plugins/
>>> F: tests/tcg/plugins/
>>> +F: tests/tcg/multiarch/plugin/
>>> F: tests/functional/aarch64/test_tcg_plugins.py
>>> F: contrib/plugins/
>>> F: scripts/qemu-plugin-symbols.py
>>> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
>>> index 6189d7a0e2..613bbf0939 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-set-pc: CFLAGS+=-marm
>>> +endif
>>> +
>>
>> Is that still needed?
>
> Yes, unfortunately. The compiler emits Thumb2 instructions where it
> deems it appropriate but the addresses of functions/labels are still at
> least 2-byte aligned and therefore don't have the bottom bit set.
> Setting the PC to an even address therefore makes the vCPU think the
> target is in arm32 mode, and it tries to execute the Thumb2 instructions
> in arm32 mode. I'd either need to ensure that the targets are always in
> Thumb2 mode and set the bottom bit myself, or ensure that everything is
> in arm32 mode from the start.
>
>>> TESTS += $(ARM_TESTS)
>>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
>>> index 07d0b27bdd..a347efbadf 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,13 +204,20 @@ 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) $<
>>> run-plugin-test-plugin-syscall-filter-with-libsyscall.so:
>>> +run-plugin-test-plugin-set-pc-with-libsetpc.so:
>>> EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
>>> - run-plugin-test-plugin-syscall-filter-with-libsyscall.so
>>> -else
>>> + run-plugin-test-plugin-syscall-filter-with-libsyscall.so \
>>> + run-plugin-test-plugin-set-pc-with-libsetpc.so
>>> +
>>> +else # CONFIG_PLUGIN=n
>>> +# Do not build the syscall skipping test if it's not tested with the setpc
>>> +# plugin because it will simply fail the test.
>>> +MULTIARCH_TESTS := $(filter-out test-plugin-set-pc, $(MULTIARCH_TESTS))
>>> +
>>> # test-plugin-syscall-filter needs syscall plugin to succeed
>>> test-plugin-syscall-filter: CFLAGS+=-DSKIP
>>> endif
>>> 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-set-pc.c b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
>>> new file mode 100644
>>> index 0000000000..40d9a9e8f0
>>> --- /dev/null
>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
>>> @@ -0,0 +1,140 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> + *
>>> + * This test set exercises the qemu_plugin_set_pc() function in four different
>>> + * contexts:
>>> + * 1. in a syscall callback,
>>> + * 2. in an instruction callback during normal execution,
>>> + * 3. in an instruction callback during signal handling,
>>> + * 4. in a memory access callback.
>>> + * Note: using the volatile guards is necessary to prevent the compiler from
>>> + * doing dead code elimination even on -O0, which would cause everything after
>>> + * the asserts and thus also the target labels to be optimized away.
>>> + */
>>> +#include <assert.h>
>>> +#include <signal.h>
>>> +#include <stdint.h>
>>> +#include <stdlib.h>
>>> +#include <stdio.h>
>>> +#include <unistd.h>
>>> +#include <setjmp.h>
>>> +
>>> +#define NOINLINE __attribute__((noinline))
>>
>> Test being compiled in O0, it should not need any noinline attribute.
>
> Ack, removed.
>
>>> +#define NORETURN __attribute__((noreturn))
>>
>> It's used in a single place, simply move the the attribute there, no need for another define.
>
> Ditto.
>
>>> +
>>> +static int signal_handled;
>>> +/*
>>> + * The volatile variable is used as a guard to prevent the compiler from
>>> + * optimizing away "unreachable" labels.
>>> + */
>>> +static volatile uint32_t guard = 1;
>>> +
>>
>> As mentioned on v4, you can simply use an external function for failing the test, which won't be tainted with noreturn attribute.
>> This way, you don't need any guard at all or assert(0), and resulting code is linear and easier to read.
>> It test completes, it all worked. If it crashes, something was wrong.
>>
>> void panic(void)
>> {
>> g_assert_not_reached();
>> }
>>
>> void test_...() {
>> ...
>> on_panic:
>> panic();
>> after_panic:
>> printf("Hello World\n");
>> }
>
> I've reworked it to look similar to this (without g_assert_not_reached()
> though, because glib is not available in the test itself). Still
> required some shuffling of stuff to make sure the compiler doesn't
> optimize things away then :) Especially on hexagon, which is the only
> target (by default) built with clang instead of gcc. Clang/LLVM seems to
> be way more aggressive even at -O0 to do deadcode elimination and
> inlining, so I had to add extra CFLAGS for the hexagon target.
>
>>> + * This test executes a magic syscall which communicates two addresses to the
>>> + * plugin via the syscall arguments. Whenever we reach the "bad" instruction
>>> + * during normal execution, the plugin should redirect control flow to the
>>> + * "good" instruction instead.
>>> + */
>>> +NOINLINE void test_insn(void)
>>> +{
>>> + long ret = syscall(4095, &&bad_insn, &&good_insn, NULL);
>>> + assert(ret == 0 && "Syscall filter did not return expected value");
>>> + if (guard) {
>>> +bad_insn:
>>> + assert(0 && "PC redirection in instruction callback failed");
>>> + } else {
>>> +good_insn:
>>> + return;
>>> + }
>>> +}
>>> +
>>> +/*
>>> + * This signal handler communicates a "bad" and a "good" address to the plugin
>>> + * similar to the previous test, and skips to the "good" address when the "bad"
>>> + * one is reached. This serves to test whether PC redirection via
>>> + * qemu_plugin_set_pc() also works properly in a signal handler context.
>>> + */
>>> +NOINLINE void usr1_handler(int signum)
>>> +{
>>> + long ret = syscall(4095, &&bad_signal, &&good_signal, NULL);
>>> + assert(ret == 0 && "Syscall filter did not return expected value");
>>> + if (guard) {
>>> +bad_signal:
>>> + assert(0 && "PC redirection in instruction callback failed");
>>> + } else {
>>> +good_signal:
>>> + signal_handled = 1;
>>> + return;
>>> + }
>>> +}
>>> +
>>> +/*
>>> + * This test sends a signal to the process, which should trigger the above
>>> + * signal handler. The signal handler should then exercise the PC redirection
>>> + * functionality in the context of a signal handler, which behaves a bit
>>> + * differently from normal execution.
>>> + */
>>> +NOINLINE void test_sighandler(void)
>>> +{
>>> + struct sigaction sa = {0};
>>> + sa.sa_handler = usr1_handler;
>>> + sigaction(SIGUSR1, &sa, NULL);
>>> + pid_t pid = getpid();
>>> + kill(pid, SIGUSR1);
>>> + assert(signal_handled == 1 && "Signal handler was not executed properly");
>>> +}
>>> +
>>> +/*
>>> + * This test communicates a "good" address and the address of a local variable
>>> + * to the plugin. Upon accessing the local variable, the plugin should then
>>> + * redirect control flow to the "good" address via qemu_plugin_set_pc().
>>> + */
>>> +NOINLINE void test_mem(void)
>>> +{
>>> + long ret = syscall(4095, NULL, &&good_mem, &guard);
>>
>> Since you use two different syscall numbers, it's worth adding two defines, that you will duplicate in test and plugin.
>> Alternatively, you can use the first parameter of syscall to identify which kind of "service" you want from plugin, with defines duplicated between test and plugin again.
>> Feel free to pick the solution you prefer.
>>
>> Reading 4095 the first time here felt like it was a typo mistake.
>
> Ack, reworked this to be clearer.
>
>>
>>> + assert(ret == 0 && "Syscall filter did not return expected value");
>>> + if (guard) {
>>> + assert(0 && "PC redirection in memory access callback failed");
>>> + } else {
>>> +good_mem:
>>> + return;
>>> + }
>>> +}
>>> +
>>> +/*
>>> + * This test executes a magic syscall which is intercepted and its actual
>>> + * execution skipped via the qemu_plugin_set_pc() API. In a proper plugin,
>>> + * syscall skipping would rather be implemented via the syscall filtering
>>> + * callback, but we want to make sure qemu_plugin_set_pc() works in different
>>> + * contexts.
>>> + */
>>> +NOINLINE NORETURN
>>> +void test_syscall(void)
>>> +{
>>> + syscall(4096, &&good_syscall);
>>> + if (guard) {
>>> + assert(0 && "PC redirection in syscall callback failed");
>>> + } else {
>>> +good_syscall:
>>> + /*
>>> + * Note: we execute this test last and exit straight from here because
>>> + * when the plugin redirects control flow upon syscall, the stack frame
>>> + * for the syscall function (and potential other functions in the call
>>> + * chain in libc) is still live and the stack is not unwound properly.
>>> + * Thus, returning from here is risky and breaks on some architectures,
>>> + * so we just exit directly from this test.
>>> + */
>>> + _exit(EXIT_SUCCESS);
>>> + }
>>> +}
>>> +
>>> +
>>> +int main(int argc, char *argv[])
>>> +{
>>> + test_insn();
>>> + test_sighandler();
>>> + test_mem();
>>> + test_syscall();
>>> +}
>>> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
>>> index c5e49753fd..b3e3a9a6d0 100644
>>> --- a/tests/tcg/plugins/meson.build
>>> +++ b/tests/tcg/plugins/meson.build
>>> @@ -7,6 +7,7 @@ test_plugins = [
>>> 'mem.c',
>>> 'patch.c',
>>> 'reset.c',
>>> +'setpc.c',
>>> 'syscall.c',
>>> ]
>>> diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c
>>> new file mode 100644
>>> index 0000000000..72ae31a0ef
>>> --- /dev/null
>>> +++ b/tests/tcg/plugins/setpc.c
>>> @@ -0,0 +1,120 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> + */
>>> +#include <assert.h>
>>> +#include <glib.h>
>>> +#include <inttypes.h>
>>> +#include <unistd.h>
>>> +
>>> +#include <qemu-plugin.h>
>>> +
>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>>> +
>>> +static uint64_t source_pc;
>>> +static uint64_t target_pc;
>>> +static uint64_t target_vaddr;
>>> +
>>> +static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>>> + int64_t num, uint64_t a1, uint64_t a2,
>>> + uint64_t a3, uint64_t a4, uint64_t a5,
>>> + uint64_t a6, uint64_t a7, uint64_t a8)
>>> +{
>>> + if (num == 4096) {
>>> + qemu_plugin_outs("Marker syscall detected, jump to clean return\n");
>>> + qemu_plugin_set_pc(a1);
>>> + }
>>> +}
>>> +
>>> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>>> + int64_t num, uint64_t a1, uint64_t a2,
>>> + uint64_t a3, uint64_t a4, uint64_t a5,
>>> + uint64_t a6, uint64_t a7, uint64_t a8,
>>> + uint64_t *sysret)
>>> +{
>>> + if (num == 4095) {
>>> + qemu_plugin_outs("Communication syscall detected, set target_pc / "
>>> + "target_vaddr\n");
>>> + source_pc = a1;
>>> + target_pc = a2;
>>> + target_vaddr = a3;
>>> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
>>> + /*
>>> + * Some architectures (e.g., m68k) use 32-bit addresses with the
>>> + * top bit set, which causes them to get sign-extended somewhere in
>>> + * the chain to this callback. We mask the top bits off here to get
>>> + * the actual addresses.
>>> + */
>>
>> This looks like an actual bug. Using a backtrace here, can you find which function extended it?
>> Parameter should be treated as unsigned everywhere, else something is really going wrong.
>
> tl;dr: register values are considered signed in the syscall handling
> code, which seems incorrect to me. Details below.
>
> This seems to be a bigger issue in the syscall handling code and I'm
> surprised this never surfaced before. Generally, the CPUArchState struct
> in target/*/cpu.h defines the registers as unsigned types. When a
> syscall is encountered, the main cpu loop calls do_syscall() from
> linux-user/syscall.c, which takes and returns abi_long values. abi_long
> is defined as either int32_t or target_long in include/user/abitypes.h
> and therefore a signed type, so the register values get converted from
> unsigned to signed types here. do_syscall() passes those abi_long values
> on, e.g., to record_syscall_start() and send_through_syscall_filters(),
> which still take the register values in as abi_long. Those in turn
> however pass the values on to qemu_plugin_vcpu_syscall() and
> qemu_plugin_vcpu_syscall_filter(), respectively. Those two functions now
> take uint64_t values for the arguments, so for 32-bit architectures the
> conversion chain is uint32_t (CPUArchState) -> int32_t (do_syscall) ->
> uint64_t (callbacks).
>
> This seems to be some bigger refactoring changing all those abi_longs to
> abi_ulongs. Would you like me to prepare a separate patch series for
> this?
As a follow-up on this, changing this seems to be a bit more complicated
than I initially thought. The arguments could be simply made unsigned,
but the return value (which is also just a register content and should
therefore be unsigned if I understand correctly) can't easily be made
unsigned. The per-target main loop relies on the return value of
do_syscall() to be signed to determine whether one of the QEMU-specific
errors (e.g., QEMU_ERESTARTSYS) was returned.
>>
>>> + qemu_plugin_outs("High bit in addresses detected: possible sign "
>>> + "extension in syscall, masking off top bits\n");
>>> + source_pc &= UINT32_MAX;
>>> + target_pc &= UINT32_MAX;
>>> + target_vaddr &= UINT32_MAX;
>>> + }
>>> + *sysret = 0;
>>> + return true;
>>> + }
>>> + return false;
>>> +}
>>> +
>>> +static void vcpu_insn_exec(unsigned int vcpu_index, void *userdata)
>>> +{
>>> + uint64_t vaddr = (uint64_t)userdata;
>>> + if (vaddr == source_pc) {
>>> + g_assert(target_pc != 0);
>>> + g_assert(target_vaddr == 0);
>>> +
>>> + qemu_plugin_outs("Marker instruction detected, jump to clean return\n");
>>> + qemu_plugin_set_pc(target_pc);
>>> + }
>>> +}
>>> +
>>> +static void vcpu_mem_access(unsigned int vcpu_index,
>>> + qemu_plugin_meminfo_t info,
>>> + uint64_t vaddr, void *userdata)
>>> +{
>>> + if (vaddr != 0 && vaddr == target_vaddr) {
>>> + g_assert(source_pc == 0);
>>> + g_assert(target_pc != 0);
>>> + qemu_plugin_mem_value val = qemu_plugin_mem_get_value(info);
>>> + /* target_vaddr points to our volatile guard ==> should always be 1 */
>>> + g_assert(val.type == QEMU_PLUGIN_MEM_VALUE_U32);
>>> + g_assert(val.data.u32 == 1);
>>> +
>>> + qemu_plugin_outs("Marker mem access detected, jump to clean return\n");
>>> + qemu_plugin_set_pc(target_pc);
>>> + }
>>
>> Thanks for adding this case also!
>> So you'll definitely need a read for this precise test, to be able to keep a load. If it would work with a local variable, that's good, else, try with a local static (eventually marked volatile if it helps), and in last case, use a global variable.
>
> Works fine with a local variable!
>
>>
>>> +}
>>> +
>>> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>>> +{
>>> + size_t insns = qemu_plugin_tb_n_insns(tb);
>>> + for (size_t i = 0; i < insns; i++) {
>>> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
>>> + uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn);
>>> + /*
>>> + * Note: we cannot only register the callbacks if the instruction is
>>> + * in one of the functions of interest, because symbol lookup for
>>> + * filtering does not work for all architectures (e.g., ppc64).
>>> + */
>>
>> Too sad :)
>> It's not a problem to instrument all instructions though.
>>
>>> + qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
>>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>>> + (void *)insn_vaddr);
>>> + qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem_access,
>>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>>> + QEMU_PLUGIN_MEM_R, NULL);
>>> + }
>>> +}
>>> +
>>> +
>>> +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_syscall_cb(id, vcpu_syscall);
>>> + qemu_plugin_register_vcpu_syscall_filter_cb(id, vcpu_syscall_filter);
>>> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>>> + return 0;
>>> +}
>>>
>>
>> Thanks for the update.
>> Test looks good, and minus the style nits reported, it looks quite ready.
>> It would be nice if we could sort the signed parameter extension also to avoid a hack on this.
>>
>> Regards,
>> Pierrick
>
> Best regards,
> Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-04 15:45 ` Florian Hofhammer
@ 2026-03-04 16:18 ` Florian Hofhammer
2026-03-04 17:56 ` Pierrick Bouvier
0 siblings, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-04 16:18 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
[-- Attachment #1: Type: text/plain, Size: 22105 bytes --]
On 04/03/2026 16:45, Florian Hofhammer wrote:
> On 04/03/2026 16:35, Florian Hofhammer wrote:
>> On 03/03/2026 20:46, Pierrick Bouvier wrote:
>>> On 3/3/26 5:07 AM, Florian Hofhammer wrote:
>>>> The test plugin intercepts execution in different contexts. Without the
>>>> plugin, any of the implemented test functions would trigger an assert
>>>> and fail. With the plugin, control flow is redirected to skip the assert
>>>> and return cleanly via the qemu_plugin_set_pc() API.
>>>>
>>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>> ---
>>>> MAINTAINERS | 1 +
>>>> tests/tcg/arm/Makefile.target | 6 +
>>>> tests/tcg/multiarch/Makefile.target | 17 ++-
>>>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>>>> tests/tcg/plugins/meson.build | 1 +
>>>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>>>> 8 files changed, 282 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 6698e5ff69..63c0af4d86 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -4104,6 +4104,7 @@ S: Maintained
>>>> F: docs/devel/tcg-plugins.rst
>>>> F: plugins/
>>>> F: tests/tcg/plugins/
>>>> +F: tests/tcg/multiarch/plugin/
>>>> F: tests/functional/aarch64/test_tcg_plugins.py
>>>> F: contrib/plugins/
>>>> F: scripts/qemu-plugin-symbols.py
>>>> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target
>>>> index 6189d7a0e2..613bbf0939 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-set-pc: CFLAGS+=-marm
>>>> +endif
>>>> +
>>>
>>> Is that still needed?
>>
>> Yes, unfortunately. The compiler emits Thumb2 instructions where it
>> deems it appropriate but the addresses of functions/labels are still at
>> least 2-byte aligned and therefore don't have the bottom bit set.
>> Setting the PC to an even address therefore makes the vCPU think the
>> target is in arm32 mode, and it tries to execute the Thumb2 instructions
>> in arm32 mode. I'd either need to ensure that the targets are always in
>> Thumb2 mode and set the bottom bit myself, or ensure that everything is
>> in arm32 mode from the start.
>>
>>>> TESTS += $(ARM_TESTS)
>>>> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
>>>> index 07d0b27bdd..a347efbadf 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,13 +204,20 @@ 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) $<
>>>> run-plugin-test-plugin-syscall-filter-with-libsyscall.so:
>>>> +run-plugin-test-plugin-set-pc-with-libsetpc.so:
>>>> EXTRA_RUNS_WITH_PLUGIN += run-plugin-test-plugin-mem-access-with-libmem.so \
>>>> - run-plugin-test-plugin-syscall-filter-with-libsyscall.so
>>>> -else
>>>> + run-plugin-test-plugin-syscall-filter-with-libsyscall.so \
>>>> + run-plugin-test-plugin-set-pc-with-libsetpc.so
>>>> +
>>>> +else # CONFIG_PLUGIN=n
>>>> +# Do not build the syscall skipping test if it's not tested with the setpc
>>>> +# plugin because it will simply fail the test.
>>>> +MULTIARCH_TESTS := $(filter-out test-plugin-set-pc, $(MULTIARCH_TESTS))
>>>> +
>>>> # test-plugin-syscall-filter needs syscall plugin to succeed
>>>> test-plugin-syscall-filter: CFLAGS+=-DSKIP
>>>> endif
>>>> 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-set-pc.c b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
>>>> new file mode 100644
>>>> index 0000000000..40d9a9e8f0
>>>> --- /dev/null
>>>> +++ b/tests/tcg/multiarch/plugin/test-plugin-set-pc.c
>>>> @@ -0,0 +1,140 @@
>>>> +/*
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + *
>>>> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>> + *
>>>> + * This test set exercises the qemu_plugin_set_pc() function in four different
>>>> + * contexts:
>>>> + * 1. in a syscall callback,
>>>> + * 2. in an instruction callback during normal execution,
>>>> + * 3. in an instruction callback during signal handling,
>>>> + * 4. in a memory access callback.
>>>> + * Note: using the volatile guards is necessary to prevent the compiler from
>>>> + * doing dead code elimination even on -O0, which would cause everything after
>>>> + * the asserts and thus also the target labels to be optimized away.
>>>> + */
>>>> +#include <assert.h>
>>>> +#include <signal.h>
>>>> +#include <stdint.h>
>>>> +#include <stdlib.h>
>>>> +#include <stdio.h>
>>>> +#include <unistd.h>
>>>> +#include <setjmp.h>
>>>> +
>>>> +#define NOINLINE __attribute__((noinline))
>>>
>>> Test being compiled in O0, it should not need any noinline attribute.
>>
>> Ack, removed.
>>
>>>> +#define NORETURN __attribute__((noreturn))
>>>
>>> It's used in a single place, simply move the the attribute there, no need for another define.
>>
>> Ditto.
>>
>>>> +
>>>> +static int signal_handled;
>>>> +/*
>>>> + * The volatile variable is used as a guard to prevent the compiler from
>>>> + * optimizing away "unreachable" labels.
>>>> + */
>>>> +static volatile uint32_t guard = 1;
>>>> +
>>>
>>> As mentioned on v4, you can simply use an external function for failing the test, which won't be tainted with noreturn attribute.
>>> This way, you don't need any guard at all or assert(0), and resulting code is linear and easier to read.
>>> It test completes, it all worked. If it crashes, something was wrong.
>>>
>>> void panic(void)
>>> {
>>> g_assert_not_reached();
>>> }
>>>
>>> void test_...() {
>>> ...
>>> on_panic:
>>> panic();
>>> after_panic:
>>> printf("Hello World\n");
>>> }
>>
>> I've reworked it to look similar to this (without g_assert_not_reached()
>> though, because glib is not available in the test itself). Still
>> required some shuffling of stuff to make sure the compiler doesn't
>> optimize things away then :) Especially on hexagon, which is the only
>> target (by default) built with clang instead of gcc. Clang/LLVM seems to
>> be way more aggressive even at -O0 to do deadcode elimination and
>> inlining, so I had to add extra CFLAGS for the hexagon target.
>>
>>>> + * This test executes a magic syscall which communicates two addresses to the
>>>> + * plugin via the syscall arguments. Whenever we reach the "bad" instruction
>>>> + * during normal execution, the plugin should redirect control flow to the
>>>> + * "good" instruction instead.
>>>> + */
>>>> +NOINLINE void test_insn(void)
>>>> +{
>>>> + long ret = syscall(4095, &&bad_insn, &&good_insn, NULL);
>>>> + assert(ret == 0 && "Syscall filter did not return expected value");
>>>> + if (guard) {
>>>> +bad_insn:
>>>> + assert(0 && "PC redirection in instruction callback failed");
>>>> + } else {
>>>> +good_insn:
>>>> + return;
>>>> + }
>>>> +}
>>>> +
>>>> +/*
>>>> + * This signal handler communicates a "bad" and a "good" address to the plugin
>>>> + * similar to the previous test, and skips to the "good" address when the "bad"
>>>> + * one is reached. This serves to test whether PC redirection via
>>>> + * qemu_plugin_set_pc() also works properly in a signal handler context.
>>>> + */
>>>> +NOINLINE void usr1_handler(int signum)
>>>> +{
>>>> + long ret = syscall(4095, &&bad_signal, &&good_signal, NULL);
>>>> + assert(ret == 0 && "Syscall filter did not return expected value");
>>>> + if (guard) {
>>>> +bad_signal:
>>>> + assert(0 && "PC redirection in instruction callback failed");
>>>> + } else {
>>>> +good_signal:
>>>> + signal_handled = 1;
>>>> + return;
>>>> + }
>>>> +}
>>>> +
>>>> +/*
>>>> + * This test sends a signal to the process, which should trigger the above
>>>> + * signal handler. The signal handler should then exercise the PC redirection
>>>> + * functionality in the context of a signal handler, which behaves a bit
>>>> + * differently from normal execution.
>>>> + */
>>>> +NOINLINE void test_sighandler(void)
>>>> +{
>>>> + struct sigaction sa = {0};
>>>> + sa.sa_handler = usr1_handler;
>>>> + sigaction(SIGUSR1, &sa, NULL);
>>>> + pid_t pid = getpid();
>>>> + kill(pid, SIGUSR1);
>>>> + assert(signal_handled == 1 && "Signal handler was not executed properly");
>>>> +}
>>>> +
>>>> +/*
>>>> + * This test communicates a "good" address and the address of a local variable
>>>> + * to the plugin. Upon accessing the local variable, the plugin should then
>>>> + * redirect control flow to the "good" address via qemu_plugin_set_pc().
>>>> + */
>>>> +NOINLINE void test_mem(void)
>>>> +{
>>>> + long ret = syscall(4095, NULL, &&good_mem, &guard);
>>>
>>> Since you use two different syscall numbers, it's worth adding two defines, that you will duplicate in test and plugin.
>>> Alternatively, you can use the first parameter of syscall to identify which kind of "service" you want from plugin, with defines duplicated between test and plugin again.
>>> Feel free to pick the solution you prefer.
>>>
>>> Reading 4095 the first time here felt like it was a typo mistake.
>>
>> Ack, reworked this to be clearer.
>>
>>>
>>>> + assert(ret == 0 && "Syscall filter did not return expected value");
>>>> + if (guard) {
>>>> + assert(0 && "PC redirection in memory access callback failed");
>>>> + } else {
>>>> +good_mem:
>>>> + return;
>>>> + }
>>>> +}
>>>> +
>>>> +/*
>>>> + * This test executes a magic syscall which is intercepted and its actual
>>>> + * execution skipped via the qemu_plugin_set_pc() API. In a proper plugin,
>>>> + * syscall skipping would rather be implemented via the syscall filtering
>>>> + * callback, but we want to make sure qemu_plugin_set_pc() works in different
>>>> + * contexts.
>>>> + */
>>>> +NOINLINE NORETURN
>>>> +void test_syscall(void)
>>>> +{
>>>> + syscall(4096, &&good_syscall);
>>>> + if (guard) {
>>>> + assert(0 && "PC redirection in syscall callback failed");
>>>> + } else {
>>>> +good_syscall:
>>>> + /*
>>>> + * Note: we execute this test last and exit straight from here because
>>>> + * when the plugin redirects control flow upon syscall, the stack frame
>>>> + * for the syscall function (and potential other functions in the call
>>>> + * chain in libc) is still live and the stack is not unwound properly.
>>>> + * Thus, returning from here is risky and breaks on some architectures,
>>>> + * so we just exit directly from this test.
>>>> + */
>>>> + _exit(EXIT_SUCCESS);
>>>> + }
>>>> +}
>>>> +
>>>> +
>>>> +int main(int argc, char *argv[])
>>>> +{
>>>> + test_insn();
>>>> + test_sighandler();
>>>> + test_mem();
>>>> + test_syscall();
>>>> +}
>>>> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
>>>> index c5e49753fd..b3e3a9a6d0 100644
>>>> --- a/tests/tcg/plugins/meson.build
>>>> +++ b/tests/tcg/plugins/meson.build
>>>> @@ -7,6 +7,7 @@ test_plugins = [
>>>> 'mem.c',
>>>> 'patch.c',
>>>> 'reset.c',
>>>> +'setpc.c',
>>>> 'syscall.c',
>>>> ]
>>>> diff --git a/tests/tcg/plugins/setpc.c b/tests/tcg/plugins/setpc.c
>>>> new file mode 100644
>>>> index 0000000000..72ae31a0ef
>>>> --- /dev/null
>>>> +++ b/tests/tcg/plugins/setpc.c
>>>> @@ -0,0 +1,120 @@
>>>> +/*
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + *
>>>> + * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
>>>> + */
>>>> +#include <assert.h>
>>>> +#include <glib.h>
>>>> +#include <inttypes.h>
>>>> +#include <unistd.h>
>>>> +
>>>> +#include <qemu-plugin.h>
>>>> +
>>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>>>> +
>>>> +static uint64_t source_pc;
>>>> +static uint64_t target_pc;
>>>> +static uint64_t target_vaddr;
>>>> +
>>>> +static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>>>> + int64_t num, uint64_t a1, uint64_t a2,
>>>> + uint64_t a3, uint64_t a4, uint64_t a5,
>>>> + uint64_t a6, uint64_t a7, uint64_t a8)
>>>> +{
>>>> + if (num == 4096) {
>>>> + qemu_plugin_outs("Marker syscall detected, jump to clean return\n");
>>>> + qemu_plugin_set_pc(a1);
>>>> + }
>>>> +}
>>>> +
>>>> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>>>> + int64_t num, uint64_t a1, uint64_t a2,
>>>> + uint64_t a3, uint64_t a4, uint64_t a5,
>>>> + uint64_t a6, uint64_t a7, uint64_t a8,
>>>> + uint64_t *sysret)
>>>> +{
>>>> + if (num == 4095) {
>>>> + qemu_plugin_outs("Communication syscall detected, set target_pc / "
>>>> + "target_vaddr\n");
>>>> + source_pc = a1;
>>>> + target_pc = a2;
>>>> + target_vaddr = a3;
>>>> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
>>>> + /*
>>>> + * Some architectures (e.g., m68k) use 32-bit addresses with the
>>>> + * top bit set, which causes them to get sign-extended somewhere in
>>>> + * the chain to this callback. We mask the top bits off here to get
>>>> + * the actual addresses.
>>>> + */
>>>
>>> This looks like an actual bug. Using a backtrace here, can you find which function extended it?
>>> Parameter should be treated as unsigned everywhere, else something is really going wrong.
>>
>> tl;dr: register values are considered signed in the syscall handling
>> code, which seems incorrect to me. Details below.
>>
>> This seems to be a bigger issue in the syscall handling code and I'm
>> surprised this never surfaced before. Generally, the CPUArchState struct
>> in target/*/cpu.h defines the registers as unsigned types. When a
>> syscall is encountered, the main cpu loop calls do_syscall() from
>> linux-user/syscall.c, which takes and returns abi_long values. abi_long
>> is defined as either int32_t or target_long in include/user/abitypes.h
>> and therefore a signed type, so the register values get converted from
>> unsigned to signed types here. do_syscall() passes those abi_long values
>> on, e.g., to record_syscall_start() and send_through_syscall_filters(),
>> which still take the register values in as abi_long. Those in turn
>> however pass the values on to qemu_plugin_vcpu_syscall() and
>> qemu_plugin_vcpu_syscall_filter(), respectively. Those two functions now
>> take uint64_t values for the arguments, so for 32-bit architectures the
>> conversion chain is uint32_t (CPUArchState) -> int32_t (do_syscall) ->
>> uint64_t (callbacks).
>>
>> This seems to be some bigger refactoring changing all those abi_longs to
>> abi_ulongs. Would you like me to prepare a separate patch series for
>> this?
>
> As a follow-up on this, changing this seems to be a bit more complicated
> than I initially thought. The arguments could be simply made unsigned,
> but the return value (which is also just a register content and should
> therefore be unsigned if I understand correctly) can't easily be made
> unsigned. The per-target main loop relies on the return value of
> do_syscall() to be signed to determine whether one of the QEMU-specific
> errors (e.g., QEMU_ERESTARTSYS) was returned.
Note: some architectures already define the return value as unsigned in
the main cpu loop and implicitly cast the return value of do_syscall().
Given that in theory, a syscall's return value could always coincide
with the values of QEMU's special errors, I'm wondering whether it would
make sense to make do_syscall()'s return value unsigned and signal those
special error codes out-of-band instead of in the syscall return value,
e.g., either via a separate pointer argument to do_syscall() or by
making do_syscall() return a struct containing both the return value and
a potential error code.
But maybe I'm getting ahead of myself here, please let me know what you
think!
>
>>>
>>>> + qemu_plugin_outs("High bit in addresses detected: possible sign "
>>>> + "extension in syscall, masking off top bits\n");
>>>> + source_pc &= UINT32_MAX;
>>>> + target_pc &= UINT32_MAX;
>>>> + target_vaddr &= UINT32_MAX;
>>>> + }
>>>> + *sysret = 0;
>>>> + return true;
>>>> + }
>>>> + return false;
>>>> +}
>>>> +
>>>> +static void vcpu_insn_exec(unsigned int vcpu_index, void *userdata)
>>>> +{
>>>> + uint64_t vaddr = (uint64_t)userdata;
>>>> + if (vaddr == source_pc) {
>>>> + g_assert(target_pc != 0);
>>>> + g_assert(target_vaddr == 0);
>>>> +
>>>> + qemu_plugin_outs("Marker instruction detected, jump to clean return\n");
>>>> + qemu_plugin_set_pc(target_pc);
>>>> + }
>>>> +}
>>>> +
>>>> +static void vcpu_mem_access(unsigned int vcpu_index,
>>>> + qemu_plugin_meminfo_t info,
>>>> + uint64_t vaddr, void *userdata)
>>>> +{
>>>> + if (vaddr != 0 && vaddr == target_vaddr) {
>>>> + g_assert(source_pc == 0);
>>>> + g_assert(target_pc != 0);
>>>> + qemu_plugin_mem_value val = qemu_plugin_mem_get_value(info);
>>>> + /* target_vaddr points to our volatile guard ==> should always be 1 */
>>>> + g_assert(val.type == QEMU_PLUGIN_MEM_VALUE_U32);
>>>> + g_assert(val.data.u32 == 1);
>>>> +
>>>> + qemu_plugin_outs("Marker mem access detected, jump to clean return\n");
>>>> + qemu_plugin_set_pc(target_pc);
>>>> + }
>>>
>>> Thanks for adding this case also!
>>> So you'll definitely need a read for this precise test, to be able to keep a load. If it would work with a local variable, that's good, else, try with a local static (eventually marked volatile if it helps), and in last case, use a global variable.
>>
>> Works fine with a local variable!
>>
>>>
>>>> +}
>>>> +
>>>> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>>>> +{
>>>> + size_t insns = qemu_plugin_tb_n_insns(tb);
>>>> + for (size_t i = 0; i < insns; i++) {
>>>> + struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
>>>> + uint64_t insn_vaddr = qemu_plugin_insn_vaddr(insn);
>>>> + /*
>>>> + * Note: we cannot only register the callbacks if the instruction is
>>>> + * in one of the functions of interest, because symbol lookup for
>>>> + * filtering does not work for all architectures (e.g., ppc64).
>>>> + */
>>>
>>> Too sad :)
>>> It's not a problem to instrument all instructions though.
>>>
>>>> + qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
>>>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>>>> + (void *)insn_vaddr);
>>>> + qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem_access,
>>>> + QEMU_PLUGIN_CB_RW_REGS_PC,
>>>> + QEMU_PLUGIN_MEM_R, NULL);
>>>> + }
>>>> +}
>>>> +
>>>> +
>>>> +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_syscall_cb(id, vcpu_syscall);
>>>> + qemu_plugin_register_vcpu_syscall_filter_cb(id, vcpu_syscall_filter);
>>>> + qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>>>> + return 0;
>>>> +}
>>>>
>>>
>>> Thanks for the update.
>>> Test looks good, and minus the style nits reported, it looks quite ready.
>>> It would be nice if we could sort the signed parameter extension also to avoid a hack on this.
>>>
>>> Regards,
>>> Pierrick
>>
>> Best regards,
>> Florian
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-04 16:18 ` Florian Hofhammer
@ 2026-03-04 17:56 ` Pierrick Bouvier
2026-03-04 19:38 ` Pierrick Bouvier
0 siblings, 1 reply; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-04 17:56 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/4/26 8:18 AM, Florian Hofhammer wrote:
>>>>> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>>>>> + int64_t num, uint64_t a1, uint64_t a2,
>>>>> + uint64_t a3, uint64_t a4, uint64_t a5,
>>>>> + uint64_t a6, uint64_t a7, uint64_t a8,
>>>>> + uint64_t *sysret)
>>>>> +{
>>>>> + if (num == 4095) {
>>>>> + qemu_plugin_outs("Communication syscall detected, set target_pc / "
>>>>> + "target_vaddr\n");
>>>>> + source_pc = a1;
>>>>> + target_pc = a2;
>>>>> + target_vaddr = a3;
>>>>> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
>>>>> + /*
>>>>> + * Some architectures (e.g., m68k) use 32-bit addresses with the
>>>>> + * top bit set, which causes them to get sign-extended somewhere in
>>>>> + * the chain to this callback. We mask the top bits off here to get
>>>>> + * the actual addresses.
>>>>> + */
>>>>
>>>> This looks like an actual bug. Using a backtrace here, can you find which function extended it?
>>>> Parameter should be treated as unsigned everywhere, else something is really going wrong.
>>>
>>> tl;dr: register values are considered signed in the syscall handling
>>> code, which seems incorrect to me. Details below.
>>>
>>> This seems to be a bigger issue in the syscall handling code and I'm
>>> surprised this never surfaced before. Generally, the CPUArchState struct
>>> in target/*/cpu.h defines the registers as unsigned types. When a
>>> syscall is encountered, the main cpu loop calls do_syscall() from
>>> linux-user/syscall.c, which takes and returns abi_long values. abi_long
>>> is defined as either int32_t or target_long in include/user/abitypes.h
>>> and therefore a signed type, so the register values get converted from
>>> unsigned to signed types here. do_syscall() passes those abi_long values
>>> on, e.g., to record_syscall_start() and send_through_syscall_filters(),
>>> which still take the register values in as abi_long. Those in turn
>>> however pass the values on to qemu_plugin_vcpu_syscall() and
>>> qemu_plugin_vcpu_syscall_filter(), respectively. Those two functions now
>>> take uint64_t values for the arguments, so for 32-bit architectures the
>>> conversion chain is uint32_t (CPUArchState) -> int32_t (do_syscall) ->
>>> uint64_t (callbacks).
>>>
>>> This seems to be some bigger refactoring changing all those abi_longs to
>>> abi_ulongs. Would you like me to prepare a separate patch series for
>>> this?
>>
Indeed, they are signed in linux-user, so maybe my original assumption
that it *should* be unsigned is wrong. Let me take a look to see what
should be the correct semantic here.
>> As a follow-up on this, changing this seems to be a bit more complicated
>> than I initially thought. The arguments could be simply made unsigned,
>> but the return value (which is also just a register content and should
>> therefore be unsigned if I understand correctly) can't easily be made
>> unsigned. The per-target main loop relies on the return value of
>> do_syscall() to be signed to determine whether one of the QEMU-specific
>> errors (e.g., QEMU_ERESTARTSYS) was returned.
>
> Note: some architectures already define the return value as unsigned in
> the main cpu loop and implicitly cast the return value of do_syscall().
> Given that in theory, a syscall's return value could always coincide
> with the values of QEMU's special errors, I'm wondering whether it would
> make sense to make do_syscall()'s return value unsigned and signal those
> special error codes out-of-band instead of in the syscall return value,
> e.g., either via a separate pointer argument to do_syscall() or by
> making do_syscall() return a struct containing both the return value and
> a potential error code.
>
It could be a solution. I'm just a little bit relunctant to make such a
change now because we have limited time (soft freeze is coming next
week), so not the right time to make such deep changes and... it has
been working like that for long time.
> But maybe I'm getting ahead of myself here, please let me know what you
> think!
>
Thanks for your very good analysis. I'll come back once I have more
information about what should be the correct fix.
Meanwhile, and because we are close from softfreeze, just keep the
current hack in the test. This way, we can focus on sending your series
and the additional comment patch you sent.
Thanks,
Pierrick
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-04 17:56 ` Pierrick Bouvier
@ 2026-03-04 19:38 ` Pierrick Bouvier
2026-03-05 7:14 ` Florian Hofhammer
0 siblings, 1 reply; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-04 19:38 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
[-- Attachment #1: Type: text/plain, Size: 5099 bytes --]
On 3/4/26 9:56 AM, Pierrick Bouvier wrote:
> On 3/4/26 8:18 AM, Florian Hofhammer wrote:
>>>>>> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>>>>>> + int64_t num, uint64_t a1, uint64_t a2,
>>>>>> + uint64_t a3, uint64_t a4, uint64_t a5,
>>>>>> + uint64_t a6, uint64_t a7, uint64_t a8,
>>>>>> + uint64_t *sysret)
>>>>>> +{
>>>>>> + if (num == 4095) {
>>>>>> + qemu_plugin_outs("Communication syscall detected, set target_pc / "
>>>>>> + "target_vaddr\n");
>>>>>> + source_pc = a1;
>>>>>> + target_pc = a2;
>>>>>> + target_vaddr = a3;
>>>>>> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
>>>>>> + /*
>>>>>> + * Some architectures (e.g., m68k) use 32-bit addresses with the
>>>>>> + * top bit set, which causes them to get sign-extended somewhere in
>>>>>> + * the chain to this callback. We mask the top bits off here to get
>>>>>> + * the actual addresses.
>>>>>> + */
>>>>>
>>>>> This looks like an actual bug. Using a backtrace here, can you find which function extended it?
>>>>> Parameter should be treated as unsigned everywhere, else something is really going wrong.
>>>>
>>>> tl;dr: register values are considered signed in the syscall handling
>>>> code, which seems incorrect to me. Details below.
>>>>
>>>> This seems to be a bigger issue in the syscall handling code and I'm
>>>> surprised this never surfaced before. Generally, the CPUArchState struct
>>>> in target/*/cpu.h defines the registers as unsigned types. When a
>>>> syscall is encountered, the main cpu loop calls do_syscall() from
>>>> linux-user/syscall.c, which takes and returns abi_long values. abi_long
>>>> is defined as either int32_t or target_long in include/user/abitypes.h
>>>> and therefore a signed type, so the register values get converted from
>>>> unsigned to signed types here. do_syscall() passes those abi_long values
>>>> on, e.g., to record_syscall_start() and send_through_syscall_filters(),
>>>> which still take the register values in as abi_long. Those in turn
>>>> however pass the values on to qemu_plugin_vcpu_syscall() and
>>>> qemu_plugin_vcpu_syscall_filter(), respectively. Those two functions now
>>>> take uint64_t values for the arguments, so for 32-bit architectures the
>>>> conversion chain is uint32_t (CPUArchState) -> int32_t (do_syscall) ->
>>>> uint64_t (callbacks).
>>>>
>>>> This seems to be some bigger refactoring changing all those abi_longs to
>>>> abi_ulongs. Would you like me to prepare a separate patch series for
>>>> this?
>>>
>
> Indeed, they are signed in linux-user, so maybe my original assumption
> that it *should* be unsigned is wrong. Let me take a look to see what
> should be the correct semantic here.
>
>>> As a follow-up on this, changing this seems to be a bit more complicated
>>> than I initially thought. The arguments could be simply made unsigned,
>>> but the return value (which is also just a register content and should
>>> therefore be unsigned if I understand correctly) can't easily be made
>>> unsigned. The per-target main loop relies on the return value of
>>> do_syscall() to be signed to determine whether one of the QEMU-specific
>>> errors (e.g., QEMU_ERESTARTSYS) was returned.
>>
>> Note: some architectures already define the return value as unsigned in
>> the main cpu loop and implicitly cast the return value of do_syscall().
>> Given that in theory, a syscall's return value could always coincide
>> with the values of QEMU's special errors, I'm wondering whether it would
>> make sense to make do_syscall()'s return value unsigned and signal those
>> special error codes out-of-band instead of in the syscall return value,
>> e.g., either via a separate pointer argument to do_syscall() or by
>> making do_syscall() return a struct containing both the return value and
>> a potential error code.
>>
>
> It could be a solution. I'm just a little bit relunctant to make such a
> change now because we have limited time (soft freeze is coming next
> week), so not the right time to make such deep changes and... it has
> been working like that for long time.
>
>> But maybe I'm getting ahead of myself here, please let me know what you
>> think!
>>
> Thanks for your very good analysis. I'll come back once I have more
> information about what should be the correct fix.
>
> Meanwhile, and because we are close from softfreeze, just keep the
> current hack in the test. This way, we can focus on sending your series
> and the additional comment patch you sent.
>
> Thanks,
> Pierrick
The attached patch fixes the problem by clamping syscall arguments for
32-bit targets. Return type has to be signed anyway, so no change is
needed on this side.
You can add it as the first patch on your next version, and remove the
hack checking the highest bit in this test, now that the problem is solved.
Regards,
Pierrick
[-- Attachment #2: 0001-plugins-core-clamp-syscall-arguments-if-target-is-32.patch --]
[-- Type: text/x-patch, Size: 2245 bytes --]
From 9814b90693bb5a6949f1bcdc9a31bef558fc5539 Mon Sep 17 00:00:00 2001
From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Date: Wed, 4 Mar 2026 11:18:35 -0800
Subject: [PATCH] plugins/core: clamp syscall arguments if target is 32-bit
Syscall arguments are abi_long in user code, and plugin syscall
interface works with uint64_t only.
According to C integer promotion rules, the value is sign extended
before becoming unsigned, thus setting high bits when only 32-bit lower
ones should have a significant value.
As a result, we need to clamp values we receive from user-code
accordingly.
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
plugins/core.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/plugins/core.c b/plugins/core.c
index 7220b9dbb4a..2324bbffa3d 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -516,6 +516,23 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
}
}
+static void clamp_syscall_arguments(uint64_t *a1, uint64_t *a2, uint64_t *a3,
+ uint64_t *a4, uint64_t *a5, uint64_t *a6,
+ uint64_t *a7, uint64_t *a8)
+{
+ if (target_long_bits() == 32) {
+ const uint64_t mask = UINT32_MAX;
+ *a1 &= mask;
+ *a2 &= mask;
+ *a3 &= mask;
+ *a4 &= mask;
+ *a5 &= mask;
+ *a6 &= mask;
+ *a7 &= mask;
+ *a8 &= mask;
+ }
+}
+
/*
* Disable CFI checks.
* The callback function has been loaded from an external library so we do not
@@ -534,6 +551,8 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
return;
}
+ clamp_syscall_arguments(&a1, &a2, &a3, &a4, &a5, &a6, &a7, &a8);
+
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
qemu_plugin_vcpu_syscall_cb_t func = cb->f.vcpu_syscall;
@@ -587,6 +606,8 @@ qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
return false;
}
+ clamp_syscall_arguments(&a1, &a2, &a3, &a4, &a5, &a6, &a7, &a8);
+
qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS_PC);
QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
--
2.47.3
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-04 19:38 ` Pierrick Bouvier
@ 2026-03-05 7:14 ` Florian Hofhammer
0 siblings, 0 replies; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-05 7:14 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 04/03/2026 20:38, Pierrick Bouvier wrote:
> On 3/4/26 9:56 AM, Pierrick Bouvier wrote:
>> On 3/4/26 8:18 AM, Florian Hofhammer wrote:
>>>>>>> +static bool vcpu_syscall_filter(qemu_plugin_id_t id, unsigned int vcpu_index,
>>>>>>> + int64_t num, uint64_t a1, uint64_t a2,
>>>>>>> + uint64_t a3, uint64_t a4, uint64_t a5,
>>>>>>> + uint64_t a6, uint64_t a7, uint64_t a8,
>>>>>>> + uint64_t *sysret)
>>>>>>> +{
>>>>>>> + if (num == 4095) {
>>>>>>> + qemu_plugin_outs("Communication syscall detected, set target_pc / "
>>>>>>> + "target_vaddr\n");
>>>>>>> + source_pc = a1;
>>>>>>> + target_pc = a2;
>>>>>>> + target_vaddr = a3;
>>>>>>> + if (source_pc >> 63 || target_pc >> 63 || target_vaddr >> 63) {
>>>>>>> + /*
>>>>>>> + * Some architectures (e.g., m68k) use 32-bit addresses with the
>>>>>>> + * top bit set, which causes them to get sign-extended somewhere in
>>>>>>> + * the chain to this callback. We mask the top bits off here to get
>>>>>>> + * the actual addresses.
>>>>>>> + */
>>>>>>
>>>>>> This looks like an actual bug. Using a backtrace here, can you find which function extended it?
>>>>>> Parameter should be treated as unsigned everywhere, else something is really going wrong.
>>>>>
>>>>> tl;dr: register values are considered signed in the syscall handling
>>>>> code, which seems incorrect to me. Details below.
>>>>>
>>>>> This seems to be a bigger issue in the syscall handling code and I'm
>>>>> surprised this never surfaced before. Generally, the CPUArchState struct
>>>>> in target/*/cpu.h defines the registers as unsigned types. When a
>>>>> syscall is encountered, the main cpu loop calls do_syscall() from
>>>>> linux-user/syscall.c, which takes and returns abi_long values. abi_long
>>>>> is defined as either int32_t or target_long in include/user/abitypes.h
>>>>> and therefore a signed type, so the register values get converted from
>>>>> unsigned to signed types here. do_syscall() passes those abi_long values
>>>>> on, e.g., to record_syscall_start() and send_through_syscall_filters(),
>>>>> which still take the register values in as abi_long. Those in turn
>>>>> however pass the values on to qemu_plugin_vcpu_syscall() and
>>>>> qemu_plugin_vcpu_syscall_filter(), respectively. Those two functions now
>>>>> take uint64_t values for the arguments, so for 32-bit architectures the
>>>>> conversion chain is uint32_t (CPUArchState) -> int32_t (do_syscall) ->
>>>>> uint64_t (callbacks).
>>>>>
>>>>> This seems to be some bigger refactoring changing all those abi_longs to
>>>>> abi_ulongs. Would you like me to prepare a separate patch series for
>>>>> this?
>>>>
>>
>> Indeed, they are signed in linux-user, so maybe my original assumption
>> that it *should* be unsigned is wrong. Let me take a look to see what
>> should be the correct semantic here.
>>
>>>> As a follow-up on this, changing this seems to be a bit more complicated
>>>> than I initially thought. The arguments could be simply made unsigned,
>>>> but the return value (which is also just a register content and should
>>>> therefore be unsigned if I understand correctly) can't easily be made
>>>> unsigned. The per-target main loop relies on the return value of
>>>> do_syscall() to be signed to determine whether one of the QEMU-specific
>>>> errors (e.g., QEMU_ERESTARTSYS) was returned.
>>>
>>> Note: some architectures already define the return value as unsigned in
>>> the main cpu loop and implicitly cast the return value of do_syscall().
>>> Given that in theory, a syscall's return value could always coincide
>>> with the values of QEMU's special errors, I'm wondering whether it would
>>> make sense to make do_syscall()'s return value unsigned and signal those
>>> special error codes out-of-band instead of in the syscall return value,
>>> e.g., either via a separate pointer argument to do_syscall() or by
>>> making do_syscall() return a struct containing both the return value and
>>> a potential error code.
>>>
>>
>> It could be a solution. I'm just a little bit relunctant to make such a
>> change now because we have limited time (soft freeze is coming next
>> week), so not the right time to make such deep changes and... it has
>> been working like that for long time.
>>
>>> But maybe I'm getting ahead of myself here, please let me know what you
>>> think!
>>>
>> Thanks for your very good analysis. I'll come back once I have more
>> information about what should be the correct fix.
>>
>> Meanwhile, and because we are close from softfreeze, just keep the
>> current hack in the test. This way, we can focus on sending your series
>> and the additional comment patch you sent.
>>
>> Thanks,
>> Pierrick
>
> The attached patch fixes the problem by clamping syscall arguments for 32-bit targets. Return type has to be signed anyway, so no change is needed on this side.
>
> You can add it as the first patch on your next version, and remove the hack checking the highest bit in this test, now that the problem is solved.
>
> Regards,
> Pierrick
Ack, thanks for the patch!
Best regards,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API
2026-03-04 15:35 ` Florian Hofhammer
2026-03-04 15:45 ` Florian Hofhammer
@ 2026-03-04 17:44 ` Pierrick Bouvier
1 sibling, 0 replies; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-04 17:44 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/4/26 7:35 AM, Florian Hofhammer wrote:
> On 03/03/2026 20:46, Pierrick Bouvier wrote:
>> On 3/3/26 5:07 AM, Florian Hofhammer wrote:
>>> The test plugin intercepts execution in different contexts. Without the
>>> plugin, any of the implemented test functions would trigger an assert
>>> and fail. With the plugin, control flow is redirected to skip the assert
>>> and return cleanly via the qemu_plugin_set_pc() API.
>>>
>>> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
>>> ---
>>> MAINTAINERS | 1 +
>>> tests/tcg/arm/Makefile.target | 6 +
>>> tests/tcg/multiarch/Makefile.target | 17 ++-
>>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>>> tests/tcg/plugins/meson.build | 1 +
>>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>>> 8 files changed, 282 insertions(+), 3 deletions(-)
>>>
...
>>
>> As mentioned on v4, you can simply use an external function for failing the test, which won't be tainted with noreturn attribute.
>> This way, you don't need any guard at all or assert(0), and resulting code is linear and easier to read.
>> It test completes, it all worked. If it crashes, something was wrong.
>>
>> void panic(void)
>> {
>> g_assert_not_reached();
>> }
>>
>> void test_...() {
>> ...
>> on_panic:
>> panic();
>> after_panic:
>> printf("Hello World\n");
>> }
>
> I've reworked it to look similar to this (without g_assert_not_reached()
> though, because glib is not available in the test itself). Still
> required some shuffling of stuff to make sure the compiler doesn't
> optimize things away then :) Especially on hexagon, which is the only
> target (by default) built with clang instead of gcc. Clang/LLVM seems to
> be way more aggressive even at -O0 to do deadcode elimination and
> inlining, so I had to add extra CFLAGS for the hexagon target.
>
You're right, I forgot glib is not available there.
assert(0), exit for abort works instead of g_assert_not_reached().
Sounds good for hexagon target.
We can always update the test later if needed, and at least it stays
simple. Thanks for doing this extra step.
Regards,
Pierrick
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v6 5/7] plugins: add read-only property for registers
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (3 preceding siblings ...)
2026-03-03 13:07 ` [PATCH v6 4/7] tests/tcg: add tests for qemu_plugin_set_pc API Florian Hofhammer
@ 2026-03-03 13:07 ` Florian Hofhammer
2026-03-03 18:08 ` Pierrick Bouvier
2026-03-03 13:07 ` [PATCH v6 6/7] plugins: prohibit writing to read-only registers Florian Hofhammer
` (2 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-03 13:07 UTC (permalink / raw)
To: qemu-devel
Cc: Florian Hofhammer, Alex Bennée, Pierrick Bouvier,
Laurent Vivier, berrange, richard.henderson, imp
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.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
include/plugins/qemu-plugin.h | 3 +++
plugins/api.c | 16 ++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
index f083c30fd3..791d223df4 100644
--- a/include/plugins/qemu-plugin.h
+++ b/include/plugins/qemu-plugin.h
@@ -979,11 +979,14 @@ 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 23c291f644..85b34949cb 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -410,6 +410,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 PC */
+static const char rip_str[] = "rip"; /* x86_64-specific name for PC */
+static const char pswa_str[] = "pswa"; /* s390x-specific name for PC */
+static const char iaoq_str[] = "iaoq"; /* HP/PA-specific name for PC */
+static const char rpc_str[] = "rpc"; /* microblaze-specific name for PC */
static GArray *create_register_handles(GArray *gdbstub_regs)
{
GArray *find_data = g_array_new(true, true,
@@ -427,6 +433,16 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
/* Create a record for the plugin */
desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
desc.name = g_intern_string(grd->name);
+ desc.is_readonly = false;
+ if (g_strcmp0(desc.name, pc_str) == 0
+ || g_strcmp0(desc.name, eip_str) == 0
+ || g_strcmp0(desc.name, rip_str) == 0
+ || g_strcmp0(desc.name, pswa_str) == 0
+ || g_strcmp0(desc.name, iaoq_str) == 0
+ || g_strcmp0(desc.name, rpc_str) == 0
+ ) {
+ desc.is_readonly = true;
+ }
desc.feature = g_intern_string(grd->feature_name);
g_array_append_val(find_data, desc);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v6 5/7] plugins: add read-only property for registers
2026-03-03 13:07 ` [PATCH v6 5/7] plugins: add read-only property for registers Florian Hofhammer
@ 2026-03-03 18:08 ` Pierrick Bouvier
0 siblings, 0 replies; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-03 18:08 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/3/26 5:07 AM, Florian Hofhammer wrote:
> 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.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> include/plugins/qemu-plugin.h | 3 +++
> plugins/api.c | 16 ++++++++++++++++
> 2 files changed, 19 insertions(+)
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v6 6/7] plugins: prohibit writing to read-only registers
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (4 preceding siblings ...)
2026-03-03 13:07 ` [PATCH v6 5/7] plugins: add read-only property for registers Florian Hofhammer
@ 2026-03-03 13:07 ` Florian Hofhammer
2026-03-03 13:57 ` Alex Bennée
2026-03-03 18:09 ` Pierrick Bouvier
2026-03-03 13:07 ` [PATCH v6 7/7] tests/tcg/plugins: test register accesses Florian Hofhammer
2026-03-03 19:20 ` [PATCH v6 0/7] Enable PC diversion via the plugin API Pierrick Bouvier
7 siblings, 2 replies; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-03 13:07 UTC (permalink / raw)
To: qemu-devel
Cc: Florian Hofhammer, Alex Bennée, Pierrick Bouvier,
Laurent Vivier, berrange, richard.henderson, imp
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 | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/plugins/api.c b/plugins/api.c
index 85b34949cb..0c348a789b 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -424,6 +424,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
for (int i = 0; i < gdbstub_regs->len; i++) {
GDBRegDesc *grd = &g_array_index(gdbstub_regs, GDBRegDesc, i);
qemu_plugin_reg_descriptor desc;
+ gint plugin_ro_bit = 0;
/* skip "un-named" regs */
if (!grd->name) {
@@ -431,7 +432,6 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
}
/* Create a record for the plugin */
- desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
desc.name = g_intern_string(grd->name);
desc.is_readonly = false;
if (g_strcmp0(desc.name, pc_str) == 0
@@ -442,7 +442,9 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
|| g_strcmp0(desc.name, rpc_str) == 0
) {
desc.is_readonly = true;
+ plugin_ro_bit = 1;
}
+ desc.handle = GINT_TO_POINTER((grd->gdb_reg << 1) | plugin_ro_bit);
desc.feature = g_intern_string(grd->feature_name);
g_array_append_val(find_data, desc);
}
@@ -467,7 +469,7 @@ bool qemu_plugin_read_register(struct qemu_plugin_register *reg,
return false;
}
- return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1) > 0);
+ return (gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) >> 1) > 0);
}
bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
@@ -475,13 +477,16 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *reg,
{
g_assert(current_cpu);
+ /* Read-only property is encoded in least significant bit */
+ g_assert((GPOINTER_TO_INT(reg) & 1) == 0);
+
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 false;
}
- return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1) > 0);
+ return (gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) >> 1) > 0);
}
void qemu_plugin_set_pc(uint64_t vaddr)
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v6 6/7] plugins: prohibit writing to read-only registers
2026-03-03 13:07 ` [PATCH v6 6/7] plugins: prohibit writing to read-only registers Florian Hofhammer
@ 2026-03-03 13:57 ` Alex Bennée
2026-03-03 18:09 ` Pierrick Bouvier
1 sibling, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2026-03-03 13:57 UTC (permalink / raw)
To: Florian Hofhammer
Cc: qemu-devel, Pierrick Bouvier, Laurent Vivier, berrange,
richard.henderson, imp
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:
> The opaque register handle encodes whether a register is read-only in
> the lowest bit and prevents writing to the register via the plugin API
> in this case.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v6 6/7] plugins: prohibit writing to read-only registers
2026-03-03 13:07 ` [PATCH v6 6/7] plugins: prohibit writing to read-only registers Florian Hofhammer
2026-03-03 13:57 ` Alex Bennée
@ 2026-03-03 18:09 ` Pierrick Bouvier
1 sibling, 0 replies; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-03 18:09 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/3/26 5:07 AM, Florian Hofhammer wrote:
> 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 | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v6 7/7] tests/tcg/plugins: test register accesses
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (5 preceding siblings ...)
2026-03-03 13:07 ` [PATCH v6 6/7] plugins: prohibit writing to read-only registers Florian Hofhammer
@ 2026-03-03 13:07 ` Florian Hofhammer
2026-03-03 18:10 ` Pierrick Bouvier
2026-03-03 19:20 ` [PATCH v6 0/7] Enable PC diversion via the plugin API Pierrick Bouvier
7 siblings, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-03 13:07 UTC (permalink / raw)
To: qemu-devel
Cc: Florian Hofhammer, Alex Bennée, Pierrick Bouvier,
Laurent Vivier, berrange, richard.henderson, imp
The additional plugin tests register accesses, specifically both for
read-only and read-write registers. Writing to a read-only register is
currently not tested, as this would trigger an assertion and fail the
test.
Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
---
tests/tcg/plugins/meson.build | 1 +
tests/tcg/plugins/registers.c | 79 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+)
diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
index b3e3a9a6d0..d7f8f0ae0a 100644
--- a/tests/tcg/plugins/meson.build
+++ b/tests/tcg/plugins/meson.build
@@ -6,6 +6,7 @@ test_plugins = [
'insn.c',
'mem.c',
'patch.c',
+'registers.c',
'reset.c',
'setpc.c',
'syscall.c',
diff --git a/tests/tcg/plugins/registers.c b/tests/tcg/plugins/registers.c
new file mode 100644
index 0000000000..6d627c7037
--- /dev/null
+++ b/tests/tcg/plugins/registers.c
@@ -0,0 +1,79 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Copyright (C) 2026, Florian Hofhammer <florian.hofhammer@epfl.ch>
+ */
+#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 plugin tests whether we can read and write registers via the plugin
+ * API. We try to just read/write a single register, as some architectures have
+ * registers that cannot be written to, which would fail the test.
+ * See: https://lists.gnu.org/archive/html/qemu-devel/2026-02/msg07025.html
+ */
+static void vcpu_init_cb(qemu_plugin_id_t id, unsigned int vcpu_index)
+{
+ g_autoptr(GArray) regs = qemu_plugin_get_registers();
+ g_assert(regs != NULL);
+ g_autoptr(GByteArray) buf = g_byte_array_sized_new(0);
+ qemu_plugin_reg_descriptor *reg_desc = NULL;
+ bool success = false;
+
+ /* Make sure we can read and write a register not marked as readonly */
+ 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) {
+ g_byte_array_set_size(buf, 0);
+ success = qemu_plugin_read_register(reg_desc->handle, buf);
+ g_assert(success);
+ g_assert(buf->len > 0);
+ success = qemu_plugin_write_register(reg_desc->handle, buf);
+ g_assert(success);
+ break;
+ } else {
+ reg_desc = NULL;
+ }
+ }
+ g_assert(regs->len == 0 || reg_desc != NULL);
+
+ /*
+ * Check whether we can still read 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.
+ */
+ 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) {
+ g_byte_array_set_size(buf, 0);
+ success = qemu_plugin_read_register(reg_desc->handle, buf);
+ g_assert(success);
+ g_assert(buf->len > 0);
+ break;
+ } else {
+ reg_desc = NULL;
+ }
+ }
+ g_assert(regs->len == 0 || reg_desc != NULL);
+ /*
+ * Note: we currently do not test whether the read-only register can be
+ * written to, because doing so would throw an assert in the plugin API.
+ */
+}
+
+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.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v6 7/7] tests/tcg/plugins: test register accesses
2026-03-03 13:07 ` [PATCH v6 7/7] tests/tcg/plugins: test register accesses Florian Hofhammer
@ 2026-03-03 18:10 ` Pierrick Bouvier
0 siblings, 0 replies; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-03 18:10 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/3/26 5:07 AM, Florian Hofhammer wrote:
> The additional plugin tests register accesses, specifically both for
> read-only and read-write registers. Writing to a read-only register is
> currently not tested, as this would trigger an assertion and fail the
> test.
>
> Signed-off-by: Florian Hofhammer <florian.hofhammer@epfl.ch>
> ---
> tests/tcg/plugins/meson.build | 1 +
> tests/tcg/plugins/registers.c | 79 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 80 insertions(+)
>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v6 0/7] Enable PC diversion via the plugin API
2026-03-03 13:07 [PATCH v6 0/7] Enable PC diversion via the plugin API Florian Hofhammer
` (6 preceding siblings ...)
2026-03-03 13:07 ` [PATCH v6 7/7] tests/tcg/plugins: test register accesses Florian Hofhammer
@ 2026-03-03 19:20 ` Pierrick Bouvier
2026-03-04 10:36 ` Florian Hofhammer
7 siblings, 1 reply; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-03 19:20 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/3/26 5:07 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.
>
> This version v6 of the patch series addresses the requested changes from
> the previous v4 submission and an incorrect commit message from v5
> (details below).
> Note: checkpatch.pl still reports a warning about line length violations
> in patch nr. 6/7 but I did not fix this, as the line was already > 80
> characters long previously, the change added only a single character,
> and I think the readability of the code is better as it is now. Please
> let me know if you disagree and would like me to fix this!
>
> Best regards,
> Florian
>
> Changes:
> v6:
> - update commit message for patch 4/7
> v5:
> - make QEMU abort via asserts instead of just returning an error from
> the plugin API if preconditions are violated
> - extend tests for qemu_plugin_set_pc() to different contexts
> - fix issues highlighted by checkpatch.pl
> v4:
> - switch strcmp out in favor of g_strcmp0
> - split the patch introducing the qemu_plugin_set_pc() API into three
> patches, two for preparing the plugin infrastructure and the syscall
> handling code and a third introducing the actual plugin API
> 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 (7):
> plugins: add flag to specify whether PC is rw
> linux-user: make syscall emulation interruptible
> plugins: add PC diversion API function
> tests/tcg: add tests for qemu_plugin_set_pc API
> plugins: add read-only property for registers
> plugins: prohibit writing to read-only registers
> tests/tcg/plugins: test register accesses
>
> MAINTAINERS | 1 +
> include/plugins/qemu-plugin.h | 19 +++
> 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 | 1 +
> 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 | 9 +-
> linux-user/or1k/cpu_loop.c | 2 +-
> linux-user/ppc/cpu_loop.c | 10 +-
> 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 | 16 +++
> linux-user/xtensa/cpu_loop.c | 1 +
> plugins/api.c | 42 ++++++-
> plugins/core.c | 29 +++--
> tests/tcg/arm/Makefile.target | 6 +
> tests/tcg/multiarch/Makefile.target | 17 ++-
> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
> .../{ => plugin}/test-plugin-mem-access.c | 0
> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
> tests/tcg/plugins/meson.build | 2 +
> tests/tcg/plugins/registers.c | 79 ++++++++++++
> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
> 31 files changed, 495 insertions(+), 42 deletions(-)
> ---
> base-commit: 3fb456e9a0e9eef6a71d9b49bfff596a0f0046e9
> change-id: 20260303-setpc-v5-c1df30bad07f
Hi Florian,
it seems like there is a small issue building documentation with this
series, which should be trivial to fix.
https://github.com/p-b-o/qemu-ci/actions/runs/22632339221
For v7, you can run this by yourself before sending the series. It's
easy, quick and instructions are here:
https://github.com/p-b-o/qemu-ci?tab=readme-ov-file#qemu-ci
It simply saves time by making sure we don't need to pull, build and run
any test by ourselves. If any failure is left when making a pull request
on GitLab, it will be our personal responsibility to fix it.
Regards,
Pierrick
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 0/7] Enable PC diversion via the plugin API
2026-03-03 19:20 ` [PATCH v6 0/7] Enable PC diversion via the plugin API Pierrick Bouvier
@ 2026-03-04 10:36 ` Florian Hofhammer
2026-03-04 17:39 ` Pierrick Bouvier
0 siblings, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-04 10:36 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
[-- Attachment #1: Type: text/plain, Size: 6043 bytes --]
On 03/03/2026 20:20, Pierrick Bouvier wrote:
> On 3/3/26 5:07 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.
>>
>> This version v6 of the patch series addresses the requested changes from
>> the previous v4 submission and an incorrect commit message from v5
>> (details below).
>> Note: checkpatch.pl still reports a warning about line length violations
>> in patch nr. 6/7 but I did not fix this, as the line was already > 80
>> characters long previously, the change added only a single character,
>> and I think the readability of the code is better as it is now. Please
>> let me know if you disagree and would like me to fix this!
>>
>> Best regards,
>> Florian
>>
>> Changes:
>> v6:
>> - update commit message for patch 4/7
>> v5:
>> - make QEMU abort via asserts instead of just returning an error from
>> the plugin API if preconditions are violated
>> - extend tests for qemu_plugin_set_pc() to different contexts
>> - fix issues highlighted by checkpatch.pl
>> v4:
>> - switch strcmp out in favor of g_strcmp0
>> - split the patch introducing the qemu_plugin_set_pc() API into three
>> patches, two for preparing the plugin infrastructure and the syscall
>> handling code and a third introducing the actual plugin API
>> 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 (7):
>> plugins: add flag to specify whether PC is rw
>> linux-user: make syscall emulation interruptible
>> plugins: add PC diversion API function
>> tests/tcg: add tests for qemu_plugin_set_pc API
>> plugins: add read-only property for registers
>> plugins: prohibit writing to read-only registers
>> tests/tcg/plugins: test register accesses
>>
>> MAINTAINERS | 1 +
>> include/plugins/qemu-plugin.h | 19 +++
>> 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 | 1 +
>> 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 | 9 +-
>> linux-user/or1k/cpu_loop.c | 2 +-
>> linux-user/ppc/cpu_loop.c | 10 +-
>> 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 | 16 +++
>> linux-user/xtensa/cpu_loop.c | 1 +
>> plugins/api.c | 42 ++++++-
>> plugins/core.c | 29 +++--
>> tests/tcg/arm/Makefile.target | 6 +
>> tests/tcg/multiarch/Makefile.target | 17 ++-
>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>> .../{ => plugin}/test-plugin-mem-access.c | 0
>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>> tests/tcg/plugins/meson.build | 2 +
>> tests/tcg/plugins/registers.c | 79 ++++++++++++
>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>> 31 files changed, 495 insertions(+), 42 deletions(-)
>> ---
>> base-commit: 3fb456e9a0e9eef6a71d9b49bfff596a0f0046e9
>> change-id: 20260303-setpc-v5-c1df30bad07f
>
> Hi Florian,
>
> it seems like there is a small issue building documentation with this series, which should be trivial to fix.
> https://github.com/p-b-o/qemu-ci/actions/runs/22632339221
Sorry, I didn't catch this one before. I didn't have sphinx installed
locally and built without the docs.
It seems as if the issue is coming from the declaration of the new API
as "QEMU_PLUGIN_API G_NORETURN void ..." and sphinx is tripping over the
"G_NORETURN" macro. To fix this, I could either change the sphinx config
to accept the macro, or remove the attribute from the declaration. I'd
personally prefer the former but I'd be happy to get your opinion on
this.
Best regards,
Florian
> For v7, you can run this by yourself before sending the series. It's easy, quick and instructions are here:
> https://github.com/p-b-o/qemu-ci?tab=readme-ov-file#qemu-ci
>
> It simply saves time by making sure we don't need to pull, build and run any test by ourselves. If any failure is left when making a pull request on GitLab, it will be our personal responsibility to fix it.
>
> Regards,
> Pierrick
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4346 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 0/7] Enable PC diversion via the plugin API
2026-03-04 10:36 ` Florian Hofhammer
@ 2026-03-04 17:39 ` Pierrick Bouvier
2026-03-05 7:14 ` Florian Hofhammer
0 siblings, 1 reply; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-04 17:39 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/4/26 2:36 AM, Florian Hofhammer wrote:
> On 03/03/2026 20:20, Pierrick Bouvier wrote:
>> On 3/3/26 5:07 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.
>>>
>>> This version v6 of the patch series addresses the requested changes from
>>> the previous v4 submission and an incorrect commit message from v5
>>> (details below).
>>> Note: checkpatch.pl still reports a warning about line length violations
>>> in patch nr. 6/7 but I did not fix this, as the line was already > 80
>>> characters long previously, the change added only a single character,
>>> and I think the readability of the code is better as it is now. Please
>>> let me know if you disagree and would like me to fix this!
>>>
>>> Best regards,
>>> Florian
>>>
>>> Changes:
>>> v6:
>>> - update commit message for patch 4/7
>>> v5:
>>> - make QEMU abort via asserts instead of just returning an error from
>>> the plugin API if preconditions are violated
>>> - extend tests for qemu_plugin_set_pc() to different contexts
>>> - fix issues highlighted by checkpatch.pl
>>> v4:
>>> - switch strcmp out in favor of g_strcmp0
>>> - split the patch introducing the qemu_plugin_set_pc() API into three
>>> patches, two for preparing the plugin infrastructure and the syscall
>>> handling code and a third introducing the actual plugin API
>>> 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 (7):
>>> plugins: add flag to specify whether PC is rw
>>> linux-user: make syscall emulation interruptible
>>> plugins: add PC diversion API function
>>> tests/tcg: add tests for qemu_plugin_set_pc API
>>> plugins: add read-only property for registers
>>> plugins: prohibit writing to read-only registers
>>> tests/tcg/plugins: test register accesses
>>>
>>> MAINTAINERS | 1 +
>>> include/plugins/qemu-plugin.h | 19 +++
>>> 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 | 1 +
>>> 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 | 9 +-
>>> linux-user/or1k/cpu_loop.c | 2 +-
>>> linux-user/ppc/cpu_loop.c | 10 +-
>>> 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 | 16 +++
>>> linux-user/xtensa/cpu_loop.c | 1 +
>>> plugins/api.c | 42 ++++++-
>>> plugins/core.c | 29 +++--
>>> tests/tcg/arm/Makefile.target | 6 +
>>> tests/tcg/multiarch/Makefile.target | 17 ++-
>>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>>> tests/tcg/plugins/meson.build | 2 +
>>> tests/tcg/plugins/registers.c | 79 ++++++++++++
>>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>>> 31 files changed, 495 insertions(+), 42 deletions(-)
>>> ---
>>> base-commit: 3fb456e9a0e9eef6a71d9b49bfff596a0f0046e9
>>> change-id: 20260303-setpc-v5-c1df30bad07f
>>
>> Hi Florian,
>>
>> it seems like there is a small issue building documentation with this series, which should be trivial to fix.
>> https://github.com/p-b-o/qemu-ci/actions/runs/22632339221
>
> Sorry, I didn't catch this one before. I didn't have sphinx installed
> locally and built without the docs.
>
No worries, it's hard to guess all the things that you *might* miss, and
it's a common mistake even for regular contributors, including myself.
> It seems as if the issue is coming from the declaration of the new API
> as "QEMU_PLUGIN_API G_NORETURN void ..." and sphinx is tripping over the
> "G_NORETURN" macro. To fix this, I could either change the sphinx config
> to accept the macro, or remove the attribute from the declaration. I'd
> personally prefer the former but I'd be happy to get your opinion on
> this.
>
Simply move the attribute after prototype, it's the same semantic.
diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
index 791d223df40..0825bc9279d 100644
--- a/include/plugins/qemu-plugin.h
+++ b/include/plugins/qemu-plugin.h
@@ -1055,8 +1055,7 @@ bool qemu_plugin_write_register(struct
qemu_plugin_register *handle,
* resumes execution at that address. This function does not return.
*/
QEMU_PLUGIN_API
-G_NORETURN
-void qemu_plugin_set_pc(uint64_t vaddr);
+void qemu_plugin_set_pc(uint64_t vaddr) G_NORETURN;
/**
* qemu_plugin_read_memory_vaddr() - read from memory using a virtual
address
Regards,
Pierrick
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v6 0/7] Enable PC diversion via the plugin API
2026-03-04 17:39 ` Pierrick Bouvier
@ 2026-03-05 7:14 ` Florian Hofhammer
2026-03-05 8:30 ` Pierrick Bouvier
0 siblings, 1 reply; 26+ messages in thread
From: Florian Hofhammer @ 2026-03-05 7:14 UTC (permalink / raw)
To: Pierrick Bouvier, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 04/03/2026 18:39, Pierrick Bouvier wrote:
> On 3/4/26 2:36 AM, Florian Hofhammer wrote:
>> On 03/03/2026 20:20, Pierrick Bouvier wrote:
>>> On 3/3/26 5:07 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.
>>>>
>>>> This version v6 of the patch series addresses the requested changes from
>>>> the previous v4 submission and an incorrect commit message from v5
>>>> (details below).
>>>> Note: checkpatch.pl still reports a warning about line length violations
>>>> in patch nr. 6/7 but I did not fix this, as the line was already > 80
>>>> characters long previously, the change added only a single character,
>>>> and I think the readability of the code is better as it is now. Please
>>>> let me know if you disagree and would like me to fix this!
>>>>
>>>> Best regards,
>>>> Florian
>>>>
>>>> Changes:
>>>> v6:
>>>> - update commit message for patch 4/7
>>>> v5:
>>>> - make QEMU abort via asserts instead of just returning an error from
>>>> the plugin API if preconditions are violated
>>>> - extend tests for qemu_plugin_set_pc() to different contexts
>>>> - fix issues highlighted by checkpatch.pl
>>>> v4:
>>>> - switch strcmp out in favor of g_strcmp0
>>>> - split the patch introducing the qemu_plugin_set_pc() API into three
>>>> patches, two for preparing the plugin infrastructure and the syscall
>>>> handling code and a third introducing the actual plugin API
>>>> 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 (7):
>>>> plugins: add flag to specify whether PC is rw
>>>> linux-user: make syscall emulation interruptible
>>>> plugins: add PC diversion API function
>>>> tests/tcg: add tests for qemu_plugin_set_pc API
>>>> plugins: add read-only property for registers
>>>> plugins: prohibit writing to read-only registers
>>>> tests/tcg/plugins: test register accesses
>>>>
>>>> MAINTAINERS | 1 +
>>>> include/plugins/qemu-plugin.h | 19 +++
>>>> 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 | 1 +
>>>> 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 | 9 +-
>>>> linux-user/or1k/cpu_loop.c | 2 +-
>>>> linux-user/ppc/cpu_loop.c | 10 +-
>>>> 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 | 16 +++
>>>> linux-user/xtensa/cpu_loop.c | 1 +
>>>> plugins/api.c | 42 ++++++-
>>>> plugins/core.c | 29 +++--
>>>> tests/tcg/arm/Makefile.target | 6 +
>>>> tests/tcg/multiarch/Makefile.target | 17 ++-
>>>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>>>> tests/tcg/plugins/meson.build | 2 +
>>>> tests/tcg/plugins/registers.c | 79 ++++++++++++
>>>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>>>> 31 files changed, 495 insertions(+), 42 deletions(-)
>>>> ---
>>>> base-commit: 3fb456e9a0e9eef6a71d9b49bfff596a0f0046e9
>>>> change-id: 20260303-setpc-v5-c1df30bad07f
>>>
>>> Hi Florian,
>>>
>>> it seems like there is a small issue building documentation with this series, which should be trivial to fix.
>>> https://github.com/p-b-o/qemu-ci/actions/runs/22632339221
>>
>> Sorry, I didn't catch this one before. I didn't have sphinx installed
>> locally and built without the docs.
>>
>
> No worries, it's hard to guess all the things that you *might* miss, and it's a common mistake even for regular contributors, including myself.
>
>> It seems as if the issue is coming from the declaration of the new API
>> as "QEMU_PLUGIN_API G_NORETURN void ..." and sphinx is tripping over the
>> "G_NORETURN" macro. To fix this, I could either change the sphinx config
>> to accept the macro, or remove the attribute from the declaration. I'd
>> personally prefer the former but I'd be happy to get your opinion on
>> this.
>>
>
> Simply move the attribute after prototype, it's the same semantic.
>
> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
> index 791d223df40..0825bc9279d 100644
> --- a/include/plugins/qemu-plugin.h
> +++ b/include/plugins/qemu-plugin.h
> @@ -1055,8 +1055,7 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *handle,
> * resumes execution at that address. This function does not return.
> */
> QEMU_PLUGIN_API
> -G_NORETURN
> -void qemu_plugin_set_pc(uint64_t vaddr);
> +void qemu_plugin_set_pc(uint64_t vaddr) G_NORETURN;
Unfortunately, this only works if there's no C++ compiler present.
Otherwise, glib's G_NORETURN is defined as [[noreturn]] (it checks
whether the compiler supports C++11's [[noreturn]] and prioritizes this
attribute style), which cannot be placed after the function prototype.
>
> /**
> * qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
>
> Regards,
> Pierrick
Best regards,
Florian
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v6 0/7] Enable PC diversion via the plugin API
2026-03-05 7:14 ` Florian Hofhammer
@ 2026-03-05 8:30 ` Pierrick Bouvier
0 siblings, 0 replies; 26+ messages in thread
From: Pierrick Bouvier @ 2026-03-05 8:30 UTC (permalink / raw)
To: Florian Hofhammer, qemu-devel
Cc: Alex Bennée, Laurent Vivier, berrange, richard.henderson,
imp
On 3/4/26 11:14 PM, Florian Hofhammer wrote:
> On 04/03/2026 18:39, Pierrick Bouvier wrote:
>> On 3/4/26 2:36 AM, Florian Hofhammer wrote:
>>> On 03/03/2026 20:20, Pierrick Bouvier wrote:
>>>> On 3/3/26 5:07 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.
>>>>>
>>>>> This version v6 of the patch series addresses the requested changes from
>>>>> the previous v4 submission and an incorrect commit message from v5
>>>>> (details below).
>>>>> Note: checkpatch.pl still reports a warning about line length violations
>>>>> in patch nr. 6/7 but I did not fix this, as the line was already > 80
>>>>> characters long previously, the change added only a single character,
>>>>> and I think the readability of the code is better as it is now. Please
>>>>> let me know if you disagree and would like me to fix this!
>>>>>
>>>>> Best regards,
>>>>> Florian
>>>>>
>>>>> Changes:
>>>>> v6:
>>>>> - update commit message for patch 4/7
>>>>> v5:
>>>>> - make QEMU abort via asserts instead of just returning an error from
>>>>> the plugin API if preconditions are violated
>>>>> - extend tests for qemu_plugin_set_pc() to different contexts
>>>>> - fix issues highlighted by checkpatch.pl
>>>>> v4:
>>>>> - switch strcmp out in favor of g_strcmp0
>>>>> - split the patch introducing the qemu_plugin_set_pc() API into three
>>>>> patches, two for preparing the plugin infrastructure and the syscall
>>>>> handling code and a third introducing the actual plugin API
>>>>> 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 (7):
>>>>> plugins: add flag to specify whether PC is rw
>>>>> linux-user: make syscall emulation interruptible
>>>>> plugins: add PC diversion API function
>>>>> tests/tcg: add tests for qemu_plugin_set_pc API
>>>>> plugins: add read-only property for registers
>>>>> plugins: prohibit writing to read-only registers
>>>>> tests/tcg/plugins: test register accesses
>>>>>
>>>>> MAINTAINERS | 1 +
>>>>> include/plugins/qemu-plugin.h | 19 +++
>>>>> 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 | 1 +
>>>>> 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 | 9 +-
>>>>> linux-user/or1k/cpu_loop.c | 2 +-
>>>>> linux-user/ppc/cpu_loop.c | 10 +-
>>>>> 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 | 16 +++
>>>>> linux-user/xtensa/cpu_loop.c | 1 +
>>>>> plugins/api.c | 42 ++++++-
>>>>> plugins/core.c | 29 +++--
>>>>> tests/tcg/arm/Makefile.target | 6 +
>>>>> tests/tcg/multiarch/Makefile.target | 17 ++-
>>>>> .../multiarch/{ => plugin}/check-plugin-output.sh | 0
>>>>> .../{ => plugin}/test-plugin-mem-access.c | 0
>>>>> tests/tcg/multiarch/plugin/test-plugin-set-pc.c | 140 +++++++++++++++++++++
>>>>> tests/tcg/plugins/meson.build | 2 +
>>>>> tests/tcg/plugins/registers.c | 79 ++++++++++++
>>>>> tests/tcg/plugins/setpc.c | 120 ++++++++++++++++++
>>>>> 31 files changed, 495 insertions(+), 42 deletions(-)
>>>>> ---
>>>>> base-commit: 3fb456e9a0e9eef6a71d9b49bfff596a0f0046e9
>>>>> change-id: 20260303-setpc-v5-c1df30bad07f
>>>>
>>>> Hi Florian,
>>>>
>>>> it seems like there is a small issue building documentation with this series, which should be trivial to fix.
>>>> https://github.com/p-b-o/qemu-ci/actions/runs/22632339221
>>>
>>> Sorry, I didn't catch this one before. I didn't have sphinx installed
>>> locally and built without the docs.
>>>
>>
>> No worries, it's hard to guess all the things that you *might* miss, and it's a common mistake even for regular contributors, including myself.
>>
>>> It seems as if the issue is coming from the declaration of the new API
>>> as "QEMU_PLUGIN_API G_NORETURN void ..." and sphinx is tripping over the
>>> "G_NORETURN" macro. To fix this, I could either change the sphinx config
>>> to accept the macro, or remove the attribute from the declaration. I'd
>>> personally prefer the former but I'd be happy to get your opinion on
>>> this.
>>>
>>
>> Simply move the attribute after prototype, it's the same semantic.
>>
>> diff --git a/include/plugins/qemu-plugin.h b/include/plugins/qemu-plugin.h
>> index 791d223df40..0825bc9279d 100644
>> --- a/include/plugins/qemu-plugin.h
>> +++ b/include/plugins/qemu-plugin.h
>> @@ -1055,8 +1055,7 @@ bool qemu_plugin_write_register(struct qemu_plugin_register *handle,
>> * resumes execution at that address. This function does not return.
>> */
>> QEMU_PLUGIN_API
>> -G_NORETURN
>> -void qemu_plugin_set_pc(uint64_t vaddr);
>> +void qemu_plugin_set_pc(uint64_t vaddr) G_NORETURN;
>
> Unfortunately, this only works if there's no C++ compiler present.
> Otherwise, glib's G_NORETURN is defined as [[noreturn]] (it checks
> whether the compiler supports C++11's [[noreturn]] and prioritizes this
> attribute style), which cannot be placed after the function prototype.
>
Oh right, I didn't try a full build before proposing this, sorry.
In this case, let's just use the original compiler attribute which will
make everyone happy.
--- a/include/plugins/qemu-plugin.h
+++ b/include/plugins/qemu-plugin.h
@@ -1055,7 +1055,7 @@ bool qemu_plugin_write_register(struct
qemu_plugin_register *handle,
* resumes execution at that address. This function does not return.
*/
QEMU_PLUGIN_API
-G_NORETURN
+__attribute__((__noreturn__))
void qemu_plugin_set_pc(uint64_t vaddr);
Regards,
Pierrick
^ permalink raw reply [flat|nested] 26+ messages in thread